New ring state READONLY to be used by ingesters
What this PR does: This PR creates a new status in ring called READONLY. When an ingester is in READONLY it will not receive write requests but it will receive read requests. READONLY can only be set if in ACTIVE. A ingester can return from READONLY to ACTIVE.
The main idea of this feature is allow a better approach to scale down ingesters without need to set querier.query-store-after=0 or have potential partial read
Example on how a scale down can occur with new status:
Ring has 12 ingesters in 3 different AZs
We call API for 3 of them to set as READONLY
After the required time, we can remove each az at once. This is needed because when shutdown it goes to LEAVING which can cause 5xx for unhealthy ingesters.
If you also want to clean the ring from the ingester if SetUnregisterOnShutdown is false, you should call /shutdown api before scaling down
The ingester page will looks like
Which issue(s) this PR fixes: Fixes #6144
Checklist
- [X] Tests updated
- [ ] Documentation added
- [X]
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]
what should happen if -distributor.extend-writes is enabled? do we keep writing to the readonly ingester? That would be confusing 🤔
friedrichg For the extended-write, this would not change much.
The code for extended write does
Write = NewOp([]InstanceState{ACTIVE}, func(s InstanceState) bool {
return s != ACTIVE
})
It write to any active and extend to any non active. As it already extend for any status which is not ACTIVE, it will force to extend to READONLY as any ingester in LEAVING or PENDING
@danielblando I think it should never extend to readonly on writes, even with extended writes. It's one step closer to https://github.com/cortexproject/cortex/issues/5977
what do you think?
@friedrichg
If we dont extended in write for readonly, the scale down doesnt work. Because normally after this step we filter the subRing to check if we have enough ingesters for quorum. ReadOnly on that state is counted as unhealthy so the quorum fails if we dont extend.
For #5977, when @alanprot propose to not extended for non unhealthy states, i consider those to be transient states (PENDING, LEAVING). I would say that we should keep extending for unhealthy and ReadOnly state.
@danielblando I think it should never extend to readonly on writes, even with extended writes. It's one step closer to #5977
Its the other way around, no? we should always extend on read only as we wanna read only ingesters to become empty.
@danielblando I think it should never extend to readonly on writes, even with extended writes. It's one step closer to #5977
Its the other way around, no? we should always extend on read only as we wanna read only ingesters to become empty.
We do agree on making readonly ingesters empty. I think my use of the verb "extend" was not clear. What I meant was we should not add any series to read only ingesters. Which I believe we all agree. Probably obvious. Sorry not being clear.
I think this looks good.
Should we update the documentation about scaling down ingesters as well? Or we wanna do this in another PR?
Probably better to do in a separate PR. I still have another PR related to the issue opened. We can change the docs after all have been merged
LGTM
Added to the api docs