[WIP] Bound re-enqueue of usage payloads with unresolvable API key hash - #2610
Draft
PawelPeczek-Roboflow wants to merge 2 commits into
Draft
[WIP] Bound re-enqueue of usage payloads with unresolvable API key hash#2610PawelPeczek-Roboflow wants to merge 2 commits into
PawelPeczek-Roboflow wants to merge 2 commits into
Conversation
Persisted usage rows are keyed by api_key_hash. After a restart the plaintext key is unknown until re-seen, so send_usage_payload authenticates with the hash, the server rejects it, and _offload_to_api re-enqueues forever (SQLiteQueue.full() is always False). Drop such rows after a bounded number of attempts; known-key transient failures still retry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🚧 Work in progress — not ready for review
Draft PR from a batch addressing findings from an in-depth engineering review. Fixes review finding 92 (Low, Correctness).
The bug
When usage telemetry is persisted to SQLite, rows are keyed by
api_key_hash. After a process restart the in-memory_hashed_api_keysmap is empty, sohashes_to_api_keysin_offload_to_api(inference/usage_tracking/collector.py:544) cannot resolve a persisted hash to its plaintext key.send_usage_payloadthen either marks the hash failed or sends it with the SHA256 hash used as the Bearer token, which the server rejects. The failed hash is kept in the payload and re-enqueued by_offload_to_api(collector.py:594-596pre-fix). BecauseSQLiteQueue.full()is hardcoded toFalse, nothing bounds this, so rows for a key that is never re-seen re-enqueue forever, accumulating inusage.dband wasting a request every flush.Root cause
The re-enqueue loop treated an unresolvable-key failure identically to a transient failure and retried it indefinitely, even though it can never succeed until the same plaintext key issues another request.
Fix
inference/usage_tracking/collector.pyonly:_MAX_UNRESOLVED_RESEND_ATTEMPTS = 5and an instance counterself._unresolved_resend_attempts: Dict[APIKeyHash, int]._offload_to_api, split the re-enqueue decision three ways: delivered payloads are dropped and their counter cleared; failures whose hash is resolvable (transient, e.g. network down) still retry indefinitely with the counter reset; failures whose hash is not resolvable increment the counter and are dropped once it reaches the bound.No plaintext API keys are persisted to disk (avoiding a security regression), and legitimate retry behaviour for known keys is preserved.
Verification
Standalone simulation of the corrected re-enqueue loop (full package import isn't possible from the sparse worktree):
python3 -m py_compile inference/usage_tracking/collector.pypasses.Scope
Focused change; one of a coordinated batch (one PR per finding).
🤖 Generated with Claude Code