[AMORO-4348] Fix infinite retry loop for failed table processes - #4349
Open
j1wonpark wants to merge 1 commit into
Open
[AMORO-4348] Fix infinite retry loop for failed table processes#4349j1wonpark wants to merge 1 commit into
j1wonpark wants to merge 1 commit into
Conversation
Signed-off-by: Jiwon Park <jiwonpark@apache.org>
xxubai
reviewed
Sep 2, 2026
|
|
||
| @Override | ||
| public ProcessStatus getStatus(String processIdentifier) { | ||
| if (failedIdentifiers.contains(processIdentifier)) { |
Contributor
There was a problem hiding this comment.
nit: Please make the fake engine fail directly from submitTableProcess().
Returning FAILED immediately takes the fast-terminal path and produces redundant COMPLETE_FAILED transition errors.
Suggested change
| if (failedIdentifiers.contains(processIdentifier)) { | |
| throw new IllegalStateException("Submission failure"); |
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.
Why are the changes needed?
Fix #4348.
Since #4116,
ProcessServiceconstructsDefaultTableProcessStorepassing the current retry count into the constructor'smaxRetryTimeparameter. A new process therefore getsmaxRetryTime = 0and becomes terminal on its first failure, every subsequent transition is rejected, andretryNumbernever increases — while the retry branch inexecuteOrTraceProcesskeeps evaluatingretryNumber < PROCESS_MAX_RETRY_NUMBERas true and resubmits the process forever, with no backoff.Brief change log
DefaultTableProcessStorewithPROCESS_MAX_RETRY_NUMBERasmaxRetryTime, in both the creation and the recovery path (restores the pre-[Improvement]: Extract TableProcessStore from AmoroProcess interface. #4116 semantics of [Feature] Introduce a new framework that supports scheduling for Formats and Processes #3924)RETRY_REQUESTEDtransition is actually accepted, so a rejected transition can never loopPROCESS_MAX_RETRY_NUMBERtimes and then droppedHow was this patch tested?
TestDefaultProcessService#testFailedProcessRetryIsBoundedreproduces the infinite loop before the fix (the process is never dropped and the wait times out) and passes after: the failing process is submitted1 + PROCESS_MAX_RETRY_NUMBERtimes in total and then untracked. Existing process service tests all pass.Documentation