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: log errors if no nsqlookupd broadcast address #839

Merged
merged 1 commit into from
Dec 29, 2016

Conversation

ploxiln
Copy link
Member

@ploxiln ploxiln commented Dec 29, 2016

another piece to fix #826

When using nsqlookupd with no explicit broadcast address, and without the fix in #837, it looks like this:

[nsqlookupd] 2016/12/29 13:01:26.395092 CLIENT(127.0.0.1:53473): IDENTIFY Address:plo-pro.local TCP:4150 HTTP:4151 Version:0.3.8
[nsqlookupd] 2016/12/29 13:01:26.395115 DB: client(127.0.0.1:53473) REGISTER category:client key: subkey:
[nsqd] 2016/12/29 13:01:26.395325 LOOKUPD(127.0.0.1:4160): peer info {TCPPort:4160 HTTPPort:4161 Version:0.3.8 BroadcastAddress:}
[nsqd] 2016/12/29 13:01:26.395337 LOOKUPD(127.0.0.1:4160): ERROR - no broadcast address
 ...
[nsqd] 2016/12/29 13:01:27.468701 TOPIC(t1): created
[nsqd] 2016/12/29 13:01:27.468721 ERROR: no available nsqlookupd to query for channels to pre-create for topic t1

I think the ErrList used in GetLookupdTopicChannels() is a bit awkward. It turns into a multi-line error string. And if all lookupd requests experienced an error, it pre-formats it into a plain Error with a multi-line string.

The only other user of GetLookupdTopicChannels(), nsqadmin, ignores the error completely. So maybe the thing to do is have GetLookupdTopicChannels log the errors itself, maybe with a customizable prefix or using a callback ... yeah that sounds pretty messy too, but the current situation could use some kind of refactoring IMHO. Or we could just add these messages and punt :)

@mreiferson
Copy link
Member

I think the ErrList used in GetLookupdTopicChannels() is a bit awkward. It turns into a multi-line error string. And if all lookupd requests experienced an error, it pre-formats it into a plain Error with a multi-line string.

I struggled with that, agreed that it's awkward but not sure how important it is to fix.

I think this is a useful addition, thanks.

Copy link
Member

@mreiferson mreiferson left a comment

Choose a reason for hiding this comment

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

LGTM

@mreiferson mreiferson merged commit 02619e4 into nsqio:master Dec 29, 2016
@ploxiln ploxiln deleted the no_lookupd_broadcast branch January 17, 2017 06:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

nsqd: not properly creating channels registered in lookupd on seeing a new topic
2 participants