[Ready for review] Make Clone-only resume more efficient. #1719
Conversation
This file contains 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
darinyu
changed the title
metaflow/runtime.py
Outdated
| ) | ||
|
|
||
| # Check if we should be the resume leader (maybe from previous attempt) | ||
| if ds.has_metadata("_resume_leader", add_attempt=False): |
There was a problem hiding this comment.
This is a bit tricky here -- there is a possible race where the resume leader is elected but never writes out the metadata which would mean that then no one is the resume leader. We may want to comment on this here but I think we can ignore it for now as it really shouldn't happen.
There was a problem hiding this comment.
I think it is fine here because it is under "task_id_exists_already=True", which means
- We are in first attempt, since task_id_exists_already is true, we can not be leader anyways.
- We are in second attempt, and we could be leader from writes in previous attempt (no race here)
There was a problem hiding this comment.
talked offline, added code to retry (to avoid race condition)
saikonen
approved these changes
Mar 13, 2024
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.
Current clone-only resume has a few flaws we need to solve:
To tackle this, we decided to elect a resume leader (whichever tasks register _parameters task first) and also remembers the resume leader path. All non-leader will need to wait for the leader to finish resuming (which essentially is copying the datastore pointer to new place).
In the case of a failed clone, we will check whether task_id is registered and task is complete to ensure that we will still clone the old run when task_id is already registered but never complete.
To test this, one can try this flow.