client/core: safe swap and comms tweaks#663
Merged
Merged
Conversation
chappjc
force-pushed
the
safe-tick-swap
branch
from
ff29f48 to
34f8564
Compare
chappjc
commented
Sep 9, 2020
Comment on lines
-363
to
-374
| // reconnect begins reconnection immediately. | ||
| func (conn *wsConn) reconnect() { | ||
| conn.setConnected(false) | ||
| conn.reconnectCh <- struct{}{} | ||
| } | ||
|
|
||
| // queueReconnect queues a reconnection attempt. | ||
| func (conn *wsConn) queueReconnect(wait time.Duration) { | ||
| conn.setConnected(false) | ||
| log.Infof("Attempting reconnect to %s in %d seconds.", conn.cfg.URL, wait/time.Second) | ||
| time.AfterFunc(wait, func() { conn.reconnectCh <- struct{}{} }) | ||
| } |
Member
Author
There was a problem hiding this comment.
Using these from outside read() would be all kinds of problematic, so they are moved into read.
| err = conn.ws.WriteMessage(websocket.TextMessage, b) | ||
| if err != nil { | ||
| log.Errorf("write error: %v", err) | ||
| log.Errorf("Send: WriteMessage error: %v", err) |
Member
Author
There was a problem hiding this comment.
I had briefly added a conn.close() here to cause the read loop to error and trigger reconnect, but the write error itself seems to be enough to cause reads (e.g. ws.ReadJSON) to return with an error immediately.
buck54321
approved these changes
Sep 9, 2020
client/core: prevent swap bcast with stale dex conn more careful reconnect, remove max reconnects
chappjc
force-pushed
the
safe-tick-swap
branch
from
ee89dbe to
a542cb1
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Swap contracts are no longer permitted to be broadcast if the DEX connection is known to be down. This can happen when a computer or client process comes out of suspend and immediately tries to take their action, particularly the taker trying to bcast their swap. The net connection's read timeout must still put the link into a failed state quickly for this to work during a resume, but this connectivity check is also valuable with normal connectivity trouble, not just machine/process suspend.
Triggering of ticks with the tick timer is also modified to skip tick execution if a long delay since the previous timer fire is detected (computer resume) to allow time for DEX re-connection. This together with the swap check described above prevents needlessly broadcasting a swap contract for a match that is revoked unbeknownst to the client. The swap check is still necessary however since asset tip changes also trigger a tick.
client/comms.WsConn reconnect logic is tweaked to prevent back-to-back or duplicate queued reconnect attempts. This is especially necessary now since send errors in Send and the ping handler now also trigger reconnect. Previously read errors would only trigger reconnect. The (*wsConn).read loop now detects and logs timeouts immediately before all other error checks to hasten flagging the connection as down and to log the timeout as the cause in plain language.