txnsync: delete-able incoming message queue #3126
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3126 +/- ##
==========================================
+ Coverage 43.69% 43.77% +0.07%
==========================================
Files 390 391 +1
Lines 86700 86877 +177
==========================================
+ Hits 37887 38033 +146
- Misses 42788 42807 +19
- Partials 6025 6037 +12
Continue to review full report at Codecov.
|
| } | ||
| goto writeOutboundMessage | ||
| } | ||
| } |
There was a problem hiding this comment.
Should the mutex be locked while waiting?
There was a problem hiding this comment.
can you find a code-path that would get to
imq.enqueuedPeersCond.Wait()without the lock being taken ?
the imq.enqueuedPeersCond would internally unlock the mutex, wait for signal and then take back the lock.
| outboundPeerCh chan incomingMessage | ||
| enqueuedPeersMap map[*Peer]*queuedMsgEntry | ||
| messages queuedMsgList | ||
| freelist queuedMsgList |
There was a problem hiding this comment.
can you explain the need for having both the messages and freelist lists? Can we just use the list.List which also holds the length of the linked list?
There was a problem hiding this comment.
Yes, I was thinking about that; however, list.List is not that optimal ( both because of the interface{} casting, as well as for the dynamic allocation ).
There was a problem hiding this comment.
Can we just use one message linked list and keep a counter for the number of items instead of having 2 lists?
There was a problem hiding this comment.
I think that it's doable - but it does make the testing for that a bit less straight-forward. I'd like to see that this solution addresses the issue first; if it does, we can merge the two lists into a single list + counter.
## Summary Running betanet, we've seen errors such as ``` unable to enqueue incoming message into peer incoming message backlog. disconnecting from peer. ``` as well as multiple ``` Peer 162.202.32.72:56543 disconnected: SlowConnection ``` The goal of this PR is to remove from the pending incoming message queue entries that are associated with network peers that have been disconnected ( by the network package ), or have been scheduled for disconnection ( by the transaction synch package ). Removing these would increase the "available" space in the incoming message queue and would prevent redundant disconnections. ## Test Plan - [x] unit tests updated. - [x] ran s1 network with and without load, successfully.
This reverts commit ca3e877.
Summary
Running betanet, we've seen errors such as
as well as multiple
The goal of this PR is to remove from the pending incoming message queue entries that are associated with network peers that have been disconnected ( by the network package ), or have been scheduled for disconnection ( by the transaction synch package ).
Removing these would increase the "available" space in the incoming message queue and would prevent redundant disconnections.
Test Plan