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.

Sign up for GitHub

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

Jump to bottom

nsqd: close connections that don't send magic #1153

Merged
merged 1 commit into from
Mar 26, 2019
Merged

nsqd: close connections that don't send magic #1153

merged 1 commit into from
Mar 26, 2019

Conversation

fengve
Copy link
Contributor

@fengve fengve commented Mar 26, 2019

Prevent a lot of CLOSE_WAIT。

The probe service detects if nsqd is alive.
Since there is no call to close here, a lot of CLOSE_WAIT will be generated.

@fengve
Update tcp.go
5a730ec 
Prevent a lot of CLOSE_WAIT
@fengve fengve changed the title Update tcp.go Prevent a lot of CLOSE_WAIT。 Mar 26, 2019
@mreiferson
Copy link
Member

This looks OK to me, will leave for others to take a peek.

@mreiferson mreiferson changed the title Prevent a lot of CLOSE_WAIT。 nsqd: close connections that don't send magic Mar 26, 2019
jehiah
jehiah reviewed Mar 26, 2019
View reviewed changes
Copy link
Member

@jehiah jehiah left a comment

Choose a reason for hiding this comment

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

This LGTM, however i think there is one more spot to fix the same issue.

Can you also add a clientConn.Close() on line 46?

@fengve
Copy link
Contributor Author

fengve commented Mar 26, 2019

This LGTM, however i think there is one more spot to fix the same issue.

Can you also add a clientConn.Close() on line 46?

It's not necessary here.
Because close is done in prot.IOLoop(clientConn)

@jehiah jehiah merged commit 223e97f into nsqio:master Mar 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jehiah jehiah jehiah approved these changes

Assignees
No one assigned
Labels
chore
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@fengve @mreiferson @jehiah