From 35151c9c66b18a9ab7587b250ae7412f294c0b93 Mon Sep 17 00:00:00 2001 From: Nathan VanBenschoten Date: Wed, 17 Nov 2021 15:25:24 -0500 Subject: [PATCH] kv: remove clock update on BatchResponse Before this change, we were updating the local clock with each BatchResponse's WriteTimestamp. This was meant to handle cases where the batch request timestamp was forwarded during evaluation. This was unnecessary for two reasons. The first is that a BatchResponse can legitimately carry an operation timestamp that leads the local HLC clock on the leaseholder that evaluated the request. This has been true since #80706, which introduced the concept of a "local timestamp". This allowed us to remove the (broken) attempt at ensuring that the HLC on a leaseholder always leads the MVCC timestamp of all values in the leaseholder's keyspace (see the update to `pkg/kv/kvserver/uncertainty/doc.go` in that PR). The second was that it was not even correct. The idea behind bumping the HLC on the response path was to ensure that if a batch request was forwarded to a newer timestamp during evaluation and then completed a write, that forwarded timestamp would be reflected in the leaseholder's HLC. However, this ignored the fact that any forwarded timestamp must have either come from an existing value in the range or from the leaseholder's clock. So if those didn't lead the clock, the derived timestamp wouldn't either. It also ignored the fact that the clock bump here was too late (post-latch release) and if it had actually been needed (it wasn't), it wouldn't have even ensured that the timestamp on any lease transfer led the maximum time of any response served by the outgoing leaseholder. There are no mixed-version migration concerns of this change, because #80706 ensured that any future-time operation will still continue to use the synthetic bit until all nodes are running v22.2 or later. --- pkg/kv/kvserver/store_send.go | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/pkg/kv/kvserver/store_send.go b/pkg/kv/kvserver/store_send.go index d8266bfa6e53..f6d335d21da6 100644 --- a/pkg/kv/kvserver/store_send.go +++ b/pkg/kv/kvserver/store_send.go @@ -118,23 +118,6 @@ func (s *Store) SendWithWriteBytes( if br.Txn == nil { br.Txn = ba.Txn } - // Update our clock with the outgoing response txn timestamp - // (if timestamp has been forwarded). - if ba.Timestamp.Less(br.Txn.WriteTimestamp) { - if clockTS, ok := br.Txn.WriteTimestamp.TryToClockTimestamp(); ok { - s.cfg.Clock.Update(clockTS) - } - } - } - } else { - if pErr == nil { - // Update our clock with the outgoing response timestamp. - // (if timestamp has been forwarded). - if ba.Timestamp.Less(br.Timestamp) { - if clockTS, ok := br.Timestamp.TryToClockTimestamp(); ok { - s.cfg.Clock.Update(clockTS) - } - } } }