You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/06/18 21:04:38 UTC
[GitHub] [kafka] showuon opened a new pull request #10903: [WIP] KAFKA-12473: make "cooperative-sticky, range" as the default assignor
showuon opened a new pull request #10903:
URL: https://github.com/apache/kafka/pull/10903
*More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.*
*Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.*
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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:
users@infra.apache.org
[GitHub] [kafka] showuon commented on pull request #10903: KAFKA-13023: make "range, cooperative-sticky" as the default assignor in V3.0
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #10903:
URL: https://github.com/apache/kafka/pull/10903#issuecomment-875214820
@ableegoldman , I've added a test in `ConsumerConfigTest` to verify the default partition assignor. We cannot verify the assignor via `KafkaConsumer` or `ConsumerCoordinator` unless we add/update methods for testing. I think that's unnecessary. Let me know if you have other opinions. Thank you.
--
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
[GitHub] [kafka] showuon commented on pull request #10903: KAFKA-12473: make "range, cooperative-sticky" as the default assignor
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #10903:
URL: https://github.com/apache/kafka/pull/10903#issuecomment-871362981
@ableegoldman , please help review this PR.
Failed tests are unrelated.
```
Build / JDK 16 and Scala 2.13 / kafka.server.RaftClusterTest.testCreateClusterAndCreateAndManyTopicsWithManyPartitions()
Build / JDK 16 and Scala 2.13 / kafka.server.RaftClusterTest.testCreateClusterAndCreateAndManyTopics()
Build / JDK 16 and Scala 2.13 / kafka.server.RaftClusterTest.testCreateClusterAndCreateAndManyTopics()
Build / JDK 11 and Scala 2.13 / kafka.server.RaftClusterTest.testCreateClusterAndCreateAndManyTopicsWithManyPartitions()
Build / JDK 8 and Scala 2.12 / kafka.server.RaftClusterTest.testCreateClusterAndCreateListDeleteTopic()
```
--
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
[GitHub] [kafka] ableegoldman commented on pull request #10903: KAFKA-13023: make "range, cooperative-sticky" as the default assignor in V3.0
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #10903:
URL: https://github.com/apache/kafka/pull/10903#issuecomment-875272278
Merged to trunk. Thanks @showuon !
--
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
[GitHub] [kafka] dajac commented on pull request #10903: KAFKA-13023: make "range, cooperative-sticky" as the default assignor in V3.0
Posted by GitBox <gi...@apache.org>.
dajac commented on pull request #10903:
URL: https://github.com/apache/kafka/pull/10903#issuecomment-875322381
@ableegoldman Do we need to cherry-pick this one to the 3.0 branch as well? cc @kkonstantine
--
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
[GitHub] [kafka] kkonstantine commented on pull request #10903: KAFKA-13023: make "range, cooperative-sticky" as the default assignor in V3.0
Posted by GitBox <gi...@apache.org>.
kkonstantine commented on pull request #10903:
URL: https://github.com/apache/kafka/pull/10903#issuecomment-876138638
LGTM. That's what was discussed on the mailing list.
--
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
[GitHub] [kafka] showuon commented on pull request #10903: KAFKA-13023: make "range, cooperative-sticky" as the default assignor in V3.0
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #10903:
URL: https://github.com/apache/kafka/pull/10903#issuecomment-875264149
@ableegoldman , failed tests are unrelated. Thank you.
```
Build / JDK 11 and Scala 2.13 / kafka.server.RaftClusterTest.testCreateClusterAndCreateAndManyTopicsWithManyPartitions()
Build / JDK 11 and Scala 2.13 / kafka.server.RaftClusterTest.testCreateClusterAndCreateAndManyTopicsWithManyPartitions()
```
--
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
[GitHub] [kafka] ableegoldman merged pull request #10903: KAFKA-13023: make "range, cooperative-sticky" as the default assignor in V3.0
Posted by GitBox <gi...@apache.org>.
ableegoldman merged pull request #10903:
URL: https://github.com/apache/kafka/pull/10903
--
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
[GitHub] [kafka] ableegoldman commented on pull request #10903: KAFKA-13023: make "range, cooperative-sticky" as the default assignor in V3.0
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #10903:
URL: https://github.com/apache/kafka/pull/10903#issuecomment-875181165
> So, in this PR, I've added 1 test to test it: testMultiConsumerDefaultAssignorAndVerifyAssignment
Thanks Luke, but I actually meant something simpler (and stupider) that directly verifies that the chosen assignor is the RangeAssignor, rather than inferring it from the assignment that's produced. Like something in ConsumerConfigTest, or KafkaConsumerTest. Does that make sense? I just think we want to have both a test: a more complicated test that validates what we actually care about, the resulting assignment, but also a plain dumb test that we can be 100% sure will break if/when the default assignor is changed, no matter what it gets changed to (ie even if the new assignor happens to produce the same assignment as the RangeAssignor in some cases)
--
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
[GitHub] [kafka] showuon commented on a change in pull request #10903: KAFKA-12473: make "range, cooperative-sticky" as the default assignor
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #10903:
URL: https://github.com/apache/kafka/pull/10903#discussion_r661925589
##########
File path: core/src/test/scala/integration/kafka/api/AbstractConsumerTest.scala
##########
@@ -412,13 +413,15 @@ abstract class AbstractConsumerTest extends BaseRequestTest {
* 1. Every consumer got assigned at least one partition
* 2. Each partition is assigned to only one consumer
* 3. Every partition is assigned to one of the consumers
+ * 4. The assignment is the same as expected assignment (if provided)
Review comment:
Before the change, we don't verify the assignment results are the expected distribution. It's fine since we already tested that in each assignor tests. But here, I added a default assignor test, that I wanted to make sure it is using our expected assignor (i.e. Range assignor) to distribute the partitions.
--
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
[GitHub] [kafka] ableegoldman commented on pull request #10903: KAFKA-13023: make "range, cooperative-sticky" as the default assignor in V3.0
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #10903:
URL: https://github.com/apache/kafka/pull/10903#issuecomment-876002447
Cherrypicked to the 3.0 branch
--
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
[GitHub] [kafka] showuon commented on pull request #10903: KAFKA-13023: make "range, cooperative-sticky" as the default assignor in V3.0
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #10903:
URL: https://github.com/apache/kafka/pull/10903#issuecomment-875157335
@ableegoldman , as discussed, we agreed this should be put in v3.0. Please take a look when available. Thank you.
--
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
[GitHub] [kafka] ableegoldman commented on pull request #10903: KAFKA-13023: make "range, cooperative-sticky" as the default assignor in V3.0
Posted by GitBox <gi...@apache.org>.
ableegoldman commented on pull request #10903:
URL: https://github.com/apache/kafka/pull/10903#issuecomment-875339479
Ah, yes, this should definitely be cherrypicked to 3.0. I'll do it tomorrow if that's all good with @kkonstantine
Thanks for catching this
--
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
[GitHub] [kafka] showuon commented on pull request #10903: KAFKA-13023: make "range, cooperative-sticky" as the default assignor in V3.0
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #10903:
URL: https://github.com/apache/kafka/pull/10903#issuecomment-875185428
Make sense! Will add later. Thanks for the suggestion :)
--
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
[GitHub] [kafka] showuon commented on pull request #10903: KAFKA-13023: make "range, cooperative-sticky" as the default assignor in V3.0
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #10903:
URL: https://github.com/apache/kafka/pull/10903#issuecomment-875165545
@ableegoldman , no, there's no tests to verify that the RangeAssignor will be chosen by default when creating a Consumer. So, in this PR, I've added 1 test to test it: `testMultiConsumerDefaultAssignorAndVerifyAssignment`. Thank you!
--
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