-
Notifications
You must be signed in to change notification settings - Fork 26
feat(store): Sync #287
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
feat(store): Sync #287
Conversation
2ef0f04
to
af22edd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fine w this, chan chan :)
// and Store is not stopping(headers == nil) | ||
if s.pending.Len() < s.Params.WriteBatchSize && headers != nil { | ||
continue | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah okay so you want to leave not large enough un-flushed batch in waiting and exit flush loop so that we aren't writing anything to disk for the Sync
call, I see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is why the method is not called Flush
but Sync
, which initially I called in Flush
, but then realized that akcshually its not Flush
// (2) Batching header writes | ||
func (s *Store[H]) flushLoop() { | ||
defer close(s.writesDn) | ||
ctx := context.Background() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
side note - we never cancel this ctx if we exit, should we put a cancellation in defer ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can and even should if any internal calls spins up a backrground routine waiting on context, but as its not happening anywhere yet, it fine to keep as is
Adds a Sync method which ensures all pending writes are processed. It blocks until the operation completes or fails.
Also, the method gets used in
DeleteTo
, ensuring deletion always operates on the latest state.This is technically a hack for until #241 and #263 come