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

rpk: patch group seeking behavior #3404

Merged
merged 2 commits into from
Jan 6, 2022
Merged

rpk: patch group seeking behavior #3404

merged 2 commits into from
Jan 6, 2022

Conversation

twmb
Copy link
Contributor

@twmb twmb commented Jan 6, 2022

Observed in the wild: seeking sometimes errored even though a commit went through because the OffsetCommit response was not as expected, and an error while seeking panicked.

For the first issue, we relax the strictness on what we expect an OffsetCommit response to look like. For the latter, we just fix the panic.

Release notes

  • Fixes a panic in rpk group seek if there was an error during the offset commit

I thought that an embedded private struct with public fields would mean
the fields are public, but it does not, which meant that if seeking had
per-partition errors, PrintStructFields would panic.

We now use dedicated public fields.
This bumps kadm to a version that uses per-partition errors if
OffsetCommitResponse is missing some topics/partitions in the response
that were in the request.

Notably, this was seen when seeking against redpanda: redpanda does not
today either match the request ordering, or does not respond to
everything all of the time. This is tracked in #3403.
Copy link
Contributor

@0x5d 0x5d left a comment

Choose a reason for hiding this comment

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

LGTM!

@0x5d 0x5d merged commit 5aba969 into redpanda-data:dev Jan 6, 2022
@twmb twmb deleted the rpk_patches branch January 7, 2022 05:28
This was referenced Feb 8, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants