Why lease renew need to wait for apply index?

[aaronjzhang]

I'm reading etcd codes about lease keep alive flow, noticed that before renew the lease, it waits for the current apply index, if the waiting is timeout, the lease renew will be failed. if the storage has high latency issue, it will cause all leases can't be keep alive.
I can't understand why it's designed this way?

func (s *EtcdServer) LeaseRenew(ctx context.Context, id lease.LeaseID) (int64, error) {
	if s.isLeader() {
		// If s.isLeader() returns true, but we fail to ensure the current
		// member's leadership, there are a couple of possibilities:
		//   1. current member gets stuck on writing WAL entries;
		//   2. current member is in network isolation status;
		//   3. current member isn't a leader anymore (possibly due to #1 above).
		// In such case, we just return error to client, so that the client can
		// switch to another member to continue the lease keep-alive operation.
		if !s.ensureLeadership() {
			return -1, lease.ErrNotPrimary
		}

		//Why s.waitAppliedIndex is needed here？
		if err := s.waitAppliedIndex(); err != nil {
			return 0, err
		}
		ttl, err := s.lessor.Renew(id)
		if err == nil { // already requested to primary lessor(leader)
			return ttl, nil
		}
		if err != lease.ErrNotPrimary {
			return -1, err
		}
	}
	cctx, cancel := context.WithTimeout(ctx, s.Cfg.ReqTimeout())
	defer cancel()
	// renewals don't go through raft; forward to leader manually
	for cctx.Err() == nil {
		leader, lerr := s.waitLeader(cctx)
		if lerr != nil {
			return -1, lerr
		}
		for _, url := range leader.PeerURLs {
			lurl := url + leasehttp.LeasePrefix
			ttl, err := leasehttp.RenewHTTP(cctx, id, lurl, s.peerRt)
			if err == nil || err == lease.ErrLeaseNotFound {
				return ttl, err
			}
		}
		// Throttle in case of e.g. connection problems.
		time.Sleep(50 * time.Millisecond)
	}
	if cctx.Err() == context.DeadlineExceeded {
		return -1, errors.ErrTimeout
	}
	return -1, errors.ErrCanceled
}

[serathius]

https://docs.google.com/document/d/1peLDjwebnSuNR69ombuUiJ5fkSw7Rd3Slv_mUpOLsmg/edit#heading=h.jbaw5ynuj9i7

[aaronjzhang]

Thanks for the reply.
I know the issue mentioned in above document, actually I created the issue, it's different with this discussion.
I just want to understand why wait for apply index is needed in lease renew function.

[ahrtr]

@aaronjzhang Please read #13675 (comment) and #13690

[aaronjzhang]

Thanks the answer. But looks there is a side effect, when "apply entries took too long" happens and the apply time took more than the TTL of a lease will cause lease keep alive failure, for our application, it can tolerate the fake lease keep alive failure, in this case, it just grant a new lease and retry, but it can't tolerate the "apply entries took too long" blocks the keep alive for all leases.
Can we make the feature "linearizable renew lease" configurable? I can make the changes.

[ahrtr]

when "apply entries took too long" happens and the apply time took more than the TTL of a lease

It's unlikely to happen, because applying doesn't need to sync data to disk. Also usually it isn't recommended to create a lease with too short TTL (e.g < 5s).

But if the leader has network issue (e.g. in network isolation situation), then s.ensureLeadership() may timeout (7s by default if memory serves me correctly). In that case, the lease won't be mistakenly revoked and it will be renewed by new leader; it's exactly what #13690 fixed.

Do you see any real issue?

[aaronjzhang]

Yes, it indeed seldom appears, but I did encounter it.

etcd log:
{"level":"warn","ts":"2024-04-25T02:38:16.862Z","caller":"etcdserver/util.go:166","msg":"apply request took too long","took":"4.138949022s","expected-duration":"100ms","prefix":"","request":"header:<ID:6185006806275549166 > put:<key:"network-function:[SBIPHI404]:components:[sbiphi404-upc-1]:operational-state" value_size:1 >","response":"size:7"}

Client log:
<02:38:22.706 ERR ETCD_CLIENT 954:1426 0:0> [Lease:2c078eebb553be1e] Keep alive stream finished, peer: ipv4:127.0.0.1:2379, error: etcdserver: request timed out, waiting for the applied index took too long

[ahrtr]

• You need to analyze the trace of the write/put operation, to figure out which step consumed most of the time.
• disk I/O and network connectivity are important to etcd, this looks likes an environment issue which you need to resolve.
• As mentioned previously, it isn't recommended to create lease with short TTL (e.g. < 5s), because obviously it may be affected by either server side or client side delay.

[aaronjzhang]

I agree it's environment issue, we have lots of customers and their production environments are varied and uneven, they run smoothly most of the time, but it's hard to avoid occasional environmental issues. we aim to minimize the adverse affects of the occasional environmental issues.
Our products need to meet carrier-grade, service unavailability need to be detected quickly, so we create lease with 5~7s TTL, but the above accident almost make all services were unregistered, so that's why I want to add a configuration flag to enable or disable the feature, it can be enable by default, but it also can be disabled via configuration.