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 2020/06/03 02:22:37 UTC
[GitHub] [kafka] showuon opened a new pull request #8788: MINOR: Remove unused isSticky assert out from tests only do constrainedAssign
showuon opened a new pull request #8788:
URL: https://github.com/apache/kafka/pull/8788
Suggested by @ableegoldman in https://github.com/apache/kafka/pull/8778#discussion_r434008302, remove the unused isSticky assert out from tests only do `constrainedAssign`. I printed out the test name and which assign method used during tests. Below is the output. So, we can remove the `isSticky()` assertion from the tests using `constrainedAssign` method.
In summary, total 9 tests were doing `assertTrue(assignor.isSticky())` but using `constrainedAssign`. Thanks.
> StickyAssignorTest - testAssignmentWithConflictingPreviousGenerations
> 2020-06-03T09:44:03.362+0800 [DEBUG] [TestEventLogger] constrainedAssign
> StickyAssignorTest - testSchemaBackwardCompatibility
> 2020-06-03T09:44:03.376+0800 [DEBUG] [TestEventLogger] constrainedAssign
> StickyAssignorTest - testAssignmentWithMultipleGenerations1
> 2020-06-03T09:44:03.378+0800 [DEBUG] [TestEventLogger] constrainedAssign
> StickyAssignorTest - testAssignmentWithMultipleGenerations2
> 2020-06-03T09:44:03.380+0800 [DEBUG] [TestEventLogger] constrainedAssign
> AbstractStickyAssignorTest - testStickiness
> 2020-06-03T09:44:03.383+0800 [DEBUG] [TestEventLogger] constrainedAssign
> AbstractStickyAssignorTest - testAddRemoveConsumerOneTopic
> 2020-06-03T09:44:03.386+0800 [DEBUG] [TestEventLogger] constrainedAssign
> AbstractStickyAssignorTest - testReassignmentWithRandomSubscriptionsAndChanges
> 2020-06-03T09:44:03.393+0800 [DEBUG] [TestEventLogger] generalAssign
> AbstractStickyAssignorTest - testNewSubscription
> 2020-06-03T09:44:06.238+0800 [DEBUG] [TestEventLogger] generalAssign
> AbstractStickyAssignorTest - testAddRemoveTopicTwoConsumers
> 2020-06-03T09:44:06.239+0800 [DEBUG] [TestEventLogger] constrainedAssign
> AbstractStickyAssignorTest - testReassignmentAfterOneConsumerLeaves
> 2020-06-03T09:44:06.243+0800 [DEBUG] [TestEventLogger] generalAssign
> AbstractStickyAssignorTest - testLargeAssignmentWithMultipleConsumersLeavingAndRandomSubscription
> 2020-06-03T09:44:06.268+0800 [DEBUG] [TestEventLogger] generalAssign
> AbstractStickyAssignorTest - testSameSubscriptions
> 2020-06-03T09:44:08.400+0800 [DEBUG] [TestEventLogger] constrainedAssign
> AbstractStickyAssignorTest - testReassignmentAfterOneConsumerAdded
> 2020-06-03T09:44:08.403+0800 [DEBUG] [TestEventLogger] constrainedAssign
### 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 #8788: MINOR: Remove unused isSticky assert out from tests only do constrainedAssign
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #8788:
URL: https://github.com/apache/kafka/pull/8788#issuecomment-640337271
Thanks for the comment, @ableegoldman . Yes, I agree we should improve the stickiness verification, but I haven't got a better idea for that so far. I've created a ticket to track it:
[KAFKA-10118](https://issues.apache.org/jira/browse/KAFKA-10118) - Improve stickiness verification for AbstractStickyAssignorTest
For this PR, I also put the `partitionMovements` initialization back to `generalAssign` method sine we won't have NPE during testing anymore after this fix. I think we can firstly merge this PR, and then discuss the improvement in KAFKA-10118.
How do you think?
----------------------------------------------------------------
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 #8788: MINOR: Remove unused isSticky assert out from tests only do constrainedAssign
Posted by GitBox <gi...@apache.org>.
showuon commented on pull request #8788:
URL: https://github.com/apache/kafka/pull/8788#issuecomment-637913934
hi @ableegoldman , could you review this PR? Thanks.
----------------------------------------------------------------
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] guozhangwang merged pull request #8788: MINOR: Remove unused isSticky assert out from tests only do constrainedAssign
Posted by GitBox <gi...@apache.org>.
guozhangwang merged pull request #8788:
URL: https://github.com/apache/kafka/pull/8788
----------------------------------------------------------------
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] mjsax commented on pull request #8788: MINOR: Remove unused isSticky assert out from tests only do constrainedAssign
Posted by GitBox <gi...@apache.org>.
mjsax commented on pull request #8788:
URL: https://github.com/apache/kafka/pull/8788#issuecomment-640741662
Retest this please.
----------------------------------------------------------------
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