DanielLeens commented on PR #11636:
URL: https://github.com/apache/seatunnel/pull/11636#issuecomment-5229130830

   Thanks @srijan-singh for the links, and @SEZ9 for laying out exactly what's 
left to re-approve. I pulled the timing on all three runs plus the current head 
to check the timeout claim independently rather than just eyeballing 
"cancelled"/"success".
   
   **On the timeout evidence (@SEZ9's ask #1).**
   
   - Run 30775346505 (`all-connectors-it-3`, java 8, ubuntu): started 
`00:52:04Z`, cancelled `04:22:21Z` -> **210m17s**. Java 11 leg: started 
`00:52:10Z`, cancelled `04:22:25Z` -> **210m15s**. Both cancellations land 
within seconds of the old `timeout-minutes: 210` ceiling, so this is a genuine 
timeout kill, not an unrelated cancellation.
   - Run 30786984078 (the one that passed): java 8 leg ran `05:54:11Z` -> 
`09:23:15Z` = **209m4s**; java 11 leg ran `06:00:38Z` -> `09:28:37Z` = 
**207m59s**. Even the successful run finished with only 1-3 minutes of margin 
under the 210-minute cap. That's not a one-off fluke, it's a job that was 
routinely running right up against its own budget.
   
   So the headroom claim checks out: these aren't runs that failed for 
unrelated reasons, and the "passing" run barely passed.
   
   **One nuance worth flagging.** All three linked runs are from Aug 2-3, on 
commits (`c72937a7`, `f1977c55`, `eb5ced99`) that predate the 
`connector-couchbase-e2e` shard placement fix @srijan-singh described on Aug 7. 
I checked `all-connectors-it-3` on the actual current PR head (`8c3765f5`, fork 
run 31011149808, same run I cited in my APPROVED): java 8 leg ran `13:50:48Z` 
-> `16:00:43Z` = **~130m**; java 11 leg ran `13:51:37Z` -> `16:07:30Z` = 
**~136m**. That's roughly 70-80 minutes of headroom under the *old* 210-minute 
limit, well before touching the new 240-minute one.
   
   Practically: the shard reorder already fixed the acute problem, so the 
240-minute bump isn't masking an ongoing slowdown on today's head — it's a 
safety margin against the kind of variance the Aug 2-3 runs show (real-world IT 
duration for this job clearly does fluctuate close to 210m under some 
conditions), applied on top of a job that's now comfortably faster. That 
reconciles both your ask and Issue 5 from your Aug 6 review: legitimate 
headroom, not a cover for an unaddressed regression, and the numeric root cause 
(shard placement) is understood and documented in-thread rather than mysterious.
   
   **On @SEZ9's ask #2 (green Build on the current head).** This is already 
satisfied and has been since Aug 5 — no new commit needed. Fork run 31011149808 
on head `8c3765f5` is fully green: all four `unit-test` matrix legs (java 8/11 
x ubuntu/windows) and both `all-connectors-it-3` legs succeeded. The 
apache-side `Build` check-run pointer confirms the same (`conclusion: success`, 
completed `2026-08-05T20:15:48Z`), and `head` on the PR is still `8c3765f5` 
right now, so nothing has drifted since either of us last looked.
   
   Given both conditions are met with verifiable evidence rather than just 
re-stated claims, I don't see anything blocking here from my side. @SEZ9, over 
to you for the re-review you offered — happy to help track down anything else 
you want checked in the meantime.


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