Merge #64650

64650: kvserver: prevent StoreRebalancer from downreplicating r=erikgrinaker,nvanbenschoten a=tbg

When the replication factor is lowered and the StoreRebalancer
attempts a rebalance, it will accidentally perform a downreplication.
Since it wasn't ever supposed to do that, the downreplication is
pretty haphazard and doesn't safeguard quorum in the same way
that a "proper" downreplication likely would. Prevent if from
changing the number of voters and non-voters to avoid this issue.

Annoyingly, I [knew] about this problem, but instead of fixing it at the
source - as this commit does - I added a lower- level check that could
then not be backported to release-20.2, where we are now seeing this
problem.

[knew]: #54444 (comment)

#64649

Release note: None


Co-authored-by: Tobias Grieger <tobias.b.grieger@gmail.com>

pkg/kv/kvserver/store_rebalancer.go

@@ -543,14 +543,24 @@ func (sr *StoreRebalancer) chooseRangeToRebalance(
  rangeDesc, zone := replWithStats.repl.DescAndZone()
  clusterNodes := sr.rq.allocator.storePool.ClusterNodeCount()
  numDesiredVoters := GetNeededVoters(zone.GetNumVoters(), clusterNodes)
+ numDesiredNonVoters := GetNeededNonVoters(numDesiredVoters, int(zone.GetNumNonVoters()), clusterNodes)
+ if rs := rangeDesc.Replicas(); numDesiredVoters != len(rs.VoterDescriptors()) ||
+ numDesiredNonVoters != len(rs.NonVoterDescriptors()) {
+ // If the StoreRebalancer is allowed past this point, it may accidentally
+ // downreplicate and this can cause unavailable ranges.
+ //
+ // See: https://github.com/cockroachdb/cockroach/issues/54444#issuecomment-707706553
+ log.VEventf(ctx, 3, "range needs up/downreplication; not considering rebalance")
+ continue
+ }
 
  rebalanceCtx := rangeRebalanceContext{
  replWithStats: replWithStats,
  rangeDesc: rangeDesc,
  zone: zone,
  clusterNodes: clusterNodes,
  numDesiredVoters: numDesiredVoters,
- numDesiredNonVoters: GetNeededNonVoters(numDesiredVoters, int(zone.GetNumNonVoters()), clusterNodes),
+ numDesiredNonVoters: numDesiredNonVoters,
  }
  targetVoterRepls, targetNonVoterRepls := sr.getRebalanceCandidatesBasedOnQPS(
  ctx, rebalanceCtx, localDesc, storeMap, storeList, minQPS, maxQPS,