Repository: etcd-io/etcd
File: server/etcdserver/v3_server.go (Go)

Code before:
	ctx, span = traceutil.Tracer.Start(ctx, "lease_renew", trace.WithAttributes(
		attribute.Int64("id", int64(id)),
	))
	defer span.End()

	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
		}

		// This change aims to mitigate the impact of "apply request took too long"
		// while maintaining correctness:
		// 	 1. If a lease is not found, it might be in the process of being applied.
		//      We need to wait for the applied index to advance to confirm whether
		//      the lease truly doesn't exist.
		//   2. If a lease is found but in revoking state, the revoke request has
		//      been committed but not yet applied. Even if we allow the current
		//      renewal to proceed, it won't affect the eventual outcome since the
		//      lease will still be properly revoked once the revoke request is applied.
		le := s.lessor.Lookup(id)
		if le == nil {
			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 !errorspkg.Is(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
		}

Review comment (bug):
In the second scenario, there is a small chance that users might get confused when the `LeaseKeepAlive` responds no error even after they just successfuly called `LeaseRevoke`. 
The `LeaseRevoke` will eventually be applied, `KeepAlive` will quit, so it does not break correctness.
A suprising renewal failure seems a bigger threat to a metadata system.

Code after:
	ctx, span = traceutil.Tracer.Start(ctx, "lease_renew", trace.WithAttributes(
		attribute.Int64("id", int64(id)),
	))
	defer span.End()

	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
		}

		// This change aims to make lease renewal faster under high server load
		// while preserving correctness. If a lease is not found, it might still be in
		// the process of being created. We must wait for the applied index to advance
		// to verify whether the lease truly does not exist.
		if s.FeatureEnabled(features.FastLeaseKeepAlive) {
			le := s.lessor.Lookup(id)
			if le == nil {
				if err := s.waitAppliedIndex(); err != nil {
					return 0, err
				}
			}
		} else {
			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 !errorspkg.Is(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