Extend ShuffleSharding on READONLY ingesters

[danielblando]

What this PR does:
We found an issue when a tenant has ingestion_tenant_shard_size lower than the number of ACTIVE ingesters or high number of READONLY ingesters when testing the new status of READONLY.

Eg:
Failed Push
Lets assume we have a ring with
10 ACTIVE ingesters
50 READONLY ingesters
tenantA ingestion_tenant_shard_size of 20

The current subRing of this tenant can be created with only READONLY ingesters. In this case, DoBatch will fail as there will be no health ingesters to send data.

Early throttle
Lets assume we have a ring with
80 ACTIVE ingesters
20 READONLY ingesters
tenantA ingestion_tenant_shard_size of 20

The current subRing can be created as a mix of ACTIVE and READONLY ingesters. This will cause a subRing of size 20 but only 15 ACTIVE ingesters supposedly. The localLimit for each ingesters will be calculated over 20 as the shard size but only 15 ingester are receiving all data. The new scenario will create a subRing over just the 80 ACTIVE ingesters.

This PR introduce extension to READONLY instances on ShuffleShard. It works similar to lookback. On the write path as we dont use READONLY and extend instead to write, it will not send requests to REANDONLY on RemoteWrite. On read path, it will return a shard greather than expected as it happens with lookback.

We are also changing the registered timestamp of ingesters which returns from READONLY state. These ingesters are as they entered the ring again for a Write perspective. This will make the lookback on read extend on them

Checklist

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

[yeya24]

If we extend replicas if the current instance is read only status, does that work?

[danielblando]

It does @yeya24.
I have moved to that approach. The filter approach brings problem to the Read operation as we can be missing some ingesters to query.
Extending we will use a similar logic as lookback.

[CharlieTLe]

Hello @danielblando, thank you for opening this PR.

There is a release in progress. As such, please rebase your CHANGELOG entry on top of the master branch and move the CHANGELOG entry to the top under ## master / unreleased.

Thanks,
Charlie

[yeya24]

Question, what happens if we don't reset the registered time?

[danielblando]

This is to catch cases the ingester goes to READONLY and back to ACTIVE.
The query still need to extend on these cases. The change on registeredTimestamp enforce that query will continue to extend requests on these ingesters.

[yeya24]

I am a bit confused about the name. I understand the prioritization but it is weird to call it equal when you have timestamp different.

[danielblando]

Hm I get what you mean, but i dont have a better naming. I think this is ok as ReadOnly is less restrictive than EqualButTimestamporState

[yeya24]

Thanks. LGTM