SEPURI-SAI-KRISHNA commented on PR #12444: URL: https://github.com/apache/seatunnel/pull/12444#issuecomment-5852894012
This is the fix. I read it against the source rather than taking the thread's conclusion, and it holds up. `PayPalClient.execute()` returns at the transient-status branch without ever touching the entity, and its `finally` calls `request.abort()` unconditionally straight after. On the server side `serve()` counted `arrived` down only after `exchange.getResponseBody().write(reply.body)` returned, with the write inside a `catch (IOException ignored)`. `closeWakesRetryWait` replies with a plain 503, so that is the exact branch it takes, and if the abort wins the race the write throws, the exception is swallowed, and the latch is never released. Moving the `countDown()` above the write makes the latch mean what its name says, that the request reached the server, which is true the moment `serve()` is entered and does not depend on the response being writable. The `finally` countdown is the right belt and braces rather than redundancy: it also covers the `InterruptedException` branch and any failure in the streaming path above. `arrived` is a `CountDownLatch(1)`, so a second `countDown()` is a no-op and there is no double-release concern. Two things worth saying since I have been on the other side of this thread. I supported #12381 on its own thread on the strength of a 4-of-6 failure rate I measured across my own PRs. That data was real but my conclusion from it was wrong: I read a wait that always consumed its full budget as evidence the budget was too small, when it was evidence the latch never fired. @SEZ9 got to the mechanism and @goutamadwant's 50-run numbers settled it, including 20 of 20 failures for #12381 with the body write forced to throw against 0 of 20 here. Second, a disclosure: this red leg is currently the only thing blocking my #12438, so I have an interest in it landing. That is also why I have corrected the pointer on that PR from #12381 to this one. This has been open since 09-22 with no review from anyone but its author. The diff is eight lines in one test file, it fixes the mechanism rather than the symptom, and it is the variant with measured zero failures. It deserves a committer. -- 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]
