KAFKA-3076: BrokerChangeListener should log the brokers in order - #749
KAFKA-3076: BrokerChangeListener should log the brokers in order#749konradkalita wants to merge 7 commits into
Conversation
|
Thanks for the PR. Instead of sorting on every log statement, another approach is to sort it once and pass the sorted seq around. Have you considered this approach? |
There was a problem hiding this comment.
Sorting once seems cleaner I probably overcomplicated it, but it seems that we pass a Seq here only to convert it to a Set again, so maybe we just pass a Set?
There was a problem hiding this comment.
That is true, it's a bit unclear why we convert to a Seq and then immediately convert back to a Set. Another option is to introduce a method like brokerSetToString and have that method do the sorting.
|
Thanks for the patch. Yes, I agree with @ijuma . It seems it's simpler to just sort the broker ids once in BrokerChangeListener and pass them as Seq as they are now. |
# Conflicts: # core/src/main/scala/kafka/controller/ReplicaStateMachine.scala
There was a problem hiding this comment.
We should use curBrokers instead of controllerContext.liveBrokerIds here.
|
LGTM |
|
Thanks for the patch. LGTM |
With 6f81bb1 merged, we can now use c4.xlarge instances with our system tests, which are much more reliable for spot than m3.xlarge.
3 participants
No description provided.