StoreGateway: fix store gateway shuffle sharding consistency issue when shard size change by 1

[yeya24]

What this PR does:

Add a new method ShuffleShardWithZoneStability in ReadRing interface. It does the same thing as ShuffleShard but makes a slight change:

1. It doesn't round up shard size to be multiple of numZones when zone awareness is enabled.

If shardSize is divisible by numZones, the behavior should be the same.
If shardSize is not divisible by numZones, for example, shard size 8 and num zones is 3. Then 2 zones are assigned to have 3 instances and 1 zone has 2 instances. Because we iterate over zones using the same order so we can ensure the consistency.

Which issue(s) this PR fixes:
Fixes #5467

Checklist

• Tests updated
• Documentation added
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[yeya24]

ShuffleShardWithZoneStability might not be a good name and tbh I am not sure how to name the new shuffle sharding algorithm/change. Any suggestions are welcomed.
For compatibility reason I think we might still want to keep the existing ShuffleShard method.

[harry671003]

Do we need to put this behind a feature flag? Would it break some customers of Cortex who're using zone-awareness in store-gateways with shuffle-sharding?
By default, we use the old algorithm. Cortex has to be explicitly configured to use the new one.

We can remove this feature flag after two releases. ;)

[harry671003]

Great work! Thanks for the PR.

[justinjung04]

Looks good! But I thought this is more of a bug fix in ring.ShuffleShard rather than a new feature. I guess you created new function and flag just to avoid any regression for now, but is there a plan to migrate other components to use this func as well?

[yeya24]

But I thought this is more of a bug fix in ring.ShuffleShard rather than a new feature. I guess you created new function and flag just to avoid any regression for now, but is there a plan to migrate other components to use this func as well?

Yes, it is not a new feature but a bug fix specifically for Store Gateway due to consistency check.

I don't know if it is necessary for other components. But because there is no consistency check to throw 5XXs due to this issue in other components, I hope it is not as urgent as this one.

So no plan for migrations of other components at the moment. We probably need to assess the impact first for each component and see if this is really causing any issue for them. For example, I don't see a big issue of using the existing shuffle sharding algorithm for compactor. We can open another issue to track the assessment and migration.