Skip to content

Commit

Permalink
kvserver: propose empty entry on unquiescence
Browse files Browse the repository at this point in the history
During Raft unquiescence, an empty command is proposed to wake the Raft
leader. However, this empty command was encoded as a regular Raft
command with its own command ID and an empty payload. Meanwhile, the
downstream Raft application expects such noop commands to be entirely
empty (i.e. the entire Raft entry is empty), rather than an entry with
an empty payload. This caused it to interpret the command as having a
`ProposerLeaseSequence` of 0 and return a `NotLeaseHolderError` rather
than considering it a noop. Construction and processing of this error
has a non-negligible cost when unquiescing large numbers of ranges.

This patch instead proposes an empty entry on unquiescence.

Epic: none
Release note: None
  • Loading branch information
erikgrinaker committed Apr 1, 2023
1 parent 99102dd commit c4d50c9
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 8 deletions.
10 changes: 5 additions & 5 deletions pkg/kv/kvserver/kvserverbase/forced_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,11 @@ func CheckForcedErr(
requestedLease = *raftCmd.ReplicatedEvalResult.State.Lease
}
if idKey == "" {
// This is an empty Raft command (which is sent by Raft after elections
// to trigger reproposals or during concurrent configuration changes).
// Nothing to do here except making sure that the corresponding batch
// (which is bogus) doesn't get executed (for it is empty and so
// properties like key range are undefined).
// This is an empty Raft command (which is sent by Raft after elections to
// trigger reproposals, during concurrent configuration changes, or to
// unquiesce the Raft group). Nothing to do here except making sure that the
// corresponding batch (which is bogus) doesn't get executed (for it is
// empty and so properties like key range are undefined).
return ForcedErrResult{
LeaseIndex: leaseIndex,
Rejection: ProposalRejectionPermanent,
Expand Down
4 changes: 1 addition & 3 deletions pkg/kv/kvserver/replica_raft_quiesce.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import (

"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverpb"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/liveness/livenesspb"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/raftlog"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/util/hlc"
"github.com/cockroachdb/cockroach/pkg/util/log"
Expand Down Expand Up @@ -88,8 +87,7 @@ func (r *Replica) maybeUnquiesceAndWakeLeaderLocked() bool {
r.store.unquiescedReplicas.Unlock()
r.maybeCampaignOnWakeLocked(ctx)
// Propose an empty command which will wake the leader.
data := raftlog.EncodeRaftCommand(raftlog.EntryEncodingStandardWithoutAC, makeIDKey(), nil)
_ = r.mu.internalRaftGroup.Propose(data)
_ = r.mu.internalRaftGroup.Propose(nil)
return true
}

Expand Down

0 comments on commit c4d50c9

Please sign in to comment.