-
Notifications
You must be signed in to change notification settings - Fork 1
Commit
- Loading branch information
There are no files selected for viewing
2 comments
on commit 1fd1128
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the potential race condition, this seems OK.
But 😁 - I really think we should try to get onto the latest version of go-nsq
before making functional changes like this.
In the newer go-nsq
codebase, I think the approach we should take is to maintain a list of nsqd
addresses like we maintain for nsqlookupd
- and then we won't need to add the reconnect
flag because we'll be able to remove the nsqd
address from the list, close the connection, and the "reconnect logic" in the close handler will check our list to see if it's still there, otherwise drop it on the floor. This also means we don't have the potential racey overwrite of the CloseCB
.
I can whip up a PR that adds this functionality on top of go-nsq
master taking this approach.
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pssst nsqio#91
I think this assignment is racey