DanielLeens commented on PR #12316: URL: https://github.com/apache/seatunnel/pull/12316#issuecomment-5846335036
Thank you for taking the time to build an independent harness and reproduce this rather than just reading the description — that's exactly the kind of check I was hoping for. The harness numbers line up with what I traced through the code: `Thread.sleep(0)` on `dev` losing the race roughly a quarter of the time with a multi-poll tail, versus this PR bounding it to 1-2 polls every time. And I read the throughput comparison the same way you do — the fast path is one atomic read per poll with no allocation, so a difference in the noise on a loaded host is what I'd expect, not evidence either way on its own. You're also right about the boundary of what this fixes: the handoff only creates a yield point *between* `pollNext` calls, so a single call that blocks for seconds on its own (a large JDBC split, for instance) is unaffected — that's the same limitation I called out in my own review, and it's exactly why #12313's fixture (bounding rows per poll) and this PR are complementary rather than overlapping fixes for the same symptom. Your suggestion to keep one IT variant on the original `split.num = 1` workload is a good one, and I agree with the reasoning: if #12313's fixture lands and the IT no longer produces a long back-to-back lock hold, this handoff's own regression coverage would quietly disappear even though the code is still there. I'll add that variant in the next revision so the two fixes don't end up canceling out each other's test coverage. -- 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]
