Skip to content

Commit

Permalink
kvserver: bump the in-memory GC threshold as a pre-apply side effect
Browse files Browse the repository at this point in the history
This commit changes where the in-memory GC threshold on a Replica is
bumped during the application of a GC request.

Previously, the in-memory GC threshold was bumped as a post-apply side
effect. Additionally, GC requests do not acquire latches at the
timestamp that they are garbage collecting, and thus, readers need to
take additional care to ensure that results aren't served off of a
partial state.

Readers today rely on the invariant that the in-memory GC threshold is
bumped before the actual garbage collection. This today is true because
the mvccGCQueue issues GC requests in 2 phases: the first that simply
bumps the in-memory GC threshold, and the second one that performs the
actual garbage collection. If the in-memory GC threshold were bumped in
the pre-apply phase of command application, this usage quirk wouldn't
need to exist. That's what this commit does.

Relates to #55293

Release note: None
  • Loading branch information
aayushshah15 committed Jun 22, 2022
1 parent 246eda9 commit be23e6f
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 9 deletions.
10 changes: 5 additions & 5 deletions pkg/kv/kvserver/replica_application_state_machine.go
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,11 @@ func (b *replicaAppBatch) runPreApplyTriggersAfterStagingWriteBatch(
)
}

if res.State != nil && res.State.GCThreshold != nil {
b.r.handleGCThresholdResult(ctx, res.State.GCThreshold)
res.State.GCThreshold = nil
}

if res.State != nil && res.State.TruncatedState != nil {
var err error
// Typically one should not be checking the cluster version below raft,
Expand Down Expand Up @@ -1288,11 +1293,6 @@ func (sm *replicaStateMachine) handleNonTrivialReplicatedEvalResult(
rResult.RaftExpectedFirstIndex = 0
}

if newThresh := rResult.State.GCThreshold; newThresh != nil {
sm.r.handleGCThresholdResult(ctx, newThresh)
rResult.State.GCThreshold = nil
}

if newVersion := rResult.State.Version; newVersion != nil {
sm.r.handleVersionResult(ctx, newVersion)
rResult.State.Version = nil
Expand Down
4 changes: 0 additions & 4 deletions pkg/kv/kvserver/replica_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8649,10 +8649,6 @@ func TestGCThresholdRacesWithRead(t *testing.T) {

testutils.RunTrueAndFalse(t, "followerRead", func(t *testing.T, followerRead bool) {
testutils.RunTrueAndFalse(t, "thresholdFirst", func(t *testing.T, thresholdFirst bool) {
if !thresholdFirst {
skip.IgnoreLint(t, "the test fails, revealing that it is not safe "+
"to bump the GC threshold and to GC individual keys at the same time")
}

ctx := context.Background()
tc := serverutils.StartNewTestCluster(t, 2, base.TestClusterArgs{
Expand Down

0 comments on commit be23e6f

Please sign in to comment.