Skip to content

Commit

Permalink
kvserver: disable early ack for commit triggers
Browse files Browse the repository at this point in the history
Early-acking splits, merges, etc, isn't useful. At best, it is
inconsequential but at worst it causes flakes because the split hasn't
actually completed by the time it is acked and so it isn't yet reflected
in the descriptor, whose in-memory copy is frequently accessed during
allocation decisions.

Release note: None
  • Loading branch information
tbg committed Nov 2, 2022
1 parent 206fc07 commit fad19d5
Showing 1 changed file with 11 additions and 1 deletion.
12 changes: 11 additions & 1 deletion pkg/kv/kvserver/replica_application_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,17 @@ func (c *replicatedCmd) CanAckBeforeApplication() bool {
// We don't try to ack async consensus writes before application because we
// know that there isn't a client waiting for the result.
req := c.proposal.Request
return req.IsIntentWrite() && !req.AsyncConsensus
if !req.IsIntentWrite() || req.AsyncConsensus {
return false
}
if et, ok := req.GetArg(roachpb.EndTxn); ok && et.(*roachpb.EndTxnRequest).InternalCommitTrigger != nil {
// Don't early-ack for commit triggers, just to keep things simple - the
// caller is reasonably expecting to be able to run another replication
// change right away, and some code paths pull the descriptor out of memory
// where it may not reflect the previous operation yet.
return false
}
return true
}

// AckSuccess implements the apply.CheckedCommand interface.
Expand Down

0 comments on commit fad19d5

Please sign in to comment.