Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce sync lookup receives a single result #5777

Merged
merged 1 commit into from
May 14, 2024

Conversation

dapplion
Copy link
Collaborator

Issue Addressed

Current stable lookup requests include an internal request counter to differentiate requests. Because each lookup manages retries and streams it must handle interleaved packets from different requests.

PR removed the need for this internal counter, where network context handles streams and uniquely tags each request

However, a bug may break the invariant of 1 request at a time per lookup. If this happens, it will result in undefined behavior.

Proposed Changes

  • Each lookup keeps a unique ID of the current inflight requests.
  • If it gets a download result from any other ID, consider it an internal bug and hard error

This change is quite strict but ensures that in the case of duplicate requests, we don't mix errors and results from other requests. The hard error is easy to pick up from metrics (in sync_lookups_dropped_total{reason = "BadState"}) and logs.

@realbigsean
Copy link
Member

@mergify queue

Copy link

mergify bot commented May 14, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at ce66ab3

@mergify mergify bot merged commit ce66ab3 into sigp:unstable May 14, 2024
27 checks passed
@dapplion dapplion deleted the enforce-single-request branch May 14, 2024 10:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants