aruha-876: kafka client 10.1 #684
Conversation
Codecov Report
@@ Coverage Diff @@
## master #684 +/- ##
============================================
+ Coverage 54.4% 54.43% +0.02%
- Complexity 1473 1474 +1
============================================
Files 287 287
Lines 8045 8045
Branches 706 706
============================================
+ Hits 4377 4379 +2
+ Misses 3415 3414 -1
+ Partials 253 252 -1
Continue to review full report at Codecov.
|
| @@ -126,7 +186,8 @@ private void createTopic(final String topic, final int partitionsNum, final int | |||
| final Properties topicConfig = new Properties(); | |||
| topicConfig.setProperty("retention.ms", Long.toString(retentionMs)); | |||
| topicConfig.setProperty("segment.ms", Long.toString(rotationMs)); | |||
| AdminUtils.createTopic(zkUtils, topic, partitionsNum, replicaFactor, topicConfig); | |||
| AdminUtils.createTopic(zkUtils, topic, partitionsNum, replicaFactor, topicConfig, | |||
| RackAwareMode.Enforced$.MODULE$); | |||
There was a problem hiding this comment.
I'm reading this part of the client code https://github.com/apache/kafka/blob/cae5977ed0d6b63f992973800273769c970b0a0a/core/src/main/scala/kafka/admin/RackAwareMode.scala#L22-L39 and trying to understand the change in behaviour with RackAwareMode.Enforced.
Do you understand the difference between Safe, Disabled and Enforced? It's not clear for me.
There was a problem hiding this comment.
There is about how you configure racks in kafka config, more here
https://cwiki.apache.org/confluence/display/KAFKA/KIP-36+Rack+aware+replica+assignment
There was a problem hiding this comment.
In context of aws it will be az
There was a problem hiding this comment.
Yes, I know the feature, but do we use rack awareness? Why choosing Enforced here?
There was a problem hiding this comment.
Ok, so what I got is that once we add rack awareness, by specifying the broker.rack property in server.properties this is going to be immediately used by Nakadi when creating new topics. So sounds like a good thing. It's just not clear if it has any difference right now, since we do not have broker.rack configured.
|
👍 |
|
👎 |
|
I had to put a veto, because we need first to check how the new client version influences https://techjira.zalando.net/browse/ARUHA-854 (hash partitioning problem) |
|
Ricardo reported that ETs created with a new client version have a different order of partitions that the ones created with old client version. I will check this more deeply but for now I think that the fix for https://techjira.zalando.net/browse/ARUHA-854 should be released before this PR is merged. |
|
@rcillo @v-stepanov since the aruha-854 is merged can I get a green light ? |
|
👍 |
|
@rcillo @v-stepanov @lmontrieux @antban could you approve it? just merged from upstream |
|
👍 |
1 similar comment
|
👍 |
aruha-876