Skip to content
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

revise WriteAPI.Flush to attempt to send batches in the retry queue #291

Closed
wants to merge 2 commits into from

Conversation

pabigot
Copy link
Contributor

@pabigot pabigot commented Dec 13, 2021

Closes #289
Closes #290

Proposed Changes

After completing standard flush operations that move points from the buffer to queued batch, trigger a re-examination of retries rather than waiting for a timeout.

Also incorporates the cleanup patch for #290.

Checklist

I'll address these if it appears this is an acceptable solution to the problem described in #289.

  • CHANGELOG.md updated
  • Rebased/mergeable
  • A test has been added if appropriate
  • Tests pass
  • Commit messages are in semantic format
  • Sign CLA (if not already signed)

The length of an unbuffered channel is always zero, and all checked
channels are unbuffered, so all flush does is a transient
synchronization with the writeProc and bufferProc goroutines.  Remove
the unnecessary logic.

Signed-off-by: Peter A. Bigot <pab@pabigot.com>
Flush() is documented to force all pending writes from the buffer to
be sent.  It does force pending writes in the buffer to be submitted
for send, but if there is a pending retry with an unexpired timeout it
won't actually send anything.

Make Flush() more useful by having it trigger a re-examination of the
retry queue and initiating a retry immediately.

Depending on whether the documented behavior was intentional, this
operation might be better implemented as an additional WriteAPI method
FlushAll().

Signed-off-by: Peter A. Bigot <pab@pabigot.com>
@pabigot
Copy link
Contributor Author

pabigot commented Dec 14, 2021

After further consideration this doesn't meet my needs: it avoids data loss in successful reconnection cases, but on shutdown the flush operation would provide a callback only for one queued batch per flush, so recovery of submitted data would be tedious. Also the fact that the batch processing will discard data without programmatically notifying the client if the queue length gets too long is a showstopper. So I'm going to stick with the blocking API.

@pabigot pabigot closed this Dec 14, 2021
@pabigot pabigot deleted the pabigot/20211213b branch January 4, 2022 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WriteAPIImpl.Flush is unnecessarily complex WriteAPI.Flush doesn't do what's expected
1 participant