All leases are revoked when the etcd leader is stuck in handling raft Ready due to slow fdatasync or high CPU.

[aaronjzhang]

What happened?

We have 3 etcd nodes which are running on k8s, the etcd pod use portworx storage for db saving, we've found when rebooting the portworx node, it will cause etcd to be stuck in fdatasync, if this etcd node is leader, then it will cause below 2 lease keep alive issues.

1. the old leader still revokes lease after it steped to follower, this is the same issue as #12528

some clients connected to other etcd nodes and send keep alive to them, after the old leader steped to follower, the new leader will handle the lease keep alive, but the old leader's lessor still considered itself is the primary and revoked all leases after their ttl expired in its view, after the old leader returned from fdatasync, these revoke request will be still sent out when handling the next Ready.

2. the old leader still handle lease keep alive request from the connected client.

some clients connected to the old leader etcd node, when the old leader was stuck in fdatasync, the raft layer steped to follower, but the lessor was not demoted, the connected clients can still send keep alive request to the old leader, at the same time the other two etcd nodes elected a new leader and didn't receive the lease keep alive message which should be sent to them but they were still sent to old leader, so these leases were revoked after the ttl expired.

Essentially the above 2 issues are because of the lessor primary is not synced up with the raft leader, there is big gap between lessor primary and raft leader when etcd server is stuck in processing raft Ready.

What did you expect to happen?

1. The lease which is kept alive on old leader should return an error when its raft stepped to follower so that the clients can connect to other etcd nodes.
2. The old leader should not revoke the other leases.

How can we reproduce it (as minimally and precisely as possible)?

1. Add a piece of codes in func (w *WAL) sync() function to simulate fdatasync stuck

func (w *WAL) sync() error {
	if w.encoder != nil {
		if err := w.encoder.flush(); err != nil {
			return err
		}
	}
	start := time.Now()
	err := fileutil.Fdatasync(w.tail().File)

    //Simulate fdatasync is stuck
	if fileutil.Exist("/run/etcd/fdatasync_block") {
		f, err := ioutil.ReadFile("/run/etcd/fdatasync_block")
		if err == nil {
			str_num := strings.Trim(string(f), "\n")
			block_duration, err := strconv.Atoi(str_num)
			if err == nil {
				time.Sleep(time.Duration(block_duration) * time.Second)
			} else {
				if w.lg != nil {
					w.lg.Warn("Atoi error", zap.Error(err))
				}
			}
		}
	}

	took := time.Since(start)
	if took > warnSyncDuration {
		if w.lg != nil {
			w.lg.Warn(
				"slow fdatasync",
				zap.Duration("took", took),
				zap.Duration("expected-duration", warnSyncDuration),
			)
		} else {
			plog.Warningf("sync duration of %v, expected less than %v", took, warnSyncDuration)
		}
	}
	walFsyncSec.Observe(took.Seconds())

	return err
}

2. Compile etcd with above changes.
3. Apply the etcd patch in a cluster and restart them.
4. Grant 3 leases with 10 seconds ttl.
5. Keep different lease alive on different etcd node.
6. Start to simulate fdatasync blocking on leader etcd with below commands:

mkdir /run/etcd; echo 60 > /run/etcd/fdatasync_block

7. Put a kv to trigger fdatasync.
8. Wait for etcd leader changing.
9. Check the time to live of the lease on the new etcd leader which is kept alive on the old leader, you will see it's expired but the keeping alive is on going on the old leader.
10. Wait for about 60 seconds, the other two leases will be revoked after the old leader is recovered.

Anything else we need to know?

No response

Etcd version (please run commands below)

Details

$ etcd --version
etcd Version: 3.4.3
Git SHA: 3cf2f69b5
Go Version: go1.14
Go OS/Arch: linux/amd64




$ etcdctl version
etcdctl version: 3.4.3
API version: 3.4

Etcd configuration (command line flags or environment variables)

Details

paste your configuration here

Etcd debug information (please run commands below, feel free to obfuscate the IP address or FQDN in the output)

Details

$ etcdctl member list -w table
# paste output here

$ etcdctl --endpoints=<member list> endpoint status -w table
# paste output here

Relevant log output

No response

Solution and analysis (updated by @ahrtr)

• Resolved in Ignore old leader's leases revoking request #16822 for main (3.6)
• Summary: All leases are revoked when the etcd leader is stuck in handling raft Ready due to slow fdatasync or high CPU. #15247 (comment)

[ahrtr]

Thanks @aaronjzhang for raising this issue.

It looks like a major design issue to me. When the etcdserver is being blocked on the fdatasync, there is no chance for raft to notify etcdserver the leader changing.

I do not get time to deep dive into the new feature raft: support asynchronous storage writes, my immediate feeling is that it should can resolve such issue. @tbg @nvanbenschoten Could you double confirm this? But we also need to resolve it for etcd 3.5, which still depends on the old raft package.

Note you can reproduce this issue using the failpoint walBeforeSync or walAfterSync added in #15252. The steps to reproduce this issue are as below,

$ make gofail-enable
$ make build
$ goreman start   # Use the Procfile below
$ etcdctl lease grant 20
$ etcdctl lease keep-alive 3269862a9fd99a09  # Run this command in another terminal. The leaseid is returned in above command
$ curl http://127.0.0.1:2003/raftBeforeSave -XPUT -d'sleep("100s")'  # Note that you need to enable failpoint on the leader, assuming etcd3 is the leader
$ etcdctl put k1 v1   # wait about 100s, then you will see that the lease will be revoked

Procfile

etcd1: GOFAIL_HTTP="127.0.0.1:2001" bin/etcd --name infra1 --listen-client-urls http://127.0.0.1:2379 --advertise-client-urls http://127.0.0.1:2379 --listen-peer-urls http://127.0.0.1:12380 --initial-advertise-peer-urls http://127.0.0.1:12380 --initial-cluster-token etcd-cluster-1 --initial-cluster 'infra1=http://127.0.0.1:12380,infra2=http://127.0.0.1:22380,infra3=http://127.0.0.1:32380' --initial-cluster-state new --enable-pprof --logger=zap --log-outputs=stderr
etcd2: GOFAIL_HTTP="127.0.0.1:2002" bin/etcd --name infra2 --listen-client-urls http://127.0.0.1:22379 --advertise-client-urls http://127.0.0.1:22379 --listen-peer-urls http://127.0.0.1:22380 --initial-advertise-peer-urls http://127.0.0.1:22380 --initial-cluster-token etcd-cluster-1 --initial-cluster 'infra1=http://127.0.0.1:12380,infra2=http://127.0.0.1:22380,infra3=http://127.0.0.1:32380' --initial-cluster-state new --enable-pprof --logger=zap --log-outputs=stderr
etcd3: GOFAIL_HTTP="127.0.0.1:2003" bin/etcd --name infra3 --listen-client-urls http://127.0.0.1:32379 --advertise-client-urls http://127.0.0.1:32379 --listen-peer-urls http://127.0.0.1:32380 --initial-advertise-peer-urls http://127.0.0.1:32380 --initial-cluster-token etcd-cluster-1 --initial-cluster 'infra1=http://127.0.0.1:12380,infra2=http://127.0.0.1:22380,infra3=http://127.0.0.1:32380' --initial-cluster-state new --enable-pprof --logger=zap --log-outputs=stderr

[ahrtr]

Run make gofail-disable to cleanup the codebase.

[tbg]

do not get time to deep dive into the new feature etcd-io/raft#8, my immediate feeling is that it should can resolve such issue. @tbg @nvanbenschoten Could you double confirm this? But we also need to resolve it for etcd 3.5, which still depends on the old raft package.

It could help be more resilient to disk stalls, but it wasn't designed to do just that, and so this is just speculation. In CockroachDB we only use async log appends, but the log application is still in the same loop, so we don't have a ton of experience there either. Plus, we haven't shipped it. I don't understand exactly what the problem in etcd is that is described above, but it's probably similar to issues we have in CockroachDB when the disk stalls and turns into a grey failure. What we do there is detect disk stalls and fatal the process. We don't try to handle it gracefully.

[ahrtr]

What we do there is detect disk stalls and fatal the process.

Thanks @tbg . This seems to be the key point. Could you be a little more specific about this?

[tbg]

Sure - here's a small tour. I hope I'm not missing anything but basically

• all logging is fallible: buffered and when it can't flush in time, exit the process¹. This includes network logging (fluentd). We have observed grey failures due to all of these stalling.
• our storage engine pebble detects stalled writes and exits the process².
• additionally, in the liveness layer in CRDB, we also write to our disks before extending a lease³; this is so that we are a bit more snappy in detecting badness but doesn't fundamentally change the behavior.
• we perform end-to-end testing of disk stalls⁴⁵.

Footnotes

1. https://github.com/cockroachdb/cockroach/blob/5024b24907e9782b2c3b8eb9e097467b2570c671/pkg/util/log/file.go#L251-L268 ↩

2. https://github.com/cockroachdb/cockroach/blob/844a370ceced8e02fd5293a8854de964edae0794/pkg/storage/pebble.go#L1124-L1153 ↩

3. https://github.com/cockroachdb/cockroach/blob/a28aa6cbb4b0de3ff92b4dcc74e319d2e2a220c2/pkg/kv/kvserver/liveness/liveness.go#L1278-L1295 ↩

4. https://github.com/cockroachdb/cockroach/blob/6ed2fb0f1e98b0bdbf6d0f100bef2c1bf5dbf6de/pkg/cmd/roachtest/tests/disk_stall.go ↩

5. https://github.com/cockroachdb/cockroach/blob/eec5a47f4762922674feaeac74666e4d63d2a247/pkg/cmd/roachtest/tests/failover.go#L631-L638 ↩

[ahrtr]

Thank you @tbg ! Will take a look later.

[chaochn47]

#13527 is related.

[ahrtr]

Thanks @tbg for the input.

When there is something wrong on the storage, etcd may get stuck on syncing data on the storage files. When running into this situation, the etcd member may not be able to process any requests, even become unresponsive when it is being removed from the cluster, e.g issues/14338. If the blocked member is the leader, it may cause two issues,

• Users may get two leaders, but actually there is only one valid leader. The old leader is getting stuck on stalled writes, so the raft has no chance to notify it that the leader has changed. See 14338#issuecomment-1216006545.
• The old leader may unexpectedly revoke all leases when it recovers from stalled writes, and accordingly remove all keys bound to the leases. See issues/15247.

I am thinking to propose to terminate etcd process when it doesn’t finish syncing the data (WAL or boltdb) in a configured max duration (we can define two thresholds of duration, one for WAL and the other for boltDB). When the member starts again, it will not be elected as the leader anymore if the storage issue isn’t resolved, so the two issues mentioned above will not happen.

any immediate comments or concerns?
@mitake @ptabor @serathius @spzala @tbg
@liggitt @dims @lavalamp @wojtek-t @nikhita @neolit123 @chaochn47 @tjungblu @fuweid

If it looks overall good, then I will provide a detailed design doc.

[chaochn47]

+1 on the proposal

The issue we saw was kube-apiserver watch still connects to the old member which was stuck in disk write and serving with no updates. Terminating the "stuck in disk writing" etcd process is the right thing to do IMO. It is now fulfilled by nanny agent but could have been done in etcd.

For data consistency risks:

1. boltdb is less concerning as long as consistent_index did not persist before terminating. WAL replay will reapply to boltdb after restart.
2. Not sure what's the side effect of "meant to be persisted" WAL entries are lost especially hard state (commit index). I think the safest way to mitigate this uncertainty is to fail fast after restart. This could be achieved by removing this "bad" member permanently.

In other hand, linearizability test already has equivalent failure injection point raftBeforeSave and beforeCommit to verify correctness.