Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(consumer): follow preferred broker #1936

Merged
merged 3 commits into from
May 7, 2021
Merged

fix(consumer): follow preferred broker #1936

merged 3 commits into from
May 7, 2021

Conversation

dnwe
Copy link
Collaborator

@dnwe dnwe commented May 5, 2021

Historically (before protocol version 11) if we attempted to consume
from a follower, we would get a NotLeaderForPartition response and move
our consumer to the new leader. However, since v11 the Kafka broker
treats us just like any other follower and permits us to consume from
any replica and it is up to us to monitor metadata to determine when the
leadership has changed.

Modifying the handleResponse func to check the topic partition
leadership against the current broker (in the absence of a
preferredReadReplica) and trigger a re-create of the consumer for that
partition

Contributes-to: #1927

dnwe added 2 commits May 6, 2021 00:44
Historically (before protocol version 11) if we attempted to consume
from a follower, we would get a NotLeaderForPartition response and move
our consumer to the new leader. However, since v11 the Kafka broker
treats us just like any other follower and permits us to consume from
any replica and it is up to us to monitor metadata to determine when the
leadership has changed.

Modifying the handleResponse func to check the topic partition
leadership against the current broker (in the absence of a
preferredReadReplica) and trigger a re-create of the consumer for that
partition

Contributes-to: #1927
gofumpt is a stricter but compatible gofmt:
https://github.com/mvdan/gofumpt

Run it across the repo just to tidy things up a little more.
@dnwe dnwe requested a review from bai as a code owner May 5, 2021 23:51
@dnwe dnwe marked this pull request as draft May 5, 2021 23:51
@dnwe dnwe marked this pull request as ready for review May 7, 2021 12:19
TestConsumeMessagesTrackLeader ensures that in the event that leadership
of a topicPartition changes and no preferredReadReplica is specified,
the consumer connects back to the new leader to resume consumption and
doesn't continue consuming from the follower.

See #1927
@dnwe dnwe merged commit 83d633e into master May 7, 2021
@dnwe dnwe deleted the dnwe/follow-the-leader branch May 7, 2021 12:35
dnwe added a commit that referenced this pull request Dec 1, 2021
FetchResponse from a follower will _not_ contain a PreferredReadReplica.
It seems like the partitionConsumer would overwrite the previously
assigned value from the leader with -1 which would then trigger the
"reconnect to the current leader" changes from #1936 causing a flip-flop
effect.

Contributes-to: #2071

Signed-off-by: Dominic Evans <dominic.evans@uk.ibm.com>
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.

None yet

3 participants