fqaiser94 commented on code in PR #10792:
URL: https://github.com/apache/iceberg/pull/10792#discussion_r1695632061


##########
kafka-connect/kafka-connect/src/main/java/org/apache/iceberg/connect/channel/CoordinatorThread.java:
##########
@@ -65,5 +66,15 @@ boolean isTerminated() {
 
   void terminate() {
     terminated = true;
+
+    try {
+      join(60_000);

Review Comment:
   TLDR; I have no objections to this PR, this is still an incremental 
improvement.
   
   ---
   
   Just pointing out that this doesn't by any means guarantee that 2 
coordinators cannot be running for the same connector at the same time. 
   IIRC, Kafka Connect will wait 5 seconds (by default, configurable via 
`task.shutdown.graceful.timeout.ms`) for a Task to stop. 
   If a Task does not stop within that time, Kafka Connect will happily abandon 
that Task and move on to creating a new Task to replace it. 
   So waiting 60 seconds here is fairly irrelevant; we will really only wait 
`task.shutdown.graceful.timeout.ms` which as previously mentioned is only 5 
seconds by default. 
   
   I've said it before; there is no guaranteed way to prevent 2 Coordinators 
from running at the same time (unless you want to build a flink-like runtime on 
top of kafka-connect or bring in external locking solutions, neither of which I 
recommend). 
   In my (unfortunate) experience, solutions like this are only ever band-aids 
that fail at sufficient scale. 
   A better solution would be to fence zombie data file appends to iceberg 
tables (currently, we only fence zombie writes to the Kafka control topic 
currently). 
   The best way to do that would be to add support conditional-commit-style 
semantics to iceberg, something like this: 
https://github.com/apache/iceberg/pull/6513
   
   In the meantime however, this is the best we can do 👍 



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to