Skip to content

nsqd: log errors if no nsqlookupd broadcast address - #839

Merged
mreiferson merged 1 commit into
nsqio:masterfrom
ploxiln:no_lookupd_broadcast
Dec 29, 2016
Merged

nsqd: log errors if no nsqlookupd broadcast address#839
mreiferson merged 1 commit into
nsqio:masterfrom
ploxiln:no_lookupd_broadcast

Conversation

@ploxiln

@ploxiln ploxiln commented Dec 29, 2016

Copy link
Copy Markdown
Member

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
Copy Markdown
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.

@mreiferson mreiferson left a comment

Copy link
Copy Markdown
Member

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