Skip to content

KAFKA-7937: Fix Flaky Test ResetConsumerGroupOffsetTest.testResetOffs…#6311

Merged
vahidhashemian merged 1 commit intoapache:trunkfrom
gwenshap:KAFKA-7937-trunk
Feb 23, 2019
Merged

KAFKA-7937: Fix Flaky Test ResetConsumerGroupOffsetTest.testResetOffs…#6311
vahidhashemian merged 1 commit intoapache:trunkfrom
gwenshap:KAFKA-7937-trunk

Conversation

@gwenshap
Copy link
Copy Markdown
Contributor

Address the comments on PR-6307. Sorry for new PR, but one of the comments was to move the PR to another branch.


Since the test fails sometimes on lack of coordinator, I'm giving it a bit more attempts to find it.

I admit that I haven't been able to actually reproduce this failure, so I'm only hoping this fixes it. But it doesn't fail more often than it used to (on my machine)

Fixing on 2.2 because the intent is to fix enough flakes to allow for a clean release.

@gwenshap
Copy link
Copy Markdown
Contributor Author

CC @vahidhashemian @mjsax

Copy link
Copy Markdown
Contributor

@vahidhashemian vahidhashemian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I'll wait for the build to pass before merging. Thanks!

@gwenshap
Copy link
Copy Markdown
Contributor Author

Thanks! Don't forget to cherrypick to 2.2 while you are at it :)

@vahidhashemian
Copy link
Copy Markdown
Contributor

Sure. I haven't yet done cherry-picking when merging. That would be my learning exercise :)

@vahidhashemian
Copy link
Copy Markdown
Contributor

The failed unit test seems to be unrelated.

@vahidhashemian vahidhashemian merged commit 0150dbc into apache:trunk Feb 23, 2019
vahidhashemian pushed a commit that referenced this pull request Feb 23, 2019
@vahidhashemian
Copy link
Copy Markdown
Contributor

Cherry-picked to 2.2 branch.

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Feb 24, 2019

Isn't this a bug in the command that it doesn't retry? Seems like this change is hiding a real bug. Also, we should aim to include information in the commit messages themselves, seems like they're empty aside from the title.

@vahidhashemian
Copy link
Copy Markdown
Contributor

Agreed. The retry logic existed in the old AdminClient, but it's not there yet in the new one. There is a JIRA open for it though.

Also good point on commit title and message. If they should be there for each commit (which makes sense), I am wondering if there is a good way to enforce them as part of the test or when merging?

@ijuma
Copy link
Copy Markdown
Member

ijuma commented Feb 24, 2019

Can we add a comment to that open PR stating that we should revert this change there?

For the commit message, it's the responsibility of the committer to ensure the commit message is good. The same way you have to review the code. Personally, I often edit it myself before merging instead of going back to the PR author if the change is simple. For complex changes, I ask the author to update the PR description and I then copy and paste.

@vahidhashemian
Copy link
Copy Markdown
Contributor

vahidhashemian commented Feb 24, 2019

Created KAFKA-7993 to better track it.
Understood your point about the commit message. Will try to be more diligent about their quality.

jarekr pushed a commit to confluentinc/kafka that referenced this pull request Apr 18, 2019
* AK/trunk: (36 commits)
  KAFKA-7962: Avoid NPE for StickyAssignor (apache#6308)
  Address flakiness of CustomQuotaCallbackTest#testCustomQuotaCallback (apache#6330)
  KAFKA-7918: Inline generic parameters Pt. II: RocksDB Bytes Store and Memory LRU Caches (apache#6327)
  MINOR: fix parameter naming (apache#6316)
  KAFKA-7956 In ShutdownableThread, immediately complete the shutdown if the thread has not been started (apache#6218)
  MINOR: Refactor replica log dir fetching for improved logging (apache#6313)
  [TRIVIAL] Remove unused StreamsGraphNode#repartitionRequired (apache#6227)
  MINOR: Increase produce timeout to 120 seconds (apache#6326)
  KAFKA-7918: Inline generic parameters Pt. I: in-memory key-value store (apache#6293)
  MINOR: Fix line break issue in upgrade notes (apache#6320)
  KAFKA-7972: Use automatic RPC generation in SaslHandshake
  MINOR: Enable capture of full stack trace in StreamTask#process (apache#6310)
  KAFKA-7938: Fix test flakiness in DeleteConsumerGroupsTest (apache#6312)
  KAFKA-7937: Fix Flaky Test ResetConsumerGroupOffsetTest.testResetOffsetsNotExistingGroup (apache#6311)
  MINOR: Update docs to say 2.2 (apache#6315)
  KAFKA-7672 : force write checkpoint during StreamTask #suspend (apache#6115)
  KAFKA-7961; Ignore assignment for un-subscribed partitions (apache#6304)
  KAFKA-7672: Restoring tasks need to be closed upon task suspension (apache#6113)
  KAFKA-7864; validate partitions are 0-based (apache#6246)
  KAFKA-7492 : Updated javadocs for aggregate and reduce methods returning null behavior. (apache#6285)
  ...
pengxiaolong pushed a commit to pengxiaolong/kafka that referenced this pull request Jun 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants