[FIXED] Possible delays in delivering messages #895
Merged
Conversation
This file contains 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
There is a possibility that a partial write results in data not being sent in a timely fashion to a subscription. Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
kozlovic
force-pushed
the
fix_possible_writeloop_stall
branch
from
c4bdc8b
to
42f45ce
Compare
| @@ -917,13 +920,21 @@ func (c *client) flushOutbound() bool { | |||
| } | |||
| } | |||
|
|
|||
| // Check that if there is still data to send and writeLoop is in wait, | |||
| // we need to signal | |||
| if c.out.pb > 0 && c.out.sgw { | |||
There was a problem hiding this comment.
Why not just signal, why do we need the bool? I think we may just want to signal on partial?
| return true | ||
| } | ||
|
|
||
| // flushSignal will use server to queue the flush IO operation to a pool of flushers. | ||
| // Lock must be held. | ||
| func (c *client) flushSignal() { | ||
| c.out.sg.Signal() | ||
| if c.out.sgw { |
There was a problem hiding this comment.
Do we think this is saving us much?
There was a problem hiding this comment.
Did a quick and dirty benchmark of simulating signaling a routine going into a wait, with and without a check with boolean:
Benchmark_SignalWithoutCheck-8 50000000 41.6 ns/op
Benchmark____SignalWithCheck-8 100000000 19.9 ns/op
But more importantly, even if we don't gain much, why not checking? Signaling while not in wait is just wasted because a signal while not in wait is lost anyway. Since we use mutex to protect wait()/signal(), it makes sense to me to know if we are in wait or not.
|
LGTM |
|
I merged it but doing normal benchmarks it slows things down a bunch so may edit a bit.. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 is a possibility that a partial write results in data
not being sent in a timely fashion to a subscription.
Signed-off-by: Ivan Kozlovic ivan@synadia.com