the ring never removes old ingester even if the ingester pod is evicted

[wuyafang]

I have a similar problem as #1502
when my ingester pod was evicted , a new ingester pod will be created .
now the ring has two ingester, but only one (the new one) is healthy. the old one will not be removed from the ring, even if I delete the evict pod manually.
the ring information as follows:

`

                                        <tr>
						<td>ingester-7fc8759d7f-nzb6g</td>
						<td>ACTIVE</td>
						<td>172.16.0.62:9095</td>
						<td>2019-07-19 03:33:32 &#43;0000 UTC</td>
						<td>128</td>
						<td>45.739077787319914%</td>
						<td><button name="forget" value="ingester-7fc8759d7f-nzb6g" type="submit">Forget</button></td>
					</tr>
					
					<tr>
						<td>ingester-7fc8759d7f-wmnms</td>
						<td>Unhealthy</td>
						<td>172.16.0.93:9095</td>
						<td>2019-07-18 14:46:18 &#43;0000 UTC</td>
						<td>128</td>
						<td>54.260922212680086%</td>
						<td><button name="forget" value="ingester-7fc8759d7f-wmnms" type="submit">Forget</button></td>
					</tr>

`
and the ingester's status is always unready， with distributor's error

level=warn ts=2019-07-19T03:41:45.413839063Z caller=server.go:1995 traceID=daf4028f530860f msg="POST /api/prom/push (500) 727.847µs Response: \"at least 1 live ingesters required, could only find 0\\n\" ws: false; Connection: close; Content-Encoding: snappy; Content-Length: 3742; Content-Type: application/x-protobuf; User-Agent: Prometheus/2.11.0; X-Forwarded-For: 172.16.0.17; X-Forwarded-Host: perf.monitorefk.huawei.com; X-Forwarded-Port: 443; X-Forwarded-Proto: https; X-Original-Uri: /api/prom/push; X-Prometheus-Remote-Write-Version: 0.1.0; X-Real-Ip: 172.16.0.17; X-Request-Id: 62a470dc6de7a83c8974e3411fa63e40; X-Scheme: https; X-Scope-Orgid: custom; "

I wonder if there is any solution to deal with the situaton automatically?
maybe to check the replicas-refactor and remove unhealthy excess ingesters from the ring?

[bboreham]

If the ingester shut down cleanly, even on eviction, then it would not be in the ring. So, the first task is to find out why it did not shut down cleanly, and if possible fix that.

Everything else you report is deliberate. We return not-ready to halt a rolling update.

[bboreham]

Actually I don’t understand could only find 0, since your ring shows 1 active.

[wuyafang]

what do you mean about "shut down cleanly"?

I try to reproduce the problem by delete pod --force. and a new ingester pod is produced by deployment controller immediately.
nowI get an ring has two ingester(one is active,the other is unhealthy)，an unready state for my new ingester(because there is an unhealthy pod, so 503 returned for readiness check), and a distributor log like this
level=warn ts=2019-07-19T08:18:25.511613228Z caller=logging.go:49 traceID=5eabc2f0f6b837e9 msg="POST /api/prom/push (500) 266.759µs Response: \"at least 2 live ingesters required, could only find 1\\n\" ws: false; Connection: close; Content-Encoding: snappy; Content-Length: 5162; Content-Type: application/x-protobuf; User-Agent: Prometheus/2.11.1; X-Forwarded-For: 100.95.185.106; X-Forwarded-Host: 100.95.137.223; X-Forwarded-Port: 443; X-Forwarded-Proto: https; X-Original-Uri: /api/prom/push; X-Prometheus-Remote-Write-Version: 0.1.0; X-Real-Ip: 100.95.185.106; X-Request-Id: 89b05193a60c7935ac6a7bcd090b9a16; X-Scheme: https; X-Scope-Orgid: primary

I'm confused because my -distributor.replication-factor
is 1, so by minSuccess := (replicationFactor / 2) + 1 , my distributor only need at 1 live ingester, but the log tells me I need two.

so is there anything I misunderstood?

I wonder when the ring adds ingester and when to remove? Is consul do it by itself , or ingester tell it what to do? I notice when ingester start and shutdown, it will tell ring. But what if the ingester is shutdown unclearly ,is there any solutions to automatically clean the unhealthy pod in the ring ?

by the way , after I restart my consul , the ring will only have the active one and anything works well.

[bboreham]

I mean the ingester went through its exit sequence, rather than being abruptly terminated from outside.

There are two main cases: hand-over to another ingester, and flush to store. In both cases the time required is a function of how much data is in memory.

When using an explicitly provisioned store (eg DynamoDB) it would be nice to scale up specifically for a “save everything” operation. There’s no code to do that currently.

[wuyafang]

I try to reproduce the problem by delete pod --force. and a new ingester pod is produced by deployment controller immediately.
nowI get an ring has two ingester(one is active,the other is unhealthy)，an unready state for my new ingester(because there is an unhealthy pod, so 503 returned for readiness check), and a distributor log like this
level=warn ts=2019-07-19T08:18:25.511613228Z caller=logging.go:49 traceID=5eabc2f0f6b837e9 msg="POST /api/prom/push (500) 266.759µs Response: "at least 2 live ingesters required, could only find 1\n" ws: false; Connection: close; Content-Encoding: snappy; Content-Length: 5162; Content-Type: application/x-protobuf; User-Agent: Prometheus/2.11.1; X-Forwarded-For: 100.95.185.106; X-Forwarded-Host: 100.95.137.223; X-Forwarded-Port: 443; X-Forwarded-Proto: https; X-Original-Uri: /api/prom/push; X-Prometheus-Remote-Write-Version: 0.1.0; X-Real-Ip: 100.95.185.106; X-Request-Id: 89b05193a60c7935ac6a7bcd090b9a16; X-Scheme: https; X-Scope-Orgid: primary

I'm confused because my -distributor.replication-factor
is 1, so by minSuccess := (replicationFactor / 2) + 1 , my distributor only need at 1 live ingester, but the log tells me I need two.

so is there anything I misunderstood?

I wonder when the ring adds ingester and when to remove? Is consul do it by itself , or ingester tell it what to do? I notice when ingester start and shutdown, it will tell ring. But what if the ingester is shutdown unclearly ,is there any solutions to automatically clean the unhealthy pod in the ring ?

by the way , after I restart my consul , the ring will only have the active one and anything works well.

[wuyafang]

I know...
you do this

        replicationFactor := r.cfg.ReplicationFactor
	if len(ingesters) > replicationFactor {
		replicationFactor = len(ingesters)
	}

so the replicationFactor is 2 now, instead of what I set in -distributor.replication-factor.
it is in case of node joining/leaving， but will cause write failure in my case above.

[bboreham]

That sounds like the same problem as #1290
@tomwilkie can you remember what that check is for?

cortex/pkg/ring/replication_strategy.go

Line 20 in 7cf0690

if len(ingesters) > replicationFactor {

[YaoZengzeng]

Actually If I deploy one ingester and replicationFactor is 1, then ingester pod was evicted because of low memory and kubelet restart another ingester pod.

However the previous ingester didn't exit cleanly, actually the corresponding entry in the ring of consul will never be cleaned. So at this moment:

replicationFactor == len(ingesters) = 2 (1 eviceted ingester and 1 running ingester)

minSuccess = (replicationFactor / 2) + 1 = 2

However len(liveIngesters) = 1 < minSuccess ---> There is is a deadlock: unhealthy ingester never cleaned from the ring and we'll never reach minSuccess.

Two problems here:

1. Why len(ingester) > replicationFactor, then replicationFactor = len(ingesters). ---> In fact, the ingesters more than original replicationFactor is usually not normal.

2. If the ingester is unhealthy for a long time, why not clean it out from the ring. ---> The residual ingester will affect the replication strategy.

@bboreham @tomwilkie @csmarchbanks Any ideas ?

[bboreham]

The current design requires that you set terminationGracePeriodSeconds long enough to shut down the first ingester cleanly.

Your point 1 seems the same as #1290

Point 2 because we don't have enough experience of situations that need this. We would probably add it as an option if someone was to submit a PR.

[YaoZengzeng]

@bboreham If ingester is killed because of OOM (Actually ingester consume a lot memory and it's very common in k8s, at least very common in my k8s environment), then it will never have terminationGracePeriodSeconds to shut down gracefully 😂

For point 1, I think replicationFactor is configured by user, so set it constant maybe more reasonable.

For point 2, I may need to read more code to better understand the design intent. If it's necessary, I'd like to make an PR to fix it.

[bboreham]

"killed because of OOM" is not the same thing as "evicted". A pod that is OOM-killed will restart with the same identity on the same node, hence pick up the same entry in the Cortex ring.
Unless you have evidence to the contrary?

[YaoZengzeng]

@bboreham You are right.

In our environment, the kubelet was configured with hard eviction, so the ingester pod was evicted without graceful period.

However even configure kubelet with soft eviction, I have no idea of how to configure the eviction-max-pod-grace-period. Because the grace period needed by ingester may related to the amount of data it contains.

If after configured grace period, ingester still can't exit cleanly. Then the problem still can't be solved.

[miklosbarabasForm3]

Hi
Apparently I ran into the same issue, although I saw msg="error removing stale clients" err="too many failed ingesters" as well in the logs.

Having a look at the code my assumption is the following (please correct me if im wrong):

• pkg/ingester/client/pool.go#removeStaleClients() calls to p.ring.GetAll()
• pkg/ring/ring.go#GetAll() returns error with too many failed ingesters because of the way how the logic is implemented using maxErrors.

Some questions:

• why does GetAll need to check against the non-healthy instances at all?
• if that check is needed, what is the purpose behind calculating the maxErrors from the Unhealthy instances instead of calculating the ACTIVE ones and using the ReplicationFactor?
  

  cortex/pkg/ring/ring.go

  Lines 258 to 286 in 1ca4ad0

  	// GetAll returns all available ingesters in the ring.
  	func (r *Ring) GetAll() (ReplicationSet, error) {
  	r.mtx.RLock()
  	defer r.mtx.RUnlock()
  	if r.ringDesc == nil || len(r.ringDesc.Tokens) == 0 {
  	return ReplicationSet{}, ErrEmptyRing
  	}
  	ingesters := make([]IngesterDesc, 0, len(r.ringDesc.Ingesters))
  	maxErrors := r.cfg.ReplicationFactor / 2
  	for _, ingester := range r.ringDesc.Ingesters {
  	if !r.IsHealthy(&ingester, Read) {
  	maxErrors--
  	continue
  	}
  	ingesters = append(ingesters, ingester)
  	}
  	if maxErrors < 0 {
  	return ReplicationSet{}, fmt.Errorf("too many failed ingesters")
  	}
  	return ReplicationSet{
  	Ingesters: ingesters,
  	MaxErrors: maxErrors,
  	}, nil
  	}

After changing the aforementioned code (line 278-280) to the following, I stopped receiving "error removing stale client":

if len(ingesters) < r.cfg.ReplicationFactor / 2 + 1 {
    return ReplicationSet{}, fmt.Errorf("not enough healthy ingesters (ingesters: %d, replicationFactor: %d)", len(ingesters), r.cfg.ReplicationFactor)
}

also when I set distributor.health-check-ingesters enabled, the Unhealthy ingesters got cleaned up properly. (so #1264 might be related as well) EXCEPT when it was an OOM or using ECS and your ingester comes back with the same identity (as you mentioned), on the same IP/host. Meaning there's no logic to transition from LEAVING to ACTIVE again. Shouldn't there be some logic created around that?

Note that distributor.client-cleanup-period (def 15s) & distributor.health-check-ingesters (def: false) controls the ingester cleanup, which will clean up Unhealthy ingesters if you have the health-check-ingesters enabled.

cortex/pkg/ingester/client/pool.go

Lines 75 to 92 in 1ca4ad0

	func (p *Pool) loop() {
	defer p.done.Done()
	cleanupClients := time.NewTicker(p.cfg.ClientCleanupPeriod)
	defer cleanupClients.Stop()
	for {
	select {
	case <-cleanupClients.C:
	p.removeStaleClients()
	if p.cfg.HealthCheckIngesters {
	p.cleanUnhealthy()
	}
	case <-p.quit:
	return
	}
	}
	}

[wuyafang]

Note that distributor.client-cleanup-period (def 15s) & distributor.health-check-ingesters (def: false) controls the ingester cleanup, which will clean up Unhealthy ingesters if you have the health-check-ingesters enabled.

I tried this. but it just removes unhealthy ingester from distirbutor pool( which holds ingester clients ) instead of removing them from consul ring. It doesn't work for me.

[bboreham]

Don't read too much into the words - that's removing them from one data structure in memory. There is no code to remove ingesters from the ring when they are suspected to be dead, and this was deliberate.