[GitHub] [kafka] rhauch commented on a change in pull request #11933: KAFKA-13759: Disable idempotency by default in producers instantiated by Connect

[GitBox <gi...@apache.org>]

rhauch commented on a change in pull request #11933:
URL: https://github.com/apache/kafka/pull/11933#discussion_r832738114



##########
File path: connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java
##########
@@ -648,6 +648,14 @@ private WorkerTask buildWorkerTask(ClusterConfigState configState,
         // These settings will execute infinite retries on retriable exceptions. They *may* be overridden via configs passed to the worker,
         // but this may compromise the delivery guarantees of Kafka Connect.
         producerProps.put(ProducerConfig.MAX_BLOCK_MS_CONFIG, Long.toString(Long.MAX_VALUE));
+        // By default, producers that are instantiated and used by Connect have idempotency disabled even after idempotency became
+        // default for Kafka producers. This is chosen to avoid breaking changes when Connect contacts Kafka brokers that do not support

Review comment:
       Maybe rephrase to be more positive? Like:
   ```suggestion
           // default for Kafka producers. This ensures Connect continues to work with many Kafka broker versions, including older brokers that do not support
   ```
   If you make a change like this, please change the other places to be consistent.

##########
File path: connect/runtime/src/main/java/org/apache/kafka/connect/runtime/Worker.java
##########
@@ -648,6 +648,14 @@ private WorkerTask buildWorkerTask(ClusterConfigState configState,
         // These settings will execute infinite retries on retriable exceptions. They *may* be overridden via configs passed to the worker,
         // but this may compromise the delivery guarantees of Kafka Connect.
         producerProps.put(ProducerConfig.MAX_BLOCK_MS_CONFIG, Long.toString(Long.MAX_VALUE));
+        // By default, producers that are instantiated and used by Connect have idempotency disabled even after idempotency became
+        // default for Kafka producers. This is chosen to avoid breaking changes when Connect contacts Kafka brokers that do not support
+        // idempotent producers or require explicit steps to enable them (e.g. adding the IDEMPOTENT_WRITE ACL to brokers older than 2.8).
+        // These settings might change when https://cwiki.apache.org/confluence/display/KAFKA/KIP-318%3A+Make+Kafka+Connect+Source+idempotent
+        // gets approved and scheduled for release.
+        producerProps.put(ProducerConfig.ENABLE_IDEMPOTENCE_CONFIG, "false");
+        producerProps.put(ProducerConfig.ACKS_CONFIG, "all");
+        producerProps.put(ProducerConfig.MAX_IN_FLIGHT_REQUESTS_PER_CONNECTION, "1");

Review comment:
       Didn't #11932 add these two lines back in? If so, the only change should be the addition of:
   ```
   producerProps.put(ProducerConfig.ENABLE_IDEMPOTENCE_CONFIG, "false");
   ```
   Maybe this PR just needs to merge the latest from `trunk`.




-- 
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: jira-unsubscribe@kafka.apache.org

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