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-3219: Fix long topic name validation #898

Closed
wants to merge 1 commit into from

Conversation

vahidhashemian
Copy link
Contributor

This fixes an issue with long topic names by considering, during topic
validation, the '-' and the partition id that is appended to the log
folder created for each topic partition.

@vahidhashemian
Copy link
Contributor Author

@granthenke Thanks for the suggestion. I did not want to assume anything on the first try, but if it is acceptable to assume a max value for partition count I'd be happy to update the pull request.

@granthenke
Copy link
Member

@vahidhashemian I would be interested in thoughts from others.

@gwenshap
Copy link
Contributor

I think 249 characters and 99,999 partitions is a good solution.
We need to document this as a limitation, but it sounds acceptable to me.

@vahidhashemian
Copy link
Contributor Author

Sounds good to me. With 249 characters for topic name we can actually have 100,000 partitions (since partition ids start from 0). I'll try to find the right place in the documentation to mention this limitation.

This fixes an issue with long topic names by considering, during topic
validation, the '-' and the partition id that is appended to the log
folder created for each topic partition. Assuming that the partition
count can not go over 100,000 the maximum length of a topic will be
249.
@gwenshap
Copy link
Contributor

Oh, I think you removed the behavior where long names cause an error when creating the topic. I actually preferred that to silent truncating of the name. Any reason you removed this part?

@vahidhashemian
Copy link
Contributor Author

@gwenshap Well, the only change to the actual code now is the topic name length limit which is decreased from 255 to 249. Topic names longer than the limit would cause an error like before, and would not be truncated. Could you please clarify? I might be missing something.

@granthenke
Copy link
Member

@gwenshap Any reason this can't get into 0.10?

@gwenshap
Copy link
Contributor

@granthenke Well, lack of LGTM at the moment.
I'll try to get to it this week, but can't promise.

@granthenke
Copy link
Member

@gwenshap gotcha. This patch reduces the valid topic name length to 249 (from 255) and adds test coverage and documentation. It's a LGTM for me.

@vahidhashemian Anything else you need to do yet for this patch?

@gwenshap
Copy link
Contributor

LGTM

asfgit pushed a commit that referenced this pull request Mar 22, 2016
This fixes an issue with long topic names by considering, during topic
validation, the '-' and the partition id that is appended to the log
folder created for each topic partition.

Author: Vahid Hashemian <vahidhashemian@us.ibm.com>

Reviewers: Gwen Shapira, Grant Henke

Closes #898 from vahidhashemian/KAFKA-3219

(cherry picked from commit ad3dfc6)
Signed-off-by: Gwen Shapira <cshapi@gmail.com>
@asfgit asfgit closed this in ad3dfc6 Mar 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants