From d7d5ade74aeae6b7b939b3113d9cc3dbe0497c7b Mon Sep 17 00:00:00 2001 From: Austen McClernon Date: Thu, 12 Jan 2023 18:18:27 +0000 Subject: [PATCH] kvserver: remove deprecated last split qps This patch removes the deprecated 'lastSplitQPS' value throughout the split/merge code. This field was deprecated in 22.1 in favor or `maxSplitQPS` and stopped being consulted in 22.2. A remaining field `max_queries_per_second_set` will remain until 23.2 where it can be removed as no node should consult it. Now only `maxSplitQPS` is consulted and set in `RangeStatsResponse`. Release note: None --- pkg/kv/kvserver/batcheval/cmd_range_stats.go | 1 - pkg/kv/kvserver/batcheval/eval_context.go | 11 ----------- pkg/kv/kvserver/merge_queue.go | 9 ++------- pkg/kv/kvserver/replica.go | 10 ---------- pkg/kv/kvserver/replica_eval_context_span.go | 6 ------ pkg/kv/kvserver/split_queue.go | 1 + pkg/roachpb/api.proto | 15 ++++----------- 7 files changed, 7 insertions(+), 46 deletions(-) diff --git a/pkg/kv/kvserver/batcheval/cmd_range_stats.go b/pkg/kv/kvserver/batcheval/cmd_range_stats.go index 1e1d14611232..9456c59dc81e 100644 --- a/pkg/kv/kvserver/batcheval/cmd_range_stats.go +++ b/pkg/kv/kvserver/batcheval/cmd_range_stats.go @@ -44,7 +44,6 @@ func RangeStats( ) (result.Result, error) { reply := resp.(*roachpb.RangeStatsResponse) reply.MVCCStats = cArgs.EvalCtx.GetMVCCStats() - reply.DeprecatedLastQueriesPerSecond = cArgs.EvalCtx.GetLastSplitQPS(ctx) if qps, ok := cArgs.EvalCtx.GetMaxSplitQPS(ctx); ok { reply.MaxQueriesPerSecond = qps } else { diff --git a/pkg/kv/kvserver/batcheval/eval_context.go b/pkg/kv/kvserver/batcheval/eval_context.go index 3831691a1a73..6e5c9fa48b39 100644 --- a/pkg/kv/kvserver/batcheval/eval_context.go +++ b/pkg/kv/kvserver/batcheval/eval_context.go @@ -94,14 +94,6 @@ type EvalContext interface { // is disabled. GetMaxSplitQPS(context.Context) (float64, bool) - // GetLastSplitQPS returns the Replica's most recent queries/s request rate. - // - // NOTE: This should not be used when the load based splitting cluster setting - // is disabled. - // - // TODO(nvanbenschoten): remove this method in v22.1. - GetLastSplitQPS(context.Context) float64 - GetGCThreshold() hlc.Timestamp ExcludeDataFromBackup() bool GetLastReplicaGCTimestamp(context.Context) (hlc.Timestamp, error) @@ -248,9 +240,6 @@ func (m *mockEvalCtxImpl) GetMVCCStats() enginepb.MVCCStats { func (m *mockEvalCtxImpl) GetMaxSplitQPS(context.Context) (float64, bool) { return m.QPS, true } -func (m *mockEvalCtxImpl) GetLastSplitQPS(context.Context) float64 { - return m.QPS -} func (m *mockEvalCtxImpl) CanCreateTxnRecord( context.Context, uuid.UUID, []byte, hlc.Timestamp, ) (bool, roachpb.TransactionAbortedReason) { diff --git a/pkg/kv/kvserver/merge_queue.go b/pkg/kv/kvserver/merge_queue.go index 1e3b4e51696c..4ab4cc0f196b 100644 --- a/pkg/kv/kvserver/merge_queue.go +++ b/pkg/kv/kvserver/merge_queue.go @@ -199,13 +199,8 @@ func (mq *mergeQueue) requestRangeStats( desc = &res.RangeInfo.Desc stats = res.MVCCStats - if res.MaxQueriesPerSecondSet { - qps = res.MaxQueriesPerSecond - qpsOK = qps >= 0 - } else { - qps = res.DeprecatedLastQueriesPerSecond - qpsOK = true - } + qps = res.MaxQueriesPerSecond + qpsOK = qps >= 0 return desc, stats, qps, qpsOK, nil } diff --git a/pkg/kv/kvserver/replica.go b/pkg/kv/kvserver/replica.go index 4043399b974b..b51f0b1fd0d8 100644 --- a/pkg/kv/kvserver/replica.go +++ b/pkg/kv/kvserver/replica.go @@ -1298,16 +1298,6 @@ func (r *Replica) GetMaxSplitQPS(ctx context.Context) (float64, bool) { return r.loadBasedSplitter.MaxQPS(ctx, r.Clock().PhysicalTime()) } -// GetLastSplitQPS returns the Replica's most recent queries/s request rate. -// -// NOTE: This should only be used for load based splitting, only -// works when the load based splitting cluster setting is enabled. -// -// Use QueriesPerSecond() for current QPS stats for all other purposes. -func (r *Replica) GetLastSplitQPS(ctx context.Context) float64 { - return r.loadBasedSplitter.LastQPS(ctx, r.Clock().PhysicalTime()) -} - // ContainsKey returns whether this range contains the specified key. // // TODO(bdarnell): This is not the same as RangeDescriptor.ContainsKey. diff --git a/pkg/kv/kvserver/replica_eval_context_span.go b/pkg/kv/kvserver/replica_eval_context_span.go index a2525072e35b..7551a07c402b 100644 --- a/pkg/kv/kvserver/replica_eval_context_span.go +++ b/pkg/kv/kvserver/replica_eval_context_span.go @@ -135,12 +135,6 @@ func (rec SpanSetReplicaEvalContext) GetMaxSplitQPS(ctx context.Context) (float6 return rec.i.GetMaxSplitQPS(ctx) } -// GetLastSplitQPS returns the Replica's most recent queries/s rate for -// splitting and merging purposes. -func (rec SpanSetReplicaEvalContext) GetLastSplitQPS(ctx context.Context) float64 { - return rec.i.GetLastSplitQPS(ctx) -} - // CanCreateTxnRecord determines whether a transaction record can be created // for the provided transaction information. See Replica.CanCreateTxnRecord // for details about its arguments, return values, and preconditions. diff --git a/pkg/kv/kvserver/split_queue.go b/pkg/kv/kvserver/split_queue.go index b729b55bebee..56adf14df73b 100644 --- a/pkg/kv/kvserver/split_queue.go +++ b/pkg/kv/kvserver/split_queue.go @@ -267,6 +267,7 @@ func (sq *splitQueue) processAttempt( batchHandledQPS := loadStats.QueriesPerSecond raftAppliedQPS := loadStats.WriteKeysPerSecond splitQPS := r.loadBasedSplitter.LastQPS(ctx, now) + reason := fmt.Sprintf( "load at key %s (%.2f splitQPS, %.2f batches/sec, %.2f raft mutations/sec)", splitByLoadKey, diff --git a/pkg/roachpb/api.proto b/pkg/roachpb/api.proto index 38ca9ffa1fde..1e3ae8260480 100644 --- a/pkg/roachpb/api.proto +++ b/pkg/roachpb/api.proto @@ -2104,13 +2104,6 @@ message RangeStatsResponse { (gogoproto.customname) = "MVCCStats" ]; - // DeprecatedLastQueriesPerSecond is the most recent rate of request/s or QPS - // for the range. The field is deprecated in favor of MaxQueriesPerSecond. - // - // TODO(nvanbenschoten): remove this field in v22.1 when all nodes in the - // cluster are guaranteed to return MaxQueriesPerSecond. - double deprecated_last_queries_per_second = 3; - // MaxQueriesPerSecond is the maximum rate of request/s or QPS that the range // has served over a configured measurement period. Set to -1 if the replica // serving the RangeStats request has not been the leaseholder long enough to @@ -2123,15 +2116,15 @@ message RangeStatsResponse { // by the server. Used to distinguish 0 qps set by a new server from the field // not being set at all by an old server. // - // TODO(nvanbenschoten): stop consulting this field on the receiver in v22.1 - // when all nodes in the cluster are guaranteed to return MaxQueriesPerSecond. - // - // TODO(nvanbenschoten): stop setting this field and remove it in v22.2 when + // TODO(kvoli): stop setting this field and remove it in v23.2 when // no nodes in the cluster consult this field. bool max_queries_per_second_set = 6; + // range_info contains descriptor and lease information. RangeInfo range_info = 4 [(gogoproto.nullable) = false]; + + reserved 3; } // MigrateRequest is used instruct all ranges overlapping with it to exercise