KAFKA-9538: Flaky test: testResetOffsetsExportImportPlan#6561
Merged
hachikuji merged 2 commits intoapache:trunkfrom Feb 13, 2020
Merged
KAFKA-9538: Flaky test: testResetOffsetsExportImportPlan#6561hachikuji merged 2 commits intoapache:trunkfrom
hachikuji merged 2 commits intoapache:trunkfrom
Conversation
Contributor
Author
|
retest this please |
Member
|
@huxihx This PR shows conflict. Can you rebase the PR to resolve them? @hachikuji @cmccabe This test fails very often -- can we bump up the priority to review this PR? Does the proposed fix make sense? I am not familiar with this test. |
https://issues.apache.org/jira/browse/KAFKA-8211 Reduced offset-committing interval from 5 seconds to 1 seoncd, hoping consumer#committed returns offset more quickly. Besides enriched the output message for the exceptional case.
Contributor
Author
|
Occasionally, the group is not in inactive state when resetting offsets. That would return the empty offset to fail the assertion. We should ensure the group inactivity before running |
hachikuji
reviewed
Feb 11, 2020
| @@ -416,6 +416,9 @@ class ResetConsumerGroupOffsetTest extends ConsumerGroupCommandTest { | |||
| produceConsumeAndShutdown(topic = topic1, group = group1, totalMessages = 100, numConsumers = 2) | |||
| produceConsumeAndShutdown(topic = topic2, group = group2, totalMessages = 100, numConsumers = 5) | |||
Contributor
There was a problem hiding this comment.
This test case doesn't really need 5 consumers. Can we change both of these to use a single consumer?
hachikuji
changed the title
Contributor
|
ok to test |
3 tasks
hachikuji
added a commit
that referenced
this pull request
Feb 12, 2020
The recent increase in the flakiness of one of the offset reset tests (KAFKA-9538) traces back to #7941. After investigation, we found that following this patch, the consumer was sending an additional metadata request prior to performing the group assignment. This slight timing difference was enough to trigger the test failures. The problem turned out to be due to a bug in `SubscriptionState.groupSubscribe`, which no longer counted the local subscription when determining if there were new topics to fetch metadata for. Hence the extra metadata update. This patch restores the old logic. Without the fix, we saw 30-50% test failures locally. With it, I could no longer reproduce the failure. However, #6561 is probably still needed to improve the resilience of this test. Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
hachikuji
approved these changes
Feb 12, 2020
Contributor
hachikuji
left a comment
hachikuji
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the patch!
Contributor
|
ok to test |
hachikuji
added a commit
that referenced
this pull request
Feb 12, 2020
The recent increase in the flakiness of one of the offset reset tests (KAFKA-9538) traces back to #7941. After investigation, we found that following this patch, the consumer was sending an additional metadata request prior to performing the group assignment. This slight timing difference was enough to trigger the test failures. The problem turned out to be due to a bug in `SubscriptionState.groupSubscribe`, which no longer counted the local subscription when determining if there were new topics to fetch metadata for. Hence the extra metadata update. This patch restores the old logic. Without the fix, we saw 30-50% test failures locally. With it, I could no longer reproduce the failure. However, #6561 is probably still needed to improve the resilience of this test. Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
rite2nikhil
pushed a commit
to confluentinc/kafka
that referenced
this pull request
Feb 12, 2020
…#8095) The recent increase in the flakiness of one of the offset reset tests (KAFKA-9538) traces back to apache#7941. After investigation, we found that following this patch, the consumer was sending an additional metadata request prior to performing the group assignment. This slight timing difference was enough to trigger the test failures. The problem turned out to be due to a bug in `SubscriptionState.groupSubscribe`, which no longer counted the local subscription when determining if there were new topics to fetch metadata for. Hence the extra metadata update. This patch restores the old logic. Without the fix, we saw 30-50% test failures locally. With it, I could no longer reproduce the failure. However, apache#6561 is probably still needed to improve the resilience of this test. Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
hachikuji
added a commit
that referenced
this pull request
Feb 12, 2020
The recent increase in the flakiness of one of the offset reset tests (KAFKA-9538) traces back to #7941. After investigation, we found that following this patch, the consumer was sending an additional metadata request prior to performing the group assignment. This slight timing difference was enough to trigger the test failures. The problem turned out to be due to a bug in `SubscriptionState.groupSubscribe`, which no longer counted the local subscription when determining if there were new topics to fetch metadata for. Hence the extra metadata update. This patch restores the old logic. Without the fix, we saw 30-50% test failures locally. With it, I could no longer reproduce the failure. However, #6561 is probably still needed to improve the resilience of this test. Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
hachikuji
added a commit
that referenced
this pull request
Feb 12, 2020
The recent increase in the flakiness of one of the offset reset tests (KAFKA-9538) traces back to #7941. After investigation, we found that following this patch, the consumer was sending an additional metadata request prior to performing the group assignment. This slight timing difference was enough to trigger the test failures. The problem turned out to be due to a bug in `SubscriptionState.groupSubscribe`, which no longer counted the local subscription when determining if there were new topics to fetch metadata for. Hence the extra metadata update. This patch restores the old logic. Without the fix, we saw 30-50% test failures locally. With it, I could no longer reproduce the failure. However, #6561 is probably still needed to improve the resilience of this test. Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
Contributor
|
retest this please |
Contributor
|
Having a hard time getting a build here. I verified the fix works locally, so I will go ahead and merge. |
stanislavkozlovski
pushed a commit
to stanislavkozlovski/kafka
that referenced
this pull request
Feb 18, 2020
…#8095) The recent increase in the flakiness of one of the offset reset tests (KAFKA-9538) traces back to apache#7941. After investigation, we found that following this patch, the consumer was sending an additional metadata request prior to performing the group assignment. This slight timing difference was enough to trigger the test failures. The problem turned out to be due to a bug in `SubscriptionState.groupSubscribe`, which no longer counted the local subscription when determining if there were new topics to fetch metadata for. Hence the extra metadata update. This patch restores the old logic. Without the fix, we saw 30-50% test failures locally. With it, I could no longer reproduce the failure. However, apache#6561 is probably still needed to improve the resilience of this test. Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
qq619618919
pushed a commit
to qq619618919/kafka
that referenced
this pull request
May 12, 2020
…#8095) The recent increase in the flakiness of one of the offset reset tests (KAFKA-9538) traces back to apache#7941. After investigation, we found that following this patch, the consumer was sending an additional metadata request prior to performing the group assignment. This slight timing difference was enough to trigger the test failures. The problem turned out to be due to a bug in `SubscriptionState.groupSubscribe`, which no longer counted the local subscription when determining if there were new topics to fetch metadata for. Hence the extra metadata update. This patch restores the old logic. Without the fix, we saw 30-50% test failures locally. With it, I could no longer reproduce the failure. However, apache#6561 is probably still needed to improve the resilience of this test. Reviewers: Rajini Sivaram <rajinisivaram@googlemail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://issues.apache.org/jira/browse/KAFKA-8211
Reduced offset-committing interval from 5 seconds to 1 second, hoping consumer#committed returns offset more quickly. Besides, enriched the output message for the exceptional case.
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)