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

   Thanks for taking a look, @SEZ9. Just flagging that my most recent review 
round on this exact head (`27e1482342d8`, unchanged since) found two 
High-severity blockers that are still open:
   
   1. `SourceSplitEnumeratorTask.java:342-350` now holds `synchronized 
(enumeratorContext)` across the entire duration of `enumerator.run()`. For 
enumerators whose `run()` is a long-running loop by design (e.g. 
`IcebergStreamSplitEnumerator.run()`, 
`IcebergStreamSplitEnumerator.java:79-104`), that monitor is never released 
during normal operation, so every subsequent `triggerBarrier()` call 
(`SourceSplitEnumeratorTask.java:159-167`) blocks forever on the same lock — 
checkpointing hangs deterministically starting from the first checkpoint after 
the job reaches RUNNING.
   2. The same lock also creates a lock-order inversion with connector-owned 
locks, e.g. Pulsar's periodic discovery thread: 
`PulsarSplitEnumerator.java:157-163` takes `stateLock` first and then calls 
back into `context.assignSplit(...)` -> `enumeratorContext`, while 
`triggerBarrier()` takes `enumeratorContext` first and then `stateLock` inside 
`snapshotState()` (`PulsarSplitEnumerator.java:309-314`). That's a permanent 
AB/BA deadlock between the discovery thread and the checkpoint-barrier thread.
   
   Both hit real, already-shipped connectors on a normal successful run, not 
just an edge case. Full detail and a suggested narrower-lock fix are in my 
review above. Since neither issue has a code change addressing it yet on this 
head, I'd hold off calling this ready to merge until at least those two are 
resolved — happy to take another pass as soon as a follow-up commit lands.


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