Fix M8: ensure FINISHED status only after successful score upload#2424
Open
hanane-ca wants to merge 1 commit into
Open
Fix M8: ensure FINISHED status only after successful score upload#2424hanane-ca wants to merge 1 commit into
hanane-ca wants to merge 1 commit into
Conversation
…e upload Problem: - compute_worker set status to FINISHED before uploading scores - If push_scores() failed (network, timeout, API error), submission stayed Finished with no scores → data integrity violation Solution: - Move _update_status(FINISHED) to AFTER push_scores() and push_output() - Add retry logic to push_scores() with 3 attempts and exponential backoff - Add HTTP status validation and explicit 30s timeout Changes: - compute_worker/compute_worker.py: * Removed _update_status(FINISHED) from start() method * Added _update_status(FINISHED) after successful uploads in run_wrapper() * Enhanced push_scores() with retry logic and error handling * Cleaned up comments to be code-focused (removed M8 references) - tests/k6/: * test_m8_finished_has_scores.js: K6 test for integrity invariant * run_m8_test.sh: Bash test orchestrator * README_finished_has_scores.md: Test documentation * All test files cleaned up (removed M8 references per guidelines) Test validates: - finished_without_scores == 0 (integrity invariant) - conservation_rate == 100% (all submissions terminal) Fixes codalab#2423
487fb00 to
24b3ce0
Compare
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.
Reviewers
@codalab/maintainers
Description
Fixes a data integrity bug where submissions could be marked as
Finishedwithout having scores or outputs uploaded.Problem: The compute worker set the submission status to
FINISHEDbefore uploading scores and outputs. If the upload failed (network error, timeout, API failure), the submission remainedFinishedwith no data.Root cause: Status update and result upload were decoupled in the workflow.
Solution:
FINISHEDstatusCode changes:
compute_worker/compute_worker.py:_update_status(FINISHED)to run_wrapper() after successful uploadspush_scores()with retry logic (3 attempts, exponential backoff, HTTP validation, 30s timeout)tests/k6/:Issues this PR resolves
Fixes #2423
Background
This fix addresses a submission lifecycle bug discovered during the EEG Foundation Challenge incident analysis:
The fix ensures the invariant:
status == Finished ⟹ scores existChecklist for hand testing
Finishedsubmissions have scoresRelevant files for testing
Integration test suite in
tests/k6/:test_m8_finished_has_scores.js— K6 test scriptrun_m8_test.sh— Test orchestratorREADME_finished_has_scores.md— Test documentationRun tests:
cd tests/k6 ./run_m8_test.shChecklist