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

nsqd: topic name length and channel name length #355

Closed
svmehta opened this issue May 23, 2014 · 4 comments
Closed

nsqd: topic name length and channel name length #355

svmehta opened this issue May 23, 2014 · 4 comments

Comments

@svmehta
Copy link
Contributor

svmehta commented May 23, 2014

We find ourself in need of larger topic and channel name lengths (https://github.com/bitly/go-nsq/blob/97ea060927af3af0ad2c25debf94014a22e891ca/protocol.go#L30).

They are currently capped at 32 characters, what's the design reasoning behind these limits?

128 characters would be ideal for our application. Even 64 would help.

Thanks!

@mreiferson
Copy link
Member

hi @svmehta - there is no legitimate technical reason why they cannot be longer, it was more of a visual aesthetic issue in the variety of places we end up displaying them.

I've often thought about increasing the limit, now is as good a time as any if you're interested in sending a PR.

Note that most client libraries do client-side validation of topic/channel names, so most would need to be patched as well if this change was made.

cc @jehiah for input

@mreiferson mreiferson changed the title topic name length and channel name length nsqd: topic name length and channel name length May 23, 2014
@jehiah
Copy link
Member

jehiah commented May 23, 2014

I agree that 32 is/was arbitrary, and i'm ok going to 64. I'll also mention that some needs to communicate information about the client via channel name (the only spot where this was originally possible) are also now handled by the ability for clients to report a 'user agent' string along with other identify when subscribing.

@mreiferson
Copy link
Member

@svmehta - we merged #357 if you want to go ahead and make this change

@mreiferson
Copy link
Member

fixed in #358

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants