Describe the bug
segment-writer doesn't use ring.NewAutoForgetDelegate().
Even if an instance is configured to unregister on shutdown, a SIGKILL can result in the ring member perpetually lingering in an unhealthy state and never being deleted.
To Reproduce
Set up segment-writer to horizontal scale Pod count.
SIGKILL a segment-writer while replicas are scaling down.
Expected behavior
Every other Pyroscope component seems to use ring.NewAutoForgetDelegate() so orphan instances can be removed automatically.
The exception is the ingesters, which have a web UI for easily removing orphaned ring members.
One can e.g. curl -s -X POST -d 'forget=<id>' http://localhost:<port>/ring-segment-writer to manually remove orphaned instances. But this is cumbersome.
Environment
Kubernetes.
Additional Context
This problem is not likely to occur when using StatefulSets and static replica counts since the replacement Pod will assume the previous ring identity.
However, it is a very realistic scenario when you auto-scale segment writers.
I would also question why segment-writers need to be a StatefulSet instead of a Deployment. AFAICT Pyroscope handles dynamic segment-writer Pods pretty well with the exception of unclean process exit orphaning the memberlist entry. Since segment-writers don't have persistent volume state and the static Pod names aren't a hard requirement, IMO it makes more sense to deploy them as a Deployment so you can avoid all the quirks with StatefulSet.
Describe the bug
segment-writerdoesn't usering.NewAutoForgetDelegate().Even if an instance is configured to unregister on shutdown, a SIGKILL can result in the ring member perpetually lingering in an
unhealthystate and never being deleted.To Reproduce
Set up
segment-writerto horizontal scale Pod count.SIGKILLasegment-writerwhile replicas are scaling down.Expected behavior
Every other Pyroscope component seems to use
ring.NewAutoForgetDelegate()so orphan instances can be removed automatically.The exception is the ingesters, which have a web UI for easily removing orphaned ring members.
One can e.g.
curl -s -X POST -d 'forget=<id>' http://localhost:<port>/ring-segment-writerto manually remove orphaned instances. But this is cumbersome.Environment
Kubernetes.
Additional Context
This problem is not likely to occur when using StatefulSets and static replica counts since the replacement Pod will assume the previous ring identity.
However, it is a very realistic scenario when you auto-scale segment writers.
I would also question why segment-writers need to be a StatefulSet instead of a Deployment. AFAICT Pyroscope handles dynamic segment-writer Pods pretty well with the exception of unclean process exit orphaning the memberlist entry. Since segment-writers don't have persistent volume state and the static Pod names aren't a hard requirement, IMO it makes more sense to deploy them as a Deployment so you can avoid all the quirks with StatefulSet.