Skip to content

fix(ENGKNOW-3979): never cache a parallel part interrupted by a failed sibling - #151

Merged
gmagnu merged 4 commits into
mainfrom
ENGKNOW-3979-parallel-failed-part-cache
Oct 7, 2026
Merged

gmagnu merged 4 commits into
mainfrom
ENGKNOW-3979-parallel-failed-part-cache

Conversation

@gmagnu

@gmagnu gmagnu commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Jira: ENGKNOW-3979

Problem

In create ##x## = parallel -parts [...] <( ... ), when one part 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.

Fix

  • BatchedPipeStepIteratorAdaptor.hasNext() now throws GorCancelledException on interrupt instead of reporting end-of-stream, so a cancel is not reported as a system error.
  • ParallelExecutor takes a shared cancel flag. It sets the flag before interrupting siblings and stops starting queued parts once it is set.
  • GeneralQueryHandler passes that flag into runCommand and refuses to commit if the flag is set or the thread is interrupted, so the guard does not depend on the interrupt flag surviving:
    • plain queries: checked before the temp file is moved into place (the temp file is deleted);
    • 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.
  • Each new test fails with the new logic disabled and passes with it.
  • Also fixes a flaky UTestSignature.testSignatureWithVersionedLinkFile (same-tick mtime) that failed CI.
  • Full :gortools:test: 2645 tests, 0 failures.

🤖 Generated with Claude Code

Test and others added 3 commits October 4, 2026 01:49
…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>
…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>
@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Junit Tests - Summary

4 917 tests  +5   4 746 ✅ +5   21m 34s ⏱️ + 1m 11s
  511 suites +2     171 💤 ±0 
  511 files   +2       0 ❌ ±0 

Results for commit fb26ae9. ± Comparison against base commit e2d22eb.

♻️ This comment has been updated with latest results.

…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
gmagnu marked this pull request as ready for review October 7, 2026 00:19
@gmagnu
gmagnu merged commit 55cc372 into main Oct 7, 2026
14 checks passed
@gmagnu
gmagnu deleted the ENGKNOW-3979-parallel-failed-part-cache branch October 7, 2026 12:52
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.

2 participants