From 086903ec5af4137e311ad5f382bebda407ac0d13 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Tue, 29 Dec 2020 00:13:59 -0500 Subject: [PATCH] hlc: replace Timestamp.flags bit field with synthetic boolean This commit replaces Timestamp's (and LegacyTimestamp's) `flags` field with a single boolean field called `synthetic`. This shaves a byte off the encoded message when the field is set and makes synthetic timestamps easier to work with. If we ever decide that we need to go back on this because we need more flags (unlikely), we can without breaking anything. The commit also starts the process of updating tests to handle the fact that `Timestamp.Add` should set the synthetic flag. However, it stops before completing the change because it turned out to be fairly large and not a pre-requisite for non-blocking transactions. --- pkg/ccl/changefeedccl/kvfeed/buffer.go | 4 - pkg/ccl/changefeedccl/sink.go | 2 +- .../kvcoord/dist_sender_server_test.go | 2 +- .../kvclient/kvcoord/txn_coord_sender_test.go | 4 +- .../txn_interceptor_span_refresher_test.go | 2 +- pkg/kv/kvserver/below_raft_protos_test.go | 14 +- pkg/kv/kvserver/client_replica_test.go | 112 ++++++++++- .../kvserver/rangefeed/resolved_timestamp.go | 2 +- .../kvserver/rditer/replica_data_iter_test.go | 4 +- pkg/kv/kvserver/store_test.go | 4 +- pkg/kv/kvserver/tscache/cache_test.go | 2 +- pkg/roachpb/data_test.go | 40 ++-- pkg/roachpb/string_test.go | 4 +- pkg/storage/batch.go | 20 +- pkg/storage/batch_test.go | 2 +- pkg/storage/engine_key.go | 16 +- pkg/storage/engine_key_test.go | 8 +- pkg/storage/enginepb/decode.go | 2 +- pkg/storage/mvcc.go | 10 +- pkg/storage/mvcc_test.go | 4 +- pkg/util/hlc/hlc_test.go | 2 +- pkg/util/hlc/legacy_timestamp.pb.go | 83 ++++---- pkg/util/hlc/legacy_timestamp.proto | 20 +- pkg/util/hlc/timestamp.go | 179 +++++++----------- pkg/util/hlc/timestamp.pb.go | 123 +++++------- pkg/util/hlc/timestamp.proto | 57 ++---- pkg/util/hlc/timestamp_test.go | 153 ++++++--------- 27 files changed, 435 insertions(+), 440 deletions(-) diff --git a/pkg/ccl/changefeedccl/kvfeed/buffer.go b/pkg/ccl/changefeedccl/kvfeed/buffer.go index 638569b5b4db..f3510f0d4e0a 100644 --- a/pkg/ccl/changefeedccl/kvfeed/buffer.go +++ b/pkg/ccl/changefeedccl/kvfeed/buffer.go @@ -211,7 +211,6 @@ var memBufferColTypes = []*types.T{ types.Bytes, // span.EndKey types.Int, // ts.WallTime types.Int, // ts.Logical - types.Int, // ts.Flags } // memBuffer is an in-memory buffer for changed KV and Resolved timestamp @@ -267,7 +266,6 @@ func (b *memBuffer) AddKV( tree.DNull, b.allocMu.a.NewDInt(tree.DInt(kv.Value.Timestamp.WallTime)), b.allocMu.a.NewDInt(tree.DInt(kv.Value.Timestamp.Logical)), - b.allocMu.a.NewDInt(tree.DInt(kv.Value.Timestamp.Flags)), } b.allocMu.Unlock() return b.addRow(ctx, row) @@ -286,7 +284,6 @@ func (b *memBuffer) AddResolved( b.allocMu.a.NewDBytes(tree.DBytes(span.EndKey)), b.allocMu.a.NewDInt(tree.DInt(ts.WallTime)), b.allocMu.a.NewDInt(tree.DInt(ts.Logical)), - b.allocMu.a.NewDInt(tree.DInt(ts.Flags)), } b.allocMu.Unlock() return b.addRow(ctx, row) @@ -303,7 +300,6 @@ func (b *memBuffer) Get(ctx context.Context) (Event, error) { ts := hlc.Timestamp{ WallTime: int64(*row[5].(*tree.DInt)), Logical: int32(*row[6].(*tree.DInt)), - Flags: uint32(*row[7].(*tree.DInt)), } if row[2] != tree.DNull { e.prevVal = roachpb.Value{ diff --git a/pkg/ccl/changefeedccl/sink.go b/pkg/ccl/changefeedccl/sink.go index b14c6e5f0a7e..b5904005cda2 100644 --- a/pkg/ccl/changefeedccl/sink.go +++ b/pkg/ccl/changefeedccl/sink.go @@ -822,7 +822,7 @@ func (s *bufferSink) EmitRow( {Datum: tree.DNull}, // resolved span {Datum: s.alloc.NewDString(tree.DString(topic))}, // topic {Datum: s.alloc.NewDBytes(tree.DBytes(key))}, // key - {Datum: s.alloc.NewDBytes(tree.DBytes(value))}, //value + {Datum: s.alloc.NewDBytes(tree.DBytes(value))}, // value }) return nil } diff --git a/pkg/kv/kvclient/kvcoord/dist_sender_server_test.go b/pkg/kv/kvclient/kvcoord/dist_sender_server_test.go index 6a0c893098bb..177ff8e548b1 100644 --- a/pkg/kv/kvclient/kvcoord/dist_sender_server_test.go +++ b/pkg/kv/kvclient/kvcoord/dist_sender_server_test.go @@ -1190,7 +1190,7 @@ func TestMultiRangeScanReverseScanInconsistent(t *testing.T) { t.Fatal(err) } ts[i] = s.Clock().Now() - log.Infof(ctx, "%d: %s %d", i, key, ts[i]) + log.Infof(ctx, "%d: %s %s", i, key, ts[i]) if i == 0 { testutils.SucceedsSoon(t, func() error { // Enforce that when we write the second key, it's written diff --git a/pkg/kv/kvclient/kvcoord/txn_coord_sender_test.go b/pkg/kv/kvclient/kvcoord/txn_coord_sender_test.go index 0eff49ec39fe..14fe94f00e41 100644 --- a/pkg/kv/kvclient/kvcoord/txn_coord_sender_test.go +++ b/pkg/kv/kvclient/kvcoord/txn_coord_sender_test.go @@ -697,8 +697,8 @@ func TestTxnCoordSenderTxnUpdatedOnError(t *testing.T) { defer log.Scope(t).Close(t) ctx := context.Background() origTS := makeTS(123, 0) - plus10 := origTS.Add(10, 10) - plus20 := origTS.Add(20, 0) + plus10 := origTS.Add(10, 10).SetSynthetic(false) + plus20 := origTS.Add(20, 0).SetSynthetic(false) testCases := []struct { // The test's name. name string diff --git a/pkg/kv/kvclient/kvcoord/txn_interceptor_span_refresher_test.go b/pkg/kv/kvclient/kvcoord/txn_interceptor_span_refresher_test.go index f851b1d39ff5..57831025b6b6 100644 --- a/pkg/kv/kvclient/kvcoord/txn_interceptor_span_refresher_test.go +++ b/pkg/kv/kvclient/kvcoord/txn_interceptor_span_refresher_test.go @@ -164,7 +164,7 @@ func TestTxnSpanRefresherRefreshesTransactions(t *testing.T) { return pErr }, expRefresh: true, - expRefreshTS: txn.WriteTimestamp.Add(20, 0), // see UpdateObservedTimestamp + expRefreshTS: txn.WriteTimestamp.Add(20, 0).SetSynthetic(false), // see UpdateObservedTimestamp }, { pErr: func() *roachpb.Error { diff --git a/pkg/kv/kvserver/below_raft_protos_test.go b/pkg/kv/kvserver/below_raft_protos_test.go index f7e29f139e64..53ec62441351 100644 --- a/pkg/kv/kvserver/below_raft_protos_test.go +++ b/pkg/kv/kvserver/below_raft_protos_test.go @@ -63,11 +63,15 @@ var belowRaftGoldenProtos = map[reflect.Type]fixture{ populatedConstructor: func(r *rand.Rand) protoutil.Message { m := enginepb.NewPopulatedMVCCMetadata(r, false) m.Txn = nil // never populated below Raft + m.Timestamp.Synthetic = nil // never populated below Raft + if m.MergeTimestamp != nil { + m.MergeTimestamp.Synthetic = nil // never populated below Raft + } m.TxnDidNotUpdateMeta = nil // never populated below Raft return m }, emptySum: 7551962144604783939, - populatedSum: 11599955036265189084, + populatedSum: 12366000535951165621, }, reflect.TypeOf(&enginepb.RangeAppliedState{}): { populatedConstructor: func(r *rand.Rand) protoutil.Message { @@ -124,10 +128,14 @@ var belowRaftGoldenProtos = map[reflect.Type]fixture{ }, reflect.TypeOf(&enginepb.MVCCMetadataSubsetForMergeSerialization{}): { populatedConstructor: func(r *rand.Rand) protoutil.Message { - return enginepb.NewPopulatedMVCCMetadataSubsetForMergeSerialization(r, false) + m := enginepb.NewPopulatedMVCCMetadataSubsetForMergeSerialization(r, false) + if m.MergeTimestamp != nil { + m.MergeTimestamp.Synthetic = nil // never populated below Raft + } + return m }, emptySum: 14695981039346656037, - populatedSum: 834545685817460463, + populatedSum: 6109178572734990978, }, } diff --git a/pkg/kv/kvserver/client_replica_test.go b/pkg/kv/kvserver/client_replica_test.go index 44b89fd8b469..28104c846643 100644 --- a/pkg/kv/kvserver/client_replica_test.go +++ b/pkg/kv/kvserver/client_replica_test.go @@ -60,10 +60,103 @@ import ( "go.etcd.io/etcd/raft/v3/raftpb" ) -// TestRangeCommandClockUpdate verifies that followers update their -// clocks when executing a command, even if the lease holder's clock is far -// in the future. -func TestRangeCommandClockUpdate(t *testing.T) { +// TestReplicaClockUpdates verifies that the leaseholder and followers both +// update their clocks when executing a command to the command's timestamp, as +// long as the request timestamp is from a clock (i.e. is not synthetic). +func TestReplicaClockUpdates(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + + run := func(t *testing.T, write bool, synthetic bool) { + const numNodes = 3 + const maxOffset = 100 * time.Millisecond + var manuals []*hlc.ManualClock + var clocks []*hlc.Clock + for i := 0; i < numNodes; i++ { + manuals = append(manuals, hlc.NewManualClock(1)) + clocks = append(clocks, hlc.NewClock(manuals[i].UnixNano, maxOffset)) + } + ctx := context.Background() + cfg := kvserver.TestStoreConfig(nil) + cfg.TestingKnobs.DisableReplicateQueue = true + cfg.Clock = nil + mtc := &multiTestContext{ + storeConfig: &cfg, + clocks: clocks, + // This test was written before the multiTestContext started creating many + // system ranges at startup, and hasn't been update to take that into + // account. + startWithSingleRange: true, + } + defer mtc.Stop() + mtc.Start(t, numNodes) + mtc.replicateRange(1, 1, 2) + + // Pick a timestamp in the future of all nodes by less than the + // MaxOffset. Set the synthetic flag according to the test case. + reqTS := clocks[0].Now().Add(int64(maxOffset/2), 0).SetSynthetic(synthetic) + h := roachpb.Header{Timestamp: reqTS} + + // Execute the command. + var req roachpb.Request + reqKey := roachpb.Key("a") + if write { + req = incrementArgs(reqKey, 5) + } else { + req = getArgs(reqKey) + } + if _, err := kv.SendWrappedWith(ctx, mtc.stores[0].TestSender(), h, req); err != nil { + t.Fatal(err) + } + + // If writing, wait for that command to execute on all the replicas. + // Consensus is asynchronous outside of the majority quorum, and Raft + // application is asynchronous on all nodes. + if write { + testutils.SucceedsSoon(t, func() error { + var values []int64 + for _, eng := range mtc.engines { + val, _, err := storage.MVCCGet(ctx, eng, reqKey, reqTS, storage.MVCCGetOptions{}) + if err != nil { + return err + } + values = append(values, mustGetInt(val)) + } + if !reflect.DeepEqual(values, []int64{5, 5, 5}) { + return errors.Errorf("expected (5, 5, 5), got %v", values) + } + return nil + }) + } + + // Verify that clocks were updated as expected. Check all clocks if we + // issued a write, but only the leaseholder's if we issued a read. In + // theory, we should be able to assert that _only_ the leaseholder's + // clock is updated by a read, but in practice an assertion against + // followers' clocks being updated is very difficult to make without + // being flaky because it's difficult to prevent other channels + // (background work, etc.) from carrying the clock update. + expUpdated := !synthetic + clocksToCheck := clocks + if !write { + clocksToCheck = clocks[:1] + } + for _, c := range clocksToCheck { + require.Equal(t, expUpdated, reqTS.Less(c.Now())) + } + } + + testutils.RunTrueAndFalse(t, "write", func(t *testing.T, write bool) { + testutils.RunTrueAndFalse(t, "synthetic", func(t *testing.T, synthetic bool) { + run(t, write, synthetic) + }) + }) +} + +// TestFollowersDontRejectClockUpdateWithJump verifies that followers update +// their clocks when executing a command, even if the leaseholder's clock is +// far in the future. +func TestFollowersDontRejectClockUpdateWithJump(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) @@ -126,9 +219,9 @@ func TestRangeCommandClockUpdate(t *testing.T) { } } -// TestRejectFutureCommand verifies that lease holders reject commands that -// would cause a large time jump. -func TestRejectFutureCommand(t *testing.T) { +// TestLeaseholdersRejectClockUpdateWithJump verifies that leaseholders reject +// commands that would cause a large time jump. +func TestLeaseholdersRejectClockUpdateWithJump(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) @@ -149,7 +242,7 @@ func TestRejectFutureCommand(t *testing.T) { const numCmds = 3 clockOffset := clock.MaxOffset() / numCmds for i := int64(1); i <= numCmds; i++ { - ts := ts1.Add(i*clockOffset.Nanoseconds(), 0) + ts := ts1.Add(i*clockOffset.Nanoseconds(), 0).SetSynthetic(false) if _, err := kv.SendWrappedWith(context.Background(), mtc.stores[0].TestSender(), roachpb.Header{Timestamp: ts}, incArgs); err != nil { t.Fatal(err) } @@ -161,7 +254,8 @@ func TestRejectFutureCommand(t *testing.T) { } // Once the accumulated offset reaches MaxOffset, commands will be rejected. - _, pErr := kv.SendWrappedWith(context.Background(), mtc.stores[0].TestSender(), roachpb.Header{Timestamp: ts1.Add(clock.MaxOffset().Nanoseconds()+1, 0)}, incArgs) + tsFuture := ts1.Add(clock.MaxOffset().Nanoseconds()+1, 0).SetSynthetic(false) + _, pErr := kv.SendWrappedWith(context.Background(), mtc.stores[0].TestSender(), roachpb.Header{Timestamp: tsFuture}, incArgs) if !testutils.IsPError(pErr, "remote wall time is too far ahead") { t.Fatalf("unexpected error %v", pErr) } diff --git a/pkg/kv/kvserver/rangefeed/resolved_timestamp.go b/pkg/kv/kvserver/rangefeed/resolved_timestamp.go index 120cab958357..22f22a25a57f 100644 --- a/pkg/kv/kvserver/rangefeed/resolved_timestamp.go +++ b/pkg/kv/kvserver/rangefeed/resolved_timestamp.go @@ -237,7 +237,7 @@ func (rts *resolvedTimestamp) recompute() bool { func (rts *resolvedTimestamp) assertNoChange() { before := rts.resolvedTS changed := rts.recompute() - if changed || (before != rts.resolvedTS) { + if changed || !before.EqOrdering(rts.resolvedTS) { panic(fmt.Sprintf("unexpected resolved timestamp change on recomputation, "+ "was %s, recomputed as %s", before, rts.resolvedTS)) } diff --git a/pkg/kv/kvserver/rditer/replica_data_iter_test.go b/pkg/kv/kvserver/rditer/replica_data_iter_test.go index 806809427fa2..84163a137bb3 100644 --- a/pkg/kv/kvserver/rditer/replica_data_iter_test.go +++ b/pkg/kv/kvserver/rditer/replica_data_iter_test.go @@ -170,7 +170,7 @@ func verifyRDReplicatedOnlyMVCCIter( if key := iter.Key(); !key.Equal(expectedKeys[i]) { k1, ts1 := key.Key, key.Timestamp k2, ts2 := expectedKeys[i].Key, expectedKeys[i].Timestamp - t.Errorf("%d: expected %q(%d); got %q(%d)", i, k2, ts2, k1, ts1) + t.Errorf("%d: expected %q(%s); got %q(%s)", i, k2, ts2, k1, ts1) } if reverse { i-- @@ -220,7 +220,7 @@ func verifyRDEngineIter( if !k.Equal(expectedKeys[i]) { k1, ts1 := k.Key, k.Timestamp k2, ts2 := expectedKeys[i].Key, expectedKeys[i].Timestamp - t.Errorf("%d: expected %q(%d); got %q(%d)", i, k2, ts2, k1, ts1) + t.Errorf("%d: expected %q(%s); got %q(%s)", i, k2, ts2, k1, ts1) } i++ iter.Next() diff --git a/pkg/kv/kvserver/store_test.go b/pkg/kv/kvserver/store_test.go index b7c74665bb68..6a2259a77718 100644 --- a/pkg/kv/kvserver/store_test.go +++ b/pkg/kv/kvserver/store_test.go @@ -1340,7 +1340,7 @@ func TestStoreSendUpdateTime(t *testing.T) { defer stopper.Stop(context.Background()) store, _ := createTestStore(t, testStoreOpts{createSystemRanges: true}, stopper) args := getArgs([]byte("a")) - reqTS := store.cfg.Clock.Now().Add(store.cfg.Clock.MaxOffset().Nanoseconds(), 0) + reqTS := store.cfg.Clock.Now().Add(store.cfg.Clock.MaxOffset().Nanoseconds(), 0).SetSynthetic(false) _, pErr := kv.SendWrappedWith(context.Background(), store.TestSender(), roachpb.Header{Timestamp: reqTS}, &args) if pErr != nil { t.Fatal(pErr) @@ -1386,7 +1386,7 @@ func TestStoreSendWithClockOffset(t *testing.T) { store, _ := createTestStore(t, testStoreOpts{createSystemRanges: true}, stopper) args := getArgs([]byte("a")) // Set args timestamp to exceed max offset. - reqTS := store.cfg.Clock.Now().Add(store.cfg.Clock.MaxOffset().Nanoseconds()+1, 0) + reqTS := store.cfg.Clock.Now().Add(store.cfg.Clock.MaxOffset().Nanoseconds()+1, 0).SetSynthetic(false) _, pErr := kv.SendWrappedWith(context.Background(), store.TestSender(), roachpb.Header{Timestamp: reqTS}, &args) if !testutils.IsPError(pErr, "remote wall time is too far ahead") { t.Errorf("unexpected error: %v", pErr) diff --git a/pkg/kv/kvserver/tscache/cache_test.go b/pkg/kv/kvserver/tscache/cache_test.go index 2d7a163b1c87..4bce972bf75a 100644 --- a/pkg/kv/kvserver/tscache/cache_test.go +++ b/pkg/kv/kvserver/tscache/cache_test.go @@ -545,7 +545,7 @@ func TestTimestampCacheImplsIdentical(t *testing.T) { to = nil } - ts := start.Add(int64(j), 100) + ts := start.Add(int64(j), 100).SetSynthetic(false) if useClock { ts = clock.Now() } diff --git a/pkg/roachpb/data_test.go b/pkg/roachpb/data_test.go index b6ae8d733c2e..3dfffeeab5c8 100644 --- a/pkg/roachpb/data_test.go +++ b/pkg/roachpb/data_test.go @@ -47,20 +47,19 @@ func makeClockTS(walltime int64, logical int32) hlc.ClockTimestamp { } } -func makeClockTSWithFlag(walltime int64, logical int32) hlc.ClockTimestamp { - return hlc.ClockTimestamp{ +func makeTS(walltime int64, logical int32) hlc.Timestamp { + return hlc.Timestamp{ WallTime: walltime, Logical: logical, - Flags: uint32(hlc.TimestampFlag_SYNTHETIC), } } -func makeTS(walltime int64, logical int32) hlc.Timestamp { - return makeClockTS(walltime, logical).ToTimestamp() -} - -func makeTSWithFlag(walltime int64, logical int32) hlc.Timestamp { - return makeClockTSWithFlag(walltime, logical).ToTimestamp() +func makeSynTS(walltime int64, logical int32) hlc.Timestamp { + return hlc.Timestamp{ + WallTime: walltime, + Logical: logical, + Synthetic: true, + } } // TestKeyNext tests that the method for creating lexicographic @@ -474,17 +473,24 @@ var nonZeroTxn = Transaction{ Key: Key("foo"), ID: uuid.MakeV4(), Epoch: 2, - WriteTimestamp: makeTSWithFlag(20, 21), - MinTimestamp: makeTSWithFlag(10, 11), + WriteTimestamp: makeSynTS(20, 21), + MinTimestamp: makeSynTS(10, 11), Priority: 957356782, Sequence: 123, }, - Name: "name", - Status: COMMITTED, - LastHeartbeat: makeTSWithFlag(1, 2), - ReadTimestamp: makeTSWithFlag(20, 22), - MaxTimestamp: makeTSWithFlag(40, 41), - ObservedTimestamps: []ObservedTimestamp{{NodeID: 1, Timestamp: makeClockTSWithFlag(1, 2)}}, + Name: "name", + Status: COMMITTED, + LastHeartbeat: makeSynTS(1, 2), + ReadTimestamp: makeSynTS(20, 22), + MaxTimestamp: makeSynTS(40, 41), + ObservedTimestamps: []ObservedTimestamp{{ + NodeID: 1, + Timestamp: hlc.ClockTimestamp{ + WallTime: 1, + Logical: 2, + Synthetic: true, // normally not set, but needed for zerofields.NoZeroField + }, + }}, WriteTooOld: true, LockSpans: []Span{{Key: []byte("a"), EndKey: []byte("b")}}, InFlightWrites: []SequencedWrite{{Key: []byte("c"), Sequence: 1}}, diff --git a/pkg/roachpb/string_test.go b/pkg/roachpb/string_test.go index 6f7305104279..f48b1dc6ae36 100644 --- a/pkg/roachpb/string_test.go +++ b/pkg/roachpb/string_test.go @@ -43,10 +43,10 @@ func TestTransactionString(t *testing.T) { Status: roachpb.COMMITTED, LastHeartbeat: hlc.Timestamp{WallTime: 10, Logical: 11}, ReadTimestamp: hlc.Timestamp{WallTime: 30, Logical: 31}, - MaxTimestamp: hlc.Timestamp{WallTime: 40, Logical: 41}, + MaxTimestamp: hlc.Timestamp{WallTime: 40, Logical: 41, Synthetic: true}, } expStr := `"name" meta={id=d7aa0f5e key="foo" pri=44.58039917 epo=2 ts=0.000000020,21 min=0.000000010,11 seq=15}` + - ` lock=true stat=COMMITTED rts=0.000000030,31 wto=false max=0.000000040,41` + ` lock=true stat=COMMITTED rts=0.000000030,31 wto=false max=0.000000040,41?` if str := txn.String(); str != expStr { t.Errorf( diff --git a/pkg/storage/batch.go b/pkg/storage/batch.go index 0d75f023315d..59296ff21288 100644 --- a/pkg/storage/batch.go +++ b/pkg/storage/batch.go @@ -77,12 +77,12 @@ const ( // The keys encoded into the batch are MVCC keys: a string key with a timestamp // suffix. MVCC keys are encoded as: // -// [[[]]]<#timestamp-bytes> +// [[[]]]<#timestamp-bytes> // -// The , , and portions of the key are encoded as -// 64-bit, 32-bit, and 8-bit big-endian integers, respectively. A custom RocksDB -// comparator is used to maintain the desired ordering as these keys do not sort -// lexicographically correctly. +// The , , and portions of the key are encoded +// as 64-bit, 32-bit, and 8-bit big-endian integers, respectively. A custom +// RocksDB comparator is used to maintain the desired ordering as these keys do +// not sort lexicographically correctly. // // TODO(bilal): This struct exists mostly as a historic artifact. Transition the // remaining few test uses of this struct over to pebble.Batch, and remove it @@ -145,7 +145,7 @@ func encodeKeyToBuf(buf []byte, key MVCCKey, keyLen int) { timestampSentinelLen = 1 walltimeEncodedLen = 8 logicalEncodedLen = 4 - flagsEncodedLen = 1 + syntheticEncodedLen = 1 ) copy(buf, key.Key) @@ -157,13 +157,13 @@ func encodeKeyToBuf(buf []byte, key MVCCKey, keyLen int) { pos += timestampSentinelLen binary.BigEndian.PutUint64(buf[pos:], uint64(key.Timestamp.WallTime)) pos += walltimeEncodedLen - if key.Timestamp.Logical != 0 || key.Timestamp.Flags != 0 { + if key.Timestamp.Logical != 0 || key.Timestamp.Synthetic { binary.BigEndian.PutUint32(buf[pos:], uint32(key.Timestamp.Logical)) pos += logicalEncodedLen } - if key.Timestamp.Flags != 0 { - buf[pos] = uint8(key.Timestamp.Flags) - pos += flagsEncodedLen + if key.Timestamp.Synthetic { + buf[pos] = 1 + pos += syntheticEncodedLen } } buf[len(buf)-1] = byte(timestampLength) diff --git a/pkg/storage/batch_test.go b/pkg/storage/batch_test.go index 961ae1a1b0b9..2fc359164db4 100644 --- a/pkg/storage/batch_test.go +++ b/pkg/storage/batch_test.go @@ -1207,7 +1207,7 @@ func TestDecodeKey(t *testing.T) { {Key: []byte("foo")}, {Key: []byte("foo"), Timestamp: hlc.Timestamp{WallTime: 1}}, {Key: []byte("foo"), Timestamp: hlc.Timestamp{WallTime: 1, Logical: 1}}, - {Key: []byte("foo"), Timestamp: hlc.Timestamp{WallTime: 1, Logical: 1, Flags: 3}}, + {Key: []byte("foo"), Timestamp: hlc.Timestamp{WallTime: 1, Logical: 1, Synthetic: true}}, } for _, test := range tests { t.Run(test.String(), func(t *testing.T) { diff --git a/pkg/storage/engine_key.go b/pkg/storage/engine_key.go index c5db58045770..b851f6b7180f 100644 --- a/pkg/storage/engine_key.go +++ b/pkg/storage/engine_key.go @@ -39,11 +39,11 @@ type EngineKey struct { } const ( - engineKeyNoVersion = 0 - engineKeyVersionWallTimeLen = 8 - engineKeyVersionWallAndLogicalTimeLen = 12 - engineKeyVersionWallLogicalAndFlagsTimeLen = 13 - engineKeyVersionLockTableLen = 17 + engineKeyNoVersion = 0 + engineKeyVersionWallTimeLen = 8 + engineKeyVersionWallAndLogicalTimeLen = 12 + engineKeyVersionWallLogicalAndSyntheticTimeLen = 13 + engineKeyVersionLockTableLen = 17 ) // Format implements the fmt.Formatter interface @@ -131,7 +131,7 @@ func (k EngineKey) IsMVCCKey() bool { return l == engineKeyNoVersion || l == engineKeyVersionWallTimeLen || l == engineKeyVersionWallAndLogicalTimeLen || - l == engineKeyVersionWallLogicalAndFlagsTimeLen + l == engineKeyVersionWallLogicalAndSyntheticTimeLen } // IsLockTableKey returns true if the key can be decoded as a LockTableKey. @@ -150,10 +150,10 @@ func (k EngineKey) ToMVCCKey() (MVCCKey, error) { case engineKeyVersionWallAndLogicalTimeLen: key.Timestamp.WallTime = int64(binary.BigEndian.Uint64(k.Version[0:8])) key.Timestamp.Logical = int32(binary.BigEndian.Uint32(k.Version[8:12])) - case engineKeyVersionWallLogicalAndFlagsTimeLen: + case engineKeyVersionWallLogicalAndSyntheticTimeLen: key.Timestamp.WallTime = int64(binary.BigEndian.Uint64(k.Version[0:8])) key.Timestamp.Logical = int32(binary.BigEndian.Uint32(k.Version[8:12])) - key.Timestamp.Flags = uint32(k.Version[12]) + key.Timestamp.Synthetic = k.Version[12] != 0 default: return MVCCKey{}, errors.Errorf("version is not an encoded timestamp %x", k.Version) } diff --git a/pkg/storage/engine_key_test.go b/pkg/storage/engine_key_test.go index d6108c664429..3a68a2bfcb5d 100644 --- a/pkg/storage/engine_key_test.go +++ b/pkg/storage/engine_key_test.go @@ -87,14 +87,14 @@ func TestMVCCAndEngineKeyEncodeDecode(t *testing.T) { {key: MVCCKey{Key: roachpb.Key("a")}}, {key: MVCCKey{Key: roachpb.Key("glue"), Timestamp: hlc.Timestamp{WallTime: 89999}}}, {key: MVCCKey{Key: roachpb.Key("foo"), Timestamp: hlc.Timestamp{WallTime: 99, Logical: 45}}}, - {key: MVCCKey{Key: roachpb.Key("flags"), Timestamp: hlc.Timestamp{WallTime: 99, Logical: 45, Flags: 3}}}, + {key: MVCCKey{Key: roachpb.Key("bar"), Timestamp: hlc.Timestamp{WallTime: 99, Logical: 45, Synthetic: true}}}, } for _, test := range testCases { t.Run("", func(t *testing.T) { var encodedTS []byte if !test.key.Timestamp.IsEmpty() { var size int - if test.key.Timestamp.Flags != 0 { + if test.key.Timestamp.Synthetic { size = 13 } else if test.key.Timestamp.Logical != 0 { size = 12 @@ -106,8 +106,8 @@ func TestMVCCAndEngineKeyEncodeDecode(t *testing.T) { if test.key.Timestamp.Logical != 0 { binary.BigEndian.PutUint32(encodedTS[8:], uint32(test.key.Timestamp.Logical)) } - if test.key.Timestamp.Flags != 0 { - encodedTS[12] = uint8(test.key.Timestamp.Flags) + if test.key.Timestamp.Synthetic { + encodedTS[12] = 1 } } eKey := EngineKey{Key: test.key.Key, Version: encodedTS} diff --git a/pkg/storage/enginepb/decode.go b/pkg/storage/enginepb/decode.go index 0e321ecd7960..5857ed34f270 100644 --- a/pkg/storage/enginepb/decode.go +++ b/pkg/storage/enginepb/decode.go @@ -59,7 +59,7 @@ func DecodeKey(encodedKey []byte) (key []byte, timestamp hlc.Timestamp, _ error) case 13: timestamp.WallTime = int64(binary.BigEndian.Uint64(ts[0:8])) timestamp.Logical = int32(binary.BigEndian.Uint32(ts[8:12])) - timestamp.Flags = uint32(ts[12]) + timestamp.Synthetic = ts[12] != 0 default: return nil, timestamp, errors.Errorf( "invalid encoded mvcc key: %x bad timestamp %x", encodedKey, ts) diff --git a/pkg/storage/mvcc.go b/pkg/storage/mvcc.go index b95f0428b69c..5d57be97d8a4 100644 --- a/pkg/storage/mvcc.go +++ b/pkg/storage/mvcc.go @@ -173,18 +173,18 @@ func (k MVCCKey) Len() int { timestampSentinelLen = 1 walltimeEncodedLen = 8 logicalEncodedLen = 4 - flagsEncodedLen = 1 + syntheticEncodedLen = 1 timestampEncodedLengthLen = 1 ) n := len(k.Key) + timestampEncodedLengthLen if !k.Timestamp.IsEmpty() { n += timestampSentinelLen + walltimeEncodedLen - if k.Timestamp.Logical != 0 || k.Timestamp.Flags != 0 { + if k.Timestamp.Logical != 0 || k.Timestamp.Synthetic { n += logicalEncodedLen } - if k.Timestamp.Flags != 0 { - n += flagsEncodedLen + if k.Timestamp.Synthetic { + n += syntheticEncodedLen } } return n @@ -1677,7 +1677,7 @@ func mvccPutInternal( txnMeta = &txn.TxnMeta // If we bumped the WriteTimestamp, we update both the TxnMeta and the // MVCCMetadata.Timestamp. - if txnMeta.WriteTimestamp.Less(writeTimestamp) { + if txnMeta.WriteTimestamp != writeTimestamp { txnMetaCpy := *txnMeta txnMetaCpy.WriteTimestamp.Forward(writeTimestamp) txnMeta = &txnMetaCpy diff --git a/pkg/storage/mvcc_test.go b/pkg/storage/mvcc_test.go index 280ff0dd9767..27c421ce3424 100644 --- a/pkg/storage/mvcc_test.go +++ b/pkg/storage/mvcc_test.go @@ -3268,7 +3268,7 @@ func TestMVCCWriteWithDiffTimestampsAndEpochs(t *testing.T) { t.Fatal(err) } - expTS := txne2Commit.WriteTimestamp.Add(0, 1) + expTS := txne2Commit.WriteTimestamp.Next() // Now try writing an earlier value without a txn--should get WriteTooOldError. err := MVCCPut(ctx, engine, nil, testKey1, hlc.Timestamp{Logical: 1}, value4, nil) @@ -3285,7 +3285,7 @@ func TestMVCCWriteWithDiffTimestampsAndEpochs(t *testing.T) { } // Now write an intent with exactly the same timestamp--ties also get WriteTooOldError. err = MVCCPut(ctx, engine, nil, testKey1, txn2.ReadTimestamp, value5, txn2) - intentTS := expTS.Add(0, 1) + intentTS := expTS.Next() if wtoErr := (*roachpb.WriteTooOldError)(nil); !errors.As(err, &wtoErr) { t.Fatal("unexpected success") } else if wtoErr.ActualTimestamp != intentTS { diff --git a/pkg/util/hlc/hlc_test.go b/pkg/util/hlc/hlc_test.go index 4c41ac847f2e..bf2c07b01ffe 100644 --- a/pkg/util/hlc/hlc_test.go +++ b/pkg/util/hlc/hlc_test.go @@ -54,7 +54,7 @@ func ExampleNewClock() { } if t.WallTime-s.WallTime > 0 { - log.Fatalf(context.Background(), "HLC timestamp %d deviates from physical clock %d", s, t) + log.Fatalf(context.Background(), "HLC timestamp %s deviates from physical clock %s", s, t) } if s.Logical > 0 { diff --git a/pkg/util/hlc/legacy_timestamp.pb.go b/pkg/util/hlc/legacy_timestamp.pb.go index 560ee8531492..7def852477af 100644 --- a/pkg/util/hlc/legacy_timestamp.pb.go +++ b/pkg/util/hlc/legacy_timestamp.pb.go @@ -31,25 +31,23 @@ type LegacyTimestamp struct { // skew)/(minimal ns between events) and nearly impossible to // overflow. Logical int32 `protobuf:"varint,2,opt,name=logical" json:"logical"` - // A collection of bit flags that provide details about the timestamp - // and its meaning. The data type is a uint32, but the number of flags - // is limited to 8 so that the flags can be encoded into a single byte. + // Indicates that the Timestamp did not come from an HLC clock somewhere + // in the system and, therefore, does not have has the ability to update + // a peer's HLC clock. If set to true, the "synthetic timestamp" may be + // arbitrarily disconnected from real time. // - // Flags do not affect the sort order of Timestamps. However, they are - // considered when performing structural equality checks (e.g. using the - // == operator). Consider use of the EqOrdering method when testing for - // equality. + // See the commentary on Timestamp.synthetic for more information. // - // The field is nullable so that it is not serialized when no flags are - // set. This ensures that the timestamp encoding does not change across - // nodes that are and are not aware of this field. - Flags *uint32 `protobuf:"varint,3,opt,name=flags" json:"flags,omitempty"` + // The field is nullable so that it is not serialized when set to false. + // This ensures that the timestamp encoding does not change across nodes + // that are and are not aware of this field. + Synthetic *bool `protobuf:"varint,3,opt,name=synthetic" json:"synthetic,omitempty"` } func (m *LegacyTimestamp) Reset() { *m = LegacyTimestamp{} } func (*LegacyTimestamp) ProtoMessage() {} func (*LegacyTimestamp) Descriptor() ([]byte, []int) { - return fileDescriptor_legacy_timestamp_d72283f54eaf58e6, []int{0} + return fileDescriptor_legacy_timestamp_2982002a070840ce, []int{0} } func (m *LegacyTimestamp) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -102,13 +100,13 @@ func (this *LegacyTimestamp) Equal(that interface{}) bool { if this.Logical != that1.Logical { return false } - if this.Flags != nil && that1.Flags != nil { - if *this.Flags != *that1.Flags { + if this.Synthetic != nil && that1.Synthetic != nil { + if *this.Synthetic != *that1.Synthetic { return false } - } else if this.Flags != nil { + } else if this.Synthetic != nil { return false - } else if that1.Flags != nil { + } else if that1.Synthetic != nil { return false } return true @@ -134,10 +132,15 @@ func (m *LegacyTimestamp) MarshalTo(dAtA []byte) (int, error) { dAtA[i] = 0x10 i++ i = encodeVarintLegacyTimestamp(dAtA, i, uint64(m.Logical)) - if m.Flags != nil { + if m.Synthetic != nil { dAtA[i] = 0x18 i++ - i = encodeVarintLegacyTimestamp(dAtA, i, uint64(*m.Flags)) + if *m.Synthetic { + dAtA[i] = 1 + } else { + dAtA[i] = 0 + } + i++ } return i, nil } @@ -162,8 +165,8 @@ func NewPopulatedLegacyTimestamp(r randyLegacyTimestamp, easy bool) *LegacyTimes this.Logical *= -1 } if r.Intn(10) != 0 { - v1 := uint32(r.Uint32()) - this.Flags = &v1 + v1 := bool(bool(r.Intn(2) == 0)) + this.Synthetic = &v1 } if !easy && r.Intn(10) != 0 { } @@ -250,8 +253,8 @@ func (m *LegacyTimestamp) Size() (n int) { _ = l n += 1 + sovLegacyTimestamp(uint64(m.WallTime)) n += 1 + sovLegacyTimestamp(uint64(m.Logical)) - if m.Flags != nil { - n += 1 + sovLegacyTimestamp(uint64(*m.Flags)) + if m.Synthetic != nil { + n += 2 } return n } @@ -338,9 +341,9 @@ func (m *LegacyTimestamp) Unmarshal(dAtA []byte) error { } case 3: if wireType != 0 { - return fmt.Errorf("proto: wrong wireType = %d for field Flags", wireType) + return fmt.Errorf("proto: wrong wireType = %d for field Synthetic", wireType) } - var v uint32 + var v int for shift := uint(0); ; shift += 7 { if shift >= 64 { return ErrIntOverflowLegacyTimestamp @@ -350,12 +353,13 @@ func (m *LegacyTimestamp) Unmarshal(dAtA []byte) error { } b := dAtA[iNdEx] iNdEx++ - v |= (uint32(b) & 0x7F) << shift + v |= (int(b) & 0x7F) << shift if b < 0x80 { break } } - m.Flags = &v + b := bool(v != 0) + m.Synthetic = &b default: iNdEx = preIndex skippy, err := skipLegacyTimestamp(dAtA[iNdEx:]) @@ -483,23 +487,24 @@ var ( ) func init() { - proto.RegisterFile("util/hlc/legacy_timestamp.proto", fileDescriptor_legacy_timestamp_d72283f54eaf58e6) + proto.RegisterFile("util/hlc/legacy_timestamp.proto", fileDescriptor_legacy_timestamp_2982002a070840ce) } -var fileDescriptor_legacy_timestamp_d72283f54eaf58e6 = []byte{ - // 221 bytes of a gzipped FileDescriptorProto +var fileDescriptor_legacy_timestamp_2982002a070840ce = []byte{ + // 226 bytes of a gzipped FileDescriptorProto 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xe2, 0x92, 0x2f, 0x2d, 0xc9, 0xcc, 0xd1, 0xcf, 0xc8, 0x49, 0xd6, 0xcf, 0x49, 0x4d, 0x4f, 0x4c, 0xae, 0x8c, 0x2f, 0xc9, 0xcc, 0x4d, 0x2d, 0x2e, 0x49, 0xcc, 0x2d, 0xd0, 0x2b, 0x28, 0xca, 0x2f, 0xc9, 0x17, 0x12, 0x4a, 0xce, 0x4f, 0xce, 0x2e, 0xca, 0x4f, 0x4c, 0xce, 0xd0, 0x03, 0x29, 0xd5, 0xcb, 0xc8, 0x49, 0x96, 0x12, 0x49, - 0xcf, 0x4f, 0xcf, 0x07, 0x4b, 0xeb, 0x83, 0x58, 0x10, 0x95, 0x4a, 0x15, 0x5c, 0xfc, 0x3e, 0x60, - 0x33, 0x42, 0x60, 0x46, 0x08, 0x29, 0x72, 0x71, 0x96, 0x27, 0xe6, 0xe4, 0x80, 0x0d, 0x95, 0x60, - 0x54, 0x60, 0xd4, 0x60, 0x76, 0x62, 0x39, 0x71, 0x4f, 0x9e, 0x21, 0x88, 0x03, 0x24, 0x0c, 0x52, - 0x27, 0x24, 0xc7, 0xc5, 0x9e, 0x93, 0x9f, 0x9e, 0x99, 0x9c, 0x98, 0x23, 0xc1, 0xa4, 0xc0, 0xa8, - 0xc1, 0x0a, 0x55, 0x00, 0x13, 0x14, 0x12, 0xe1, 0x62, 0x4d, 0xcb, 0x49, 0x4c, 0x2f, 0x96, 0x60, - 0x56, 0x60, 0xd4, 0xe0, 0x0d, 0x82, 0x70, 0xac, 0x78, 0x66, 0x2c, 0x90, 0x67, 0xd8, 0xb1, 0x40, - 0x9e, 0xf1, 0xc5, 0x02, 0x79, 0x46, 0x27, 0xd5, 0x13, 0x0f, 0xe5, 0x18, 0x4e, 0x3c, 0x92, 0x63, - 0xbc, 0xf0, 0x48, 0x8e, 0xf1, 0xc6, 0x23, 0x39, 0xc6, 0x07, 0x8f, 0xe4, 0x18, 0x27, 0x3c, 0x96, - 0x63, 0xb8, 0xf0, 0x58, 0x8e, 0xe1, 0xc6, 0x63, 0x39, 0x86, 0x28, 0xe6, 0x8c, 0x9c, 0x64, 0x40, - 0x00, 0x00, 0x00, 0xff, 0xff, 0x05, 0x5c, 0x3e, 0x65, 0xec, 0x00, 0x00, 0x00, + 0xcf, 0x4f, 0xcf, 0x07, 0x4b, 0xeb, 0x83, 0x58, 0x10, 0x95, 0x4a, 0x0d, 0x8c, 0x5c, 0xfc, 0x3e, + 0x60, 0x43, 0x42, 0x60, 0x66, 0x08, 0x29, 0x72, 0x71, 0x96, 0x27, 0xe6, 0xe4, 0x80, 0x4d, 0x95, + 0x60, 0x54, 0x60, 0xd4, 0x60, 0x76, 0x62, 0x39, 0x71, 0x4f, 0x9e, 0x21, 0x88, 0x03, 0x24, 0x0c, + 0x52, 0x27, 0x24, 0xc7, 0xc5, 0x9e, 0x93, 0x9f, 0x9e, 0x99, 0x9c, 0x98, 0x23, 0xc1, 0xa4, 0xc0, + 0xa8, 0xc1, 0x0a, 0x55, 0x00, 0x13, 0x14, 0x92, 0xe1, 0xe2, 0x2c, 0xae, 0xcc, 0x2b, 0xc9, 0x48, + 0x2d, 0xc9, 0x4c, 0x96, 0x60, 0x56, 0x60, 0xd4, 0xe0, 0x08, 0x42, 0x08, 0x58, 0xf1, 0xcc, 0x58, + 0x20, 0xcf, 0xb0, 0x63, 0x81, 0x3c, 0xe3, 0x8b, 0x05, 0xf2, 0x8c, 0x4e, 0xaa, 0x27, 0x1e, 0xca, + 0x31, 0x9c, 0x78, 0x24, 0xc7, 0x78, 0xe1, 0x91, 0x1c, 0xe3, 0x8d, 0x47, 0x72, 0x8c, 0x0f, 0x1e, + 0xc9, 0x31, 0x4e, 0x78, 0x2c, 0xc7, 0x70, 0xe1, 0xb1, 0x1c, 0xc3, 0x8d, 0xc7, 0x72, 0x0c, 0x51, + 0xcc, 0x19, 0x39, 0xc9, 0x80, 0x00, 0x00, 0x00, 0xff, 0xff, 0xe9, 0x42, 0x31, 0x94, 0xf5, 0x00, + 0x00, 0x00, } diff --git a/pkg/util/hlc/legacy_timestamp.proto b/pkg/util/hlc/legacy_timestamp.proto index ea90e4182658..a20119dbe18e 100644 --- a/pkg/util/hlc/legacy_timestamp.proto +++ b/pkg/util/hlc/legacy_timestamp.proto @@ -30,17 +30,15 @@ message LegacyTimestamp { // skew)/(minimal ns between events) and nearly impossible to // overflow. optional int32 logical = 2 [(gogoproto.nullable) = false]; - // A collection of bit flags that provide details about the timestamp - // and its meaning. The data type is a uint32, but the number of flags - // is limited to 8 so that the flags can be encoded into a single byte. + // Indicates that the Timestamp did not come from an HLC clock somewhere + // in the system and, therefore, does not have has the ability to update + // a peer's HLC clock. If set to true, the "synthetic timestamp" may be + // arbitrarily disconnected from real time. // - // Flags do not affect the sort order of Timestamps. However, they are - // considered when performing structural equality checks (e.g. using the - // == operator). Consider use of the EqOrdering method when testing for - // equality. + // See the commentary on Timestamp.synthetic for more information. // - // The field is nullable so that it is not serialized when no flags are - // set. This ensures that the timestamp encoding does not change across - // nodes that are and are not aware of this field. - optional uint32 flags = 3; + // The field is nullable so that it is not serialized when set to false. + // This ensures that the timestamp encoding does not change across nodes + // that are and are not aware of this field. + optional bool synthetic = 3; } diff --git a/pkg/util/hlc/timestamp.go b/pkg/util/hlc/timestamp.go index 1372fc89042d..061289468fff 100644 --- a/pkg/util/hlc/timestamp.go +++ b/pkg/util/hlc/timestamp.go @@ -15,13 +15,11 @@ import ( "math" "regexp" "strconv" - "strings" "time" "unsafe" "github.com/cockroachdb/cockroach/pkg/util/timeutil" "github.com/cockroachdb/errors" - "google.golang.org/protobuf/proto" ) // Timestamp constant values. @@ -54,17 +52,6 @@ func (t Timestamp) LessEq(s Timestamp) bool { return t.WallTime < s.WallTime || (t.WallTime == s.WallTime && t.Logical <= s.Logical) } -var flagStrings = map[TimestampFlag]string{ - TimestampFlag_SYNTHETIC: "syn", -} -var flagStringsInverted = func() map[string]TimestampFlag { - m := make(map[string]TimestampFlag) - for k, v := range flagStrings { - m[v] = k - } - return m -}() - // String implements the fmt.Formatter interface. func (t Timestamp) String() string { // The following code was originally written as @@ -72,7 +59,7 @@ func (t Timestamp) String() string { // The main problem with the original code was that it would put // a negative sign in the middle (after the decimal point) if // the value happened to be negative. - buf := make([]byte, 0, 20) + buf := make([]byte, 0, 21) w := t.WallTime if w == 0 { @@ -114,20 +101,8 @@ func (t Timestamp) String() string { } buf = strconv.AppendInt(buf, int64(t.Logical), 10) - if t.Flags != 0 { - buf = append(buf, '[') - comma := false - for i := 0; i < 8; i++ { - f := TimestampFlag(1 << i) - if t.IsFlagSet(f) { - if comma { - buf = append(buf, ',') - } - comma = true - buf = append(buf, flagStrings[f]...) - } - } - buf = append(buf, ']') + if t.Synthetic { + buf = append(buf, '?') } return *(*string)(unsafe.Pointer(&buf)) @@ -138,12 +113,12 @@ func (Timestamp) SafeValue() {} var ( timestampRegexp = regexp.MustCompile( - `^(?P-)?(?P\d{1,19})(?:\.(?P\d{1,20}))?(?:,(?P-?\d{1,10}))?(?:\[(?P[\w,]+)\])?$`) - signSubexp = 1 - secsSubexp = 2 - nanosSubexp = 3 - logicalSubexp = 4 - flagsSubexp = 5 + `^(?P-)?(?P\d{1,19})(?:\.(?P\d{1,20}))?(?:,(?P-?\d{1,10}))?(?P\?)?$`) + signSubexp = 1 + secsSubexp = 2 + nanosSubexp = 3 + logicalSubexp = 4 + syntheticSubexp = 5 ) // ParseTimestamp attempts to parse the string generated from @@ -180,25 +155,11 @@ func ParseTimestamp(str string) (_ Timestamp, err error) { return Timestamp{}, err } } + synthetic := matches[syntheticSubexp] != "" t := Timestamp{ - WallTime: wallTime, - Logical: int32(logical), - } - if flagsMatch := matches[flagsSubexp]; flagsMatch != "" { - flagStrs := strings.Split(flagsMatch, ",") - for _, flagStr := range flagStrs { - if flagStr == "" { - return Timestamp{}, errors.Errorf("empty flag provided") - } - flagMatch, ok := flagStringsInverted[flagStr] - if !ok { - return Timestamp{}, errors.Errorf("unknown flag %q provided", flagStr) - } - if t.IsFlagSet(flagMatch) { - return Timestamp{}, errors.Errorf("duplicate flag %q provided", flagStr) - } - t = t.SetFlag(flagMatch) - } + WallTime: wallTime, + Logical: int32(logical), + Synthetic: synthetic, } return t, nil } @@ -213,30 +174,30 @@ func (t Timestamp) IsEmpty() bool { return t == Timestamp{} } -// IsFlagSet returns whether the specified flag is set on the timestamp. -func (t Timestamp) IsFlagSet(f TimestampFlag) bool { - return t.Flags&uint32(f) != 0 -} - // Add returns a timestamp with the WallTime and Logical components increased. // wallTime is expressed in nanos. func (t Timestamp) Add(wallTime int64, logical int32) Timestamp { - return Timestamp{ - WallTime: t.WallTime + wallTime, - Logical: t.Logical + logical, - Flags: t.Flags, + s := Timestamp{ + WallTime: t.WallTime + wallTime, + Logical: t.Logical + logical, + Synthetic: t.Synthetic, } + // TODO(nvanbenschoten): adding to a timestamp should make it synthetic. + // This breaks a number of tests, so make this change in a separate PR. We + // might also want to wait until we've migrated in the Synthetic flag so we + // don't risk setting it when doing so could cause complications in a mixed + // version cluster. + // + // if t.Less(s) { + // // Adding a positive value to a Timestamp adds the Synthetic flag. + // s.Synthetic = true + // } + return s } -// SetFlag returns a timestamp with the specified flag set. -func (t Timestamp) SetFlag(f TimestampFlag) Timestamp { - t.Flags = t.Flags | uint32(f) - return t -} - -// ClearFlag returns a timestamp with the specified flag cleared. -func (t Timestamp) ClearFlag(f TimestampFlag) Timestamp { - t.Flags = t.Flags &^ uint32(f) +// SetSynthetic returns a timestamp with the Synthetic flag set to val. +func (t Timestamp) SetSynthetic(val bool) Timestamp { + t.Synthetic = val return t } @@ -252,14 +213,14 @@ func (t Timestamp) Next() Timestamp { panic("cannot take the next value to a max timestamp") } return Timestamp{ - WallTime: t.WallTime + 1, - Flags: t.Flags, + WallTime: t.WallTime + 1, + Synthetic: t.Synthetic, } } return Timestamp{ - WallTime: t.WallTime, - Logical: t.Logical + 1, - Flags: t.Flags, + WallTime: t.WallTime, + Logical: t.Logical + 1, + Synthetic: t.Synthetic, } } @@ -267,15 +228,15 @@ func (t Timestamp) Next() Timestamp { func (t Timestamp) Prev() Timestamp { if t.Logical > 0 { return Timestamp{ - WallTime: t.WallTime, - Logical: t.Logical - 1, - Flags: t.Flags, + WallTime: t.WallTime, + Logical: t.Logical - 1, + Synthetic: t.Synthetic, } } else if t.WallTime > 0 { return Timestamp{ - WallTime: t.WallTime - 1, - Logical: math.MaxInt32, - Flags: t.Flags, + WallTime: t.WallTime - 1, + Logical: math.MaxInt32, + Synthetic: t.Synthetic, } } panic("cannot take the previous value to a zero timestamp") @@ -287,15 +248,15 @@ func (t Timestamp) Prev() Timestamp { func (t Timestamp) FloorPrev() Timestamp { if t.Logical > 0 { return Timestamp{ - WallTime: t.WallTime, - Logical: t.Logical - 1, - Flags: t.Flags, + WallTime: t.WallTime, + Logical: t.Logical - 1, + Synthetic: t.Synthetic, } } else if t.WallTime > 0 { return Timestamp{ - WallTime: t.WallTime - 1, - Logical: 0, - Flags: t.Flags, + WallTime: t.WallTime - 1, + Logical: 0, + Synthetic: t.Synthetic, } } panic("cannot take the previous value to a zero timestamp") @@ -308,10 +269,8 @@ func (t *Timestamp) Forward(s Timestamp) bool { if t.Less(s) { *t = s return true - } else if t.EqOrdering(s) && onlyLeftSynthetic(*t, s) { - // If the times are equal but t is synthetic while s is not, remove the - // synthtic flag but continue to return false. - *t = t.ClearFlag(TimestampFlag_SYNTHETIC) + } else if t.EqOrdering(s) { + t.Synthetic = bothSynthetic(*t, s) } return false } @@ -319,20 +278,15 @@ func (t *Timestamp) Forward(s Timestamp) bool { // Backward replaces the receiver with the argument, if that moves it backwards // in time. func (t *Timestamp) Backward(s Timestamp) { + syn := bothSynthetic(*t, s) if s.Less(*t) { - // Replace t with s. If s is synthetic while t is not, remove the - // synthtic flag. - if onlyLeftSynthetic(s, *t) { - s = s.ClearFlag(TimestampFlag_SYNTHETIC) - } *t = s - } else if onlyLeftSynthetic(*t, s) { - *t = t.ClearFlag(TimestampFlag_SYNTHETIC) } + t.Synthetic = syn } -func onlyLeftSynthetic(l, r Timestamp) bool { - return l.IsFlagSet(TimestampFlag_SYNTHETIC) && !r.IsFlagSet(TimestampFlag_SYNTHETIC) +func bothSynthetic(l, r Timestamp) bool { + return l.Synthetic && r.Synthetic } // GoTime converts the timestamp to a time.Time. @@ -340,22 +294,24 @@ func (t Timestamp) GoTime() time.Time { return timeutil.Unix(0, t.WallTime) } +var trueBool = true + // ToLegacyTimestamp converts a Timestamp to a LegacyTimestamp. func (t Timestamp) ToLegacyTimestamp() LegacyTimestamp { - var flags *uint32 - if t.Flags != 0 { - flags = proto.Uint32(t.Flags) + var synthetic *bool + if t.Synthetic { + synthetic = &trueBool } - return LegacyTimestamp{WallTime: t.WallTime, Logical: t.Logical, Flags: flags} + return LegacyTimestamp{WallTime: t.WallTime, Logical: t.Logical, Synthetic: synthetic} } // ToTimestamp converts a LegacyTimestamp to a Timestamp. func (t LegacyTimestamp) ToTimestamp() Timestamp { - var flags uint32 - if t.Flags != nil { - flags = *t.Flags + var synthetic bool + if t.Synthetic != nil { + synthetic = *t.Synthetic } - return Timestamp{WallTime: t.WallTime, Logical: t.Logical, Flags: flags} + return Timestamp{WallTime: t.WallTime, Logical: t.Logical, Synthetic: synthetic} } // EqOrdering returns whether the receiver sorts equally to the parameter. @@ -386,7 +342,7 @@ type ClockTimestamp Timestamp // TryToClockTimestamp attempts to downcast a Timestamp into a ClockTimestamp. // Returns the result and a boolean indicating whether the cast succeeded. func (t Timestamp) TryToClockTimestamp() (ClockTimestamp, bool) { - if t.IsFlagSet(TimestampFlag_SYNTHETIC) { + if t.Synthetic { return ClockTimestamp{}, false } return ClockTimestamp(t), true @@ -396,12 +352,15 @@ func (t Timestamp) TryToClockTimestamp() (ClockTimestamp, bool) { // of whether such a cast would be legal according to the Synthetic flag. The // method should only be used in tests. func (t Timestamp) UnsafeToClockTimestamp() ClockTimestamp { - // TODO(nvanbenschoten): unset the Synthetic flag here. + t.Synthetic = false return ClockTimestamp(t) } // ToTimestamp upcasts a ClockTimestamp into a Timestamp. func (t ClockTimestamp) ToTimestamp() Timestamp { + if t.Synthetic { + panic("ClockTimestamp with Synthetic flag set") + } return Timestamp(t) } diff --git a/pkg/util/hlc/timestamp.pb.go b/pkg/util/hlc/timestamp.pb.go index 65050bbff5b5..db4aaa31d5f7 100644 --- a/pkg/util/hlc/timestamp.pb.go +++ b/pkg/util/hlc/timestamp.pb.go @@ -20,43 +20,6 @@ var _ = math.Inf // proto package needs to be updated. const _ = proto.GoGoProtoPackageIsVersion2 // please upgrade the proto package -// TimestampFlag is used to provide extra classification for Timestamps. -type TimestampFlag int32 - -const ( - TimestampFlag_UNKNOWN TimestampFlag = 0 - // A synthetic timestamp is defined as a timestamp that makes no claim - // about the value of clocks in the system. While standard timestamps - // are pulled from HLC clocks and indicate that some node in the system - // has a clock with a reading equal to or above its value, a synthetic - // timestamp makes no such indication. - // - // Synthetic timestamps are central to non-blocking transactions, which - // write at "future timestamps". They are also used to disconnect some - // committed versions from observed timestamps, where they indicate that - // versions were moved from the timestamp at which they were originally - // written. Only synthetic timestamps require observing the full - // uncertainty interval, whereas readings off the leaseholders's clock - // can tighten it for non-synthetic versions. - TimestampFlag_SYNTHETIC TimestampFlag = 1 -) - -var TimestampFlag_name = map[int32]string{ - 0: "UNKNOWN", - 1: "SYNTHETIC", -} -var TimestampFlag_value = map[string]int32{ - "UNKNOWN": 0, - "SYNTHETIC": 1, -} - -func (x TimestampFlag) String() string { - return proto.EnumName(TimestampFlag_name, int32(x)) -} -func (TimestampFlag) EnumDescriptor() ([]byte, []int) { - return fileDescriptor_timestamp_2a42725d81a17263, []int{0} -} - // Timestamp represents a state of the hybrid logical clock. type Timestamp struct { // Holds a wall time, typically a unix epoch time expressed in @@ -74,29 +37,36 @@ type Timestamp struct { // methods on Timestamp, which ensure that the synthetic flag is updated // appropriately. Logical int32 `protobuf:"varint,2,opt,name=logical,proto3" json:"logical,omitempty"` - // A collection of bit flags that provide details about the timestamp - // and its meaning. The data type is a uint32, but the number of flags - // is limited to 8 so that the flags can be encoded into a single byte. - // - // Flags do not affect the sort order of Timestamps. However, they are - // considered when performing structural equality checks (e.g. using the - // == operator). Consider use of the EqOrdering method when testing for - // equality. + // Indicates that the Timestamp did not come from an HLC clock somewhere + // in the system and, therefore, does not have has the ability to update + // a peer's HLC clock. If set to true, the "synthetic timestamp" may be + // arbitrarily disconnected from real time. // - // TODO(nvanbenschoten): use a bool to shave off a - // byte when set. This will allow the flag to serve as the dynamically - // typed version of ClockTimestamp. See TryToClockTimestamp. + // The flag serves as the dynamically typed version of a ClockTimestamp + // (but inverted). Only Timestamps with this flag set to false can be + // downcast to a ClockTimestamp successfully (see TryToClockTimestamp). // - // Should look like: - // bool synthetic = 3; + // Synthetic timestamps with this flag set to true are central to + // non-blocking transactions, which write "into the future". Setting the + // flag to true is also used to disconnect some committed MVCC versions + // from observed timestamps by indicating that those versions were moved + // from the timestamp at which they were originally written. Committed + // MVCC versions with synthetic timestamps require observing the full + // uncertainty interval, whereas readings off the leaseholders's clock + // can tighten the uncertainty interval that is applied to MVCC versions + // with clock timestamp. // - Flags uint32 `protobuf:"varint,3,opt,name=flags,proto3" json:"flags,omitempty"` + // This flag does not affect the sort order of Timestamps. However, it + // is considered when performing structural equality checks (e.g. using + // the == operator). Consider use of the EqOrdering method when testing + // for equality. + Synthetic bool `protobuf:"varint,3,opt,name=synthetic,proto3" json:"synthetic,omitempty"` } func (m *Timestamp) Reset() { *m = Timestamp{} } func (*Timestamp) ProtoMessage() {} func (*Timestamp) Descriptor() ([]byte, []int) { - return fileDescriptor_timestamp_2a42725d81a17263, []int{0} + return fileDescriptor_timestamp_dc8d3245e740e753, []int{0} } func (m *Timestamp) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -123,7 +93,6 @@ var xxx_messageInfo_Timestamp proto.InternalMessageInfo func init() { proto.RegisterType((*Timestamp)(nil), "cockroach.util.hlc.Timestamp") - proto.RegisterEnum("cockroach.util.hlc.TimestampFlag", TimestampFlag_name, TimestampFlag_value) } func (this *Timestamp) Equal(that interface{}) bool { if that == nil { @@ -150,7 +119,7 @@ func (this *Timestamp) Equal(that interface{}) bool { if this.Logical != that1.Logical { return false } - if this.Flags != that1.Flags { + if this.Synthetic != that1.Synthetic { return false } return true @@ -180,10 +149,15 @@ func (m *Timestamp) MarshalTo(dAtA []byte) (int, error) { i++ i = encodeVarintTimestamp(dAtA, i, uint64(m.Logical)) } - if m.Flags != 0 { + if m.Synthetic { dAtA[i] = 0x18 i++ - i = encodeVarintTimestamp(dAtA, i, uint64(m.Flags)) + if m.Synthetic { + dAtA[i] = 1 + } else { + dAtA[i] = 0 + } + i++ } return i, nil } @@ -207,7 +181,7 @@ func NewPopulatedTimestamp(r randyTimestamp, easy bool) *Timestamp { if r.Intn(2) == 0 { this.Logical *= -1 } - this.Flags = uint32(r.Uint32()) + this.Synthetic = bool(bool(r.Intn(2) == 0)) if !easy && r.Intn(10) != 0 { } return this @@ -297,8 +271,8 @@ func (m *Timestamp) Size() (n int) { if m.Logical != 0 { n += 1 + sovTimestamp(uint64(m.Logical)) } - if m.Flags != 0 { - n += 1 + sovTimestamp(uint64(m.Flags)) + if m.Synthetic { + n += 2 } return n } @@ -385,9 +359,9 @@ func (m *Timestamp) Unmarshal(dAtA []byte) error { } case 3: if wireType != 0 { - return fmt.Errorf("proto: wrong wireType = %d for field Flags", wireType) + return fmt.Errorf("proto: wrong wireType = %d for field Synthetic", wireType) } - m.Flags = 0 + var v int for shift := uint(0); ; shift += 7 { if shift >= 64 { return ErrIntOverflowTimestamp @@ -397,11 +371,12 @@ func (m *Timestamp) Unmarshal(dAtA []byte) error { } b := dAtA[iNdEx] iNdEx++ - m.Flags |= (uint32(b) & 0x7F) << shift + v |= (int(b) & 0x7F) << shift if b < 0x80 { break } } + m.Synthetic = bool(v != 0) default: iNdEx = preIndex skippy, err := skipTimestamp(dAtA[iNdEx:]) @@ -529,25 +504,23 @@ var ( ) func init() { - proto.RegisterFile("util/hlc/timestamp.proto", fileDescriptor_timestamp_2a42725d81a17263) + proto.RegisterFile("util/hlc/timestamp.proto", fileDescriptor_timestamp_dc8d3245e740e753) } -var fileDescriptor_timestamp_2a42725d81a17263 = []byte{ - // 247 bytes of a gzipped FileDescriptorProto +var fileDescriptor_timestamp_dc8d3245e740e753 = []byte{ + // 213 bytes of a gzipped FileDescriptorProto 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xe2, 0x92, 0x28, 0x2d, 0xc9, 0xcc, 0xd1, 0xcf, 0xc8, 0x49, 0xd6, 0x2f, 0xc9, 0xcc, 0x4d, 0x2d, 0x2e, 0x49, 0xcc, 0x2d, 0xd0, 0x2b, 0x28, 0xca, 0x2f, 0xc9, 0x17, 0x12, 0x4a, 0xce, 0x4f, 0xce, 0x2e, 0xca, 0x4f, 0x4c, 0xce, 0xd0, 0x03, 0xa9, 0xd1, 0xcb, 0xc8, 0x49, 0x96, 0x12, 0x49, 0xcf, 0x4f, 0xcf, 0x07, 0x4b, 0xeb, 0x83, - 0x58, 0x10, 0x95, 0x4a, 0x69, 0x5c, 0x9c, 0x21, 0x30, 0xcd, 0x42, 0xd2, 0x5c, 0x9c, 0xe5, 0x89, + 0x58, 0x10, 0x95, 0x4a, 0x79, 0x5c, 0x9c, 0x21, 0x30, 0xcd, 0x42, 0xd2, 0x5c, 0x9c, 0xe5, 0x89, 0x39, 0x39, 0xf1, 0x20, 0xe3, 0x24, 0x18, 0x15, 0x18, 0x35, 0x98, 0x83, 0x38, 0x40, 0x02, 0x20, 0x15, 0x42, 0x12, 0x5c, 0xec, 0x39, 0xf9, 0xe9, 0x99, 0xc9, 0x89, 0x39, 0x12, 0x4c, 0x0a, 0x8c, - 0x1a, 0xac, 0x41, 0x30, 0xae, 0x90, 0x08, 0x17, 0x6b, 0x5a, 0x4e, 0x62, 0x7a, 0xb1, 0x04, 0xb3, - 0x02, 0xa3, 0x06, 0x6f, 0x10, 0x84, 0x63, 0xc5, 0x33, 0x63, 0x81, 0x3c, 0xc3, 0x8e, 0x05, 0xf2, - 0x8c, 0x2f, 0x16, 0xc8, 0x33, 0x6a, 0x69, 0x73, 0xf1, 0xc2, 0xed, 0x71, 0xcb, 0x49, 0x4c, 0x17, - 0xe2, 0xe6, 0x62, 0x0f, 0xf5, 0xf3, 0xf6, 0xf3, 0x0f, 0xf7, 0x13, 0x60, 0x10, 0xe2, 0xe5, 0xe2, - 0x0c, 0x8e, 0xf4, 0x0b, 0xf1, 0x70, 0x0d, 0xf1, 0x74, 0x16, 0x60, 0x74, 0x52, 0x3d, 0xf1, 0x50, - 0x8e, 0xe1, 0xc4, 0x23, 0x39, 0xc6, 0x0b, 0x8f, 0xe4, 0x18, 0x6f, 0x3c, 0x92, 0x63, 0x7c, 0xf0, - 0x48, 0x8e, 0x71, 0xc2, 0x63, 0x39, 0x86, 0x0b, 0x8f, 0xe5, 0x18, 0x6e, 0x3c, 0x96, 0x63, 0x88, - 0x62, 0xce, 0xc8, 0x49, 0x4e, 0x62, 0x03, 0x7b, 0xc1, 0x18, 0x10, 0x00, 0x00, 0xff, 0xff, 0xf4, - 0x8d, 0x21, 0xb8, 0x08, 0x01, 0x00, 0x00, + 0x1a, 0xac, 0x41, 0x30, 0xae, 0x90, 0x0c, 0x17, 0x67, 0x71, 0x65, 0x5e, 0x49, 0x46, 0x6a, 0x49, + 0x66, 0xb2, 0x04, 0xb3, 0x02, 0xa3, 0x06, 0x47, 0x10, 0x42, 0xc0, 0x8a, 0x67, 0xc6, 0x02, 0x79, + 0x86, 0x1d, 0x0b, 0xe4, 0x19, 0x5f, 0x2c, 0x90, 0x67, 0x74, 0x52, 0x3d, 0xf1, 0x50, 0x8e, 0xe1, + 0xc4, 0x23, 0x39, 0xc6, 0x0b, 0x8f, 0xe4, 0x18, 0x6f, 0x3c, 0x92, 0x63, 0x7c, 0xf0, 0x48, 0x8e, + 0x71, 0xc2, 0x63, 0x39, 0x86, 0x0b, 0x8f, 0xe5, 0x18, 0x6e, 0x3c, 0x96, 0x63, 0x88, 0x62, 0xce, + 0xc8, 0x49, 0x4e, 0x62, 0x03, 0xbb, 0xce, 0x18, 0x10, 0x00, 0x00, 0xff, 0xff, 0xa2, 0x14, 0x0e, + 0x21, 0xe3, 0x00, 0x00, 0x00, } diff --git a/pkg/util/hlc/timestamp.proto b/pkg/util/hlc/timestamp.proto index e3711d78c635..83bf75ecd622 100644 --- a/pkg/util/hlc/timestamp.proto +++ b/pkg/util/hlc/timestamp.proto @@ -36,43 +36,28 @@ message Timestamp { // methods on Timestamp, which ensure that the synthetic flag is updated // appropriately. int32 logical = 2; - // A collection of bit flags that provide details about the timestamp - // and its meaning. The data type is a uint32, but the number of flags - // is limited to 8 so that the flags can be encoded into a single byte. + // Indicates that the Timestamp did not come from an HLC clock somewhere + // in the system and, therefore, does not have has the ability to update + // a peer's HLC clock. If set to true, the "synthetic timestamp" may be + // arbitrarily disconnected from real time. // - // Flags do not affect the sort order of Timestamps. However, they are - // considered when performing structural equality checks (e.g. using the - // == operator). Consider use of the EqOrdering method when testing for - // equality. + // The flag serves as the dynamically typed version of a ClockTimestamp + // (but inverted). Only Timestamps with this flag set to false can be + // downcast to a ClockTimestamp successfully (see TryToClockTimestamp). // - // TODO(nvanbenschoten): use a bool to shave off a - // byte when set. This will allow the flag to serve as the dynamically - // typed version of ClockTimestamp. See TryToClockTimestamp. - // - // Should look like: - // bool synthetic = 3; - // - uint32 flags = 3; -} - -// TimestampFlag is used to provide extra classification for Timestamps. -enum TimestampFlag { - UNKNOWN = 0x00; - // A synthetic timestamp is defined as a timestamp that makes no claim - // about the value of clocks in the system. While standard timestamps - // are pulled from HLC clocks and indicate that some node in the system - // has a clock with a reading equal to or above its value, a synthetic - // timestamp makes no such indication. - // - // Synthetic timestamps are central to non-blocking transactions, which - // write at "future timestamps". They are also used to disconnect some - // committed versions from observed timestamps, where they indicate that - // versions were moved from the timestamp at which they were originally - // written. Only synthetic timestamps require observing the full + // Synthetic timestamps with this flag set to true are central to + // non-blocking transactions, which write "into the future". Setting the + // flag to true is also used to disconnect some committed MVCC versions + // from observed timestamps by indicating that those versions were moved + // from the timestamp at which they were originally written. Committed + // MVCC versions with synthetic timestamps require observing the full // uncertainty interval, whereas readings off the leaseholders's clock - // can tighten it for non-synthetic versions. - SYNTHETIC = 0x01; - // ... = 0x02; - // ... = 0x04; - // max = 0x80; + // can tighten the uncertainty interval that is applied to MVCC versions + // with clock timestamp. + // + // This flag does not affect the sort order of Timestamps. However, it + // is considered when performing structural equality checks (e.g. using + // the == operator). Consider use of the EqOrdering method when testing + // for equality. + bool synthetic = 3; } diff --git a/pkg/util/hlc/timestamp_test.go b/pkg/util/hlc/timestamp_test.go index f950ec2f8108..8ca523553e5e 100644 --- a/pkg/util/hlc/timestamp_test.go +++ b/pkg/util/hlc/timestamp_test.go @@ -18,18 +18,11 @@ import ( ) func makeTS(walltime int64, logical int32) Timestamp { - return Timestamp{ - WallTime: walltime, - Logical: logical, - } + return Timestamp{WallTime: walltime, Logical: logical} } -func makeTSWithFlags(walltime int64, logical int32, flags TimestampFlag) Timestamp { - return Timestamp{ - WallTime: walltime, - Logical: logical, - Flags: uint32(flags), - } +func makeSynTS(walltime int64, logical int32) Timestamp { + return makeTS(walltime, logical).SetSynthetic(true) } func TestEqOrdering(t *testing.T) { @@ -46,7 +39,7 @@ func TestEqOrdering(t *testing.T) { if a.EqOrdering(b) { t.Errorf("expected %+v != %+v", b, a) } - b = makeTSWithFlags(1, 1, 3) + b = makeSynTS(1, 1) if !a.EqOrdering(b) { t.Errorf("expected %+v == %+v", b, a) } @@ -66,7 +59,7 @@ func TestLess(t *testing.T) { if !b.Less(a) { t.Errorf("expected %+v < %+v", b, a) } - b = makeTSWithFlags(1, 1, 3) + b = makeSynTS(1, 1) if a.Less(b) || b.Less(a) { t.Errorf("expected %+v == %+v", a, b) } @@ -86,34 +79,23 @@ func TestLessEq(t *testing.T) { if !b.LessEq(a) || a.LessEq(b) { t.Errorf("expected %+v < %+v", b, a) } - b = makeTSWithFlags(1, 1, 3) + b = makeSynTS(1, 1) if !a.LessEq(b) || !b.LessEq(a) { t.Errorf("expected %+v == %+v", a, b) } } func TestIsEmpty(t *testing.T) { - a := Timestamp{} + a := makeTS(0, 0) assert.True(t, a.IsEmpty()) a = makeTS(1, 0) assert.False(t, a.IsEmpty()) a = makeTS(0, 1) assert.False(t, a.IsEmpty()) - a = makeTSWithFlags(0, 0, 3) + a = makeSynTS(0, 0) assert.False(t, a.IsEmpty()) } -func TestSetAndClearFlag(t *testing.T) { - a := Timestamp{} - assert.False(t, a.IsFlagSet(TimestampFlag_SYNTHETIC)) - a = a.SetFlag(TimestampFlag_UNKNOWN) - assert.False(t, a.IsFlagSet(TimestampFlag_SYNTHETIC)) - a = a.SetFlag(TimestampFlag_SYNTHETIC) - assert.True(t, a.IsFlagSet(TimestampFlag_SYNTHETIC)) - a = a.ClearFlag(TimestampFlag_SYNTHETIC) - assert.False(t, a.IsFlagSet(TimestampFlag_SYNTHETIC)) -} - func TestTimestampNext(t *testing.T) { testCases := []struct { ts, expNext Timestamp @@ -122,10 +104,10 @@ func TestTimestampNext(t *testing.T) { {makeTS(1, math.MaxInt32-1), makeTS(1, math.MaxInt32)}, {makeTS(1, math.MaxInt32), makeTS(2, 0)}, {makeTS(math.MaxInt32, math.MaxInt32), makeTS(math.MaxInt32+1, 0)}, - {makeTSWithFlags(1, 2, 3), makeTSWithFlags(1, 3, 3)}, - {makeTSWithFlags(1, math.MaxInt32-1, 3), makeTSWithFlags(1, math.MaxInt32, 3)}, - {makeTSWithFlags(1, math.MaxInt32, 3), makeTSWithFlags(2, 0, 3)}, - {makeTSWithFlags(math.MaxInt32, math.MaxInt32, 3), makeTSWithFlags(math.MaxInt32+1, 0, 3)}, + {makeSynTS(1, 2), makeSynTS(1, 3)}, + {makeSynTS(1, math.MaxInt32-1), makeSynTS(1, math.MaxInt32)}, + {makeSynTS(1, math.MaxInt32), makeSynTS(2, 0)}, + {makeSynTS(math.MaxInt32, math.MaxInt32), makeSynTS(math.MaxInt32+1, 0)}, } for _, c := range testCases { assert.Equal(t, c.expNext, c.ts.Next()) @@ -139,9 +121,9 @@ func TestTimestampPrev(t *testing.T) { {makeTS(1, 2), makeTS(1, 1)}, {makeTS(1, 1), makeTS(1, 0)}, {makeTS(1, 0), makeTS(0, math.MaxInt32)}, - {makeTSWithFlags(1, 2, 3), makeTSWithFlags(1, 1, 3)}, - {makeTSWithFlags(1, 1, 3), makeTSWithFlags(1, 0, 3)}, - {makeTSWithFlags(1, 0, 3), makeTSWithFlags(0, math.MaxInt32, 3)}, + {makeSynTS(1, 2), makeSynTS(1, 1)}, + {makeSynTS(1, 1), makeSynTS(1, 0)}, + {makeSynTS(1, 0), makeSynTS(0, math.MaxInt32)}, } for _, c := range testCases { assert.Equal(t, c.expPrev, c.ts.Prev()) @@ -156,10 +138,10 @@ func TestTimestampFloorPrev(t *testing.T) { {makeTS(1, 2), makeTS(1, 1)}, {makeTS(1, 1), makeTS(1, 0)}, {makeTS(1, 0), makeTS(0, 0)}, - {makeTSWithFlags(2, 0, 3), makeTSWithFlags(1, 0, 3)}, - {makeTSWithFlags(1, 2, 3), makeTSWithFlags(1, 1, 3)}, - {makeTSWithFlags(1, 1, 3), makeTSWithFlags(1, 0, 3)}, - {makeTSWithFlags(1, 0, 3), makeTSWithFlags(0, 0, 3)}, + {makeSynTS(2, 0), makeSynTS(1, 0)}, + {makeSynTS(1, 2), makeSynTS(1, 1)}, + {makeSynTS(1, 1), makeSynTS(1, 0)}, + {makeSynTS(1, 0), makeSynTS(0, 0)}, } for _, c := range testCases { assert.Equal(t, c.expPrev, c.ts.FloorPrev()) @@ -167,7 +149,6 @@ func TestTimestampFloorPrev(t *testing.T) { } func TestTimestampForward(t *testing.T) { - flagSyn := TimestampFlag_SYNTHETIC testCases := []struct { ts, arg Timestamp expFwd Timestamp @@ -178,21 +159,21 @@ func TestTimestampForward(t *testing.T) { {makeTS(2, 0), makeTS(2, 0), makeTS(2, 0), false}, {makeTS(2, 0), makeTS(2, 1), makeTS(2, 1), true}, {makeTS(2, 0), makeTS(3, 0), makeTS(3, 0), true}, - {makeTSWithFlags(2, 0, flagSyn), makeTS(1, 0), makeTSWithFlags(2, 0, flagSyn), false}, - {makeTSWithFlags(2, 0, flagSyn), makeTS(1, 1), makeTSWithFlags(2, 0, flagSyn), false}, - {makeTSWithFlags(2, 0, flagSyn), makeTS(2, 0), makeTS(2, 0), false}, - {makeTSWithFlags(2, 0, flagSyn), makeTS(2, 1), makeTS(2, 1), true}, - {makeTSWithFlags(2, 0, flagSyn), makeTS(3, 0), makeTS(3, 0), true}, - {makeTS(2, 0), makeTSWithFlags(1, 0, flagSyn), makeTS(2, 0), false}, - {makeTS(2, 0), makeTSWithFlags(1, 1, flagSyn), makeTS(2, 0), false}, - {makeTS(2, 0), makeTSWithFlags(2, 0, flagSyn), makeTS(2, 0), false}, - {makeTS(2, 0), makeTSWithFlags(2, 1, flagSyn), makeTSWithFlags(2, 1, flagSyn), true}, - {makeTS(2, 0), makeTSWithFlags(3, 0, flagSyn), makeTSWithFlags(3, 0, flagSyn), true}, - {makeTSWithFlags(2, 0, flagSyn), makeTSWithFlags(1, 0, flagSyn), makeTSWithFlags(2, 0, flagSyn), false}, - {makeTSWithFlags(2, 0, flagSyn), makeTSWithFlags(1, 1, flagSyn), makeTSWithFlags(2, 0, flagSyn), false}, - {makeTSWithFlags(2, 0, flagSyn), makeTSWithFlags(2, 0, flagSyn), makeTSWithFlags(2, 0, flagSyn), false}, - {makeTSWithFlags(2, 0, flagSyn), makeTSWithFlags(2, 1, flagSyn), makeTSWithFlags(2, 1, flagSyn), true}, - {makeTSWithFlags(2, 0, flagSyn), makeTSWithFlags(3, 0, flagSyn), makeTSWithFlags(3, 0, flagSyn), true}, + {makeSynTS(2, 0), makeTS(1, 0), makeSynTS(2, 0), false}, + {makeSynTS(2, 0), makeTS(1, 1), makeSynTS(2, 0), false}, + {makeSynTS(2, 0), makeTS(2, 0), makeTS(2, 0), false}, + {makeSynTS(2, 0), makeTS(2, 1), makeTS(2, 1), true}, + {makeSynTS(2, 0), makeTS(3, 0), makeTS(3, 0), true}, + {makeTS(2, 0), makeSynTS(1, 0), makeTS(2, 0), false}, + {makeTS(2, 0), makeSynTS(1, 1), makeTS(2, 0), false}, + {makeTS(2, 0), makeSynTS(2, 0), makeTS(2, 0), false}, + {makeTS(2, 0), makeSynTS(2, 1), makeSynTS(2, 1), true}, + {makeTS(2, 0), makeSynTS(3, 0), makeSynTS(3, 0), true}, + {makeSynTS(2, 0), makeSynTS(1, 0), makeSynTS(2, 0), false}, + {makeSynTS(2, 0), makeSynTS(1, 1), makeSynTS(2, 0), false}, + {makeSynTS(2, 0), makeSynTS(2, 0), makeSynTS(2, 0), false}, + {makeSynTS(2, 0), makeSynTS(2, 1), makeSynTS(2, 1), true}, + {makeSynTS(2, 0), makeSynTS(3, 0), makeSynTS(3, 0), true}, } for _, c := range testCases { ts := c.ts @@ -202,7 +183,6 @@ func TestTimestampForward(t *testing.T) { } func TestTimestampBackward(t *testing.T) { - flagSyn := TimestampFlag_SYNTHETIC testCases := []struct { ts, arg, expBwd Timestamp }{ @@ -211,21 +191,21 @@ func TestTimestampBackward(t *testing.T) { {makeTS(2, 0), makeTS(2, 0), makeTS(2, 0)}, {makeTS(2, 0), makeTS(2, 1), makeTS(2, 0)}, {makeTS(2, 0), makeTS(3, 0), makeTS(2, 0)}, - {makeTSWithFlags(2, 0, flagSyn), makeTS(1, 0), makeTS(1, 0)}, - {makeTSWithFlags(2, 0, flagSyn), makeTS(1, 1), makeTS(1, 1)}, - {makeTSWithFlags(2, 0, flagSyn), makeTS(2, 0), makeTS(2, 0)}, - {makeTSWithFlags(2, 0, flagSyn), makeTS(2, 1), makeTS(2, 0)}, - {makeTSWithFlags(2, 0, flagSyn), makeTS(3, 0), makeTS(2, 0)}, - {makeTS(2, 0), makeTSWithFlags(1, 0, flagSyn), makeTS(1, 0)}, - {makeTS(2, 0), makeTSWithFlags(1, 1, flagSyn), makeTS(1, 1)}, - {makeTS(2, 0), makeTSWithFlags(2, 0, flagSyn), makeTS(2, 0)}, - {makeTS(2, 0), makeTSWithFlags(2, 1, flagSyn), makeTS(2, 0)}, - {makeTS(2, 0), makeTSWithFlags(3, 0, flagSyn), makeTS(2, 0)}, - {makeTSWithFlags(2, 0, flagSyn), makeTSWithFlags(1, 0, flagSyn), makeTSWithFlags(1, 0, flagSyn)}, - {makeTSWithFlags(2, 0, flagSyn), makeTSWithFlags(1, 1, flagSyn), makeTSWithFlags(1, 1, flagSyn)}, - {makeTSWithFlags(2, 0, flagSyn), makeTSWithFlags(2, 0, flagSyn), makeTSWithFlags(2, 0, flagSyn)}, - {makeTSWithFlags(2, 0, flagSyn), makeTSWithFlags(2, 1, flagSyn), makeTSWithFlags(2, 0, flagSyn)}, - {makeTSWithFlags(2, 0, flagSyn), makeTSWithFlags(3, 0, flagSyn), makeTSWithFlags(2, 0, flagSyn)}, + {makeSynTS(2, 0), makeTS(1, 0), makeTS(1, 0)}, + {makeSynTS(2, 0), makeTS(1, 1), makeTS(1, 1)}, + {makeSynTS(2, 0), makeTS(2, 0), makeTS(2, 0)}, + {makeSynTS(2, 0), makeTS(2, 1), makeTS(2, 0)}, + {makeSynTS(2, 0), makeTS(3, 0), makeTS(2, 0)}, + {makeTS(2, 0), makeSynTS(1, 0), makeTS(1, 0)}, + {makeTS(2, 0), makeSynTS(1, 1), makeTS(1, 1)}, + {makeTS(2, 0), makeSynTS(2, 0), makeTS(2, 0)}, + {makeTS(2, 0), makeSynTS(2, 1), makeTS(2, 0)}, + {makeTS(2, 0), makeSynTS(3, 0), makeTS(2, 0)}, + {makeSynTS(2, 0), makeSynTS(1, 0), makeSynTS(1, 0)}, + {makeSynTS(2, 0), makeSynTS(1, 1), makeSynTS(1, 1)}, + {makeSynTS(2, 0), makeSynTS(2, 0), makeSynTS(2, 0)}, + {makeSynTS(2, 0), makeSynTS(2, 1), makeSynTS(2, 0)}, + {makeSynTS(2, 0), makeSynTS(3, 0), makeSynTS(2, 0)}, } for _, c := range testCases { ts := c.ts @@ -268,10 +248,9 @@ func TestTimestampFormatParseRoundTrip(t *testing.T) { {makeTS(-1234567890, 0), "-1.234567890,0"}, {makeTS(6661234567890, 0), "6661.234567890,0"}, {makeTS(-6661234567890, 0), "-6661.234567890,0"}, - {makeTSWithFlags(0, 0, TimestampFlag_SYNTHETIC), "0,0[syn]"}, - {makeTSWithFlags(0, 123, TimestampFlag_SYNTHETIC), "0,123[syn]"}, - {makeTSWithFlags(1, 0, TimestampFlag_SYNTHETIC), "0.000000001,0[syn]"}, - {makeTSWithFlags(1, 123, TimestampFlag_SYNTHETIC), "0.000000001,123[syn]"}, + {makeSynTS(0, 123), "0,123?"}, + {makeSynTS(1, 0), "0.000000001,0?"}, + {makeSynTS(1, 123), "0.000000001,123?"}, } for _, c := range testCases { str := c.ts.String() @@ -300,10 +279,10 @@ func TestTimestampParseFormatNonRoundTrip(t *testing.T) { // Other cases. {"0.000000001", makeTS(1, 0), "0.000000001,0"}, {"99.000000001", makeTS(99000000001, 0), "99.000000001,0"}, - {"0[syn]", makeTSWithFlags(0, 0, TimestampFlag_SYNTHETIC), "0,0[syn]"}, - {"99[syn]", makeTSWithFlags(99000000000, 0, TimestampFlag_SYNTHETIC), "99.000000000,0[syn]"}, - {"0.000000001[syn]", makeTSWithFlags(1, 0, TimestampFlag_SYNTHETIC), "0.000000001,0[syn]"}, - {"99.000000001[syn]", makeTSWithFlags(99000000001, 0, TimestampFlag_SYNTHETIC), "99.000000001,0[syn]"}, + {"0?", makeSynTS(0, 0), "0,0?"}, + {"99?", makeSynTS(99000000000, 0), "99.000000000,0?"}, + {"0.000000001?", makeSynTS(1, 0), "0.000000001,0?"}, + {"99.000000001?", makeSynTS(99000000001, 0), "99.000000001,0?"}, } for _, c := range testCases { parsed, err := ParseTimestamp(c.s) @@ -337,20 +316,12 @@ func TestTimestampParseError(t *testing.T) { "failed to parse \"1.9999999999999999999,0\" as Timestamp: strconv.ParseInt: parsing \"9999999999999999999\": value out of range", }, { - "0,123[]", - "failed to parse \"0,123\\[\\]\" as Timestamp", - }, - { - "0,123[bad]", - "failed to parse \"0,123\\[bad\\]\" as Timestamp: unknown flag \"bad\" provided", - }, - { - "0,123[syn,]", - "failed to parse \"0,123\\[syn,\\]\" as Timestamp: empty flag provided", + "0,123[?]", + "failed to parse \"0,123\\[\\?\\]\" as Timestamp", }, { - "0,123[syn,syn]", - "failed to parse \"0,123\\[syn,syn\\]\" as Timestamp: duplicate flag \"syn\" provided", + "0,123??", + "failed to parse \"0,123\\?\\?\" as Timestamp", }, } { _, err := ParseTimestamp(c.s) @@ -368,8 +339,8 @@ func BenchmarkTimestampString(b *testing.B) { } } -func BenchmarkTimestampStringWithFlags(b *testing.B) { - ts := makeTSWithFlags(-6661234567890, 0, TimestampFlag_SYNTHETIC) +func BenchmarkTimestampStringSynthetic(b *testing.B) { + ts := makeSynTS(-6661234567890, 0) for i := 0; i < b.N; i++ { _ = ts.String()