forked from tstranex/gozmq
-
Notifications
You must be signed in to change notification settings - Fork 9
zmq: prevent unnecessary timeouts and return EOF on connection termination #3
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
Merged
Roasbeef
merged 4 commits into
lightninglabs:master
from
wpaulino:conn-timeout-block-read
Jul 10, 2019
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
5c6ccf3
build: init go module
wpaulino e047b84
zmq: block on the first frame when reading new messages
wpaulino 09dc028
zmq: prevent reconnecting when connection is explicitly terminated
wpaulino 6e9a863
zmq: expose underlying connection RemoteAddr method
wpaulino File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
module github.com/lightninglabs/gozmq | ||
|
||
go 1.12 | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ import ( | |
"os" | ||
"regexp" | ||
"strings" | ||
"sync" | ||
"time" | ||
) | ||
|
||
|
@@ -64,6 +65,9 @@ type Conn struct { | |
conn net.Conn | ||
topics []string | ||
timeout time.Duration | ||
|
||
closeConn sync.Once | ||
quit chan struct{} | ||
} | ||
|
||
func (c *Conn) writeAll(buf []byte) error { | ||
|
@@ -193,7 +197,7 @@ func (c *Conn) subscribe(prefix string) error { | |
} | ||
|
||
func (c *Conn) readCommand() (string, []byte, error) { | ||
flag, buf, err := c.readFrame() | ||
flag, buf, err := c.readFrame(true) | ||
if err != nil { | ||
return "", nil, err | ||
} | ||
|
@@ -256,10 +260,20 @@ func (c *Conn) readReady() error { | |
} | ||
|
||
// Read a frame from the socket, setting deadline before each read to prevent | ||
// timeouts during or between frames. | ||
func (c *Conn) readFrame() (byte, []byte, error) { | ||
// timeouts during or between frames. The initialFrame should be used to denote | ||
// whether this is the first frame we'll read for a _new_ message. | ||
// | ||
// NOTE: This is a blocking call if there is nothing to read from the | ||
// connection. | ||
func (c *Conn) readFrame(initialFrame bool) (byte, []byte, error) { | ||
// We'll only set a read deadline if this is not the first frame of a | ||
// message. We do this to ensure we receive complete messages in a | ||
// timely manner. | ||
if !initialFrame { | ||
c.conn.SetReadDeadline(time.Now().Add(c.timeout)) | ||
} | ||
|
||
var flagBuf [1]byte | ||
c.conn.SetReadDeadline(time.Now().Add(c.timeout)) | ||
if _, err := io.ReadFull(c.conn, flagBuf[:1]); err != nil { | ||
return 0, nil, err | ||
} | ||
|
@@ -305,11 +319,22 @@ func (c *Conn) readFrame() (byte, []byte, error) { | |
return flag, buf, nil | ||
} | ||
|
||
// Read a message from the socket. | ||
// readMessage reads a new message from the connection. | ||
// | ||
// NOTE: This is a blocking call if there is nothing to read from the | ||
// connection. | ||
func (c *Conn) readMessage() ([][]byte, error) { | ||
// We'll only set read deadlines on the underlying connection when | ||
// reading messages of multiple frames after the first frame has been | ||
// read. This is done to ensure we receive all of the frames of a | ||
// message within a reasonable time frame. When reading the first frame, | ||
// we want to avoid setting them as we don't know when a new message | ||
// will be available for us to read. | ||
initialFrame := true | ||
|
||
var parts [][]byte | ||
for { | ||
flag, buf, err := c.readFrame() | ||
flag, buf, err := c.readFrame(initialFrame) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
@@ -326,6 +351,8 @@ func (c *Conn) readMessage() ([][]byte, error) { | |
if len(parts) > 16 { | ||
return nil, errors.New("message has too many parts") | ||
} | ||
|
||
initialFrame = false | ||
} | ||
return parts, nil | ||
} | ||
|
@@ -339,7 +366,12 @@ func Subscribe(addr string, topics []string, timeout time.Duration) (*Conn, erro | |
|
||
conn.SetDeadline(time.Now().Add(10 * time.Second)) | ||
|
||
c := &Conn{conn, topics, timeout} | ||
c := &Conn{ | ||
conn: conn, | ||
topics: topics, | ||
timeout: timeout, | ||
quit: make(chan struct{}), | ||
} | ||
|
||
if err := c.writeGreeting(); err != nil { | ||
conn.Close() | ||
|
@@ -373,36 +405,58 @@ func Subscribe(addr string, topics []string, timeout time.Duration) (*Conn, erro | |
} | ||
|
||
// Receive a message from the publisher. It blocks until a new message is | ||
// received. | ||
// received. If the connection times out and it was not explicitly terminated, | ||
// then a timeout error is returned. Otherwise, if it was explicitly terminated, | ||
// then io.EOF is returned. | ||
func (c *Conn) Receive() ([][]byte, error) { | ||
messages, err := c.readMessage() | ||
// If the error is either nil or a non-EOF error, we return it as-is. | ||
if err != io.EOF { | ||
return messages, err | ||
} | ||
// We got an EOF, so our socket is disconnected. We attempt to | ||
// reconnect. If successful, replace the existing connection with the | ||
// new one. Either way, return a timeout error. | ||
|
||
// We got an EOF, so our socket is disconnected. If the connection was | ||
// explicitly terminated, we'll return the EOF error. | ||
select { | ||
case <-c.quit: | ||
return nil, io.EOF | ||
default: | ||
} | ||
|
||
// Otherwise, we'll attempt to reconnect. If successful, we'll replace | ||
// the existing connection with the new one. Either way, return a | ||
// timeout error. | ||
errTimeout := &net.OpError{ | ||
Op: "read", | ||
Net: c.conn.LocalAddr().Network(), | ||
Source: c.conn.LocalAddr(), | ||
Addr: c.conn.RemoteAddr(), | ||
Err: &reconnectError{err}, | ||
} | ||
newConn, err := Subscribe(c.conn.RemoteAddr().String(), c.topics, | ||
c.timeout) | ||
newConn, err := Subscribe( | ||
c.conn.RemoteAddr().String(), c.topics, c.timeout, | ||
) | ||
if err != nil { | ||
// Prevent CPU overuse by refused reconnection attempts. | ||
time.Sleep(c.timeout) | ||
} else { | ||
c.Close() | ||
*c = *newConn | ||
c.conn.Close() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No error to return here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We still want to return |
||
c.conn = newConn.conn | ||
} | ||
return nil, errTimeout | ||
} | ||
|
||
// Close the underlying connection. Any further operations will fail. | ||
func (c *Conn) Close() error { | ||
return c.conn.Close() | ||
var err error | ||
c.closeConn.Do(func() { | ||
close(c.quit) | ||
err = c.conn.Close() | ||
}) | ||
return err | ||
} | ||
|
||
// RemoteAddr returns the remote network address. | ||
func (c *Conn) RemoteAddr() net.Addr { | ||
return c.conn.RemoteAddr() | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
why is this defined here?
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.
See golang/go#23969 and https://go-review.googlesource.com/c/go/+/125940/.