[fix][evaluation]: retry-running-lock #539
Open
xueyizheng wants to merge 4 commits into
Open
Conversation
… items Without this, retrying a single item on a Running experiment quietly fails: the item id is appended to expt_run_log.item_ids but no ExptScheduleEvent is published, so the scheduler never seeds expt_item_run_logs / resets turn results. The retry "succeeds" at the API layer while the item stays in its old (failed) state. Calling RetryItems with the same runID is safe and idempotent — the event carries only the user's requested itemIDs, and resetEvalItems uses a fresh schedule scope per (exptID, exptRunID).
…e-reset ExptRetryItemsExec.ExptStart's idem key was (exptID, exptRunID) only. Once the first retry on a run wrote the key, every subsequent retry on the same run short-circuited at the idem.Exist check, skipping resetEvalItems entirely. The user-facing effect was that an item appended to a Running experiment got its row into expt_run_log.item_ids but its expt_run_id / expt_item_result_run_log / turn_result.status were never updated — UI stayed on the stale state. Switch the retry-items idem key to (exptID, exptRunID, sha1(sorted itemIDs)). MQ redelivery protection is preserved because identical event payloads produce identical hashes; the only relaxation is letting genuinely-new item sets through. Other schedule modes (Submit / FailRetry / Append / RetryAll) keep using makeStartIdemKey, so their behavior is unchanged.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #539 +/- ##
=======================================
Coverage 77.41% 77.41%
=======================================
Files 662 662
Lines 74864 74871 +7
=======================================
+ Hits 57956 57965 +9
+ Misses 13487 13486 -1
+ Partials 3421 3420 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
xueyizheng
changed the title
Co-Authored-By: Claude Opus 4.7 (1M context) <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
1 participant
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.
What type of PR is this?
Check the PR title
(Optional) Translate the PR title into Chinese
(Optional) More detailed description for this PR(en: English/zh: Chinese)
en:
zh(optional):
(Optional) Which issue(s) this PR fixes