zhangyue19921010 commented on a change in pull request #10551:
URL: https://github.com/apache/druid/pull/10551#discussion_r518780893



##########
File path: 
extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaConsumerConfigs.java
##########
@@ -38,7 +38,6 @@
     props.put("group.id", StringUtils.format("kafka-supervisor-%s", 
IdUtils.getRandomId()));
     props.put("auto.offset.reset", "none");
     props.put("enable.auto.commit", "false");
-    props.put("isolation.level", "read_committed");

Review comment:
       Yes, change the current behavior is what I want. 
   
   You are right, most people really don't care this Kafka configuration(Let it 
be default behavior). As I know the default logic of the Kafka Producer is not 
to enable transactions feature and the same as Kafka Consumer. So that maybe 
Druid Kafka indexing service keep the same default logic is more reasonable.
   
   If a Druid user want Druid to consume transactional Kafka topics, I think it 
is more reasonable to let users set this parameter in consumerProperties like 
'bootstrap.servers' because he knows what he is going to do and why.
   
   By the way, Druid from 0.15 to better can't consume old version Kafka is 
really confused me(I believe it's not just me). Generally speaking, higher 
Kafka consumer client is able to consume old version Kafka cluster unless it 
involves high-version-specific api.
   
   Thanks for reviewing!




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

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to