Skip to content

fix(prefix): prevent evicted hashes from being re-added to hashToPos in indexer#2579

Closed
gyliu513 wants to merge 1 commit intokubernetes-sigs:mainfrom
gyliu513:indexer
Closed

fix(prefix): prevent evicted hashes from being re-added to hashToPos in indexer#2579
gyliu513 wants to merge 1 commit intokubernetes-sigs:mainfrom
gyliu513:indexer

Conversation

@gyliu513
Copy link
Copy Markdown
Member

  • Fix a race condition in indexer.Add() where the split-lock pattern (unlock between LRU insertion and hashToPods update) allows evicted hashes to be re-added to hashToPods. When a single Add call passes more hashes than the LRU capacity, later insertions evict earlier ones. The eviction callback correctly removes entries from hashToPods, but the second lock section unconditionally re-adds all input hashes — including the evicted ones. This causes hashToPods to reference hashes no longer present in the LRU, leading to incorrect prefix-aware routing decisions.
  • Add lruForPod.Contains(hash) check before inserting into hashToPods to skip evicted hashes.
  • Add TestIndexer_AddEvictsWithinSingleCall to cover the intra-call eviction scenario.

/kind bug

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

NONE

… in indexer

- Fix a race condition in `indexer.Add()` where the split-lock pattern (unlock
  between LRU insertion and `hashToPods` update) allows evicted hashes to be
  re-added to `hashToPods`. When a single `Add` call passes more hashes than
  the LRU capacity, later insertions evict earlier ones. The eviction callback
  correctly removes entries from `hashToPods`, but the second lock section
  unconditionally re-adds all input hashes — including the evicted ones. This
  causes `hashToPods` to reference hashes no longer present in the LRU, leading
  to incorrect prefix-aware routing decisions.
- Add `lruForPod.Contains(hash)` check before inserting into `hashToPods` to
  skip evicted hashes.
- Add `TestIndexer_AddEvictsWithinSingleCall` to cover the intra-call eviction
  scenario.
@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Mar 13, 2026
@netlify
Copy link
Copy Markdown

netlify Bot commented Mar 13, 2026

Deploy Preview for gateway-api-inference-extension ready!

Name Link
🔨 Latest commit c4b219c
🔍 Latest deploy log https://app.netlify.com/projects/gateway-api-inference-extension/deploys/69b4772678eeca000803be81
😎 Deploy Preview https://deploy-preview-2579--gateway-api-inference-extension.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: gyliu513
Once this PR has been reviewed and has the lgtm label, please assign liu-cong for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot requested review from elevran and shmuelk March 13, 2026 20:44
@k8s-ci-robot k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Mar 13, 2026
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

Hi @gyliu513. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 13, 2026
@gyliu513
Copy link
Copy Markdown
Member Author

This has been fixed in #2501

@gyliu513 gyliu513 closed this Mar 14, 2026
@gyliu513 gyliu513 deleted the indexer branch March 14, 2026 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/bug Categorizes issue or PR as related to a bug. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants