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


##########
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:
   Since Coordinator is doing the commit and based on the table properties on 
commit retries it can take longer for ex: 
https://iceberg.apache.org/docs/nightly/configuration/#table-behavior-properties
 
   so it might make sense to make it configurable to me but in past we have 
seen we have gone ahead with static timeouts like 10 min's check this commit 
https://github.com/apache/iceberg/commit/25eaebacbd1e250d2d884b49fc753a23b6aa6eaf#diff-6ee62ee6bfe801211a8725679859de808af3689a36bb5784342ab4977c7828d0R201
 
   
   but 1 minutes seems to be too low in this case tbh, can we bump this to like 
a large number instead if we don't want to make it configurable, do you whats 
the max timeout of these `thread wait timeouts elsewhere like the REST catalog` 
 ? 



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