storage: rebalancing only considers range leader store

[petermattis]

Rebalancing is performed by the "replicate queue" (storage/replicate_queue.go) which periodically checks every range it is a leader for and calls Allocator.ShouldRebalance. ShouldRebalance retrieves the store descriptor for the leader replica and calls balancer.improve which compares the store capacity/range-count against cluster averages. If rebalancing should be performed, a new replica is added and a subsequent pass of the "replicate queue" will take care of removing the "worst" replica in an over-replicated range.

There is a glitch in the above, though. If the leader replica for a range is on a "good" store while one of the other replicas is on a "bad" store, we won't perform rebalancing. balancer.improve should consider the worst replica store when making the rebalancing decision.

[mrtracy]

I don't think there's any reason for the replicate queue to run exclusively on leaders. Everything it does is already safe against races.

In fact, I already thought this was the case (that replicate_queue was considering non-leaders).

[bdarnell]

It's safe to run replicate_queue on followers, but it's a bit wasteful for two nodes to try to make ChangeReplicas transactions simultaneously. I think it would be best for the replication queue to continue to run on the leader but to consider the health of all replicas (assuming that information is available at the leader).

Allocator.ShouldRebalance is called for each replica in shouldQueue. So as long as the store is the leader of some ranges, it will find opportunities for rebalancing. However, even though this helps avoid the problem pointed out in this issue, it's not ideal because it's more disruptive to rebalance a range away from its leader than from a follower. So it would be be better to do the opposite and prefer to find rebalancing opportunities in follower replicas.

[BramGruneir]

There are no plans right now to consider moving the range lease or to allow all replicas to run via the replicate queue. I think we can consider this closed for now.

Closing this issue.