SEPURI-SAI-KRISHNA commented on PR #12290:
URL: https://github.com/apache/seatunnel/pull/12290#issuecomment-5758227983
Thanks for the approval.
Happy to rebase whenever you want, though it would be a no-op for this job
today. #12290 is exactly 3 commits behind dev and all three are mine: #12349
and #12392 (RocketMQ) and #12410 (MQTT). None touch `seatunnel-engine`, so a
sync cannot move `engine-v2-it` either way.
I also need to correct something I wrote above. I said the differing
failures looked like runner load. That was wrong, and I found it by pulling the
two job logs in full instead of reading the summaries again. Both legs are
real, and both are already filed.
**JDK 8,
`SplitClusterFaultToleranceIT.testStreamJobCancelResolvesWhenWorkerCrashesBeforeCancelAck`**
```
expected: <CANCELED> but was: <FAILED>
at
CoordinatorService.lambda$makeTasksFailed$25(CoordinatorService.java:2144)
at CoordinatorService.makeTasksFailed(CoordinatorService.java:2129)
at
CoordinatorService.failedTaskOnMemberRemoved(CoordinatorService.java:2112)
at CoordinatorService.memberRemoved(CoordinatorService.java:2155)
```
Not a timeout. FAILED is terminal, so the Awaitility condition could never
have become true afterwards. This is #12353, which @tomatotomata reproduced on
unmodified dev.
What the log does establish is the path: the member-removed handler assigned
FAILED to the SplitEnumerator vertex, and `SubPlan` then logged `Task ...
Failed ... Begin to cancel other tasks in this pipeline`. Two sibling vertices
were already in terminal CANCELED at that instant and a third was still in the
`CheckTaskGroupIsExecutingOperation` retry, so the cancel was well underway.
What it does not establish is the state of that vertex at the moment the
member was removed, because vertex transitions are not logged at this level.
That matters for #12311, which redirects a vertex only when it was already
CANCELING; traced through `SubPlan.addPhysicalVertexCallBack`, that case does
end as CANCELED, since the callback then takes the CANCELED branch and never
sets the pipeline to FAILING. I also have a concern about whether #12377's
approach survives that same callback, but that belongs on its own PR rather
than here, so I will raise it there.
**JDK 11,
`BackpressureSlowSinkIT.testCheckpointsKeepCompletingUnderSustainedBackpressure`**
Fails at line 193, the first-checkpoint gate, not the sustained-window
assertion. The `JobStatus.RUNNING` check on the line above passed, so the job
was healthy and simply never completed a checkpoint in 2 minutes against a 15s
`checkpoint.interval`.
Dev matches the starvation preconditions #12313 describes: the conf sets
`row.num = 10000000` with `split.num = 1`, one split far above
`FakeSourceReader.MAX_ROWS_PER_POLL = 4096`, which keeps `splitInProgress`
true, skips the `Thread.sleep(1000L)` at `FakeSourceReader.java:173`, and lets
`pollNext` re-enter `synchronized (output.getCheckpointLock())` back to back.
#12313 addresses that by sizing splits under the cap, #12316 in the engine.
So both failures have open fixes already and I am not going to add another
PR in the same area.
On helping, what I have that those PRs do not is an independent reproduction
of both from a change that touches one line of `seatunnel-api`. I am glad to
attach that to whichever threads are useful and to review any of the four
properly. If you would rather I pick something up directly, tell me which.
For this PR in the meantime, the diff is `(object.hashCode() &
Integer.MAX_VALUE) % blockingQueues.size()` becoming
`HashUtils.bucketIndex(object.hashCode(), blockingQueues.size())`, and neither
failing test reaches `MultiTableSinkWriter`.
--
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]
