Skip to content

Commit

Permalink
kvserver: don't GC if protected timestamp information isn't available
Browse files Browse the repository at this point in the history
We only want to run GC on a replica that some PTS information (even if
it's stale). We don't want to run GC on a replica if no PTS information
is available however. This can happen if a Replica is being considered
for GC before the initial scan of the KVSubscriber has completed.

This wasn't an issue before this patch for implicit reasons -- this
patch just makes the check explicit and adds a test. Previously, we
wouldn't run GC if no PTS information was available because our lease
was guaranteed to be newer than the empty timestamp.

Release note: None
  • Loading branch information
arulajmani committed Mar 23, 2022
1 parent e1e6f39 commit 101aef7
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 0 deletions.
11 changes: 11 additions & 0 deletions pkg/kv/kvserver/replica_protected_timestamp.go
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,17 @@ func (r *Replica) checkProtectedTimestampsForGC(
if err != nil {
return false, hlc.Timestamp{}, hlc.Timestamp{}, hlc.Timestamp{}, hlc.Timestamp{}, err
}

if read.readAt.IsEmpty() {
// We don't want to allow GC to proceed if no protected timestamp
// information is available. This can happen if the initial scan of the
// rangefeed established by the spanconfig.KVSubscriber hasn't completed
// yet.
log.VEventf(ctx, 1,
"not gc'ing replica %v because protected timestamp information is unavailable", r)
return false, hlc.Timestamp{}, hlc.Timestamp{}, hlc.Timestamp{}, hlc.Timestamp{}, nil
}

gcTimestamp = read.readAt
if !read.earliestProtectionTimestamp.IsEmpty() {
// NB: we want to allow GC up to the timestamp preceding the earliest valid
Expand Down
10 changes: 10 additions & 0 deletions pkg/kv/kvserver/replica_protected_timestamp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,16 @@ func TestCheckProtectedTimestampsForGC(t *testing.T) {
require.Zero(t, gcTimestamp)
},
},
{
name: "no PTS information is available",
test: func(t *testing.T, r *Replica, mp *manualPTSReader) {
mp.asOf = hlc.Timestamp{}
canGC, _, gcTimestamp, _, _, err := r.checkProtectedTimestampsForGC(ctx, makeTTLDuration(10))
require.NoError(t, err)
require.False(t, canGC)
require.Zero(t, gcTimestamp)
},
},
{
name: "have overlapping but new enough that it's okay",
test: func(t *testing.T, r *Replica, mp *manualPTSReader) {
Expand Down

0 comments on commit 101aef7

Please sign in to comment.