Skip to content

Commit

Permalink
kv/closedts/tracker: remove propagation of synthetic timestamp bit
Browse files Browse the repository at this point in the history
Informs #101938.

This commit removes the handling of synthetic timestamps from the closed
timestamp tracker data structure.

This flag has been deprecated since v22.2 and is no longer consulted in
uncertainty interval checks or by transaction commit-wait. It does not
need to be propagated from evaluating requests to the closed timestamp.

Release note: None
  • Loading branch information
nvanbenschoten committed Dec 27, 2023
1 parent 28d6800 commit fc45b31
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 44 deletions.
33 changes: 7 additions & 26 deletions pkg/kv/kvserver/closedts/tracker/lockfree_tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,13 +156,13 @@ func (t *lockfreeTracker) Track(ctx context.Context, ts hlc.Timestamp) RemovalTo
// b1 and the rest (the "high" ones) go on to create and join b2). But that's
// harder to implement.
if !initialized || wts <= t1 {
return b1.extendAndJoin(ctx, wts, ts.Synthetic)
return b1.extendAndJoin(ctx, wts)
}

// We know that b1 < wts. We can technically join either bucket, but we always
// prefer b2 in order to let b1 drain as soon as possible (at which point
// we'll be able to create a new bucket).
return b2.extendAndJoin(ctx, wts, ts.Synthetic)
return b2.extendAndJoin(ctx, wts)
}

// Untrack is part of the Tracker interface.
Expand All @@ -176,7 +176,6 @@ func (t *lockfreeTracker) Untrack(ctx context.Context, tok RemovalToken) {
if b.refcnt == 0 {
// Reset the bucket, so that future Track() calls can create a new one.
b.ts = 0
b.synthetic = 0
// If we reset b1, swap the pointers, so that, if b2 is currently
// initialized, it becomes b1. If a single bucket is initialized, we want it
// to be b1.
Expand All @@ -196,9 +195,8 @@ func (t *lockfreeTracker) LowerBound(ctx context.Context) hlc.Timestamp {
return hlc.Timestamp{}
}
return hlc.Timestamp{
WallTime: ts,
Logical: 0,
Synthetic: t.b1.isSynthetic(),
WallTime: ts,
Logical: 0,
}
}

Expand All @@ -213,9 +211,8 @@ func (t *lockfreeTracker) Count() int {
// A bucket can be initialized or uninitialized. It's initialized when the ts is
// set.
type bucket struct {
ts int64 // atomic, nanos
refcnt int32 // atomic
synthetic int32 // atomic
ts int64 // atomic, nanos
refcnt int32 // atomic
}

func (b *bucket) String() string {
Expand All @@ -234,18 +231,12 @@ func (b *bucket) timestamp() (int64, bool) {
return ts, ts != 0
}

// isSynthetic returns true if the bucket's timestamp (i.e. the bucket's lower
// bound) should be considered a synthetic timestamp.
func (b *bucket) isSynthetic() bool {
return atomic.LoadInt32(&b.synthetic) != 0
}

// extendAndJoin extends the bucket downwards (if necessary) so that its
// timestamp is <= ts, and then adds a timestamp to the bucket. It returns a
// token to be used for removing the timestamp from the bucket.
//
// If the bucket it not initialized, it will be initialized to ts.
func (b *bucket) extendAndJoin(ctx context.Context, ts int64, synthetic bool) lockfreeToken {
func (b *bucket) extendAndJoin(ctx context.Context, ts int64) lockfreeToken {
// Loop until either we set the bucket's timestamp, or someone else sets it to
// an even lower value.
var t int64
Expand All @@ -258,16 +249,6 @@ func (b *bucket) extendAndJoin(ctx context.Context, ts int64, synthetic bool) lo
break
}
}
// If we created the bucket, then we dictate if its lower bound will be
// considered a synthetic timestamp or not. It's possible that we're now
// inserting a synthetic timestamp into the bucket but, over time, a higher
// non-synthetic timestamp joins. Or, that a lower non-synthetic timestamp
// joins. In either case, the bucket will remain "synthetic" although it'd be
// correct to make it non-synthetic. We don't make an effort to keep the
// synthetic bit up to date within a bucket.
if t == 0 && synthetic {
atomic.StoreInt32(&b.synthetic, 1)
}
atomic.AddInt32(&b.refcnt, 1)
return lockfreeToken{b: b}
}
Expand Down
5 changes: 0 additions & 5 deletions pkg/kv/kvserver/closedts/tracker/tracker.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,6 @@ type Tracker interface {
// The returned timestamp might be smaller than the lowest timestamp ever
// inserted into the set. Implementations are allowed to round timestamps
// down.
//
// Synthetic timestamps: The Tracker doesn't necessarily track synthetic /
// physical timestamps precisely; the only guarantee implementations need to
// make is that, if no synthethic timestamp is inserted into the tracked set
// for a while, eventually the LowerBound value will not be synthetic.
LowerBound(context.Context) hlc.Timestamp

// Count returns the current size of the tracked set.
Expand Down
13 changes: 0 additions & 13 deletions pkg/kv/kvserver/closedts/tracker/tracker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,19 +79,6 @@ func testTracker(ctx context.Context, t *testing.T, tr Tracker) {
}
tr.Untrack(ctx, tok30)
require.True(t, tr.LowerBound(ctx).IsEmpty())

// Check that synthetic timestamps are tracked as such.
synthTS := hlc.Timestamp{
WallTime: 10,
Synthetic: true,
}
tok := tr.Track(ctx, synthTS)
require.Equal(t, synthTS, tr.LowerBound(ctx))
// Check that after the Tracker is emptied, lowerbounds are not synthetic any
// more.
tr.Untrack(ctx, tok)
tr.Track(ctx, ts(10))
require.Equal(t, ts(10), tr.LowerBound(ctx))
}

// Test the tracker by throwing random requests at it. We verify that, at all
Expand Down

0 comments on commit fc45b31

Please sign in to comment.