Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KAFKA-3226: Replicas collections should use List instead of Set in or… #900

Closed
wants to merge 1 commit into from

Conversation

granthenke
Copy link
Member

…der to maintain order

@granthenke
Copy link
Member Author

@ijuma This is the jira/pr related to #896

@gwenshap
Copy link
Contributor

I thought PartitionState contains a leader value specifically so we won't need to make assumptions on order of replicas?

@granthenke
Copy link
Member Author

@gwenshap Good point...let me look what going on there and report back

@ijuma
Copy link
Contributor

ijuma commented Feb 10, 2016

@gwenshap I thought that too, but @granthenke said there were test failures in #896 when the server started using the Java classes. It could be that the tests are wrong or that it was working accidentally because small immutable sets (up to 4 elements) in Scala maintain ordering.

@granthenke
Copy link
Member Author

@gwenshap @ijuma It looks like the actual required behavior is correct. The test failure was due to replicas that did not match the same order. The change in #896 can definitely impact the order from what it was, but its being handled correctly by Kafka. I will close this and update the test in my original PR and leave replicas as a Set.

@granthenke granthenke closed this Feb 10, 2016
@ijuma
Copy link
Contributor

ijuma commented Feb 10, 2016

Awesome, thanks for checking this Grant.

@granthenke granthenke deleted the replicas-list branch February 17, 2016 06:05
efeg added a commit to efeg/kafka that referenced this pull request Jan 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants