nsqd: do not print EOF when closing cleanly #560
Conversation
| @@ -113,6 +113,9 @@ func (p *protocolV2) IOLoop(conn net.Conn) error { | |||
| if client.Channel != nil { | |||
| client.Channel.RemoveClient(client.ID) | |||
| } | |||
| if err == io.EOF && atomic.LoadInt32(&client.State) == stateClosing { | |||
There was a problem hiding this comment.
This is exactly what I had in mind, but I'm not sure io.EOF is the only error possible in this state - if we're in stateClosing I think all errors should be ignored, right?
There was a problem hiding this comment.
Ignoring all errors would mask if a connection issued CLS followed by FIN or other messages (which error with cannot XYZ in current state). Also, it looks like PUB/MPUB can still be issued while closing.
There was a problem hiding this comment.
If we move this code into readLoop it can more explicitly ignore all errors related to the socket after a CLS exchange.
Thoughts?
|
fixes #521 |
mreiferson
changed the title
| @@ -62,7 +62,11 @@ func (p *protocolV2) IOLoop(conn net.Conn) error { | |||
| // ie. the returned slice is only valid until the next call to it | |||
| line, err = client.Reader.ReadSlice('\n') | |||
| if err != nil { | |||
| err = fmt.Errorf("failed to read command - %s", err) | |||
| if err == io.EOF && atomic.LoadInt32(&client.State) == stateClosing { | |||
There was a problem hiding this comment.
cool, so now we can just do this for any error, right?
There was a problem hiding this comment.
I'm not so sure that ErrBufFull should be ignored, but since we're closing I guess it really doesn't matter.
There was a problem hiding this comment.
right, I think the idea is to clean up logging here - we've already done the CLS exchange at this point and I think any read-based error is just noise.
When nsqd shuts down, protocol_v2 responds to CLS with CLOSE_WAIT and then the IOLoop waits for another command with ReadSlice on the buffered client connection. ReadSlice could return an error because the client closes the connection or if the read buffer is full. This error is returned from IOLoop to the tcp handler and printed. Because we are shutting down, we do not care about the error.
|
👍 |
nsqd: do not print EOF when closing cleanly
|
Thanks @twmb |
When nsqd shuts down, protocol_v2 responds to CLS with CLOSE_WAIT and then the
IOLoop waits for another command with ReadSlice on the buffered client
connection. ReadSlice returns EOF because the client closes the connection;
this error is returned from IOLoop to the tcp handler and printed.
Because the presence of EOF is expected when the client is in stateClosing, the
error should be ignored.