Add function to fsync all segments until the current one

[fionaliao]

fsyncs for the WAL/WBL do not happen every time a record is logged, see: prometheus/prometheus#5869

This means that if Mimir is shut down uncleanly, it's possible for there to be WAL corruption errors due to not all data being written to disk before shutdown.

Adding a FsyncSegmentsUntilCurrent() function which will ensure all segments up until the current one have been fsycned before returning. Also added a corresponding function for the tsdb head, so it can be called from Mimir code.

Corresponding Mimir PR: grafana/mimir#12816

[fionaliao]

@bboreham replying here to comments from grafana/mimir#12816

I have updated the logic a bit from the version you reviewed - the current segment fsync is now queued through the actor channel. The mutex is held while queuing the  - the actual fsync operations happen asynchronously after the mutex is released.

---

For consistency, you could do the fsync inside the actor function.
I can't come up with any concrete reason why this would be better; it just seems nicer that all fsyncs would happen in order.

I think one reason is that this allows the fsync to be executed without holding the mutex but still avoids race conditions where the segment could be closed before it's fsynced (the write log Close() function waits for the actor channel to drain before closing the current segment)

---

grafana/mimir#12816 (comment)

How can this happen?

I'm not sure what this refers too - w.segement being nil? w.segment changing or being closed?

[dimitarvdimitrov]

i'm not sure about where we acquire and release the lock.

The potential problems i can think of right now are

1. there's a race between taking a copy of w.segment, unlocking the mutex and running the fsync. it's possible another goroutine replaced the w.segment; so at that point we no longer have the current segment fsync'd. it may not be a huge problem since, there's still the same race right before returning from FsyncSegmentsUntilCurrent - we can get a new segment.
2. We end up with a deadlock where pushing to the channel blocks because the currently executed function from actorc is trying to acquire the lock. This currently doesn't happen, but i don't see

i don't think it's possible to solve both. From reading the only other place which uses actorc and the places which do operations on the WL while holding the lock, I think that 2. is assumed to never happen. So I think we should release the lock only after getting done closed

[dimitarvdimitrov]

something i forgot - if you end up taking a copy of w.segment, then you should do that while holding a lock inside the actor func instead of before creating the actor closure - this will help avoid races