Skip to content

Commit

Permalink
Merge #35626
Browse files Browse the repository at this point in the history
35626: storage: Sync to disk before returning in WaitForApplication r=tbg a=bdarnell

This prevents on-disk inconsistencies when a node crashes in the
middle of a merge and the lease applied index temporarily regresses.

Fixes #33120

Release note (bug fix): Fixed an on-disk inconsistency that could
result from a crash during a range merge.


The second commit is a manual test (adapted from @tbg) that demonstrates the "overlapping range" failure if the WriteSyncNoop call is removed, and also shows that this call fixes the problem. It's obviously not mergeable in its current state, but I'm not sure how to test this without some gross hacks. 

Co-authored-by: Ben Darnell <[email protected]>
  • Loading branch information
craig[bot] and bdarnell committed Mar 12, 2019
2 parents 0c6efb2 + 68e34eb commit dc908f4
Showing 1 changed file with 13 additions and 1 deletion.
14 changes: 13 additions & 1 deletion pkg/storage/stores_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"time"

"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/storage/engine"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/retry"
)
Expand Down Expand Up @@ -102,7 +103,18 @@ func (is Server) WaitForApplication(
leaseAppliedIndex := repl.mu.state.LeaseAppliedIndex
repl.mu.RUnlock()
if leaseAppliedIndex >= req.LeaseIndex {
return nil
// For performance reasons, we don't sync to disk when
// applying raft commands. This means that if a node restarts
// after applying but before the next sync, its
// LeaseAppliedIndex could temporarily regress (until it
// reapplies its latest raft log entries).
//
// Merging relies on the monotonicity of the log applied
// index, so before returning ensure that rocksdb has synced
// everything up to this point to disk.
//
// https://github.com/cockroachdb/cockroach/issues/33120
return engine.WriteSyncNoop(ctx, s.engine)
}
}
if ctx.Err() == nil {
Expand Down

0 comments on commit dc908f4

Please sign in to comment.