SEZ9 commented on PR #12381:
URL: https://github.com/apache/seatunnel/pull/12381#issuecomment-5882433708

   Thanks @zhangshenghang — re-reviewed the current head `b77b3c2cd4`.
   
   Confirmed on the PayPal side: with the `PayPalClientTest` changes dropped 
entirely, the blocking latch finding (F1) and the two related 
`ARRIVAL_WAIT_SECONDS` points (F3: overlap with `request_timeout_ms`; F4: 
blocking the full budget after the worker has died) are all resolved by 
removal. Nothing more needed there, and deferring the real fix to the 
root-cause change in `dev` is the right call.
   
   That leaves the two `FileCollectReaderBehaviorTest` points, which the 
remaining diff doesn't address yet:
   
   1. **F2 – `rediscoversFileAfterInactiveCursorClosed` ordering.** The awaits 
are still pure wall-clock conditions, so a scheduling gap around 
`idleDeadlineMs` can let the file be re-discovered *after* "second" is appended 
and the cursor seeks past it. Widening `atMost` to 10s makes the flake rarer 
but doesn't remove that ordering. Ask: gate the append on an observable state 
transition (e.g. wait until the cursor is actually closed / the file is no 
longer tracked) before writing "second", rather than relying on elapsed time 
alone.
   
   2. **F5 – final `untilAsserted` consumes events on every poll.** Once a poll 
returns something unexpected the assertion can never recover, and the failure 
message (`expected 1 but was 0`) hides what was actually observed. Ask: 
accumulate polled events into a list outside the lambda and assert on the 
accumulated collection (including its contents in the message), so the 
assertion is idempotent and diagnostic.
   
   If you'd prefer to land the ceiling widening as-is and handle F2/F5 in a 
follow-up, I'm fine with that — just say so and I'll approve this one; 
otherwise happy to re-review once those two are in.
   
   <!-- streview-comment:1387 -->


-- 
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]

Reply via email to