Skip to content

KAFKA-10218: Stop reading config topic in every subsequent tick if catchup fails once - #8973

Merged
rhauch merged 1 commit into
apache:trunkfrom
C0urante:kafka-10218
Sep 28, 2020
Merged

KAFKA-10218: Stop reading config topic in every subsequent tick if catchup fails once#8973
rhauch merged 1 commit into
apache:trunkfrom
C0urante:kafka-10218

Conversation

@C0urante

@C0urante C0urante commented Jul 1, 2020

Copy link
Copy Markdown
Contributor

Jira

We added a canReadConfigs field to the DistributedHerder a while back that gets set to false if the herder fails to catch up to the end of the config topic, but never gets set to true again if catchup succeeds at a later point.

This PR adds logic to reset it to true when it's safe to.

A single unit test is tweaked to ensure that the herder doesn't keep unnecessarily trying to catch up to the end of the config topic during every tick.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@kkonstantine

Copy link
Copy Markdown
Contributor

ok to test

@C0urante

C0urante commented Jul 9, 2020

Copy link
Copy Markdown
Contributor Author

@gharris1727 @chia7712 @ncliang would any of you be interested in reviewing this?

@ncliang ncliang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for finding and fixing @C0urante . LGTM.

@rhauch

rhauch commented Jul 13, 2020

Copy link
Copy Markdown
Contributor

retest this please

@rhauch rhauch left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Thanks, @C0urante

@rhauch
rhauch merged commit 17fa3d9 into apache:trunk Sep 28, 2020
rhauch pushed a commit that referenced this pull request Sep 28, 2020
…tchup fails once (#8973)

Add logic to reset the existing `canReadConfigs` in `DistributedHerder` once the herder is able to successfully read the configs again. Added unit test to verify the functionality.

Author: Chris Egerton <chrise@confluent.io>
Reviewer: Nigel Liang <nigel@nigelliang.com>, Randall Hauch <rhauch@gmail.com>
rhauch pushed a commit that referenced this pull request Sep 28, 2020
…tchup fails once (#8973)

Add logic to reset the existing `canReadConfigs` in `DistributedHerder` once the herder is able to successfully read the configs again. Added unit test to verify the functionality.

Author: Chris Egerton <chrise@confluent.io>
Reviewer: Nigel Liang <nigel@nigelliang.com>, Randall Hauch <rhauch@gmail.com>
rhauch pushed a commit that referenced this pull request Sep 29, 2020
…tchup fails once (#8973)

Add logic to reset the existing `canReadConfigs` in `DistributedHerder` once the herder is able to successfully read the configs again. Added unit test to verify the functionality.

Author: Chris Egerton <chrise@confluent.io>
Reviewer: Nigel Liang <nigel@nigelliang.com>, Randall Hauch <rhauch@gmail.com>
@C0urante
C0urante deleted the kafka-10218 branch September 29, 2020 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants