Waiting for a WaterMark might return prematurely

[srh]

You'll note in places like waitForAppliedMark or any other code that waits for a WaterMark to be satisfied, we have for loops that begin, for n.applied.WaitingFor() {.

What that function WaitingFor() does is read an atomic variable that doesn't get set to 1 until the first Mark makes it over the channel and gets processed. Since we don't force these marks over the channel (WaterMark::Ch) to get processed, it's possible that the atomic variable has not yet been set to 1. Thus it's possible (if we imagine the mark channel never getting consumed) all watermarks return immediately.

I don't understand the purpose of WaitingFor, so there might be something here I'm missing.

Edit: There's one place that waits on WaitingFor exclusively. Looking at the history it seems the variable was not introduced with a strong intent behind what it accomplishes, so I would feel comfortable reworking it. Snce I've already made waiting for watermarks not sleep-wait I'll figure this out too.

[manishrjain]

Those are useful for testing purposes. @janardhan1993: Can you investigate this along with Sam?

[srh]

Is it related to IsTestRun/SetTestRun type stuff? What is the point of turning off watermark behavior during test runs? As far as I can tell, testing works fine if we get rid of IsTestRun/SetTestRun.

[manishrjain]

Speaking roughly, mutations might not get proposed to Raft, but expect to be applied/synced to disk.

[janardhan1993]

I had a look at the code. WaitingFor is used in mostly for reindexing,delete predicate mutation(We need previous entries to be synced to disk since we iterate over badger): Since everything is serialised by raft, if you look at the code we force the pending marks over the channel before processing anything.

Atleast for our present use cases it works. But as @srh pointed out it won't work if we want to wait for an index (for read linearazability) for which pending mark has not been pushed over the channel.
I guess the intent of waitingFor was to check for all indices for which mark was pushed over the channel. That is the first thing we do in draft.go:Run method.

 for _, entry := range rd.CommittedEntries {
   4         status := x.Mark{Index: entry.Index, Done: false}

[srh]

How do we force pending marks over the channel?

[janardhan1993]

My bad we just push it to the buffered channel and don't force it to be processed.

[srh]

Fixed by #1377.