Repository navigation
fix(ENGKNOW-3979): never cache a parallel part interrupted by a failed sibling - #151
Merged
Merged
Conversation
…d sibling When one part of a `parallel -parts` create failed, ParallelExecutor interrupted the sibling threads. BatchedPipeStepIteratorAdaptor.hasNext() turned the InterruptedException into end-of-stream, so the sibling's runner returned normally and GeneralQueryHandler moved its header-only temp file into the result cache. A rerun after fixing the failure reused that empty part and silently dropped its rows. - BatchedPipeStepIteratorAdaptor.hasNext() now throws on interrupt, like BatchedReadSource does. - GeneralQueryHandler refuses to commit output when the thread is interrupted, covering other adaptors that only restore the interrupt flag; the temp file is deleted by the existing error path. - Regression test in UTestParallel: one part throws while the other is running, rerun after the fix must return all rows. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…-failed-part-cache
…o avoid flaky CI The test appended to a file and expected the signature to change, but on fast CI runners the append can land in the same timestamp tick, giving an identical signature. Bump the mtime by 1s like the sibling test already does. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…nly interrupts Review follow-up. The commit guard relied on the thread's interrupt flag surviving until the result was committed, which is easy to lose (code that calls Thread.interrupted() or swallows InterruptedException). It also only covered plain queries, not dictionary commands, and reported user cancellations as system errors. - ParallelExecutor takes a shared cancel flag, sets it before interrupting siblings, and stops starting queued commands once it is set. - GeneralQueryHandler passes the flag to runCommand and refuses to commit when it is set or the thread is interrupted, for plain queries (before the temp file is moved into place) and for GORDICT/GORDICTPART/NORDICT output (the in-place dictionary file is deleted). - Interrupts and cancels now raise GorCancelledException instead of GorSystemException. - Tests: UTestParallelExecutor (sibling that clears its interrupt still sees the cancel, queued commands are not started) and UTestGeneralQueryHandler (cancelled query and dictionary are not committed). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
gmagnu
marked this pull request as ready for review
October 7, 2026 00:19
bragnarsson
approved these changes
Oct 7, 2026
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.
Jira: ENGKNOW-3979
Problem
In
create ##x## = parallel -parts [...] <( ... ), when one part failed,ParallelExecutorinterrupted the sibling threads.BatchedPipeStepIteratorAdaptor.hasNext()turned theInterruptedExceptioninto end-of-stream, so the sibling's runner returned normally andGeneralQueryHandlermoved its header-only temp file into the result cache. A rerun after fixing the failure reused that empty part and silently dropped its rows.Fix
BatchedPipeStepIteratorAdaptor.hasNext()now throwsGorCancelledExceptionon interrupt instead of reporting end-of-stream, so a cancel is not reported as a system error.ParallelExecutortakes a shared cancel flag. It sets the flag before interrupting siblings and stops starting queued parts once it is set.GeneralQueryHandlerpasses that flag intorunCommandand refuses to commit if the flag is set or the thread is interrupted, so the guard does not depend on the interrupt flag surviving:GORDICT/GORDICTPART/NORDICT: checked after the in-place write, and the partial dictionary file is deleted.Tests
UTestParallel.testFailedPartDoesNotLeaveSiblingPartInCache: end-to-end; one part throws while the other runs, and the rerun must return all rows. Without the fix the rerun returns 1 row instead of 4.UTestParallelExecutor: a sibling that clears its own interrupt flag still sees the cancel, and queued parts are not started.UTestGeneralQueryHandler: a cancelled query and a cancelled dictionary are not committed; a query that isn't cancelled is committed.UTestSignature.testSignatureWithVersionedLinkFile(same-tick mtime) that failed CI.:gortools:test: 2645 tests, 0 failures.🤖 Generated with Claude Code