fix(google-batch): recover the job when the createJob response is lost - #7610
Draft
bentsherman wants to merge 1 commit into
Draft
bentsherman wants to merge 1 commit into
bentsherman wants to merge 1 commit into
Conversation
Batch API calls are wrapped in a retry policy that retries UNAVAILABLE, DEADLINE_EXCEEDED, IOException and TimeoutException. All of these are ambiguous: the request may have been applied server-side before the response was lost. Since `createJob` is not idempotent, the retry is then rejected with ALREADY_EXISTS, which is not retryable and aborts the task submission - even though the job was created and is running. Handle ALREADY_EXISTS in `submitJob` by fetching the job created by the previous attempt. The job id is fixed per task handler, so that error can only mean an earlier attempt of the same submission succeeded. The lookup is nested within the retry policy on purpose, so that a NOT_FOUND response retries the create instead of aborting the task. Signed-off-by: Ben Sherman <bentshermann@gmail.com>
✅ Deploy Preview for nextflow-docs canceled.
|
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.
Closes #6916
Problem
A run on
google-batchaborts with anALREADY_EXISTSerror naming a job that Nextflow itself just generated:Nothing external created that job — Nextflow collided with its own earlier create request.
BatchClient.apply()wraps every Batch API call in a failsafe retry policy that retriesUnavailableException,DeadlineExceededException,IOExceptionandTimeoutException. Every one of those is ambiguous: the request may have been applied server-side before the response was lost.createJobis not idempotent under such a retry, so the second attempt is rejected withALREADY_EXISTS, which is not in the retry predicate and propagates out ofsubmitJob()→GoogleBatchTaskHandler.submit()→TaskPollingMonitor, aborting the session.The job it complains about keeps running and billing while the run is torn down. A reporter on the issue confirmed this with Cloud Logging: a
502:Bad Gateway, thenALREADY_EXISTSon the same job name, andgcloud batch describeshowing that job eventuallySUCCEEDED.process.errorStrategydoes not help, since a gaxApiExceptionis not aProcessExceptionand so never reachescheckErrorStrategy().This was flagged as a residual gap while triaging the sibling issue #6878; the
DeadlineExceededExceptionbranch added in 944f48f adds another path into it but did not create it —UNAVAILABLEalready reached it on 24.04.x.Change
Handle
AlreadyExistsExceptioninsubmitJob()by fetching the job the previous attempt created. The job id is fixed per task handler (nf-<task hash>-<currentTimeMillis>,GoogleBatchTaskHandler:135), soALREADY_EXISTSon that name can only mean an earlier attempt of the same submission succeeded.The lookup sits inside the retry policy deliberately: if it comes back
NOT_FOUND—ALREADY_EXISTSwas reported but the job is not actually there —NotFoundExceptionis already retryable, so failsafe re-runs the supplier and re-attempts the create. Wrappingapply()from the outside would retry only the lookup and then abort the task.The recovery is scoped to
submitJob;apply()stays generic, soALREADY_EXISTSfromgetJob/listTasks/deleteJobstill surfaces as before.Tests
New
BatchClientSubmitJobTest.BatchServiceClient.createJobandgetJobarefinaland cannot be mocked, andBatchServiceGrpcis not on the classpath (google-cloud-batchbuilds its method descriptors by hand; onlyproto-google-cloud-batch-v1is pulled in), so there is no generated service base to subclass either. The test instead injects a fakeManagedChannelthat answers RPCs from a scripted queue and builds a realBatchServiceClientover it viaFixedTransportChannelProvider— exercising the actual API client, the gax call plumbing and the failsafe policy, with no new dependency.Six cases: plain submit; recovery after
UNAVAILABLE; recovery afterDEADLINE_EXCEEDED;NOT_FOUNDon the lookup falling back to a fresh create; the bounded give-up path; andALREADY_EXISTSstill surfacing from a generic call.Verified red/green — with the fix reverted, the four recovery cases fail with
com.google.api.gax.rpc.AlreadyExistsException(the reported error) while the two control cases still pass. With it applied all six pass, and the full:plugins:nf-google:testsuite is green.Notes for review
jobNameclosure a user can produce colliding names across concurrent runs, and this makes Nextflow adopt the pre-existing job rather than failing. The previous behaviour aborted the whole session in that case, so it was not a useful guard either — but if a louder signal is wanted,log.warnwhen the job'screateTimepredates this submit would cover it.createBatchService()buildsBatchServiceSettingswithout anyRetrySettings, so every call is pinned to gax's 60stotalTimeoutandgoogle.batch.requestTimeout(which several users set) is not a real config option.