DanielLeens opened a new pull request, #11726:
URL: https://github.com/apache/seatunnel/pull/11726
### Purpose of this pull request
Fix a `unit-test (11, windows-latest)` failure unrelated to the PRs it
blocks.
`MultiTableSinkWriterSchemaChangeBroadcastTest#schemaChangeKeepsIOExceptionContractWhenWorkerAlreadyFailed`
(added by #11015) intermittently fails with:
```
[ERROR] schemaChangeKeepsIOExceptionContractWhenWorkerAlreadyFailed Time
elapsed: 0.178 s <<< ERROR!
java.lang.RuntimeException: java.io.IOException: boom-before-schema-entry
at
MultiTableSinkWriterSchemaChangeBroadcastTest.schemaChangeKeepsIOExceptionContractWhenWorkerAlreadyFailed(MultiTableSinkWriterSchemaChangeBroadcastTest.java:412)
Caused by: java.io.IOException: boom-before-schema-entry
```
Observed on Windows-JDK11 across four unrelated PRs: #10453, #11557, #11559,
#11626.
### Root cause
Line 412 is a bare `coordinator.close();` at the end of the test, after the
test has already asserted (lines 400–411) that `applySchemaChange()` correctly
turns the worker's stored row-write failure into an `IOException`. The
`close()` call is incidental cleanup — nothing in the test asserts on it.
`MultiTableSinkWriter.close()` calls `checkQueueRemain()`:
```java
private void checkQueueRemain() throws IOException {
while (hasPendingRuntimeWrites()) {
Thread.sleep(100);
subSinkErrorCheck(); // only reachable while a write still looks
"pending"
}
}
```
`subSinkErrorCheck()` is only invoked *inside* the loop, i.e. only while
`hasPendingRuntimeWrites()` (which checks
`MultiTableWriterRunnable.isProcessingRow()`) is still `true`. In
`MultiTableWriterRunnable.run()`, a failing row sets the worker's `throwable`
field, and only afterward — in a **separate** volatile write — clears
`processingRow`:
```java
} catch (Throwable error) {
...
throwable = error; // stored first
failPendingSchemaChangeRequests(queueElement, error);
...
processingRow = false; // cleared afterward
break;
}
```
The test's own poll loop (lines 396–398) only waits for `getThrowable() !=
null`; it says nothing about `processingRow`. So there is a real window where a
concurrent `close()` can observe the stored failure while `processingRow` still
reads `true`, re-enter `checkQueueRemain()`'s loop, call `subSinkErrorCheck()`,
and re-throw the *same* failure the test already asserted on two lines earlier
— now escaping as an unhandled `RuntimeException` instead of an assertion.
This is **pre-existing, long-standing behavior**, not a regression from
#11015: `checkQueueRemain()`'s shape (`subSinkErrorCheck()` reachable only
inside the pending-writes loop) is unchanged back to January 2026
(`eca4e2b3421414d0b5690b1bf621845da09879be`). Whether `close()` re-surfaces an
already-observed failure has always been timing-dependent; what changed in
#11015 is a new test that implicitly assumed it wouldn't.
I deliberately did not touch
`MultiTableSinkWriter`/`MultiTableWriterRunnable` production code — several
other open PRs (#10306, #11206, #11721, #11725) are independently modifying
those same classes, and closing this specific timing window is a separate,
higher-risk change than this CI-stability fix warrants.
### Does this PR introduce _any_ user-facing change?
No. Test-only change; no production code is touched.
- `assertThrows(IOException.class, () ->
coordinator.applySchemaChange(...))` and the message assertion on it are
untouched — that is the behavior this test's name describes, and it is
unaffected by this change.
- The trailing `coordinator.close()` is now wrapped to accept its two
legitimate outcomes: returning normally, or re-throwing the *exact same*
`IOException` instance already asserted above (checked with `assertSame`, not
just a message match — this is a strictly stronger check than the
message-equality assertion already used earlier in the test, not a weaker one).
Any other exception still fails the test.
### How was this patch tested?
Existing test keeps every assertion it had, plus the new `assertSame` check
on the tolerated `close()` path. Correctness verified by CI on this PR head.
### Check list
* [x] If any new Jar binary package adding in your PR, please add License
Notice according [New License
Guide](https://github.com/apache/seatunnel/blob/dev/docs/en/contribution/new-license.md)
* [x] If necessary, please update the documentation to describe the new
feature. https://github.com/apache/seatunnel/tree/dev/docs
* [x] If necessary, please update `incompatible-changes.md` to describe the
incompatibility caused by this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
