storage: Jitter the StoreRebalancer loop's timing

[a-robinson]

Release note: None

Just as a best practice. It may make failures like #31006 even less likely, although it's hard to say for sure.

[cockroach-teamcity]

This change is 

[cockroach-teamcity]

This change is 

[a-robinson]

@tschottdorf any thoughts on backporting this? It's much more likely to avoid problems than cause them, but man has it gotten late in the cycle.

bors r+

[a-robinson]

@tschottdorf any thoughts on backporting this? It's much more likely to avoid problems than cause them, but man has it gotten late in the cycle.

bors r+

[craig]

Build succeeded

• GitHub CI (Cockroach)

[craig]

Build succeeded

• GitHub CI (Cockroach)

[tschottdorf]

Is there a specific situation in which you expect it to cause problems?
Generally agreed that the jittering, if anything, is going to help.

[tschottdorf]

Is there a specific situation in which you expect it to cause problems?
Generally agreed that the jittering, if anything, is going to help.

[a-robinson]

I don't think it would ever get persistently stuck given that the goal for the StoreRebalancer is to rebalance the store within one or two rounds of work, but due to delays in propagating information about the number of replicas and load on each store, running store x's rebalancer right after store y's means that store x probably doesn't know about any changes that may have been triggered by store y. If both store x and store y need a few rounds of changes, this means that all x's decisions may be suboptimal for multiple minutes, rather than just for a subset of those rounds.

tl;dr It might reduce flakiness of the rebalance-replicas-by-load roachtest (since that only has 5 minutes to succeed), and it might allow load to rebalance more quickly in edge cases in a real cluster where multiple stores are overloaded, and it's generally just good practice. It's not solving any big problems.

[a-robinson]

I don't think it would ever get persistently stuck given that the goal for the StoreRebalancer is to rebalance the store within one or two rounds of work, but due to delays in propagating information about the number of replicas and load on each store, running store x's rebalancer right after store y's means that store x probably doesn't know about any changes that may have been triggered by store y. If both store x and store y need a few rounds of changes, this means that all x's decisions may be suboptimal for multiple minutes, rather than just for a subset of those rounds.

tl;dr It might reduce flakiness of the rebalance-replicas-by-load roachtest (since that only has 5 minutes to succeed), and it might allow load to rebalance more quickly in edge cases in a real cluster where multiple stores are overloaded, and it's generally just good practice. It's not solving any big problems.