goutamadwant commented on PR #12316: URL: https://github.com/apache/seatunnel/pull/12316#issuecomment-5841704369
I tried to reproduce the `BackpressureSlowSinkIT` flake locally, to compare this PR and #12313 alone and together. Setup: macOS, a loaded 10-core host, one fresh JVM per run. Base was `dev` at `deb16a3c3`. Each head was the PR diff(s) applied to that commit. | variant | JDK 8 | JDK 11 (`-XX:ActiveProcessorCount=2`) | |---|---|---| | base | 0 / 8 | 0 / 8 (+ 0 / 3 with `ActiveProcessorCount=1` and `nice 19`) | | this PR | 0 / 8 | 0 / 8 | | #12313 | 0 / 8 | 0 / 8 | | both | 0 / 8 | 0 / 7 | Base never failed for me, so this is not red-to-green evidence for the IT. It only shows that neither change breaks it. **Handoff alone:** to check the mechanism I wrote a small harness around `SourceCheckpointLockHandoff`. A reader holds the monitor for 5 ms per poll, and I counted how many polls an injector waits for the monitor (200 trials each). - `Thread.sleep(0)` (current `dev`): about 25% of trials waited more than one poll; the worst case was 8-10 polls. - this PR: every trial got the lock within 1-2 polls. So the handoff does what the description says. **Throughput:** I ran a 5M-row FakeSource to InMemory batch job with `checkpoint.interval = 200`, alternating `dev` and this PR, in two batches. - Batch 1, 7 pairs, heavily loaded host: median 66.5 s on `dev`, 68.1 s with this PR. - Batch 2, 5 pairs, quieter host: median 72.0 s and 72.2 s. I read that as no measurable difference, not as a clean benchmark. The fast path is one `AtomicInteger` read per poll. The reader only parks while an injector is registered, and it would be blocked on the monitor anyway in that case. The `finally` release looks right. One limit, if I read it correctly: the handoff only helps between `pollNext` calls. A single `pollNext` that blocks on a full queue for seconds still holds the lock for that time. That is the case #12313 avoids by sizing splits under `MAX_ROWS_PER_POLL`. With #12313's fixture, though, the IT no longer exercises the long back-to-back lock hold, so it would keep passing even if this handoff regressed. If both land, it may be worth keeping one variant of the test on the original `split.num = 1` workload so this change stays covered. -- 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]
