Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
73362: kv: don't unquiesce uninitialized replicas r=tbg a=nvanbenschoten

In a [support issue](https://github.com/cockroachlabs/support/issues/1340), we saw that 10s of thousands of uninitialized replicas were being ticked regularly and creating a large amount of background work on a node, driving up CPU. This commit updates the Raft quiescence logic to disallow uninitialized replicas from being unquiesced and Tick()'ing themselves.

Keeping uninitialized replicas quiesced even in the presence of Raft traffic avoids wasted work. We could Tick() these replicas, but doing so is unnecessary because uninitialized replicas can never win elections, so there is no reason for them to ever call an election. In fact, uninitialized replicas do not even know who their peers are, so there would be no way for them to call an election or for them to send any other non-reactive message. As a result, all work performed by an uninitialized replica is reactive and in response to incoming messages (see `processRequestQueue`).

There are multiple ways for an uninitialized replica to be created and then abandoned, and we don't do a good job garbage collecting them at a later point (see #73424), so it is important that they are cheap. Keeping them quiesced instead of letting them unquiesce and tick every 200ms indefinitely avoids a meaningful amount of periodic work for each uninitialized replica.

Release notes (bug fix): uninitialized replicas that are abandoned after an unsuccessful snapshot no longer perform periodic background work, so they no longer have a non-negligible cost.

73641: circuit: add probing-based circuit breaker r=erikgrinaker a=tbg

These are initially for use in #71806 but were originally conceived of
for #70485, which we are not currently prioritizing.

Importantly, this circuit breaker does not recruit a fraction of
requests to do the probing, which is desirable for both PR #71806 and
PR #70485; requests recruited as probes tend to incur high latency and
errors, and we don't want SQL client traffic to experience those.

Release note: None


73718: kv: pass roachpb.Header by pointer to DeclareKeysFunc r=nvanbenschoten a=nvanbenschoten

The `roachpb.Header` struct is up to 160 bytes in size. That's a little too
large to be passing by value repeatedly when doing so is easy to avoid. This
commit switches to passing roachpb.Header structs by pointer through the
DeclareKeysFunc implementations.

Co-authored-by: Nathan VanBenschoten <nvanbenschoten@gmail.com>
Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>
  • Loading branch information
3 people committed Dec 14, 2021
4 parents a60e8d7 + 95a7671 + eed8603 + 3def463 commit acf40e9
Show file tree
Hide file tree
Showing 54 changed files with 1,069 additions and 122 deletions.
1 change: 1 addition & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,7 @@ ALL_TESTS = [
"//pkg/util/cache:cache_test",
"//pkg/util/caller:caller_test",
"//pkg/util/cgroups:cgroups_test",
"//pkg/util/circuit:circuit_test",
"//pkg/util/cloudinfo:cloudinfo_test",
"//pkg/util/contextutil:contextutil_test",
"//pkg/util/ctxgroup:ctxgroup_test",
Expand Down
5 changes: 1 addition & 4 deletions pkg/kv/kvserver/batcheval/cmd_barrier.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,7 @@ func init() {
}

func declareKeysBarrier(
rs ImmutableRangeState,
h roachpb.Header,
req roachpb.Request,
latchSpans, lockSpans *spanset.SpanSet,
_ ImmutableRangeState, _ *roachpb.Header, req roachpb.Request, latchSpans, _ *spanset.SpanSet,
) {
// Barrier is special-cased in the concurrency manager to *not* actually
// grab these latches. Instead, any conflicting latches with these are waited
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_clear_range.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func init() {

func declareKeysClearRange(
rs ImmutableRangeState,
header roachpb.Header,
header *roachpb.Header,
req roachpb.Request,
latchSpans, lockSpans *spanset.SpanSet,
) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_compute_checksum.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func init() {
}

func declareKeysComputeChecksum(
rs ImmutableRangeState, _ roachpb.Header, _ roachpb.Request, latchSpans, _ *spanset.SpanSet,
rs ImmutableRangeState, _ *roachpb.Header, _ roachpb.Request, latchSpans, _ *spanset.SpanSet,
) {
// The correctness of range merges depends on the lease applied index of a
// range not being bumped while the RHS is subsumed. ComputeChecksum bumps a
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_conditional_put.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func init() {

func declareKeysConditionalPut(
rs ImmutableRangeState,
header roachpb.Header,
header *roachpb.Header,
req roachpb.Request,
latchSpans, lockSpans *spanset.SpanSet,
) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_delete_range.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func init() {

func declareKeysDeleteRange(
rs ImmutableRangeState,
header roachpb.Header,
header *roachpb.Header,
req roachpb.Request,
latchSpans, lockSpans *spanset.SpanSet,
) {
Expand Down
4 changes: 2 additions & 2 deletions pkg/kv/kvserver/batcheval/cmd_end_transaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ func init() {
// declareKeysWriteTransaction is the shared portion of
// declareKeys{End,Heartbeat}Transaction.
func declareKeysWriteTransaction(
_ ImmutableRangeState, header roachpb.Header, req roachpb.Request, latchSpans *spanset.SpanSet,
_ ImmutableRangeState, header *roachpb.Header, req roachpb.Request, latchSpans *spanset.SpanSet,
) {
if header.Txn != nil {
header.Txn.AssertInitialized(context.TODO())
Expand All @@ -53,7 +53,7 @@ func declareKeysWriteTransaction(

func declareKeysEndTxn(
rs ImmutableRangeState,
header roachpb.Header,
header *roachpb.Header,
req roachpb.Request,
latchSpans, _ *spanset.SpanSet,
) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_export.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func init() {

func declareKeysExport(
rs ImmutableRangeState,
header roachpb.Header,
header *roachpb.Header,
req roachpb.Request,
latchSpans, lockSpans *spanset.SpanSet,
) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_gc.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func init() {

func declareKeysGC(
rs ImmutableRangeState,
header roachpb.Header,
header *roachpb.Header,
req roachpb.Request,
latchSpans, _ *spanset.SpanSet,
) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_heartbeat_txn.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func init() {

func declareKeysHeartbeatTransaction(
rs ImmutableRangeState,
header roachpb.Header,
header *roachpb.Header,
req roachpb.Request,
latchSpans, _ *spanset.SpanSet,
) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_lease_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func init() {
}

func declareKeysLeaseInfo(
rs ImmutableRangeState, _ roachpb.Header, _ roachpb.Request, latchSpans, _ *spanset.SpanSet,
rs ImmutableRangeState, _ *roachpb.Header, _ roachpb.Request, latchSpans, _ *spanset.SpanSet,
) {
latchSpans.AddNonMVCC(spanset.SpanReadOnly, roachpb.Span{Key: keys.RangeLeaseKey(rs.GetRangeID())})
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_lease_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func init() {
}

func declareKeysRequestLease(
rs ImmutableRangeState, _ roachpb.Header, _ roachpb.Request, latchSpans, _ *spanset.SpanSet,
rs ImmutableRangeState, _ *roachpb.Header, _ roachpb.Request, latchSpans, _ *spanset.SpanSet,
) {
// NOTE: RequestLease is run on replicas that do not hold the lease, so
// acquiring latches would not help synchronize with other requests. As
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_lease_transfer.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func init() {
}

func declareKeysTransferLease(
rs ImmutableRangeState, _ roachpb.Header, _ roachpb.Request, latchSpans, _ *spanset.SpanSet,
_ ImmutableRangeState, _ *roachpb.Header, _ roachpb.Request, latchSpans, _ *spanset.SpanSet,
) {
// TransferLease must not run concurrently with any other request so it uses
// latches to synchronize with all other reads and writes on the outgoing
Expand Down
5 changes: 1 addition & 4 deletions pkg/kv/kvserver/batcheval/cmd_migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,7 @@ func init() {
}

func declareKeysMigrate(
rs ImmutableRangeState,
_ roachpb.Header,
_ roachpb.Request,
latchSpans, lockSpans *spanset.SpanSet,
rs ImmutableRangeState, _ *roachpb.Header, _ roachpb.Request, latchSpans, _ *spanset.SpanSet,
) {
// TODO(irfansharif): This will eventually grow to capture the super set of
// all keys accessed by all migrations defined here. That could get
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_probe.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import (
)

func declareKeysProbe(
_ ImmutableRangeState, _ roachpb.Header, _ roachpb.Request, _, _ *spanset.SpanSet,
_ ImmutableRangeState, _ *roachpb.Header, _ roachpb.Request, _, _ *spanset.SpanSet,
) {
// Declare no keys. This means that we're not even serializing with splits
// (i.e. a probe could be directed at a key that will become the right-hand
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_push_txn.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func init() {
}

func declareKeysPushTransaction(
rs ImmutableRangeState, _ roachpb.Header, req roachpb.Request, latchSpans, _ *spanset.SpanSet,
rs ImmutableRangeState, _ *roachpb.Header, req roachpb.Request, latchSpans, _ *spanset.SpanSet,
) {
pr := req.(*roachpb.PushTxnRequest)
latchSpans.AddNonMVCC(spanset.SpanReadWrite, roachpb.Span{Key: keys.TransactionKey(pr.PusheeTxn.Key, pr.PusheeTxn.ID)})
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_put.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func init() {

func declareKeysPut(
rs ImmutableRangeState,
header roachpb.Header,
header *roachpb.Header,
req roachpb.Request,
latchSpans, lockSpans *spanset.SpanSet,
) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_query_intent.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func init() {
}

func declareKeysQueryIntent(
_ ImmutableRangeState, _ roachpb.Header, req roachpb.Request, latchSpans, _ *spanset.SpanSet,
_ ImmutableRangeState, _ *roachpb.Header, req roachpb.Request, latchSpans, _ *spanset.SpanSet,
) {
// QueryIntent requests read the specified keys at the maximum timestamp in
// order to read any intent present, if one exists, regardless of the
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_query_txn.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func init() {
}

func declareKeysQueryTransaction(
_ ImmutableRangeState, _ roachpb.Header, req roachpb.Request, latchSpans, _ *spanset.SpanSet,
_ ImmutableRangeState, _ *roachpb.Header, req roachpb.Request, latchSpans, _ *spanset.SpanSet,
) {
qr := req.(*roachpb.QueryTxnRequest)
latchSpans.AddNonMVCC(spanset.SpanReadOnly, roachpb.Span{Key: keys.TransactionKey(qr.Txn.Key, qr.Txn.ID)})
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_range_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func init() {

func declareKeysRangeStats(
rs ImmutableRangeState,
header roachpb.Header,
header *roachpb.Header,
req roachpb.Request,
latchSpans, lockSpans *spanset.SpanSet,
) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_recompute_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func init() {
}

func declareKeysRecomputeStats(
rs ImmutableRangeState, _ roachpb.Header, _ roachpb.Request, latchSpans, _ *spanset.SpanSet,
rs ImmutableRangeState, _ *roachpb.Header, _ roachpb.Request, latchSpans, _ *spanset.SpanSet,
) {
// We don't declare any user key in the range. This is OK since all we're doing is computing a
// stats delta, and applying this delta commutes with other operations on the same key space.
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_recover_txn.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func init() {
}

func declareKeysRecoverTransaction(
rs ImmutableRangeState, _ roachpb.Header, req roachpb.Request, latchSpans, _ *spanset.SpanSet,
rs ImmutableRangeState, _ *roachpb.Header, req roachpb.Request, latchSpans, _ *spanset.SpanSet,
) {
rr := req.(*roachpb.RecoverTxnRequest)
latchSpans.AddNonMVCC(spanset.SpanReadWrite, roachpb.Span{Key: keys.TransactionKey(rr.Txn.Key, rr.Txn.ID)})
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_resolve_intent.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func declareKeysResolveIntentCombined(
}

func declareKeysResolveIntent(
rs ImmutableRangeState, _ roachpb.Header, req roachpb.Request, latchSpans, _ *spanset.SpanSet,
rs ImmutableRangeState, _ *roachpb.Header, req roachpb.Request, latchSpans, _ *spanset.SpanSet,
) {
declareKeysResolveIntentCombined(rs, req, latchSpans)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_resolve_intent_range.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ func init() {
}

func declareKeysResolveIntentRange(
rs ImmutableRangeState, _ roachpb.Header, req roachpb.Request, latchSpans, _ *spanset.SpanSet,
rs ImmutableRangeState, _ *roachpb.Header, req roachpb.Request, latchSpans, _ *spanset.SpanSet,
) {
declareKeysResolveIntentCombined(rs, req, latchSpans)
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/kv/kvserver/batcheval/cmd_resolve_intent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,15 +109,15 @@ func TestDeclareKeysResolveIntent(t *testing.T) {

if !ranged {
cArgs.Args = &ri
declareKeysResolveIntent(&desc, h, &ri, &latchSpans, &lockSpans)
declareKeysResolveIntent(&desc, &h, &ri, &latchSpans, &lockSpans)
batch := spanset.NewBatch(engine.NewBatch(), &latchSpans)
defer batch.Close()
if _, err := ResolveIntent(ctx, batch, cArgs, &roachpb.ResolveIntentResponse{}); err != nil {
t.Fatal(err)
}
} else {
cArgs.Args = &rir
declareKeysResolveIntentRange(&desc, h, &rir, &latchSpans, &lockSpans)
declareKeysResolveIntentRange(&desc, &h, &rir, &latchSpans, &lockSpans)
batch := spanset.NewBatch(engine.NewBatch(), &latchSpans)
defer batch.Close()
if _, err := ResolveIntentRange(ctx, batch, cArgs, &roachpb.ResolveIntentRangeResponse{}); err != nil {
Expand Down Expand Up @@ -203,7 +203,7 @@ func TestResolveIntentAfterPartialRollback(t *testing.T) {
}
ri.Key = k

declareKeysResolveIntent(&desc, h, &ri, &spans, nil)
declareKeysResolveIntent(&desc, &h, &ri, &spans, nil)
rbatch = spanset.NewBatch(db.NewBatch(), &spans)
defer rbatch.Close()

Expand All @@ -227,7 +227,7 @@ func TestResolveIntentAfterPartialRollback(t *testing.T) {
rir.Key = k
rir.EndKey = endKey

declareKeysResolveIntentRange(&desc, h, &rir, &spans, nil)
declareKeysResolveIntentRange(&desc, &h, &rir, &spans, nil)
rbatch = spanset.NewBatch(db.NewBatch(), &spans)
defer rbatch.Close()

Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_revert_range.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func init() {

func declareKeysRevertRange(
rs ImmutableRangeState,
header roachpb.Header,
header *roachpb.Header,
req roachpb.Request,
latchSpans, lockSpans *spanset.SpanSet,
) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_scan_interleaved_intents.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func init() {
}

func declareKeysScanInterleavedIntents(
rs ImmutableRangeState, _ roachpb.Header, _ roachpb.Request, latchSpans, _ *spanset.SpanSet,
rs ImmutableRangeState, _ *roachpb.Header, _ roachpb.Request, latchSpans, _ *spanset.SpanSet,
) {
latchSpans.AddNonMVCC(spanset.SpanReadOnly, roachpb.Span{Key: keys.RangeDescriptorKey(rs.GetStartKey())})
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_subsume.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func init() {
}

func declareKeysSubsume(
_ ImmutableRangeState, header roachpb.Header, req roachpb.Request, latchSpans, _ *spanset.SpanSet,
_ ImmutableRangeState, _ *roachpb.Header, _ roachpb.Request, latchSpans, _ *spanset.SpanSet,
) {
// Subsume must not run concurrently with any other command. It declares a
// non-MVCC write over every addressable key in the range; this guarantees
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/cmd_truncate_log.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ func init() {
}

func declareKeysTruncateLog(
rs ImmutableRangeState, _ roachpb.Header, req roachpb.Request, latchSpans, _ *spanset.SpanSet,
rs ImmutableRangeState, _ *roachpb.Header, _ roachpb.Request, latchSpans, _ *spanset.SpanSet,
) {
prefix := keys.RaftLogPrefix(rs.GetRangeID())
latchSpans.AddNonMVCC(spanset.SpanReadWrite, roachpb.Span{Key: prefix, EndKey: prefix.PrefixEnd()})
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
// isolation from conflicting transactions to the lockSpans set.
type DeclareKeysFunc func(
rs ImmutableRangeState,
header roachpb.Header,
header *roachpb.Header,
request roachpb.Request,
latchSpans, lockSpans *spanset.SpanSet,
)
Expand Down
9 changes: 6 additions & 3 deletions pkg/kv/kvserver/batcheval/declare.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ import (

// DefaultDeclareKeys is the default implementation of Command.DeclareKeys.
func DefaultDeclareKeys(
_ ImmutableRangeState, header roachpb.Header, req roachpb.Request, latchSpans, _ *spanset.SpanSet,
_ ImmutableRangeState,
header *roachpb.Header,
req roachpb.Request,
latchSpans, _ *spanset.SpanSet,
) {
access := spanset.SpanReadWrite
if roachpb.IsReadOnly(req) && !roachpb.IsLocking(req) {
Expand All @@ -38,7 +41,7 @@ func DefaultDeclareKeys(
// when it evaluated.
func DefaultDeclareIsolatedKeys(
_ ImmutableRangeState,
header roachpb.Header,
header *roachpb.Header,
req roachpb.Request,
latchSpans, lockSpans *spanset.SpanSet,
) {
Expand Down Expand Up @@ -73,7 +76,7 @@ func DefaultDeclareIsolatedKeys(
// touches to the given SpanSet. This does not include keys touched during the
// processing of the batch's individual commands.
func DeclareKeysForBatch(
rs ImmutableRangeState, header roachpb.Header, latchSpans *spanset.SpanSet,
rs ImmutableRangeState, header *roachpb.Header, latchSpans *spanset.SpanSet,
) {
if header.Txn != nil {
header.Txn.AssertInitialized(context.TODO())
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/batcheval/declare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestRequestsSerializeWithAllKeys(t *testing.T) {
Sequence: 0,
})

command.DeclareKeys(desc, header, otherRequest, &otherLatchSpans, &otherLockSpans)
command.DeclareKeys(desc, &header, otherRequest, &otherLatchSpans, &otherLockSpans)
if !allLatchSpans.Intersects(&otherLatchSpans) {
t.Errorf("%s does not serialize with declareAllKeys", method)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/kv/kvserver/client_merge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ func mergeCheckingTimestampCaches(
// Make sure the LHS range in uniquiesced so that it elects a new
// Raft leader after the partition is established.
for _, r := range lhsRepls {
r.UnquiesceAndWakeLeader()
r.MaybeUnquiesceAndWakeLeader()
}

// Issue an increment on the range. The leaseholder should evaluate
Expand Down
Loading

0 comments on commit acf40e9

Please sign in to comment.