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

kafka/server: Improve kafka::map_topic_error_code #2422

Closed

Conversation

BenPope
Copy link
Member

@BenPope BenPope commented Sep 23, 2021

Note: Let's wait for this to bake on test-staging for a bit before merge.

Cover letter

Map cluster::errc::no_leader_controller to kafka::error_code::not_controller so that the error is retriable, with metadata refresh.

Fixes #2406

Also:

  • Improve logging by using vlog
  • Improve switch case to force compilation error when new cluster::errc is introduced.

See #2425 for followup work.

Changes in force-push

  • Change the mapping to cluster::errc::not_controller
  • Change base to dev

Release notes

Release note: redpanda/kafka: leader_not_available is now a retriable error.

Signed-off-by: Ben Pope <ben@vectorized.io>
@BenPope BenPope changed the title kafka/server: Improve kafka_map_topic_error_code kafka/server: Improve kafka::map_topic_error_code Sep 23, 2021
src/v/kafka/server/errors.h Outdated Show resolved Hide resolved
src/v/kafka/server/errors.h Show resolved Hide resolved
@jcsp
Copy link
Contributor

jcsp commented Sep 24, 2021

This change looks good to me, modulo the discussion about NotController above.

@BenPope when you update this, please can you close this one and re-open against dev. With test-staging I'm intending to merge PRs into it by hand, so that we don't have to keep track of PRs that merged into test-staging but would later need re-opening for dev (keep a 1:1 relationship between issue and PR, so that when the PR closes the issue closes)

Instead of returning `unknown_server_error`, return `not_controller`

This is a retriable error code, with metadata refresh.

Fixes redpanda-data#2406

Signed-off-by: Ben Pope <ben@vectorized.io>
When a new `cluster::errc` is added, a compilation error will force
consideration of the mapping here.

Signed-off-by: Ben Pope <ben@vectorized.io>
Signed-off-by: Ben Pope <ben@vectorized.io>
@BenPope BenPope force-pushed the improve_kafka_map_topic_error_code branch from 24fed00 to 7c7c503 Compare September 24, 2021 10:55
@BenPope BenPope changed the base branch from test-staging to dev September 24, 2021 10:55
@BenPope
Copy link
Member Author

BenPope commented Sep 24, 2021

This change looks good to me, modulo the discussion about NotController above.

@BenPope when you update this, please can you close this one and re-open against dev.

I moved the base to dev.

@BenPope
Copy link
Member Author

BenPope commented Sep 24, 2021

Closed in favour of #2428 since buildkite didn't spot that I changed the base to dev.

@BenPope BenPope closed this Sep 24, 2021
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.

Failure in RetentionPolicyTest.test_changing_topic_retention_with_restart
5 participants