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

Wrong error message in rpk when too many partitions created #4017

Closed
travisdowns opened this issue Mar 15, 2022 · 9 comments · Fixed by #4662
Closed

Wrong error message in rpk when too many partitions created #4017

travisdowns opened this issue Mar 15, 2022 · 9 comments · Fixed by #4662
Assignees
Labels
area/rpk kind/bug Something isn't working

Comments

@travisdowns
Copy link
Member

Version & Environment

Redpanda version: dev branch @ 1fae709

What went wrong?

When redpanda is asked to create too many partitions (more than 1 per MB of memory, among other limits) it will fail out, but the message says Number of partitions is below 1. instead of the actual error:

$ rpk topic create foobar -p 100000 --config rp.yaml
TOPIC   STATUS
foobar  INVALID_PARTITIONS: Number of partitions is below 1.

What should have happened instead?

The error message should say more than the supported number of partitions or something like that, even better if it includes the limit that was hit (memory, etc) and the calculated cap value.

How to reproduce the issue?

As above on a vanilla or local install.

@travisdowns travisdowns added kind/bug Something isn't working area/rpk labels Mar 15, 2022
@jcsp
Copy link
Contributor

jcsp commented Mar 22, 2022

Huh, I thought we fixed this but perhaps not. Basically INVALID_PARTITIONS is the error code, and "Number of partitions is below 1" is an overly-specific interpretation of the code that is hardcoded into franz-go (and also other clients).

@travisdowns
Copy link
Member Author

Basically INVALID_PARTITIONS is the error code, and "Number of partitions is below 1" is an overly-specific interpretation of the code that is hardcoded into franz-go (and also other clients).

Right, I have seen this several times: the error message reflects the only known reason an error could occur but then as more reasons are added (or simply discovered, if they already existed) it is no longer valid. Do you know if there is some kind of "sub error code" which can indicate a more specific reason (that would be nice), or do we just have to make the message more generic, like "Too many or too few partitions".

It would be a bit of a shame if the latter since the reasons the partition count can be rejected as too high are quite varied these days with all the new limits, so it could be quite tough for an end user to determine what went wrong.

@dotnwat
Copy link
Member

dotnwat commented Mar 23, 2022

The creatable_topic_result structure which is part of the response to create topic request contains:

struct creatable_topic_result {
    model::topic name{};
    kafka::error_code error_code{};
    std::optional<ss::sstring> error_message{};
    int16_t topic_config_error_code{};                          
...

and my guess is that we only set the error_code to kafka::invalid_partitions which has the pre-defined error message. presumably we can experiment with both the error_message and topic_config_error_code fields to help provide better feedback to the client.

the extent to which clients make use of these to provide better error reporting is probably different across clients. but generally the upstream kafka client and franz-go implement many of these detailed aspects. cc @twmb

@twmb
Copy link
Contributor

twmb commented Mar 29, 2022

The error message can be used to make the output a lot nicer,
and the general wording of error messages themselves are taken directly from the kafka docs. We could have an override / extension of the kerr package within rpk to tailor any high level error codes (INVALID_PARTITIONS) a little bit more, but making using of the error message field is more preferred (and I think really the reason it was introduced, but I somewhat forget here).

@dotnwat
Copy link
Member

dotnwat commented Mar 29, 2022

but making using of the error message field is more preferred (and I think really the reason it was introduced, but I somewhat forget here).

makes sense. we already have pre-baked messages for most of our error scenarios so it shouldn't be hard to route those messages into the response. thanks travis.

@travisdowns
Copy link
Member Author

I guess rpk is English-only at the moment, but I guess the "server provided error message" is a problem when clients are localized into other languages. Perhaps that's a problem for future-us that we can shelve for now.

@dotnwat
Copy link
Member

dotnwat commented Mar 29, 2022

Perhaps that's a problem for future-us that we can shelve for now.

having a better error message for this particular issue makes sense. I haven't heard of any discussion about i18n for the CLI, but that'd be cool. I've never seen i18n in CLIs. Have you encountered any? That'd be cool to look at.

@travisdowns
Copy link
Member Author

travisdowns commented Mar 30, 2022

I've never seen i18n in CLIs. Have you encountered any? That'd be cool to look at.

I sort of felt like all the mainstream GNU userspace CLIs were translated? Certainly I've seen terminal copy/pastes from non-English users with non-English messages.

As a random example using ls and cat on my system:

tdowns@p1:~$ ls foo
ls: cannot access 'foo': No such file or directory
tdowns@p1:~$ LANGUAGE=fr_CA.UTF-8 ls foo
ls: impossible d'accéder à 'foo': Aucun fichier ou dossier de ce type
tdowns@p1:~$ LANGUAGE=fr_CA.UTF-8 cat --no
cat : l'option « --no » n'a pas été reconnue
Saisissez « cat --help » pour plus d'informations.

@dotnwat
Copy link
Member

dotnwat commented Mar 30, 2022

I've never seen i18n in CLIs. Have you encountered any? That'd be cool to look at.

I sort of felt like all the mainstream GNU userspace CLIs were translated? Certainly I've seen terminal copy/pastes from non-English users with non-English messages.

As a random example using ls on my system:

tdowns@p1:~$ ls foo
ls: cannot access 'foo': No such file or directory
tdowns@p1:~$ LANGUAGE=fr_CA.UTF-8 ls foo
ls: impossible d'accéder à 'foo': Aucun fichier ou dossier de ce type
tdowns@p1:~$ LANGUAGE=fr_CA.UTF-8 cat --no
cat : l'option « --no » n'a pas été reconnue
Saisissez « cat --help » pour plus d'informations.

Sorry, what!? This is amazing. I mean, I guess I'm not surprised, I just have never seen it in action. 🔥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/rpk kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants