From 35bdbf01eb03e661f6183a63cc3524e0481569fe Mon Sep 17 00:00:00 2001 From: George Papadrosou Date: Tue, 5 Nov 2019 09:59:22 +0100 Subject: [PATCH] storage: Migrate MVCCStats contains_estimates from bool to int64 This migration makes ContainsEstimates a counter so that the consistency checker can reset it (by returning a -ContainsEstimates) delta) without racing with another command that introduces new estimate stats. Resolves #37120 Release note: None --- .../protos/storage/engine/enginepb/mvcc.pb.cc | 8 +- .../protos/storage/engine/enginepb/mvcc.pb.h | 12 +- .../storage/engine/enginepb/mvcc3.pb.cc | 28 ++- .../protos/storage/engine/enginepb/mvcc3.pb.h | 32 +-- docs/generated/settings/settings.html | 2 +- pkg/ccl/storageccl/engineccl/rocksdb.go | 2 +- pkg/cli/debug_check_store.go | 4 +- pkg/cli/debug_check_store_test.go | 2 +- pkg/settings/cluster/cockroach_versions.go | 20 ++ pkg/settings/cluster/versionkey_string.go | 5 +- .../testdata/logic_test/builtin_function | 2 +- pkg/storage/batcheval/cmd_add_sstable.go | 9 +- pkg/storage/batcheval/cmd_add_sstable_test.go | 4 +- pkg/storage/batcheval/cmd_end_transaction.go | 10 +- pkg/storage/batcheval/cmd_recompute_stats.go | 15 +- pkg/storage/batcheval/split_stats_helper.go | 6 +- pkg/storage/below_raft_protos_test.go | 2 +- pkg/storage/consistency_queue_test.go | 11 +- pkg/storage/engine/enginepb/mvcc.go | 10 +- pkg/storage/engine/enginepb/mvcc.pb.go | 38 ++-- pkg/storage/engine/enginepb/mvcc.proto | 5 +- pkg/storage/engine/enginepb/mvcc3.pb.go | 212 +++++++++--------- pkg/storage/engine/enginepb/mvcc3.proto | 7 +- pkg/storage/engine/mvcc.go | 6 +- pkg/storage/engine/mvcc_test.go | 4 +- pkg/storage/engine/rocksdb.go | 3 +- pkg/storage/gc_queue.go | 4 +- pkg/storage/gc_queue_test.go | 10 +- .../replica_application_state_machine.go | 45 +++- pkg/storage/replica_consistency.go | 14 +- pkg/storage/replica_proposal.go | 22 ++ pkg/storage/replica_test.go | 157 ++++++++++++- 32 files changed, 469 insertions(+), 242 deletions(-) diff --git a/c-deps/libroach/protos/storage/engine/enginepb/mvcc.pb.cc b/c-deps/libroach/protos/storage/engine/enginepb/mvcc.pb.cc index c86b7b975048..d72c610f95c0 100644 --- a/c-deps/libroach/protos/storage/engine/enginepb/mvcc.pb.cc +++ b/c-deps/libroach/protos/storage/engine/enginepb/mvcc.pb.cc @@ -1072,7 +1072,7 @@ bool MVCCStats::MergePartialFromCodedStream( static_cast< ::google::protobuf::uint8>(112u /* 112 & 0xFF */)) { set_has_contains_estimates(); DO_((::google::protobuf::internal::WireFormatLite::ReadPrimitive< - bool, ::google::protobuf::internal::WireFormatLite::TYPE_BOOL>( + ::google::protobuf::int64, ::google::protobuf::internal::WireFormatLite::TYPE_INT64>( input, &contains_estimates_))); } else { goto handle_unusual; @@ -1160,7 +1160,7 @@ void MVCCStats::SerializeWithCachedSizes( } if (cached_has_bits & 0x00002000u) { - ::google::protobuf::internal::WireFormatLite::WriteBool(14, this->contains_estimates(), output); + ::google::protobuf::internal::WireFormatLite::WriteInt64(14, this->contains_estimates(), output); } output->WriteRaw(_internal_metadata_.unknown_fields().data(), @@ -1230,7 +1230,9 @@ size_t MVCCStats::ByteSizeLong() const { } if (has_contains_estimates()) { - total_size += 1 + 1; + total_size += 1 + + ::google::protobuf::internal::WireFormatLite::Int64Size( + this->contains_estimates()); } } diff --git a/c-deps/libroach/protos/storage/engine/enginepb/mvcc.pb.h b/c-deps/libroach/protos/storage/engine/enginepb/mvcc.pb.h index 85b368e36c85..8314b8f7e6e5 100644 --- a/c-deps/libroach/protos/storage/engine/enginepb/mvcc.pb.h +++ b/c-deps/libroach/protos/storage/engine/enginepb/mvcc.pb.h @@ -583,8 +583,8 @@ class MVCCStats : public ::google::protobuf::MessageLite /* @@protoc_insertion_p bool has_contains_estimates() const; void clear_contains_estimates(); static const int kContainsEstimatesFieldNumber = 14; - bool contains_estimates() const; - void set_contains_estimates(bool value); + ::google::protobuf::int64 contains_estimates() const; + void set_contains_estimates(::google::protobuf::int64 value); // @@protoc_insertion_point(class_scope:cockroach.storage.engine.enginepb.MVCCStats) private: @@ -633,7 +633,7 @@ class MVCCStats : public ::google::protobuf::MessageLite /* @@protoc_insertion_p ::google::protobuf::int64 intent_count_; ::google::protobuf::int64 sys_bytes_; ::google::protobuf::int64 sys_count_; - bool contains_estimates_; + ::google::protobuf::int64 contains_estimates_; friend struct ::protobuf_storage_2fengine_2fenginepb_2fmvcc_2eproto::TableStruct; }; // =================================================================== @@ -1079,14 +1079,14 @@ inline void MVCCStats::clear_has_contains_estimates() { _has_bits_[0] &= ~0x00002000u; } inline void MVCCStats::clear_contains_estimates() { - contains_estimates_ = false; + contains_estimates_ = GOOGLE_LONGLONG(0); clear_has_contains_estimates(); } -inline bool MVCCStats::contains_estimates() const { +inline ::google::protobuf::int64 MVCCStats::contains_estimates() const { // @@protoc_insertion_point(field_get:cockroach.storage.engine.enginepb.MVCCStats.contains_estimates) return contains_estimates_; } -inline void MVCCStats::set_contains_estimates(bool value) { +inline void MVCCStats::set_contains_estimates(::google::protobuf::int64 value) { set_has_contains_estimates(); contains_estimates_ = value; // @@protoc_insertion_point(field_set:cockroach.storage.engine.enginepb.MVCCStats.contains_estimates) diff --git a/c-deps/libroach/protos/storage/engine/enginepb/mvcc3.pb.cc b/c-deps/libroach/protos/storage/engine/enginepb/mvcc3.pb.cc index eca2316dc537..a532907c0af5 100644 --- a/c-deps/libroach/protos/storage/engine/enginepb/mvcc3.pb.cc +++ b/c-deps/libroach/protos/storage/engine/enginepb/mvcc3.pb.cc @@ -953,13 +953,13 @@ bool MVCCStatsDelta::MergePartialFromCodedStream( break; } - // bool contains_estimates = 14; + // int64 contains_estimates = 14; case 14: { if (static_cast< ::google::protobuf::uint8>(tag) == static_cast< ::google::protobuf::uint8>(112u /* 112 & 0xFF */)) { DO_((::google::protobuf::internal::WireFormatLite::ReadPrimitive< - bool, ::google::protobuf::internal::WireFormatLite::TYPE_BOOL>( + ::google::protobuf::int64, ::google::protobuf::internal::WireFormatLite::TYPE_INT64>( input, &contains_estimates_))); } else { goto handle_unusual; @@ -1057,9 +1057,9 @@ void MVCCStatsDelta::SerializeWithCachedSizes( ::google::protobuf::internal::WireFormatLite::WriteSInt64(13, this->sys_count(), output); } - // bool contains_estimates = 14; + // int64 contains_estimates = 14; if (this->contains_estimates() != 0) { - ::google::protobuf::internal::WireFormatLite::WriteBool(14, this->contains_estimates(), output); + ::google::protobuf::internal::WireFormatLite::WriteInt64(14, this->contains_estimates(), output); } output->WriteRaw((::google::protobuf::internal::GetProto3PreserveUnknownsDefault() ? _internal_metadata_.unknown_fields() : _internal_metadata_.default_instance()).data(), @@ -1157,9 +1157,11 @@ size_t MVCCStatsDelta::ByteSizeLong() const { this->sys_count()); } - // bool contains_estimates = 14; + // int64 contains_estimates = 14; if (this->contains_estimates() != 0) { - total_size += 1 + 1; + total_size += 1 + + ::google::protobuf::internal::WireFormatLite::Int64Size( + this->contains_estimates()); } int cached_size = ::google::protobuf::internal::ToCachedSize(total_size); @@ -1532,13 +1534,13 @@ bool MVCCPersistentStats::MergePartialFromCodedStream( break; } - // bool contains_estimates = 14; + // int64 contains_estimates = 14; case 14: { if (static_cast< ::google::protobuf::uint8>(tag) == static_cast< ::google::protobuf::uint8>(112u /* 112 & 0xFF */)) { DO_((::google::protobuf::internal::WireFormatLite::ReadPrimitive< - bool, ::google::protobuf::internal::WireFormatLite::TYPE_BOOL>( + ::google::protobuf::int64, ::google::protobuf::internal::WireFormatLite::TYPE_INT64>( input, &contains_estimates_))); } else { goto handle_unusual; @@ -1636,9 +1638,9 @@ void MVCCPersistentStats::SerializeWithCachedSizes( ::google::protobuf::internal::WireFormatLite::WriteInt64(13, this->sys_count(), output); } - // bool contains_estimates = 14; + // int64 contains_estimates = 14; if (this->contains_estimates() != 0) { - ::google::protobuf::internal::WireFormatLite::WriteBool(14, this->contains_estimates(), output); + ::google::protobuf::internal::WireFormatLite::WriteInt64(14, this->contains_estimates(), output); } output->WriteRaw((::google::protobuf::internal::GetProto3PreserveUnknownsDefault() ? _internal_metadata_.unknown_fields() : _internal_metadata_.default_instance()).data(), @@ -1736,9 +1738,11 @@ size_t MVCCPersistentStats::ByteSizeLong() const { this->sys_count()); } - // bool contains_estimates = 14; + // int64 contains_estimates = 14; if (this->contains_estimates() != 0) { - total_size += 1 + 1; + total_size += 1 + + ::google::protobuf::internal::WireFormatLite::Int64Size( + this->contains_estimates()); } int cached_size = ::google::protobuf::internal::ToCachedSize(total_size); diff --git a/c-deps/libroach/protos/storage/engine/enginepb/mvcc3.pb.h b/c-deps/libroach/protos/storage/engine/enginepb/mvcc3.pb.h index 8b3f31efc8d6..271594d68924 100644 --- a/c-deps/libroach/protos/storage/engine/enginepb/mvcc3.pb.h +++ b/c-deps/libroach/protos/storage/engine/enginepb/mvcc3.pb.h @@ -434,11 +434,11 @@ class MVCCStatsDelta : public ::google::protobuf::MessageLite /* @@protoc_insert ::google::protobuf::int64 sys_count() const; void set_sys_count(::google::protobuf::int64 value); - // bool contains_estimates = 14; + // int64 contains_estimates = 14; void clear_contains_estimates(); static const int kContainsEstimatesFieldNumber = 14; - bool contains_estimates() const; - void set_contains_estimates(bool value); + ::google::protobuf::int64 contains_estimates() const; + void set_contains_estimates(::google::protobuf::int64 value); // @@protoc_insertion_point(class_scope:cockroach.storage.engine.enginepb.MVCCStatsDelta) private: @@ -457,7 +457,7 @@ class MVCCStatsDelta : public ::google::protobuf::MessageLite /* @@protoc_insert ::google::protobuf::int64 intent_count_; ::google::protobuf::int64 sys_bytes_; ::google::protobuf::int64 sys_count_; - bool contains_estimates_; + ::google::protobuf::int64 contains_estimates_; mutable ::google::protobuf::internal::CachedSize _cached_size_; friend struct ::protobuf_storage_2fengine_2fenginepb_2fmvcc3_2eproto::TableStruct; }; @@ -625,11 +625,11 @@ class MVCCPersistentStats : public ::google::protobuf::MessageLite /* @@protoc_i ::google::protobuf::int64 sys_count() const; void set_sys_count(::google::protobuf::int64 value); - // bool contains_estimates = 14; + // int64 contains_estimates = 14; void clear_contains_estimates(); static const int kContainsEstimatesFieldNumber = 14; - bool contains_estimates() const; - void set_contains_estimates(bool value); + ::google::protobuf::int64 contains_estimates() const; + void set_contains_estimates(::google::protobuf::int64 value); // @@protoc_insertion_point(class_scope:cockroach.storage.engine.enginepb.MVCCPersistentStats) private: @@ -648,7 +648,7 @@ class MVCCPersistentStats : public ::google::protobuf::MessageLite /* @@protoc_i ::google::protobuf::int64 intent_count_; ::google::protobuf::int64 sys_bytes_; ::google::protobuf::int64 sys_count_; - bool contains_estimates_; + ::google::protobuf::int64 contains_estimates_; mutable ::google::protobuf::internal::CachedSize _cached_size_; friend struct ::protobuf_storage_2fengine_2fenginepb_2fmvcc3_2eproto::TableStruct; }; @@ -1966,15 +1966,15 @@ inline void TxnMeta::set_sequence(::google::protobuf::int32 value) { // MVCCStatsDelta -// bool contains_estimates = 14; +// int64 contains_estimates = 14; inline void MVCCStatsDelta::clear_contains_estimates() { - contains_estimates_ = false; + contains_estimates_ = GOOGLE_LONGLONG(0); } -inline bool MVCCStatsDelta::contains_estimates() const { +inline ::google::protobuf::int64 MVCCStatsDelta::contains_estimates() const { // @@protoc_insertion_point(field_get:cockroach.storage.engine.enginepb.MVCCStatsDelta.contains_estimates) return contains_estimates_; } -inline void MVCCStatsDelta::set_contains_estimates(bool value) { +inline void MVCCStatsDelta::set_contains_estimates(::google::protobuf::int64 value) { contains_estimates_ = value; // @@protoc_insertion_point(field_set:cockroach.storage.engine.enginepb.MVCCStatsDelta.contains_estimates) @@ -2165,15 +2165,15 @@ inline void MVCCStatsDelta::set_sys_count(::google::protobuf::int64 value) { // MVCCPersistentStats -// bool contains_estimates = 14; +// int64 contains_estimates = 14; inline void MVCCPersistentStats::clear_contains_estimates() { - contains_estimates_ = false; + contains_estimates_ = GOOGLE_LONGLONG(0); } -inline bool MVCCPersistentStats::contains_estimates() const { +inline ::google::protobuf::int64 MVCCPersistentStats::contains_estimates() const { // @@protoc_insertion_point(field_get:cockroach.storage.engine.enginepb.MVCCPersistentStats.contains_estimates) return contains_estimates_; } -inline void MVCCPersistentStats::set_contains_estimates(bool value) { +inline void MVCCPersistentStats::set_contains_estimates(::google::protobuf::int64 value) { contains_estimates_ = value; // @@protoc_insertion_point(field_set:cockroach.storage.engine.enginepb.MVCCPersistentStats.contains_estimates) diff --git a/docs/generated/settings/settings.html b/docs/generated/settings/settings.html index 0bdaa6b56b02..419f30001ee3 100644 --- a/docs/generated/settings/settings.html +++ b/docs/generated/settings/settings.html @@ -133,6 +133,6 @@ trace.debug.enablebooleanfalseif set, traces for recent requests can be seen in the /debug page trace.lightstep.tokenstringif set, traces go to Lightstep using this token trace.zipkin.collectorstringif set, traces go to the given Zipkin instance (example: '127.0.0.1:9411'); ignored if trace.lightstep.token is set -versioncustom validation19.2-1set the active cluster version in the format '.' +versioncustom validation19.2-2set the active cluster version in the format '.' diff --git a/pkg/ccl/storageccl/engineccl/rocksdb.go b/pkg/ccl/storageccl/engineccl/rocksdb.go index 52ecdebc47d5..c9a0eb356a94 100644 --- a/pkg/ccl/storageccl/engineccl/rocksdb.go +++ b/pkg/ccl/storageccl/engineccl/rocksdb.go @@ -138,7 +138,7 @@ func cStatsToGoStats(stats C.MVCCStatsResult, nowNanos int64) (enginepb.MVCCStat if err := statusToError(stats.status); err != nil { return ms, err } - ms.ContainsEstimates = false + ms.ContainsEstimates = 0 ms.LiveBytes = int64(stats.live_bytes) ms.KeyBytes = int64(stats.key_bytes) ms.ValBytes = int64(stats.val_bytes) diff --git a/pkg/cli/debug_check_store.go b/pkg/cli/debug_check_store.go index b034c71882c6..11a695b215d8 100644 --- a/pkg/cli/debug_check_store.go +++ b/pkg/cli/debug_check_store.go @@ -103,7 +103,7 @@ func (cr *checkResult) Error() error { if cr.err != nil { errs = append(errs, cr.err.Error()) } - if !cr.actMS.Equal(enginepb.MVCCStats{}) && !cr.actMS.Equal(cr.claimMS) && !cr.claimMS.ContainsEstimates { + if !cr.actMS.Equal(enginepb.MVCCStats{}) && !cr.actMS.Equal(cr.claimMS) && cr.claimMS.ContainsEstimates <= 0 { err := fmt.Sprintf("stats inconsistency:\n- stored:\n%+v\n- recomputed:\n%+v\n- diff:\n%s", cr.claimMS, cr.actMS, strings.Join(pretty.Diff(cr.claimMS, cr.actMS), ","), ) @@ -192,7 +192,7 @@ func checkStoreRangeStats( errS := err.Error() println(errS) } else { - if res.claimMS.ContainsEstimates { + if res.claimMS.ContainsEstimates > 0 { cE++ } total.Add(res.actMS) diff --git a/pkg/cli/debug_check_store_test.go b/pkg/cli/debug_check_store_test.go index a3c5ca598a01..70c7cd768d7b 100644 --- a/pkg/cli/debug_check_store_test.go +++ b/pkg/cli/debug_check_store_test.go @@ -92,7 +92,7 @@ func TestDebugCheckStore(t *testing.T) { sl := stateloader.Make(1) ms, err := sl.LoadMVCCStats(ctx, eng) require.NoError(t, err) - ms.ContainsEstimates = false + ms.ContainsEstimates = 0 ms.LiveBytes++ require.NoError(t, sl.SetMVCCStats(ctx, eng, &ms)) }() diff --git a/pkg/settings/cluster/cockroach_versions.go b/pkg/settings/cluster/cockroach_versions.go index a6480f5c0891..24e0a56af6dc 100644 --- a/pkg/settings/cluster/cockroach_versions.go +++ b/pkg/settings/cluster/cockroach_versions.go @@ -46,6 +46,7 @@ const ( VersionPartitionedBackup Version19_2 VersionStart20_1 + VersionContainsEstimatesCounter // Add new versions here (step one of two). @@ -265,6 +266,25 @@ var versionsSingleton = keyedVersions([]keyedVersion{ Key: VersionStart20_1, Version: roachpb.Version{Major: 19, Minor: 2, Unstable: 1}, }, + { + // VersionContainsEstimatesCounter is https://github.com/cockroachdb/cockroach/pull/37583. + // + // MVCCStats.ContainsEstimates has been migrated from boolean to a + // counter so that the consistency checker and splits can reset it by + // returning -ContainsEstimates, avoiding racing with other operations + // that want to also change it. + // + // The migration maintains the invariant that raft commands with + // ContainsEstimates zero or one want the bool behavior (i.e. 1+1=1). + // Before the cluster version is active, at proposal time we'll refuse + // any negative ContainsEstimates plus we clamp all others to {0,1}. + // When the version is active, and ContainsEstimates is positive, we + // multiply it by 2 (i.e. we avoid 1). Downstream of raft, we use old + // behavior for ContainsEstimates=1 and the additive behavior for + // anything else. + Key: VersionContainsEstimatesCounter, + Version: roachpb.Version{Major: 19, Minor: 2, Unstable: 2}, + }, // Add new versions here (step two of two). diff --git a/pkg/settings/cluster/versionkey_string.go b/pkg/settings/cluster/versionkey_string.go index d254ea72a6eb..e318422dc2f3 100644 --- a/pkg/settings/cluster/versionkey_string.go +++ b/pkg/settings/cluster/versionkey_string.go @@ -22,11 +22,12 @@ func _() { _ = x[VersionPartitionedBackup-11] _ = x[Version19_2-12] _ = x[VersionStart20_1-13] + _ = x[VersionContainsEstimatesCounter-14] } -const _VersionKey_name = "Version19_1VersionStart19_2VersionQueryTxnTimestampVersionStickyBitVersionParallelCommitsVersionGenerationComparableVersionLearnerReplicasVersionTopLevelForeignKeysVersionAtomicChangeReplicasTriggerVersionAtomicChangeReplicasVersionTableDescModificationTimeFromMVCCVersionPartitionedBackupVersion19_2VersionStart20_1" +const _VersionKey_name = "Version19_1VersionStart19_2VersionQueryTxnTimestampVersionStickyBitVersionParallelCommitsVersionGenerationComparableVersionLearnerReplicasVersionTopLevelForeignKeysVersionAtomicChangeReplicasTriggerVersionAtomicChangeReplicasVersionTableDescModificationTimeFromMVCCVersionPartitionedBackupVersion19_2VersionStart20_1VersionContainsEstimatesCounter" -var _VersionKey_index = [...]uint16{0, 11, 27, 51, 67, 89, 116, 138, 164, 198, 225, 265, 289, 300, 316} +var _VersionKey_index = [...]uint16{0, 11, 27, 51, 67, 89, 116, 138, 164, 198, 225, 265, 289, 300, 316, 347} func (i VersionKey) String() string { if i < 0 || i >= VersionKey(len(_VersionKey_index)-1) { diff --git a/pkg/sql/logictest/testdata/logic_test/builtin_function b/pkg/sql/logictest/testdata/logic_test/builtin_function index 59cbf1992ae0..351f6b41715b 100644 --- a/pkg/sql/logictest/testdata/logic_test/builtin_function +++ b/pkg/sql/logictest/testdata/logic_test/builtin_function @@ -2223,7 +2223,7 @@ SELECT crdb_internal.check_consistency(true, '\x03', '\x02') query ITT SELECT range_id, status, regexp_replace(detail, '[0-9]+', '', 'g') FROM crdb_internal.check_consistency(true, '\x02', '\xffff') WHERE range_id = 1 ---- -1 RANGE_CONSISTENT stats: {ContainsEstimates:false LastUpdateNanos: IntentAge: GCBytesAge: LiveBytes: LiveCount: KeyBytes: KeyCount: ValBytes: ValCount: IntentBytes: IntentCount: SysBytes: SysCount:} +1 RANGE_CONSISTENT stats: {ContainsEstimates: LastUpdateNanos: IntentAge: GCBytesAge: LiveBytes: LiveCount: KeyBytes: KeyCount: ValBytes: ValCount: IntentBytes: IntentCount: SysBytes: SysCount:} # Without explicit keys, scans all ranges (we don't test this too precisely to # avoid flaking the test when the range count changes, just want to know that diff --git a/pkg/storage/batcheval/cmd_add_sstable.go b/pkg/storage/batcheval/cmd_add_sstable.go index 777d46c625e9..c9afce18f0f2 100644 --- a/pkg/storage/batcheval/cmd_add_sstable.go +++ b/pkg/storage/batcheval/cmd_add_sstable.go @@ -15,6 +15,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/storage/batcheval/result" "github.com/cockroachdb/cockroach/pkg/storage/engine" "github.com/cockroachdb/cockroach/pkg/storage/engine/enginepb" @@ -157,7 +158,7 @@ func EvalAddSSTable( // sending an explicit recompute. // // There is a significant performance win to be achieved by ensuring that the - // stats computed are not estimates as it prevents recompuation on splits. + // stats computed are not estimates as it prevents recomputation on splits. // Running AddSSTable with disallowShadowing=true gets us close to this as we // do not allow colliding keys to be ingested. However, in the situation that // two SSTs have KV(s) which "perfectly" shadow an existing key (equal ts and @@ -170,8 +171,12 @@ func EvalAddSSTable( // cumulative for this command. These stats can then be marked as accurate. if args.DisallowShadowing { stats.Subtract(skippedKVStats) + stats.ContainsEstimates = 0 + } else { + _ = cluster.VersionContainsEstimatesCounter // see for info on ContainsEstimates migration + stats.ContainsEstimates++ } - stats.ContainsEstimates = !args.DisallowShadowing + ms.Add(stats) if args.IngestAsWrites { diff --git a/pkg/storage/batcheval/cmd_add_sstable_test.go b/pkg/storage/batcheval/cmd_add_sstable_test.go index d1bcd918c119..ef3bd2468c76 100644 --- a/pkg/storage/batcheval/cmd_add_sstable_test.go +++ b/pkg/storage/batcheval/cmd_add_sstable_test.go @@ -455,7 +455,7 @@ func TestAddSSTableMVCCStats(t *testing.T) { return afterStats }() evaledStats.Add(delta) - evaledStats.ContainsEstimates = false + evaledStats.ContainsEstimates = 0 if !afterStats.Equal(evaledStats) { t.Errorf("mvcc stats mismatch: diff(expected, actual): %s", pretty.Diff(afterStats, evaledStats)) } @@ -475,7 +475,7 @@ func TestAddSSTableMVCCStats(t *testing.T) { if _, err := batcheval.EvalAddSSTable(ctx, e, cArgsWithStats, nil); err != nil { t.Fatalf("%+v", err) } - expected := enginepb.MVCCStats{ContainsEstimates: true, KeyCount: 10} + expected := enginepb.MVCCStats{ContainsEstimates: 1, KeyCount: 10} if got := *cArgsWithStats.Stats; got != expected { t.Fatalf("expected %v got %v", expected, got) } diff --git a/pkg/storage/batcheval/cmd_end_transaction.go b/pkg/storage/batcheval/cmd_end_transaction.go index 9c956dc8b0c6..e88b3cf7d5c0 100644 --- a/pkg/storage/batcheval/cmd_end_transaction.go +++ b/pkg/storage/batcheval/cmd_end_transaction.go @@ -1035,14 +1035,10 @@ func splitTriggerHelper( RHSDelta: *h.AbsPostSplitRight(), } - // HACK(tbg): ContainsEstimates isn't an additive group (there isn't a - // -true), and instead of "-true" we'll emit a "true". This will all be - // fixed when #37583 lands (and the version is active). For now hard-code - // false and there's also code below Raft that interprets this (coming from - // a split) as a signal to reset the ContainsEstimates field to false (see - // applyRaftCommand). deltaPostSplitLeft := h.DeltaPostSplitLeft() - deltaPostSplitLeft.ContainsEstimates = false + if !cluster.Version.IsActive(ctx, rec.ClusterSettings(), cluster.VersionContainsEstimatesCounter) { + deltaPostSplitLeft.ContainsEstimates = 0 + } return deltaPostSplitLeft, pd, nil } diff --git a/pkg/storage/batcheval/cmd_recompute_stats.go b/pkg/storage/batcheval/cmd_recompute_stats.go index f15eabed7d85..023188549e08 100644 --- a/pkg/storage/batcheval/cmd_recompute_stats.go +++ b/pkg/storage/batcheval/cmd_recompute_stats.go @@ -15,6 +15,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/storage/batcheval/result" "github.com/cockroachdb/cockroach/pkg/storage/engine" "github.com/cockroachdb/cockroach/pkg/storage/engine/enginepb" @@ -93,20 +94,18 @@ func RecomputeStats( delta.Subtract(currentStats) if !dryRun { - // NB: this will never clear the ContainsEstimates flag. To be able to do this, - // we would need to guarantee that no command that sets it is in-flight in - // parallel with this command. This can be achieved by blocking all of the range - // or by using our inside knowledge that dictates that ranges which contain no - // timeseries writes never have the flag reset, or by making ContainsEstimates - // a counter (and ensuring that we're the only one subtracting at any given - // time). - // // TODO(tschottdorf): do we not want to run at all if we have estimates in // this range? I think we want to as this would give us much more realistic // stats for timeseries ranges (which go cold and the approximate stats are // wildly overcounting) and this is paced by the consistency checker, but it // means some extra engine churn. cArgs.Stats.Add(delta) + if !cluster.Version.IsActive(ctx, cArgs.EvalCtx.ClusterSettings(), cluster.VersionContainsEstimatesCounter) { + // We are running with the older version of MVCCStats.ContainsEstimates + // which was a boolean, so we should keep it in {0,1} and not reset it + // to avoid racing with another command that sets it to true. + delta.ContainsEstimates = currentStats.ContainsEstimates + } } resp.(*roachpb.RecomputeStatsResponse).AddedDelta = enginepb.MVCCStatsDelta(delta) diff --git a/pkg/storage/batcheval/split_stats_helper.go b/pkg/storage/batcheval/split_stats_helper.go index 362cd55206c5..df8fd17c90fb 100644 --- a/pkg/storage/batcheval/split_stats_helper.go +++ b/pkg/storage/batcheval/split_stats_helper.go @@ -91,7 +91,7 @@ import "github.com/cockroachdb/cockroach/pkg/storage/engine/enginepb" // CombinedErrorDelta = AbsPreSplitBothEstimated + DeltaBatchEstimated // -(AbsPostSplitLeft + AbsPostSplitRight) // -// and the fact that the second line coontains no estimates, we know that +// and the fact that the second line contains no estimates, we know that // CombinedErrorDelta is zero if the first line contains no estimates. Using // this, we can rearrange as // @@ -129,8 +129,8 @@ func makeSplitStatsHelper(input splitStatsHelperInput) (splitStatsHelper, error) in: input, } - if !h.in.AbsPreSplitBothEstimated.ContainsEstimates && - !h.in.DeltaBatchEstimated.ContainsEstimates { + if h.in.AbsPreSplitBothEstimated.ContainsEstimates == 0 && + h.in.DeltaBatchEstimated.ContainsEstimates == 0 { // We have CombinedErrorDelta zero, so use arithmetic to compute // AbsPostSplitRight(). ms := h.in.AbsPreSplitBothEstimated diff --git a/pkg/storage/below_raft_protos_test.go b/pkg/storage/below_raft_protos_test.go index 7b55d4f47ffc..f47044cfa9f6 100644 --- a/pkg/storage/below_raft_protos_test.go +++ b/pkg/storage/below_raft_protos_test.go @@ -66,7 +66,7 @@ var belowRaftGoldenProtos = map[reflect.Type]fixture{ return enginepb.NewPopulatedRangeAppliedState(r, false) }, emptySum: 615555020845646359, - populatedSum: 94706924697857278, + populatedSum: 6873743885651366543, }, reflect.TypeOf(&raftpb.HardState{}): { populatedConstructor: func(r *rand.Rand) protoutil.Message { diff --git a/pkg/storage/consistency_queue_test.go b/pkg/storage/consistency_queue_test.go index 1d97e086ca0b..94171ef4d3f2 100644 --- a/pkg/storage/consistency_queue_test.go +++ b/pkg/storage/consistency_queue_test.go @@ -372,7 +372,10 @@ func TestCheckConsistencyInconsistent(t *testing.T) { // The upreplication here is immaterial and serves only to add realism to the test. func TestConsistencyQueueRecomputeStats(t *testing.T) { defer leaktest.AfterTest(t)() + testutils.RunTrueAndFalse(t, "hadEstimates", testConsistencyQueueRecomputeStatsImpl) +} +func testConsistencyQueueRecomputeStatsImpl(t *testing.T, hadEstimates bool) { ctx := context.Background() path, cleanup := testutils.TempDir(t) @@ -482,7 +485,10 @@ func TestConsistencyQueueRecomputeStats(t *testing.T) { // not affected by the workload we run below and also does not influence the // GC queue score. ms.SysCount += sysCountGarbage - ms.ContainsEstimates = false + ms.ContainsEstimates = 0 + if hadEstimates { + ms.ContainsEstimates = 123 + } // Overwrite with the new stats; remember that this range hasn't upreplicated, // so the consistency checker won't see any replica divergence when it runs, @@ -563,7 +569,8 @@ func TestConsistencyQueueRecomputeStats(t *testing.T) { case resp := <-ccCh: assert.Contains(t, resp.Result[0].Detail, `KeyBytes`) // contains printed stats assert.Equal(t, roachpb.CheckConsistencyResponse_RANGE_CONSISTENT_STATS_INCORRECT, resp.Result[0].Status) + assert.False(t, hadEstimates) default: - t.Errorf("no response indicating the incorrect stats") + assert.True(t, hadEstimates) } } diff --git a/pkg/storage/engine/enginepb/mvcc.go b/pkg/storage/engine/enginepb/mvcc.go index bb85e7033251..d39aee9a4cd5 100644 --- a/pkg/storage/engine/enginepb/mvcc.go +++ b/pkg/storage/engine/enginepb/mvcc.go @@ -113,8 +113,9 @@ func (ms *MVCCStats) Add(oms MVCCStats) { // pre-addition state. ms.Forward(oms.LastUpdateNanos) oms.Forward(ms.LastUpdateNanos) // on local copy - // If either stats object contains estimates, their sum does too. - ms.ContainsEstimates = ms.ContainsEstimates || oms.ContainsEstimates + + ms.ContainsEstimates += oms.ContainsEstimates + // Now that we've done that, we may just add them. ms.IntentAge += oms.IntentAge ms.GCBytesAge += oms.GCBytesAge @@ -137,8 +138,9 @@ func (ms *MVCCStats) Subtract(oms MVCCStats) { // pre-subtraction state. ms.Forward(oms.LastUpdateNanos) oms.Forward(ms.LastUpdateNanos) - // If either stats object contains estimates, their difference does too. - ms.ContainsEstimates = ms.ContainsEstimates || oms.ContainsEstimates + + ms.ContainsEstimates -= oms.ContainsEstimates + // Now that we've done that, we may subtract. ms.IntentAge -= oms.IntentAge ms.GCBytesAge -= oms.GCBytesAge diff --git a/pkg/storage/engine/enginepb/mvcc.pb.go b/pkg/storage/engine/enginepb/mvcc.pb.go index 4826db0d11c6..78cf121c2f72 100644 --- a/pkg/storage/engine/enginepb/mvcc.pb.go +++ b/pkg/storage/engine/enginepb/mvcc.pb.go @@ -67,7 +67,7 @@ func (m *MVCCMetadata) Reset() { *m = MVCCMetadata{} } func (m *MVCCMetadata) String() string { return proto.CompactTextString(m) } func (*MVCCMetadata) ProtoMessage() {} func (*MVCCMetadata) Descriptor() ([]byte, []int) { - return fileDescriptor_mvcc_d6d531974b8f8580, []int{0} + return fileDescriptor_mvcc_0d3b8a25ae993fba, []int{0} } func (m *MVCCMetadata) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -108,7 +108,7 @@ func (m *MVCCMetadata_SequencedIntent) Reset() { *m = MVCCMetadata_Seque func (m *MVCCMetadata_SequencedIntent) String() string { return proto.CompactTextString(m) } func (*MVCCMetadata_SequencedIntent) ProtoMessage() {} func (*MVCCMetadata_SequencedIntent) Descriptor() ([]byte, []int) { - return fileDescriptor_mvcc_d6d531974b8f8580, []int{0, 0} + return fileDescriptor_mvcc_0d3b8a25ae993fba, []int{0, 0} } func (m *MVCCMetadata_SequencedIntent) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -167,8 +167,9 @@ type MVCCStats struct { // contains_estimates indicates that the MVCCStats object contains values // which have been estimated. This means that the stats should not be used // where complete accuracy is required, and instead should be recomputed - // when necessary. - ContainsEstimates bool `protobuf:"varint,14,opt,name=contains_estimates,json=containsEstimates" json:"contains_estimates"` + // when necessary. See cluster.VersionContainsEstimatesCounter for details + // about the migration from bool to int64. + ContainsEstimates int64 `protobuf:"varint,14,opt,name=contains_estimates,json=containsEstimates" json:"contains_estimates"` // last_update_nanos is a timestamp at which the ages were last // updated. See the comment on MVCCStats. LastUpdateNanos int64 `protobuf:"fixed64,1,opt,name=last_update_nanos,json=lastUpdateNanos" json:"last_update_nanos"` @@ -222,7 +223,7 @@ func (m *MVCCStats) Reset() { *m = MVCCStats{} } func (m *MVCCStats) String() string { return proto.CompactTextString(m) } func (*MVCCStats) ProtoMessage() {} func (*MVCCStats) Descriptor() ([]byte, []int) { - return fileDescriptor_mvcc_d6d531974b8f8580, []int{1} + return fileDescriptor_mvcc_0d3b8a25ae993fba, []int{1} } func (m *MVCCStats) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -566,12 +567,7 @@ func (m *MVCCStats) MarshalTo(dAtA []byte) (int, error) { i += 8 dAtA[i] = 0x70 i++ - if m.ContainsEstimates { - dAtA[i] = 1 - } else { - dAtA[i] = 0 - } - i++ + i = encodeVarintMvcc(dAtA, i, uint64(m.ContainsEstimates)) return i, nil } @@ -695,7 +691,10 @@ func NewPopulatedMVCCStats(r randyMvcc, easy bool) *MVCCStats { if r.Intn(2) == 0 { this.SysCount *= -1 } - this.ContainsEstimates = bool(bool(r.Intn(2) == 0)) + this.ContainsEstimates = int64(r.Int63()) + if r.Intn(2) == 0 { + this.ContainsEstimates *= -1 + } if !easy && r.Intn(10) != 0 { } return this @@ -838,7 +837,7 @@ func (m *MVCCStats) Size() (n int) { n += 9 n += 9 n += 9 - n += 2 + n += 1 + sovMvcc(uint64(m.ContainsEstimates)) return n } @@ -1384,7 +1383,7 @@ func (m *MVCCStats) Unmarshal(dAtA []byte) error { if wireType != 0 { return fmt.Errorf("proto: wrong wireType = %d for field ContainsEstimates", wireType) } - var v int + m.ContainsEstimates = 0 for shift := uint(0); ; shift += 7 { if shift >= 64 { return ErrIntOverflowMvcc @@ -1394,12 +1393,11 @@ func (m *MVCCStats) Unmarshal(dAtA []byte) error { } b := dAtA[iNdEx] iNdEx++ - v |= (int(b) & 0x7F) << shift + m.ContainsEstimates |= (int64(b) & 0x7F) << shift if b < 0x80 { break } } - m.ContainsEstimates = bool(v != 0) default: iNdEx = preIndex skippy, err := skipMvcc(dAtA[iNdEx:]) @@ -1527,10 +1525,10 @@ var ( ) func init() { - proto.RegisterFile("storage/engine/enginepb/mvcc.proto", fileDescriptor_mvcc_d6d531974b8f8580) + proto.RegisterFile("storage/engine/enginepb/mvcc.proto", fileDescriptor_mvcc_0d3b8a25ae993fba) } -var fileDescriptor_mvcc_d6d531974b8f8580 = []byte{ +var fileDescriptor_mvcc_0d3b8a25ae993fba = []byte{ // 655 bytes of a gzipped FileDescriptorProto 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x8c, 0x93, 0xcd, 0x4e, 0xdb, 0x4e, 0x10, 0xc0, 0xe3, 0x7f, 0x02, 0x38, 0x9b, 0x10, 0xfe, 0xac, 0x38, 0x44, 0x54, 0x72, 0x02, 0x1c, @@ -1569,8 +1567,8 @@ var fileDescriptor_mvcc_d6d531974b8f8580 = []byte{ 0x14, 0x55, 0xcd, 0x85, 0x69, 0x11, 0xca, 0x51, 0x15, 0x9d, 0xd1, 0xae, 0x0c, 0xd4, 0xba, 0xca, 0x9f, 0x60, 0x5a, 0x17, 0x8f, 0xb9, 0xd1, 0x55, 0xf3, 0x87, 0xf2, 0x98, 0xa7, 0x75, 0x49, 0x44, 0x8b, 0x56, 0x17, 0x10, 0x6d, 0xd9, 0x47, 0x98, 0x46, 0x4c, 0x90, 0x80, 0xf1, 0x1e, 0x70, 0x11, - 0x0c, 0x89, 0xd4, 0xd5, 0x72, 0xff, 0xee, 0x7a, 0x92, 0x7f, 0x9d, 0xa4, 0xb3, 0x39, 0xea, 0xec, + 0x0c, 0x89, 0xd4, 0xd5, 0x72, 0xbf, 0xdd, 0x7a, 0x92, 0x7f, 0x9d, 0xa4, 0xb3, 0x39, 0xea, 0xec, 0xde, 0xfc, 0x70, 0x0a, 0x37, 0x33, 0xc7, 0xba, 0x9d, 0x39, 0xd6, 0xdd, 0xcc, 0xb1, 0xbe, 0xcf, 0x1c, 0xeb, 0xf3, 0x83, 0x53, 0xb8, 0x7d, 0x70, 0x0a, 0x77, 0x0f, 0x4e, 0xe1, 0xa3, 0x9d, 0xfc, - 0x0e, 0xbf, 0x03, 0x00, 0x00, 0xff, 0xff, 0xdc, 0x01, 0xd6, 0x1d, 0x57, 0x05, 0x00, 0x00, + 0x0e, 0xbf, 0x03, 0x00, 0x00, 0xff, 0xff, 0x4b, 0x69, 0x82, 0xd8, 0x57, 0x05, 0x00, 0x00, } diff --git a/pkg/storage/engine/enginepb/mvcc.proto b/pkg/storage/engine/enginepb/mvcc.proto index 5268393ebe3d..46b3b831eb42 100644 --- a/pkg/storage/engine/enginepb/mvcc.proto +++ b/pkg/storage/engine/enginepb/mvcc.proto @@ -109,8 +109,9 @@ message MVCCStats { // contains_estimates indicates that the MVCCStats object contains values // which have been estimated. This means that the stats should not be used // where complete accuracy is required, and instead should be recomputed - // when necessary. - optional bool contains_estimates = 14 [(gogoproto.nullable) = false]; + // when necessary. See cluster.VersionContainsEstimatesCounter for details + // about the migration from bool to int64. + optional int64 contains_estimates = 14 [(gogoproto.nullable) = false]; // last_update_nanos is a timestamp at which the ages were last // updated. See the comment on MVCCStats. diff --git a/pkg/storage/engine/enginepb/mvcc3.pb.go b/pkg/storage/engine/enginepb/mvcc3.pb.go index 3f3b89732d6f..65a973b121c0 100644 --- a/pkg/storage/engine/enginepb/mvcc3.pb.go +++ b/pkg/storage/engine/enginepb/mvcc3.pb.go @@ -124,7 +124,7 @@ func (m *TxnMeta) Reset() { *m = TxnMeta{} } func (m *TxnMeta) String() string { return proto.CompactTextString(m) } func (*TxnMeta) ProtoMessage() {} func (*TxnMeta) Descriptor() ([]byte, []int) { - return fileDescriptor_mvcc3_103868dd91da76fd, []int{0} + return fileDescriptor_mvcc3_d6ebeaa8ad44885c, []int{0} } func (m *TxnMeta) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -153,10 +153,7 @@ var xxx_messageInfo_TxnMeta proto.InternalMessageInfo // encodings for most fields that make it more efficient to store negative // values. This makes the encodings incompatible. type MVCCStatsDelta struct { - // TODO(nvanbenschoten): now that we've split MVCCPersistentStats - // from this MVCCStatsDelta type, we can turn contains_estimates - // into a three-valued type ('UNCHANGED', 'NO', and 'YES'). - ContainsEstimates bool `protobuf:"varint,14,opt,name=contains_estimates,json=containsEstimates,proto3" json:"contains_estimates,omitempty"` + ContainsEstimates int64 `protobuf:"varint,14,opt,name=contains_estimates,json=containsEstimates,proto3" json:"contains_estimates,omitempty"` LastUpdateNanos int64 `protobuf:"fixed64,1,opt,name=last_update_nanos,json=lastUpdateNanos,proto3" json:"last_update_nanos,omitempty"` IntentAge int64 `protobuf:"fixed64,2,opt,name=intent_age,json=intentAge,proto3" json:"intent_age,omitempty"` GCBytesAge int64 `protobuf:"fixed64,3,opt,name=gc_bytes_age,json=gcBytesAge,proto3" json:"gc_bytes_age,omitempty"` @@ -176,7 +173,7 @@ func (m *MVCCStatsDelta) Reset() { *m = MVCCStatsDelta{} } func (m *MVCCStatsDelta) String() string { return proto.CompactTextString(m) } func (*MVCCStatsDelta) ProtoMessage() {} func (*MVCCStatsDelta) Descriptor() ([]byte, []int) { - return fileDescriptor_mvcc3_103868dd91da76fd, []int{1} + return fileDescriptor_mvcc3_d6ebeaa8ad44885c, []int{1} } func (m *MVCCStatsDelta) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -206,7 +203,7 @@ var xxx_messageInfo_MVCCStatsDelta proto.InternalMessageInfo // values but inefficient to store negative values. This makes the encodings // incompatible. type MVCCPersistentStats struct { - ContainsEstimates bool `protobuf:"varint,14,opt,name=contains_estimates,json=containsEstimates,proto3" json:"contains_estimates,omitempty"` + ContainsEstimates int64 `protobuf:"varint,14,opt,name=contains_estimates,json=containsEstimates,proto3" json:"contains_estimates,omitempty"` LastUpdateNanos int64 `protobuf:"fixed64,1,opt,name=last_update_nanos,json=lastUpdateNanos,proto3" json:"last_update_nanos,omitempty"` IntentAge int64 `protobuf:"fixed64,2,opt,name=intent_age,json=intentAge,proto3" json:"intent_age,omitempty"` GCBytesAge int64 `protobuf:"fixed64,3,opt,name=gc_bytes_age,json=gcBytesAge,proto3" json:"gc_bytes_age,omitempty"` @@ -226,7 +223,7 @@ func (m *MVCCPersistentStats) Reset() { *m = MVCCPersistentStats{} } func (m *MVCCPersistentStats) String() string { return proto.CompactTextString(m) } func (*MVCCPersistentStats) ProtoMessage() {} func (*MVCCPersistentStats) Descriptor() ([]byte, []int) { - return fileDescriptor_mvcc3_103868dd91da76fd, []int{2} + return fileDescriptor_mvcc3_d6ebeaa8ad44885c, []int{2} } func (m *MVCCPersistentStats) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -271,7 +268,7 @@ func (m *RangeAppliedState) Reset() { *m = RangeAppliedState{} } func (m *RangeAppliedState) String() string { return proto.CompactTextString(m) } func (*RangeAppliedState) ProtoMessage() {} func (*RangeAppliedState) Descriptor() ([]byte, []int) { - return fileDescriptor_mvcc3_103868dd91da76fd, []int{3} + return fileDescriptor_mvcc3_d6ebeaa8ad44885c, []int{3} } func (m *RangeAppliedState) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -308,7 +305,7 @@ func (m *MVCCWriteValueOp) Reset() { *m = MVCCWriteValueOp{} } func (m *MVCCWriteValueOp) String() string { return proto.CompactTextString(m) } func (*MVCCWriteValueOp) ProtoMessage() {} func (*MVCCWriteValueOp) Descriptor() ([]byte, []int) { - return fileDescriptor_mvcc3_103868dd91da76fd, []int{4} + return fileDescriptor_mvcc3_d6ebeaa8ad44885c, []int{4} } func (m *MVCCWriteValueOp) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -346,7 +343,7 @@ func (m *MVCCWriteIntentOp) Reset() { *m = MVCCWriteIntentOp{} } func (m *MVCCWriteIntentOp) String() string { return proto.CompactTextString(m) } func (*MVCCWriteIntentOp) ProtoMessage() {} func (*MVCCWriteIntentOp) Descriptor() ([]byte, []int) { - return fileDescriptor_mvcc3_103868dd91da76fd, []int{5} + return fileDescriptor_mvcc3_d6ebeaa8ad44885c, []int{5} } func (m *MVCCWriteIntentOp) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -382,7 +379,7 @@ func (m *MVCCUpdateIntentOp) Reset() { *m = MVCCUpdateIntentOp{} } func (m *MVCCUpdateIntentOp) String() string { return proto.CompactTextString(m) } func (*MVCCUpdateIntentOp) ProtoMessage() {} func (*MVCCUpdateIntentOp) Descriptor() ([]byte, []int) { - return fileDescriptor_mvcc3_103868dd91da76fd, []int{6} + return fileDescriptor_mvcc3_d6ebeaa8ad44885c, []int{6} } func (m *MVCCUpdateIntentOp) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -420,7 +417,7 @@ func (m *MVCCCommitIntentOp) Reset() { *m = MVCCCommitIntentOp{} } func (m *MVCCCommitIntentOp) String() string { return proto.CompactTextString(m) } func (*MVCCCommitIntentOp) ProtoMessage() {} func (*MVCCCommitIntentOp) Descriptor() ([]byte, []int) { - return fileDescriptor_mvcc3_103868dd91da76fd, []int{7} + return fileDescriptor_mvcc3_d6ebeaa8ad44885c, []int{7} } func (m *MVCCCommitIntentOp) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -460,7 +457,7 @@ func (m *MVCCAbortIntentOp) Reset() { *m = MVCCAbortIntentOp{} } func (m *MVCCAbortIntentOp) String() string { return proto.CompactTextString(m) } func (*MVCCAbortIntentOp) ProtoMessage() {} func (*MVCCAbortIntentOp) Descriptor() ([]byte, []int) { - return fileDescriptor_mvcc3_103868dd91da76fd, []int{8} + return fileDescriptor_mvcc3_d6ebeaa8ad44885c, []int{8} } func (m *MVCCAbortIntentOp) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -496,7 +493,7 @@ func (m *MVCCAbortTxnOp) Reset() { *m = MVCCAbortTxnOp{} } func (m *MVCCAbortTxnOp) String() string { return proto.CompactTextString(m) } func (*MVCCAbortTxnOp) ProtoMessage() {} func (*MVCCAbortTxnOp) Descriptor() ([]byte, []int) { - return fileDescriptor_mvcc3_103868dd91da76fd, []int{9} + return fileDescriptor_mvcc3_d6ebeaa8ad44885c, []int{9} } func (m *MVCCAbortTxnOp) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -535,7 +532,7 @@ func (m *MVCCLogicalOp) Reset() { *m = MVCCLogicalOp{} } func (m *MVCCLogicalOp) String() string { return proto.CompactTextString(m) } func (*MVCCLogicalOp) ProtoMessage() {} func (*MVCCLogicalOp) Descriptor() ([]byte, []int) { - return fileDescriptor_mvcc3_103868dd91da76fd, []int{10} + return fileDescriptor_mvcc3_d6ebeaa8ad44885c, []int{10} } func (m *MVCCLogicalOp) XXX_Unmarshal(b []byte) error { return m.Unmarshal(b) @@ -917,15 +914,10 @@ func (m *MVCCStatsDelta) MarshalTo(dAtA []byte) (int, error) { i++ i = encodeVarintMvcc3(dAtA, i, uint64((uint64(m.SysCount)<<1)^uint64((m.SysCount>>63)))) } - if m.ContainsEstimates { + if m.ContainsEstimates != 0 { dAtA[i] = 0x70 i++ - if m.ContainsEstimates { - dAtA[i] = 1 - } else { - dAtA[i] = 0 - } - i++ + i = encodeVarintMvcc3(dAtA, i, uint64(m.ContainsEstimates)) } return i, nil } @@ -1013,15 +1005,10 @@ func (m *MVCCPersistentStats) MarshalTo(dAtA []byte) (int, error) { i++ i = encodeVarintMvcc3(dAtA, i, uint64(m.SysCount)) } - if m.ContainsEstimates { + if m.ContainsEstimates != 0 { dAtA[i] = 0x70 i++ - if m.ContainsEstimates { - dAtA[i] = 1 - } else { - dAtA[i] = 0 - } - i++ + i = encodeVarintMvcc3(dAtA, i, uint64(m.ContainsEstimates)) } return i, nil } @@ -1451,7 +1438,10 @@ func NewPopulatedMVCCPersistentStats(r randyMvcc3, easy bool) *MVCCPersistentSta if r.Intn(2) == 0 { this.SysCount *= -1 } - this.ContainsEstimates = bool(bool(r.Intn(2) == 0)) + this.ContainsEstimates = int64(r.Int63()) + if r.Intn(2) == 0 { + this.ContainsEstimates *= -1 + } if !easy && r.Intn(10) != 0 { } return this @@ -1613,8 +1603,8 @@ func (m *MVCCStatsDelta) Size() (n int) { if m.SysCount != 0 { n += 1 + sozMvcc3(uint64(m.SysCount)) } - if m.ContainsEstimates { - n += 2 + if m.ContainsEstimates != 0 { + n += 1 + sovMvcc3(uint64(m.ContainsEstimates)) } return n } @@ -1664,8 +1654,8 @@ func (m *MVCCPersistentStats) Size() (n int) { if m.SysCount != 0 { n += 1 + sovMvcc3(uint64(m.SysCount)) } - if m.ContainsEstimates { - n += 2 + if m.ContainsEstimates != 0 { + n += 1 + sovMvcc3(uint64(m.ContainsEstimates)) } return n } @@ -2369,7 +2359,7 @@ func (m *MVCCStatsDelta) Unmarshal(dAtA []byte) error { if wireType != 0 { return fmt.Errorf("proto: wrong wireType = %d for field ContainsEstimates", wireType) } - var v int + m.ContainsEstimates = 0 for shift := uint(0); ; shift += 7 { if shift >= 64 { return ErrIntOverflowMvcc3 @@ -2379,12 +2369,11 @@ func (m *MVCCStatsDelta) Unmarshal(dAtA []byte) error { } b := dAtA[iNdEx] iNdEx++ - v |= (int(b) & 0x7F) << shift + m.ContainsEstimates |= (int64(b) & 0x7F) << shift if b < 0x80 { break } } - m.ContainsEstimates = bool(v != 0) default: iNdEx = preIndex skippy, err := skipMvcc3(dAtA[iNdEx:]) @@ -2659,7 +2648,7 @@ func (m *MVCCPersistentStats) Unmarshal(dAtA []byte) error { if wireType != 0 { return fmt.Errorf("proto: wrong wireType = %d for field ContainsEstimates", wireType) } - var v int + m.ContainsEstimates = 0 for shift := uint(0); ; shift += 7 { if shift >= 64 { return ErrIntOverflowMvcc3 @@ -2669,12 +2658,11 @@ func (m *MVCCPersistentStats) Unmarshal(dAtA []byte) error { } b := dAtA[iNdEx] iNdEx++ - v |= (int(b) & 0x7F) << shift + m.ContainsEstimates |= (int64(b) & 0x7F) << shift if b < 0x80 { break } } - m.ContainsEstimates = bool(v != 0) default: iNdEx = preIndex skippy, err := skipMvcc3(dAtA[iNdEx:]) @@ -3923,78 +3911,78 @@ var ( ) func init() { - proto.RegisterFile("storage/engine/enginepb/mvcc3.proto", fileDescriptor_mvcc3_103868dd91da76fd) + proto.RegisterFile("storage/engine/enginepb/mvcc3.proto", fileDescriptor_mvcc3_d6ebeaa8ad44885c) } -var fileDescriptor_mvcc3_103868dd91da76fd = []byte{ +var fileDescriptor_mvcc3_d6ebeaa8ad44885c = []byte{ // 1101 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xcc, 0x57, 0x4f, 0x6f, 0xe3, 0x44, - 0x14, 0xaf, 0x63, 0xa7, 0x75, 0x26, 0x69, 0x9b, 0xcc, 0xae, 0x44, 0xb4, 0x68, 0x93, 0x6e, 0x90, - 0x50, 0xc5, 0x1f, 0x07, 0x5a, 0xe0, 0xd0, 0x5b, 0xd2, 0xae, 0x20, 0x0b, 0xdd, 0x2e, 0xde, 0x74, - 0x57, 0x02, 0x21, 0x6b, 0xe2, 0x0c, 0xee, 0xa8, 0xf6, 0xd8, 0x6b, 0x4f, 0xba, 0xce, 0x81, 0x2f, - 0xc0, 0x89, 0x2f, 0x80, 0xd4, 0x0b, 0xdf, 0x80, 0x03, 0x1f, 0xa1, 0xc7, 0x3d, 0xae, 0x56, 0x28, - 0x82, 0xf4, 0xc2, 0x07, 0xe0, 0x54, 0x09, 0x09, 0xcd, 0x8c, 0xed, 0x24, 0x2b, 0x48, 0xff, 0x89, - 0x8a, 0x53, 0x67, 0xde, 0xef, 0xbd, 0xdf, 0x7b, 0x33, 0xbf, 0xe7, 0x79, 0x0d, 0x78, 0x2b, 0x62, - 0x7e, 0x88, 0x1c, 0xdc, 0xc4, 0xd4, 0x21, 0x34, 0xfd, 0x13, 0xf4, 0x9a, 0xde, 0x91, 0x6d, 0x6f, - 0x1a, 0x41, 0xe8, 0x33, 0x1f, 0xde, 0xb3, 0x7d, 0xfb, 0x30, 0xf4, 0x91, 0x7d, 0x60, 0x24, 0xee, - 0x86, 0xf4, 0x33, 0x52, 0xf7, 0x3b, 0xd5, 0x01, 0x23, 0x6e, 0xf3, 0xc0, 0xb5, 0x9b, 0x8c, 0x78, - 0x38, 0x62, 0xc8, 0x0b, 0x64, 0xf0, 0x9d, 0xdb, 0x8e, 0xef, 0xf8, 0x62, 0xd9, 0xe4, 0x2b, 0x69, - 0x6d, 0x7c, 0xaf, 0x82, 0xa5, 0x6e, 0x4c, 0x77, 0x31, 0x43, 0xf0, 0x4b, 0x90, 0x23, 0xfd, 0xaa, - 0xb2, 0xa6, 0xac, 0x97, 0xda, 0xad, 0x93, 0x51, 0x7d, 0xe1, 0xd5, 0xa8, 0xbe, 0xe9, 0x10, 0x76, - 0x30, 0xe8, 0x19, 0xb6, 0xef, 0x35, 0xb3, 0xec, 0xfd, 0xde, 0x64, 0xdd, 0x0c, 0x0e, 0x9d, 0xa6, - 0x48, 0x3a, 0x18, 0x90, 0xbe, 0xb1, 0xbf, 0xdf, 0xd9, 0x19, 0x8f, 0xea, 0xb9, 0xce, 0x8e, 0x99, - 0x23, 0x7d, 0x58, 0x06, 0xea, 0x21, 0x1e, 0x56, 0x55, 0xce, 0x69, 0xf2, 0x25, 0x6c, 0x80, 0x3c, - 0x0e, 0x7c, 0xfb, 0xa0, 0xaa, 0xad, 0x29, 0xeb, 0xf9, 0x76, 0xe9, 0x6c, 0x54, 0xd7, 0xbb, 0x31, - 0xbd, 0xcf, 0x6d, 0xa6, 0x84, 0x60, 0x0b, 0x14, 0xb2, 0xea, 0xab, 0xf9, 0x35, 0x65, 0xbd, 0xb8, - 0x71, 0xd7, 0x98, 0x9c, 0x9d, 0x67, 0x33, 0x0e, 0x5c, 0xdb, 0xe8, 0xa6, 0x4e, 0x6d, 0x8d, 0x97, - 0x6b, 0x4e, 0xa2, 0xe0, 0xbb, 0x40, 0x0f, 0x42, 0xe2, 0x87, 0x84, 0x0d, 0xab, 0x8b, 0x22, 0xd3, - 0xea, 0xd9, 0xa8, 0x5e, 0xec, 0xc6, 0xf4, 0x51, 0x62, 0x36, 0x33, 0x07, 0xf8, 0x36, 0xd0, 0x23, - 0xfc, 0x6c, 0x80, 0xa9, 0x8d, 0xab, 0x4b, 0xc2, 0x19, 0x9c, 0x8d, 0xea, 0x8b, 0xdd, 0x98, 0x3e, - 0xc6, 0xcf, 0xcc, 0x0c, 0x83, 0x9f, 0x81, 0x65, 0x8f, 0x50, 0x6b, 0x52, 0x5b, 0xe1, 0xe2, 0xb5, - 0x95, 0x3c, 0x42, 0x33, 0xdb, 0x96, 0xfe, 0xcb, 0x71, 0x5d, 0xf9, 0xe3, 0xb8, 0xae, 0x3c, 0xd0, - 0xf4, 0x5c, 0x59, 0x7d, 0xa0, 0xe9, 0x7a, 0xb9, 0xd0, 0xf8, 0x53, 0x05, 0x2b, 0xbb, 0x4f, 0xb6, - 0xb7, 0x1f, 0x33, 0xc4, 0xa2, 0x1d, 0xec, 0x32, 0x04, 0xdf, 0x01, 0x15, 0x17, 0x45, 0xcc, 0x1a, - 0x04, 0x7d, 0xc4, 0xb0, 0x45, 0x11, 0xf5, 0x23, 0x21, 0x51, 0xd9, 0x5c, 0xe5, 0xc0, 0xbe, 0xb0, - 0x3f, 0xe4, 0x66, 0x78, 0x17, 0x00, 0x42, 0x19, 0xa6, 0xcc, 0x42, 0x0e, 0xae, 0xe6, 0x84, 0x53, - 0x41, 0x5a, 0x5a, 0x0e, 0x86, 0x1f, 0x80, 0x92, 0x63, 0x5b, 0xbd, 0x21, 0xc3, 0x91, 0x70, 0xe0, - 0xa2, 0x94, 0xdb, 0x2b, 0xe3, 0x51, 0x1d, 0x7c, 0xba, 0xdd, 0xe6, 0xe6, 0x96, 0x83, 0x4d, 0xe0, - 0xd8, 0xe9, 0x9a, 0x13, 0xba, 0xe4, 0x08, 0xcb, 0x18, 0x21, 0x18, 0x34, 0x0b, 0xdc, 0x22, 0x3c, - 0x32, 0xd8, 0xf6, 0x07, 0x94, 0x09, 0x9d, 0x12, 0x78, 0x9b, 0x1b, 0xe0, 0x9b, 0xa0, 0x70, 0x88, - 0x87, 0x49, 0xf0, 0xa2, 0x40, 0xf5, 0x43, 0x3c, 0x94, 0xb1, 0x09, 0x28, 0x43, 0x97, 0x32, 0x30, - 0x8b, 0x3c, 0x42, 0x6e, 0x12, 0xa9, 0x4b, 0xf0, 0x08, 0xb9, 0x59, 0x24, 0x07, 0x65, 0x64, 0x21, - 0x03, 0x65, 0xe4, 0x3d, 0x50, 0x4a, 0xae, 0x40, 0x06, 0x03, 0x81, 0x17, 0xa5, 0x4d, 0xc6, 0x4f, - 0x5c, 0x24, 0x45, 0x71, 0xda, 0x25, 0xcb, 0x1f, 0x0d, 0xa3, 0x84, 0xa2, 0x24, 0x53, 0x44, 0xc3, - 0x28, 0xcb, 0xcf, 0x41, 0x19, 0xbc, 0x9c, 0x81, 0x32, 0xf2, 0x7d, 0x00, 0x6d, 0x9f, 0x32, 0x44, - 0x68, 0x64, 0xe1, 0x88, 0x11, 0x0f, 0x71, 0x8a, 0x95, 0x35, 0x65, 0x5d, 0x37, 0x2b, 0x29, 0x72, - 0x3f, 0x05, 0xb6, 0x34, 0xde, 0x02, 0x8d, 0xbf, 0x54, 0x70, 0x8b, 0xcb, 0xfe, 0x08, 0x87, 0x11, - 0x89, 0x78, 0x19, 0xa2, 0x01, 0xfe, 0x6f, 0xda, 0xab, 0xf3, 0xb5, 0x57, 0xe7, 0x6a, 0xaf, 0xce, - 0xd3, 0x5e, 0x9d, 0xa7, 0xbd, 0x3a, 0x4f, 0x7b, 0xf5, 0x1c, 0xed, 0xd5, 0xf3, 0xb5, 0x57, 0xcf, - 0xd1, 0x5e, 0x9d, 0xa7, 0xbd, 0x7a, 0x75, 0xed, 0xb3, 0x27, 0xa0, 0xf1, 0x4a, 0x01, 0x15, 0x13, - 0x51, 0x07, 0xb7, 0x82, 0xc0, 0x25, 0xb8, 0xcf, 0xd5, 0xc7, 0xf0, 0x3d, 0x00, 0x43, 0xf4, 0x2d, - 0xb3, 0x90, 0x34, 0x5a, 0x84, 0xf6, 0x71, 0x2c, 0xe4, 0xd7, 0xcc, 0x32, 0x47, 0x12, 0xef, 0x0e, - 0xb7, 0x43, 0x03, 0xdc, 0x72, 0x31, 0x8a, 0xf0, 0x6b, 0xee, 0x39, 0xe1, 0x5e, 0x11, 0xd0, 0x8c, - 0xff, 0x37, 0xa0, 0x18, 0xf2, 0x94, 0x56, 0xc4, 0x5b, 0x4d, 0xf4, 0x43, 0x71, 0xe3, 0x13, 0xe3, - 0xdc, 0x01, 0x63, 0xfc, 0x43, 0xa3, 0x26, 0x2f, 0x1c, 0x10, 0x84, 0xc2, 0x32, 0x75, 0xb8, 0xef, - 0x40, 0x99, 0x87, 0x3c, 0x0d, 0x09, 0xc3, 0x4f, 0x90, 0x3b, 0xc0, 0x7b, 0x41, 0x3a, 0x15, 0x94, - 0xc9, 0x54, 0x98, 0x79, 0xf1, 0x73, 0x57, 0x7a, 0xf1, 0x6f, 0x83, 0xfc, 0x11, 0xe7, 0x4f, 0x86, - 0x8d, 0xdc, 0x34, 0x7e, 0xcc, 0x81, 0x4a, 0x96, 0xbf, 0x23, 0x74, 0xde, 0x0b, 0xe0, 0xd7, 0x60, - 0x91, 0xc5, 0xd4, 0xca, 0xa6, 0xdd, 0xce, 0xf5, 0xa6, 0x5d, 0xbe, 0x1b, 0xd3, 0xce, 0x8e, 0x99, - 0x67, 0x31, 0xed, 0xf4, 0xe1, 0x1b, 0x60, 0x89, 0x93, 0xf3, 0x13, 0xe6, 0x44, 0x29, 0x3c, 0xd7, - 0xe7, 0xaf, 0x1f, 0x52, 0xbd, 0xd2, 0x21, 0xf7, 0x40, 0x85, 0x73, 0xcf, 0x4e, 0x21, 0xed, 0xe2, - 0x54, 0xab, 0x2c, 0xa6, 0xbb, 0x53, 0x83, 0xa8, 0xf1, 0xb3, 0x02, 0x20, 0xbf, 0x1f, 0xf9, 0x96, - 0xdc, 0xcc, 0x05, 0x5d, 0x5f, 0xec, 0xc6, 0xaf, 0x49, 0xd9, 0xdb, 0xbe, 0xe7, 0x11, 0x76, 0x33, - 0x65, 0x27, 0x5d, 0x9b, 0xfb, 0x97, 0xae, 0x55, 0xaf, 0xd7, 0xb5, 0xda, 0x74, 0xd7, 0x06, 0xb2, - 0x69, 0x5b, 0x3d, 0x3f, 0xbc, 0x99, 0xc3, 0x35, 0x3c, 0xf9, 0x9f, 0x87, 0xc8, 0xd8, 0x8d, 0xe9, - 0x7f, 0x9d, 0xee, 0x27, 0x0d, 0x2c, 0xf3, 0x7c, 0x5f, 0xf8, 0x0e, 0xb1, 0x91, 0xbb, 0x17, 0xc0, - 0x2e, 0x28, 0x3e, 0xe7, 0xdf, 0xa8, 0x25, 0xaf, 0x43, 0x11, 0xb7, 0xb9, 0x79, 0xc1, 0x07, 0x69, - 0xfa, 0x75, 0x31, 0xc1, 0xf3, 0x6c, 0x07, 0x9f, 0x82, 0x92, 0x64, 0x95, 0x4f, 0x7c, 0xd2, 0x6d, - 0x1f, 0x5d, 0x86, 0x36, 0xbd, 0x7f, 0x53, 0xd6, 0x27, 0xb7, 0xf0, 0x2b, 0xb0, 0x9c, 0x8c, 0xe5, - 0x84, 0x59, 0xca, 0xff, 0xf1, 0x05, 0x99, 0x67, 0x3f, 0x37, 0xb3, 0x34, 0x98, 0xda, 0x73, 0x6e, - 0x5b, 0xf4, 0x75, 0xca, 0xad, 0x5d, 0x8a, 0x7b, 0xf6, 0x9b, 0x30, 0x4b, 0xf6, 0xd4, 0x9e, 0x5f, - 0x08, 0xe2, 0x1a, 0xa7, 0xd4, 0xf9, 0x4b, 0x5d, 0xc8, 0x4c, 0x43, 0x9a, 0x45, 0x34, 0xd9, 0xc2, - 0x87, 0xa0, 0x20, 0x89, 0x59, 0x4c, 0xc5, 0xc4, 0x2f, 0x6e, 0x7c, 0x78, 0x19, 0x56, 0xd1, 0x74, - 0xa6, 0x8e, 0x92, 0xf5, 0x96, 0x76, 0x72, 0x5c, 0x57, 0xda, 0x6b, 0x27, 0xbf, 0xd7, 0x16, 0x4e, - 0xc6, 0x35, 0xe5, 0xc5, 0xb8, 0xa6, 0xbc, 0x1c, 0xd7, 0x94, 0xdf, 0xc6, 0x35, 0xe5, 0x87, 0xd3, - 0xda, 0xc2, 0x8b, 0xd3, 0xda, 0xc2, 0xcb, 0xd3, 0xda, 0x42, 0x6f, 0x51, 0xfc, 0x8e, 0xd9, 0xfc, - 0x3b, 0x00, 0x00, 0xff, 0xff, 0x74, 0x64, 0x63, 0xef, 0x41, 0x0d, 0x00, 0x00, + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xcc, 0x57, 0x4f, 0x6f, 0x1b, 0x45, + 0x14, 0xcf, 0x7a, 0xd7, 0x89, 0xfd, 0xec, 0x24, 0xf6, 0xb4, 0x12, 0x56, 0x51, 0xed, 0xd4, 0x48, + 0x28, 0xe2, 0xcf, 0x1a, 0x12, 0xe0, 0x90, 0x9b, 0x9d, 0x54, 0xe0, 0x42, 0x9a, 0xb2, 0x75, 0x5a, + 0x09, 0x84, 0x56, 0xe3, 0xf5, 0xb0, 0x19, 0x65, 0x3d, 0xbb, 0xf5, 0x8e, 0xd3, 0xf5, 0x81, 0x2f, + 0xc0, 0x89, 0x2f, 0x80, 0x94, 0x0b, 0xdf, 0x80, 0x03, 0x1f, 0x21, 0xc7, 0x1e, 0xab, 0x0a, 0x59, + 0xe0, 0x5c, 0xf8, 0x00, 0x9c, 0x22, 0x21, 0xa1, 0x99, 0x59, 0xaf, 0xed, 0x0a, 0x9c, 0x7f, 0x22, + 0xe2, 0x94, 0x99, 0xf7, 0x7b, 0xef, 0xf7, 0xde, 0xcc, 0xef, 0xed, 0xbc, 0x18, 0xde, 0x0a, 0xb9, + 0xdf, 0xc3, 0x2e, 0xa9, 0x11, 0xe6, 0x52, 0x36, 0xfe, 0x13, 0xb4, 0x6b, 0xdd, 0x23, 0xc7, 0xd9, + 0x34, 0x83, 0x9e, 0xcf, 0x7d, 0x74, 0xcf, 0xf1, 0x9d, 0xc3, 0x9e, 0x8f, 0x9d, 0x03, 0x33, 0x76, + 0x37, 0x95, 0x9f, 0x39, 0x76, 0xbf, 0x53, 0xea, 0x73, 0xea, 0xd5, 0x0e, 0x3c, 0xa7, 0xc6, 0x69, + 0x97, 0x84, 0x1c, 0x77, 0x03, 0x15, 0x7c, 0xe7, 0xb6, 0xeb, 0xbb, 0xbe, 0x5c, 0xd6, 0xc4, 0x4a, + 0x59, 0xab, 0xdf, 0xeb, 0xb0, 0xd4, 0x8a, 0xd8, 0x2e, 0xe1, 0x18, 0x7d, 0x09, 0x29, 0xda, 0x29, + 0x69, 0x6b, 0xda, 0x7a, 0xbe, 0x51, 0x3f, 0x19, 0x56, 0x16, 0x5e, 0x0d, 0x2b, 0x9b, 0x2e, 0xe5, + 0x07, 0xfd, 0xb6, 0xe9, 0xf8, 0xdd, 0x5a, 0x92, 0xbd, 0xd3, 0x9e, 0xac, 0x6b, 0xc1, 0xa1, 0x5b, + 0x93, 0x49, 0xfb, 0x7d, 0xda, 0x31, 0xf7, 0xf7, 0x9b, 0x3b, 0xa3, 0x61, 0x25, 0xd5, 0xdc, 0xb1, + 0x52, 0xb4, 0x83, 0x0a, 0xa0, 0x1f, 0x92, 0x41, 0x49, 0x17, 0x9c, 0x96, 0x58, 0xa2, 0x2a, 0xa4, + 0x49, 0xe0, 0x3b, 0x07, 0x25, 0x63, 0x4d, 0x5b, 0x4f, 0x37, 0xf2, 0x67, 0xc3, 0x4a, 0xa6, 0x15, + 0xb1, 0xfb, 0xc2, 0x66, 0x29, 0x08, 0xd5, 0x21, 0x9b, 0x54, 0x5f, 0x4a, 0xaf, 0x69, 0xeb, 0xb9, + 0x8d, 0xbb, 0xe6, 0xe4, 0xec, 0x22, 0x9b, 0x79, 0xe0, 0x39, 0x66, 0x6b, 0xec, 0xd4, 0x30, 0x44, + 0xb9, 0xd6, 0x24, 0x0a, 0xbd, 0x0b, 0x99, 0xa0, 0x47, 0xfd, 0x1e, 0xe5, 0x83, 0xd2, 0xa2, 0xcc, + 0xb4, 0x7a, 0x36, 0xac, 0xe4, 0x5a, 0x11, 0x7b, 0x14, 0x9b, 0xad, 0xc4, 0x01, 0xbd, 0x0d, 0x99, + 0x90, 0x3c, 0xeb, 0x13, 0xe6, 0x90, 0xd2, 0x92, 0x74, 0x86, 0xb3, 0x61, 0x65, 0xb1, 0x15, 0xb1, + 0xc7, 0xe4, 0x99, 0x95, 0x60, 0xe8, 0x33, 0x58, 0xee, 0x52, 0x66, 0x4f, 0x6a, 0xcb, 0x5e, 0xbc, + 0xb6, 0x7c, 0x97, 0xb2, 0xc4, 0xb6, 0x95, 0xf9, 0xe5, 0xb8, 0xa2, 0xfd, 0x71, 0x5c, 0xd1, 0x1e, + 0x18, 0x99, 0x54, 0x41, 0x7f, 0x60, 0x64, 0x32, 0x85, 0x6c, 0xf5, 0x4f, 0x1d, 0x56, 0x76, 0x9f, + 0x6c, 0x6f, 0x3f, 0xe6, 0x98, 0x87, 0x3b, 0xc4, 0xe3, 0x18, 0xbd, 0x03, 0x45, 0x0f, 0x87, 0xdc, + 0xee, 0x07, 0x1d, 0xcc, 0x89, 0xcd, 0x30, 0xf3, 0x43, 0x29, 0x51, 0xc1, 0x5a, 0x15, 0xc0, 0xbe, + 0xb4, 0x3f, 0x14, 0x66, 0x74, 0x17, 0x80, 0x32, 0x4e, 0x18, 0xb7, 0xb1, 0x4b, 0x4a, 0x29, 0xe9, + 0x94, 0x55, 0x96, 0xba, 0x4b, 0xd0, 0x07, 0x90, 0x77, 0x1d, 0xbb, 0x3d, 0xe0, 0x24, 0x94, 0x0e, + 0x42, 0x94, 0x42, 0x63, 0x65, 0x34, 0xac, 0xc0, 0xa7, 0xdb, 0x0d, 0x61, 0xae, 0xbb, 0xc4, 0x02, + 0xd7, 0x19, 0xaf, 0x05, 0xa1, 0x47, 0x8f, 0x88, 0x8a, 0x91, 0x82, 0x21, 0x2b, 0x2b, 0x2c, 0xd2, + 0x23, 0x81, 0x1d, 0xbf, 0xcf, 0xb8, 0xd4, 0x29, 0x86, 0xb7, 0x85, 0x01, 0xbd, 0x09, 0xd9, 0x43, + 0x32, 0x88, 0x83, 0x17, 0x25, 0x9a, 0x39, 0x24, 0x03, 0x15, 0x1b, 0x83, 0x2a, 0x74, 0x29, 0x01, + 0x93, 0xc8, 0x23, 0xec, 0xc5, 0x91, 0x19, 0x05, 0x1e, 0x61, 0x2f, 0x89, 0x14, 0xa0, 0x8a, 0xcc, + 0x26, 0xa0, 0x8a, 0xbc, 0x07, 0xf9, 0xf8, 0x0a, 0x54, 0x30, 0x48, 0x3c, 0xa7, 0x6c, 0x2a, 0x7e, + 0xe2, 0xa2, 0x28, 0x72, 0xd3, 0x2e, 0x49, 0xfe, 0x70, 0x10, 0xc6, 0x14, 0x79, 0x95, 0x22, 0x1c, + 0x84, 0x49, 0x7e, 0x01, 0xaa, 0xe0, 0xe5, 0x04, 0x54, 0x91, 0xef, 0x03, 0x72, 0x7c, 0xc6, 0x31, + 0x65, 0xa1, 0x4d, 0x42, 0x4e, 0xbb, 0x58, 0x50, 0xac, 0xac, 0x69, 0xeb, 0xba, 0x55, 0x1c, 0x23, + 0xf7, 0xc7, 0xc0, 0x96, 0x21, 0x5a, 0xa0, 0xfa, 0x97, 0x0e, 0xb7, 0x84, 0xec, 0x8f, 0x48, 0x2f, + 0xa4, 0xa1, 0x28, 0x43, 0x36, 0xc0, 0xff, 0x4d, 0x7b, 0x7d, 0xbe, 0xf6, 0xfa, 0x5c, 0xed, 0xf5, + 0x79, 0xda, 0xeb, 0xf3, 0xb4, 0xd7, 0xe7, 0x69, 0xaf, 0x9f, 0xa3, 0xbd, 0x7e, 0xbe, 0xf6, 0xfa, + 0x39, 0xda, 0xeb, 0xf3, 0xb4, 0xd7, 0xaf, 0xae, 0x7d, 0xf2, 0x04, 0x54, 0x5f, 0x69, 0x50, 0xb4, + 0x30, 0x73, 0x49, 0x3d, 0x08, 0x3c, 0x4a, 0x3a, 0x42, 0x7d, 0x82, 0xde, 0x03, 0xd4, 0xc3, 0xdf, + 0x72, 0x1b, 0x2b, 0xa3, 0x4d, 0x59, 0x87, 0x44, 0x52, 0x7e, 0xc3, 0x2a, 0x08, 0x24, 0xf6, 0x6e, + 0x0a, 0x3b, 0x32, 0xe1, 0x96, 0x47, 0x70, 0x48, 0x5e, 0x73, 0x4f, 0x49, 0xf7, 0xa2, 0x84, 0x66, + 0xfc, 0xbf, 0x81, 0x5c, 0x4f, 0xa4, 0xb4, 0x43, 0xd1, 0x6a, 0xb2, 0x1f, 0x72, 0x1b, 0x9f, 0x98, + 0xe7, 0x0e, 0x18, 0xf3, 0x1f, 0x1a, 0x35, 0x7e, 0xe1, 0x40, 0x12, 0x4a, 0xcb, 0xd4, 0xe1, 0xbe, + 0x83, 0x82, 0x08, 0x79, 0xda, 0xa3, 0x9c, 0x3c, 0xc1, 0x5e, 0x9f, 0xec, 0x05, 0xe3, 0xa9, 0xa0, + 0x4d, 0xa6, 0xc2, 0xcc, 0x8b, 0x9f, 0xba, 0xd2, 0x8b, 0x7f, 0x1b, 0xd2, 0x47, 0x82, 0x3f, 0x1e, + 0x36, 0x6a, 0x53, 0xfd, 0x31, 0x05, 0xc5, 0x24, 0x7f, 0x53, 0xea, 0xbc, 0x17, 0xa0, 0xaf, 0x61, + 0x91, 0x47, 0xcc, 0x4e, 0xa6, 0xdd, 0xce, 0xf5, 0xa6, 0x5d, 0xba, 0x15, 0xb1, 0xe6, 0x8e, 0x95, + 0xe6, 0x11, 0x6b, 0x76, 0xd0, 0x1b, 0xb0, 0x24, 0xc8, 0xc5, 0x09, 0x53, 0xb2, 0x14, 0x91, 0xeb, + 0xf3, 0xd7, 0x0f, 0xa9, 0x5f, 0xe9, 0x90, 0x7b, 0x50, 0x14, 0xdc, 0xb3, 0x53, 0xc8, 0xb8, 0x38, + 0xd5, 0x2a, 0x8f, 0xd8, 0xee, 0xd4, 0x20, 0xaa, 0xfe, 0xac, 0x01, 0x12, 0xf7, 0xa3, 0xde, 0x92, + 0x9b, 0xb9, 0xa0, 0xeb, 0x8b, 0x5d, 0xfd, 0x35, 0x2e, 0x7b, 0xdb, 0xef, 0x76, 0x29, 0xbf, 0x99, + 0xb2, 0xe3, 0xae, 0x4d, 0xfd, 0x4b, 0xd7, 0xea, 0xd7, 0xeb, 0x5a, 0x63, 0xba, 0x6b, 0x03, 0xd5, + 0xb4, 0xf5, 0xb6, 0xdf, 0xbb, 0x99, 0xc3, 0x55, 0xbb, 0xea, 0x3f, 0x0f, 0x99, 0xb1, 0x15, 0xb1, + 0xff, 0x3a, 0xdd, 0x4f, 0x06, 0x2c, 0x8b, 0x7c, 0x5f, 0xf8, 0x2e, 0x75, 0xb0, 0xb7, 0x17, 0xa0, + 0x16, 0xe4, 0x9e, 0x8b, 0x6f, 0xd4, 0x56, 0xd7, 0xa1, 0xc9, 0xdb, 0xdc, 0xbc, 0xe0, 0x83, 0x34, + 0xfd, 0xba, 0x58, 0xf0, 0x3c, 0xd9, 0xa1, 0xa7, 0x90, 0x57, 0xac, 0xea, 0x89, 0x8f, 0xbb, 0xed, + 0xa3, 0xcb, 0xd0, 0x8e, 0xef, 0xdf, 0x52, 0xf5, 0xa9, 0x2d, 0xfa, 0x0a, 0x96, 0xe3, 0xb1, 0x1c, + 0x33, 0x2b, 0xf9, 0x3f, 0xbe, 0x20, 0xf3, 0xec, 0xe7, 0x66, 0xe5, 0xfb, 0x53, 0x7b, 0xc1, 0xed, + 0xc8, 0xbe, 0x1e, 0x73, 0x1b, 0x97, 0xe2, 0x9e, 0xfd, 0x26, 0xac, 0xbc, 0x33, 0xb5, 0x17, 0x17, + 0x82, 0x85, 0xc6, 0x63, 0xea, 0xf4, 0xa5, 0x2e, 0x64, 0xa6, 0x21, 0xad, 0x1c, 0x9e, 0x6c, 0xd1, + 0x43, 0xc8, 0x2a, 0x62, 0x1e, 0x31, 0x39, 0xf1, 0x73, 0x1b, 0x1f, 0x5e, 0x86, 0x55, 0x36, 0x9d, + 0x95, 0xc1, 0xf1, 0x7a, 0xcb, 0x38, 0x39, 0xae, 0x68, 0x8d, 0xb5, 0x93, 0xdf, 0xcb, 0x0b, 0x27, + 0xa3, 0xb2, 0xf6, 0x62, 0x54, 0xd6, 0x5e, 0x8e, 0xca, 0xda, 0x6f, 0xa3, 0xb2, 0xf6, 0xc3, 0x69, + 0x79, 0xe1, 0xc5, 0x69, 0x79, 0xe1, 0xe5, 0x69, 0x79, 0xa1, 0xbd, 0x28, 0x7f, 0xc7, 0x6c, 0xfe, + 0x1d, 0x00, 0x00, 0xff, 0xff, 0x45, 0x60, 0x94, 0x6c, 0x41, 0x0d, 0x00, 0x00, } diff --git a/pkg/storage/engine/enginepb/mvcc3.proto b/pkg/storage/engine/enginepb/mvcc3.proto index 78ec096d8f3d..4bb924e4e537 100644 --- a/pkg/storage/engine/enginepb/mvcc3.proto +++ b/pkg/storage/engine/enginepb/mvcc3.proto @@ -121,10 +121,7 @@ message TxnMeta { message MVCCStatsDelta { option (gogoproto.equal) = true; - // TODO(nvanbenschoten): now that we've split MVCCPersistentStats - // from this MVCCStatsDelta type, we can turn contains_estimates - // into a three-valued type ('UNCHANGED', 'NO', and 'YES'). - bool contains_estimates = 14; + int64 contains_estimates = 14; sfixed64 last_update_nanos = 1; sfixed64 intent_age = 2; sfixed64 gc_bytes_age = 3 [(gogoproto.customname) = "GCBytesAge"]; @@ -148,7 +145,7 @@ message MVCCPersistentStats { option (gogoproto.equal) = true; option (gogoproto.populate) = true; - bool contains_estimates = 14; + int64 contains_estimates = 14; // must never go negative absent a bug sfixed64 last_update_nanos = 1; sfixed64 intent_age = 2; sfixed64 gc_bytes_age = 3 [(gogoproto.customname) = "GCBytesAge"]; diff --git a/pkg/storage/engine/mvcc.go b/pkg/storage/engine/mvcc.go index b08c7fa9400f..b4c89f068813 100644 --- a/pkg/storage/engine/mvcc.go +++ b/pkg/storage/engine/mvcc.go @@ -22,6 +22,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/keys" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/storage/engine/enginepb" "github.com/cockroachdb/cockroach/pkg/util/encoding" "github.com/cockroachdb/cockroach/pkg/util/hlc" @@ -217,7 +218,10 @@ func updateStatsOnMerge(key roachpb.Key, valSize, nowNanos int64) enginepb.MVCCS var ms enginepb.MVCCStats sys := isSysLocal(key) ms.AgeTo(nowNanos) - ms.ContainsEstimates = true + + _ = cluster.VersionContainsEstimatesCounter // see for info on ContainsEstimates migration + ms.ContainsEstimates = 1 + if sys { ms.SysBytes += valSize } else { diff --git a/pkg/storage/engine/mvcc_test.go b/pkg/storage/engine/mvcc_test.go index 1e76dd5a56eb..1002cd6e584e 100644 --- a/pkg/storage/engine/mvcc_test.go +++ b/pkg/storage/engine/mvcc_test.go @@ -173,7 +173,7 @@ var mvccGetImpls = []struct { func TestMVCCStatsAddSubForward(t *testing.T) { defer leaktest.AfterTest(t)() goldMS := enginepb.MVCCStats{ - ContainsEstimates: true, + ContainsEstimates: 1, KeyBytes: 1, KeyCount: 1, ValBytes: 1, @@ -202,7 +202,7 @@ func TestMVCCStatsAddSubForward(t *testing.T) { ms := goldMS zeroWithLU := enginepb.MVCCStats{ - ContainsEstimates: true, + ContainsEstimates: 0, LastUpdateNanos: ms.LastUpdateNanos, } diff --git a/pkg/storage/engine/rocksdb.go b/pkg/storage/engine/rocksdb.go index a07c46816ce4..90ef16879726 100644 --- a/pkg/storage/engine/rocksdb.go +++ b/pkg/storage/engine/rocksdb.go @@ -2505,7 +2505,8 @@ func cStatsToGoStats(stats C.MVCCStatsResult, nowNanos int64) (enginepb.MVCCStat if err := statusToError(stats.status); err != nil { return ms, err } - ms.ContainsEstimates = false + + ms.ContainsEstimates = 0 ms.LiveBytes = int64(stats.live_bytes) ms.KeyBytes = int64(stats.key_bytes) ms.ValBytes = int64(stats.val_bytes) diff --git a/pkg/storage/gc_queue.go b/pkg/storage/gc_queue.go index 237b102094dd..be6fc1825e26 100644 --- a/pkg/storage/gc_queue.go +++ b/pkg/storage/gc_queue.go @@ -282,7 +282,7 @@ func makeGCQueueScoreImpl( // If we GC'ed now, we can expect to delete at least this much GCByteAge. // GCByteAge - TTL*GCBytes = ExpMinGCByteAgeReduction & algebra. // - // Note that for ranges with ContainsEstimates=true, the value here may not + // Note that for ranges with ContainsEstimates > 0, the value here may not // reflect reality, and may even be nonsensical (though that's unlikely). r.ExpMinGCByteAgeReduction = r.GCByteAge - r.GCBytes*int64(r.TTL.Seconds()) @@ -292,7 +292,7 @@ func makeGCQueueScoreImpl( // completely by a DeleteRange, it should be (almost) one. // // The algebra below is complicated by the fact that ranges may contain - // stats that aren't exact (ContainsEstimates=true). + // stats that aren't exact (ContainsEstimates > 0). clamp := func(n int64) float64 { if n < 0 { return 0.0 diff --git a/pkg/storage/gc_queue_test.go b/pkg/storage/gc_queue_test.go index f8dc26c1599f..ebe2f255ee2b 100644 --- a/pkg/storage/gc_queue_test.go +++ b/pkg/storage/gc_queue_test.go @@ -118,7 +118,7 @@ func TestGCQueueMakeGCScoreInvariantQuick(t *testing.T) { func TestGCQueueMakeGCScoreAnomalousStats(t *testing.T) { defer leaktest.AfterTest(t)() - if err := quick.Check(func(keyBytes, valBytes, liveBytes int32, containsEstimates bool) bool { + if err := quick.Check(func(keyBytes, valBytes, liveBytes int32, containsEstimates int64) bool { r := makeGCQueueScoreImpl(context.Background(), 0, hlc.Timestamp{}, enginepb.MVCCStats{ ContainsEstimates: containsEstimates, LiveBytes: int64(liveBytes), @@ -153,11 +153,11 @@ func newCachedWriteSimulator(t *testing.T) *cachedWriteSimulator { cws.cache = map[gcTestCacheKey]gcTestCacheVal{ {enginepb.MVCCStats{LastUpdateNanos: 946684800000000000}, "1-1m0s-1.0 MiB"}: { first: [cacheFirstLen]enginepb.MVCCStats{ - {ContainsEstimates: false, LastUpdateNanos: 946684800000000000, IntentAge: 0, GCBytesAge: 0, LiveBytes: 1048604, LiveCount: 1, KeyBytes: 23, KeyCount: 1, ValBytes: 1048581, ValCount: 1, IntentBytes: 0, IntentCount: 0, SysBytes: 0, SysCount: 0}, - {ContainsEstimates: false, LastUpdateNanos: 946684801000000000, IntentAge: 0, GCBytesAge: 0, LiveBytes: 1048604, LiveCount: 1, KeyBytes: 35, KeyCount: 1, ValBytes: 2097162, ValCount: 2, IntentBytes: 0, IntentCount: 0, SysBytes: 0, SysCount: 0}, - {ContainsEstimates: false, LastUpdateNanos: 946684802000000000, IntentAge: 0, GCBytesAge: 1048593, LiveBytes: 1048604, LiveCount: 1, KeyBytes: 47, KeyCount: 1, ValBytes: 3145743, ValCount: 3, IntentBytes: 0, IntentCount: 0, SysBytes: 0, SysCount: 0}, + {ContainsEstimates: 0, LastUpdateNanos: 946684800000000000, IntentAge: 0, GCBytesAge: 0, LiveBytes: 1048604, LiveCount: 1, KeyBytes: 23, KeyCount: 1, ValBytes: 1048581, ValCount: 1, IntentBytes: 0, IntentCount: 0, SysBytes: 0, SysCount: 0}, + {ContainsEstimates: 0, LastUpdateNanos: 946684801000000000, IntentAge: 0, GCBytesAge: 0, LiveBytes: 1048604, LiveCount: 1, KeyBytes: 35, KeyCount: 1, ValBytes: 2097162, ValCount: 2, IntentBytes: 0, IntentCount: 0, SysBytes: 0, SysCount: 0}, + {ContainsEstimates: 0, LastUpdateNanos: 946684802000000000, IntentAge: 0, GCBytesAge: 1048593, LiveBytes: 1048604, LiveCount: 1, KeyBytes: 47, KeyCount: 1, ValBytes: 3145743, ValCount: 3, IntentBytes: 0, IntentCount: 0, SysBytes: 0, SysCount: 0}, }, - last: enginepb.MVCCStats{ContainsEstimates: false, LastUpdateNanos: 946684860000000000, IntentAge: 0, GCBytesAge: 1856009610, LiveBytes: 1048604, LiveCount: 1, KeyBytes: 743, KeyCount: 1, ValBytes: 63963441, ValCount: 61, IntentBytes: 0, IntentCount: 0, SysBytes: 0, SysCount: 0}, + last: enginepb.MVCCStats{ContainsEstimates: 0, LastUpdateNanos: 946684860000000000, IntentAge: 0, GCBytesAge: 1856009610, LiveBytes: 1048604, LiveCount: 1, KeyBytes: 743, KeyCount: 1, ValBytes: 63963441, ValCount: 61, IntentBytes: 0, IntentCount: 0, SysBytes: 0, SysCount: 0}, }, } return &cws diff --git a/pkg/storage/replica_application_state_machine.go b/pkg/storage/replica_application_state_machine.go index e4ed70d39854..553d63d94a24 100644 --- a/pkg/storage/replica_application_state_machine.go +++ b/pkg/storage/replica_application_state_machine.go @@ -16,6 +16,7 @@ import ( "time" "github.com/cockroachdb/cockroach/pkg/roachpb" + "github.com/cockroachdb/cockroach/pkg/settings/cluster" "github.com/cockroachdb/cockroach/pkg/storage/apply" "github.com/cockroachdb/cockroach/pkg/storage/engine" "github.com/cockroachdb/cockroach/pkg/storage/engine/enginepb" @@ -715,22 +716,46 @@ func (b *replicaAppBatch) stageTrivialReplicatedEvalResult( b.state.LeaseAppliedIndex = leaseAppliedIndex } res := cmd.replicatedResult() + + // Detect whether the incoming stats contain estimates that resulted from the + // evaluation of a command under the 19.1 cluster version. These were either + // evaluated on a 19.1 node (where ContainsEstimates is a bool, which maps + // to 0 and 1 in 19.2+) or on a 19.2 node which hadn't yet had its cluster + // version bumped. + // + // 19.2 nodes will never emit a ContainsEstimates outside of 0 or 1 until + // the cluster version is active (during command evaluation). When the + // version is active, they will never emit odd positive numbers (1, 3, ...). + // + // As a result, we can pinpoint exactly when the proposer of this command + // has used the old cluster version: it's when the incoming + // ContainsEstimates is 1. If so, we need to assume that an old node is processing + // the same commands (as `true + true = true`), so make sure that `1 + 1 = 1`. + _ = cluster.VersionContainsEstimatesCounter // see for info on ContainsEstimates migration + deltaStats := res.Delta.ToStats() + if deltaStats.ContainsEstimates == 1 && b.state.Stats.ContainsEstimates == 1 { + deltaStats.ContainsEstimates = 0 + } + // Special-cased MVCC stats handling to exploit commutativity of stats delta // upgrades. Thanks to commutativity, the spanlatch manager does not have to // serialize on the stats key. - b.state.Stats.Add(res.Delta.ToStats()) + b.state.Stats.Add(deltaStats) // Exploit the fact that a split will result in a full stats // recomputation to reset the ContainsEstimates flag. + // If we were running the new VersionContainsEstimatesCounter cluster version, + // the consistency checker will be able to reset the stats itself, and splits + // will as a side effect also remove estimates from both the resulting left and right hand sides. // - // TODO(tschottdorf): We want to let the usual MVCCStats-delta - // machinery update our stats for the left-hand side. But there is no - // way to pass up an MVCCStats object that will clear out the - // ContainsEstimates flag. We should introduce one, but the migration - // makes this worth a separate effort (ContainsEstimates would need to - // have three possible values, 'UNCHANGED', 'NO', and 'YES'). - // Until then, we're left with this rather crude hack. - if res.Split != nil { - b.state.Stats.ContainsEstimates = false + // TODO(tbg): this can be removed in v20.2 and not earlier. + // Consider the following scenario: + // - all nodes are running 19.2 + // - all nodes rebooted into 20.1 + // - cluster version bumped, but node1 doesn't receive the gossip update for that + // node1 runs a split that should emit ContainsEstimates=-1, but it clamps it to 0/1 because it + // doesn't know that 20.1 is active. + if res.Split != nil && deltaStats.ContainsEstimates == 0 { + b.state.Stats.ContainsEstimates = 0 } if res.State != nil && res.State.UsingAppliedStateKey && !b.state.UsingAppliedStateKey { b.migrateToAppliedStateKey = true diff --git a/pkg/storage/replica_consistency.go b/pkg/storage/replica_consistency.go index 0b657395e4a9..47a06e4ce3bf 100644 --- a/pkg/storage/replica_consistency.go +++ b/pkg/storage/replica_consistency.go @@ -191,13 +191,14 @@ func (r *Replica) CheckConsistency( if minoritySHA != "" { res.Status = roachpb.CheckConsistencyResponse_RANGE_INCONSISTENT } else if args.Mode != roachpb.ChecksumMode_CHECK_STATS && haveDelta { - if delta.ContainsEstimates { + if delta.ContainsEstimates > 0 { // When ContainsEstimates is set, it's generally expected that we'll get a different // result when we recompute from scratch. res.Status = roachpb.CheckConsistencyResponse_RANGE_CONSISTENT_STATS_ESTIMATED } else { - // When ContainsEstimates is set, it's generally expected that we'll get a different - // result when we recompute from scratch. + // When ContainsEstimates is unset, we expect the recomputation to agree with the stored stats. + // If that's not the case, that's a problem: it could be a bug in the stats computation + // or stats maintenance, but it could also hint at the replica having diverged from its peers. res.Status = roachpb.CheckConsistencyResponse_RANGE_CONSISTENT_STATS_INCORRECT } res.Detail += fmt.Sprintf("stats - recomputation: %+v\n", enginepb.MVCCStats(results[0].Response.Delta)) @@ -227,10 +228,9 @@ func (r *Replica) CheckConsistency( return resp, nil } - if !delta.ContainsEstimates && fatalOnStatsMismatch { - // ContainsEstimates is true if the replica's persisted MVCCStats had ContainsEstimates set. - // If this was *not* the case, the replica believed it had accurate stats. But we just found - // out that this isn't true. + if delta.ContainsEstimates <= 0 && fatalOnStatsMismatch { + // We just found out that the recomputation doesn't match the persisted stats, + // so ContainsEstimates should have been strictly positive. var v roachpb.Version if err := r.store.db.Txn(ctx, func(ctx context.Context, txn *client.Txn) error { diff --git a/pkg/storage/replica_proposal.go b/pkg/storage/replica_proposal.go index fd99775604d7..de893fa711a6 100644 --- a/pkg/storage/replica_proposal.go +++ b/pkg/storage/replica_proposal.go @@ -747,6 +747,28 @@ func (r *Replica) evaluateProposal( res.Replicated.Timestamp = ba.Timestamp res.Replicated.Delta = ms.ToStatsDelta() + _ = cluster.VersionContainsEstimatesCounter // see for info on ContainsEstimates migration + if cluster.Version.IsActive(ctx, r.ClusterSettings(), cluster.VersionContainsEstimatesCounter) { + // Encode that this command (and any that follow) uses regular arithmetic for ContainsEstimates + // by making sure ContainsEstimates is > 1. + // This will be interpreted during command application. + if res.Replicated.Delta.ContainsEstimates > 0 { + res.Replicated.Delta.ContainsEstimates *= 2 + } + } else { + // This range may still need to have its commands processed by nodes which treat ContainsEstimates + // as a bool, so clamp it to {0,1}. This enables use of bool semantics in command application. + if res.Replicated.Delta.ContainsEstimates > 1 { + res.Replicated.Delta.ContainsEstimates = 1 + } else if res.Replicated.Delta.ContainsEstimates < 0 { + // The caller should have checked the cluster version. At the + // time of writing, this is only RecomputeStats and the split + // trigger, which both have the check, but better safe than sorry. + return nil, false, roachpb.NewErrorf("cannot propose negative ContainsEstimates " + + "without VersionContainsEstimatesCounter") + } + } + // If the RangeAppliedState key is not being used and the cluster version is // high enough to guarantee that all current and future binaries will // understand the key, we send the migration flag through Raft. Because diff --git a/pkg/storage/replica_test.go b/pkg/storage/replica_test.go index d8aac3b43ae3..59c6393e2de5 100644 --- a/pkg/storage/replica_test.go +++ b/pkg/storage/replica_test.go @@ -35,7 +35,9 @@ import ( "github.com/cockroachdb/cockroach/pkg/rpc" "github.com/cockroachdb/cockroach/pkg/server/telemetry" "github.com/cockroachdb/cockroach/pkg/settings/cluster" + "github.com/cockroachdb/cockroach/pkg/storage/apply" "github.com/cockroachdb/cockroach/pkg/storage/batcheval" + "github.com/cockroachdb/cockroach/pkg/storage/batcheval/result" "github.com/cockroachdb/cockroach/pkg/storage/engine" "github.com/cockroachdb/cockroach/pkg/storage/engine/enginepb" "github.com/cockroachdb/cockroach/pkg/storage/intentresolver" @@ -9317,7 +9319,7 @@ func TestReplicaRecomputeStats(t *testing.T) { repl.mu.Lock() ms := repl.mu.state.Stats // intentionally mutated below disturbMS := enginepb.NewPopulatedMVCCStats(rnd, false) - disturbMS.ContainsEstimates = false + disturbMS.ContainsEstimates = 0 ms.Add(*disturbMS) err := repl.raftMu.stateLoader.SetMVCCStats(ctx, tc.engine, ms) repl.assertStateLocked(ctx, tc.engine) @@ -11947,6 +11949,159 @@ func TestReplicateQueueProcessOne(t *testing.T) { require.False(t, requeue) } +// TestContainsEstimatesClamp tests the massaging of ContainsEstimates +// before proposing a raft command. +// - If the proposing node's version is lower than the VersionContainsEstimatesCounter, +// ContainsEstimates must be clamped to {0,1}. +// - Otherwise, it should always be >1 and an even number. +func TestContainsEstimatesClampProposal(t *testing.T) { + defer leaktest.AfterTest(t)() + + _ = cluster.VersionContainsEstimatesCounter // see for details on the ContainsEstimates migration + + someRequestToProposal := func(tc *testContext, ctx context.Context) *ProposalData { + cmdIDKey := storagebase.CmdIDKey("some-cmdid-key") + var ba roachpb.BatchRequest + ba.Timestamp = tc.Clock().Now() + req := putArgs(roachpb.Key("some-key"), []byte("some-value")) + ba.Add(&req) + proposal, err := tc.repl.requestToProposal(ctx, cmdIDKey, &ba, &allSpans) + if err != nil { + t.Error(err) + } + return proposal + } + + // Mock Put command so that it always adds 2 to ContainsEstimates. Could be + // any number >1. + defer setMockPutWithEstimates(2)() + + t.Run("Pre-VersionContainsEstimatesCounter", func(t *testing.T) { + ctx := context.Background() + stopper := stop.NewStopper() + defer stopper.Stop(ctx) + cfg := TestStoreConfig(nil) + version := cluster.VersionByKey(cluster.VersionContainsEstimatesCounter - 1) + cfg.Settings = cluster.MakeClusterSettings(version, version) + var tc testContext + tc.StartWithStoreConfigAndVersion(t, stopper, cfg, version) + + proposal := someRequestToProposal(&tc, ctx) + + if proposal.command.ReplicatedEvalResult.Delta.ContainsEstimates != 1 { + t.Error("Expected ContainsEstimates to be 1, was", proposal.command.ReplicatedEvalResult.Delta.ContainsEstimates) + } + }) + + t.Run("VersionContainsEstimatesCounter", func(t *testing.T) { + ctx := context.Background() + stopper := stop.NewStopper() + defer stopper.Stop(ctx) + var tc testContext + tc.Start(t, stopper) + + proposal := someRequestToProposal(&tc, ctx) + + if proposal.command.ReplicatedEvalResult.Delta.ContainsEstimates != 4 { + t.Error("Expected ContainsEstimates to be 4, was", proposal.command.ReplicatedEvalResult.Delta.ContainsEstimates) + } + }) + +} + +// TestContainsEstimatesClampApplication tests that if the ContainsEstimates +// delta from a proposed command is 1 (and the replica state ContainsEstimates <= 1), +// ContainsEstimates will be kept 1 in the replica state. This is because +// ContainsEstimates==1 in a proposed command means that the proposer may run +// with a version older than VersionContainsEstimatesCounter, in which ContainsEstimates +// is a bool. +func TestContainsEstimatesClampApplication(t *testing.T) { + defer leaktest.AfterTest(t)() + + _ = cluster.VersionContainsEstimatesCounter // see for details on the ContainsEstimates migration + + ctx := context.Background() + stopper := stop.NewStopper() + defer stopper.Stop(ctx) + tc := testContext{} + tc.Start(t, stopper) + + // We will stage and apply 2 batches with a command that has ContainsEstimates=1 + // and expect that ReplicaState.Stats.ContainsEstimates will not become greater than 1. + applyBatch := func() { + tc.repl.raftMu.Lock() + defer tc.repl.raftMu.Unlock() + sm := tc.repl.getStateMachine() + batch := sm.NewBatch(false /* ephemeral */) + rAppbatch := batch.(*replicaAppBatch) + + lease, _ := tc.repl.GetLease() + + cmd := replicatedCmd{ + ctx: ctx, + ent: &raftpb.Entry{ + // Term: 1, + Index: rAppbatch.state.RaftAppliedIndex + 1, + Type: raftpb.EntryNormal, + }, + decodedRaftEntry: decodedRaftEntry{ + idKey: makeIDKey(), + raftCmd: storagepb.RaftCommand{ + ProposerLeaseSequence: rAppbatch.state.Lease.Sequence, + ReplicatedEvalResult: storagepb.ReplicatedEvalResult{ + Timestamp: tc.Clock().Now(), + IsLeaseRequest: true, + State: &storagepb.ReplicaState{ + Lease: &lease, + }, + Delta: enginepb.MVCCStatsDelta{ + ContainsEstimates: 1, + }, + }, + }, + }, + } + + _, err := rAppbatch.Stage(apply.Command(&cmd)) + if err != nil { + t.Fatal(err) + } + + if err := batch.ApplyToStateMachine(ctx); err != nil { + t.Fatal(err) + } + } + + applyBatch() + assert.Equal(t, int64(1), tc.repl.State().ReplicaState.Stats.ContainsEstimates) + + applyBatch() + assert.Equal(t, int64(1), tc.repl.State().ReplicaState.Stats.ContainsEstimates) +} + +// setMockPutWithEstimates mocks the Put command (could be any) to simulate a command +// that touches ContainsEstimates, in order to test request proposal behavior. +func setMockPutWithEstimates(containsEstimatesDelta int64) (undo func()) { + prev, _ := batcheval.LookupCommand(roachpb.Put) + + mockPut := func( + ctx context.Context, batch engine.ReadWriter, cArgs batcheval.CommandArgs, _ roachpb.Response, + ) (result.Result, error) { + args := cArgs.Args.(*roachpb.PutRequest) + ms := cArgs.Stats + ms.ContainsEstimates += containsEstimatesDelta + ts := cArgs.Header.Timestamp + return result.Result{}, engine.MVCCBlindPut(ctx, batch, ms, args.Key, ts, args.Value, cArgs.Header.Txn) + } + + batcheval.UnregisterCommand(roachpb.Put) + batcheval.RegisterCommand(roachpb.Put, batcheval.DefaultDeclareKeys, mockPut) + return func() { + batcheval.UnregisterCommand(roachpb.Put) + batcheval.RegisterCommand(roachpb.Put, prev.DeclareKeys, prev.Eval) + } +} + func enableTraceDebugUseAfterFree() (restore func()) { prev := trace.DebugUseAfterFinish trace.DebugUseAfterFinish = true