fix: write retry queue stops retrying #332
Conversation
There was a problem hiding this comment.
Thanks for finding the issue and the fix!
Just a few minor things:
- Fix tests, please. You can use this patch PR332TestFix.zip via
git apply - Change commit messages to be semantic.
- Small change in the error message to be consistent with other messages
…hes when server connection is lost for longer than MaxRetryTime Design error, need to evict all expired batches prior to attempting write. Otherwise if server connection drops out for longer than MaxRetryTime, can get caught in an endless loop of evicting one expired batch each time a new batch is enqueued and thus never again actually retry writing to server.
seth-hunter
force-pushed
the
write-retry-fixes
branch
2 times, most recently
from
000351a to
2b26861
Compare
Codecov Report
@@ Coverage Diff @@
## master #332 +/- ##
=======================================
Coverage 90.48% 90.48%
=======================================
Files 23 23
Lines 2490 2490
=======================================
Hits 2253 2253
Misses 178 178
Partials 59 59
Continue to review full report at Codecov.
|
|
@vlastahajek: thank you for the review and suggestions. Updated branch should address your comments and also fix an additional closely-related issue I found during further testing. Requesting your review on that, as it may be a little more subjective from a design perspective. |
|
Thanks for the update. |
seth-hunter
force-pushed
the
write-retry-fixes
branch
from
2b26861 to
a67dec1
Compare
seth-hunter
changed the title
|
Thanks! |
Fixes logic error in management of retry buffer that leads to deadlock scenario where write retries cease if server connection is lost for longer than MaxRetryTime