Move CalcJob.presubmit call from CalcJob.run to Waiting.execute
#3666
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
muhrin
reviewed
Dec 13, 2019
| calcinfo = self.prepare_for_submission(folder) | ||
| scheduler = computer.get_scheduler() | ||
| for code in codes: | ||
| if not code.can_run_on(computer): |
There was a problem hiding this comment.
is this the equivalent of the previous is_local/get_local_executable/get_content_list check?
There was a problem hiding this comment.
It is a different check, this one was just done in the run method while the one you reference was being done in presubmit. I just moved the former here as it makes more sense here. I realize now I can merge the two loops. I will do that
There was a problem hiding this comment.
@muhrin I merged the two checks in the same loop, so this should be good to go
sphuber
force-pushed
the
fix_3664_move_presubmit_call
branch
from
514f8a1
to
cf1fd5b
Compare
The `presubmit` calls through to `prepare_for_submission` which is the place where the `CalcJob` implementation writes the input files based on the inputs nodes in a temporary sandbox folder. The contents of which are then stored in the repository of the node before the process transitions to the waiting state in await of the upload task. This upload task will then copy over the contents of the node repository to the working directory on the remote computer. There is a use case to limit which files are copied to the node's repository from the sandbox folder used by `prepare_for_submission`. This means that the sandbox folder will have to be passed to the `upload_calculation`. However, since the creation of this happens in the `CalcJob.run` call, which is quite far removed from the eventual use, this opens it up for a lot of problems. The time between folder population by `prepare_for_submission` and eventual use in the upload task can be significant, among other things because the upload task will have to wait for transport. To make this feasible the creation of the sandbox folder has to be moved closer to the upload task. Here we move the `presubmit` call from `CalcJob.run` to the upload transport task which will create the sandbox folder and pass it into the `upload_calculation`. This limits the time frame in which there is a chance for the contents of the sandbox to get lost. In addition, this now gives additional freedom of deciding which files from the sandbox are permanently stored in the node's repository for extra provenance.
sphuber
force-pushed
the
fix_3664_move_presubmit_call
branch
from
cf1fd5b
to
f405504
Compare
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.
Fixes #3664
The
presubmitcalls through toprepare_for_submissionwhich is theplace where the
CalcJobimplementation writes the input files based onthe inputs nodes in a temporary sandbox folder. The contents of which
are then stored in the repository of the node before the process
transitions to the waiting state in await of the upload task. This
upload task will then copy over the contents of the node repository to
the working directory on the remote computer.
There is a use case to limit which files are copied to the node's
repository from the sandbox folder used by
prepare_for_submission.This means that the sandbox folder will have to be passed to the
upload_calculation. However, since the creation of this happens in theCalcJob.runcall, which is quite far removed from the eventual use,this opens it up for a lot of problems. The time between folder
population by
prepare_for_submissionand eventual use in the uploadtask can be significant, among other things because the upload task will
have to wait for transport. To make this feasible the creation of the
sandbox folder has to be moved closer to the upload task.
Here we move the
presubmitcall fromCalcJob.runto the uploadtransport task which will create the sandbox folder and pass it into the
upload_calculation. This limits the time frame in which there is achance for the contents of the sandbox to get lost. In addition, this
now gives additional freedom of deciding which files from the sandbox
are permanently stored in the node's repository for extra provenance.