Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

remote job submission: fix race condition #3075

Merged
merged 1 commit into from
Apr 17, 2019
Merged

remote job submission: fix race condition #3075

merged 1 commit into from
Apr 17, 2019

Conversation

davidpanderson
Copy link
Contributor

Scenario:

  • a submit_batch() creates a batch in INIT state
    and starts creating jobs
  • while it's in progress, a query_batches() RPC enumerates batches
  • while the query_batches() is in progress,
    the submit_batch() finishes and changes the state to IN_PROGRESS
  • the query_batches() finishes and rewrites the batch in INIT state

Solution: query_batches() doesn't update batches in INIT state

Fixes #

Description of the Change

Alternate Designs

Release Notes

Scenario:
- a submit_batch() creates a batch in INIT state
    and starts creating jobs
- while it's in progress, a query_batches() RPC enumerates batches
- while the query_batches() is in progress,
    the submit_batch() finishes and changes the state to IN_PROGRESS
- the query_batches() finishes and rewrites the batch in INIT state

Solution: query_batches() doesn't update batches in INIT state
@lfield
Copy link
Contributor

lfield commented Apr 10, 2019

Is there any reason why the call is done the first place? The if statement could be changed to:

    if (BATCH_STATE_COMPLETE < $batch->state and  $batch->state < BATCH_STATE_COMPLETE) {

@davidpanderson
Copy link
Contributor Author

? The function of the RPC is to find the status of batches.

@lfield
Copy link
Contributor

lfield commented Apr 10, 2019

I don’t mean the RPC call but the get_batch_params function.

@davidpanderson
Copy link
Contributor Author

That function computes the batch's fraction done,
i.e. what fraction of its jobs have finished.

@lfield
Copy link
Contributor

lfield commented Apr 11, 2019

I understand but why call that function at all if the batch is not in the state BATCH_STATE_IN_PROGRESS. Why not do the following instead?

if ($batch->state == BATCH_STATE_IN_PROGRESS) {
$batch = get_batch_params($batch, $wus);
}

@lfield lfield closed this Apr 11, 2019
@lfield lfield reopened this Apr 11, 2019
@lfield
Copy link
Contributor

lfield commented Apr 11, 2019

Sorry clicked on the wrong button.

@davidpanderson
Copy link
Contributor Author

You could do it either way. This way is fine.

@lfield
Copy link
Contributor

lfield commented Apr 17, 2019

ok, I was just considering readability of the code by having the if before the function call verses the abstraction of having it contained with the function.

@lfield lfield merged commit 6fc464a into master Apr 17, 2019
@AenBleidd AenBleidd deleted the dpa_submit8 branch May 21, 2019 20:57
@AenBleidd AenBleidd added this to the Server Release 1.2.0 milestone Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants