etcd-io / etcd-io/etcd

mvcc: compaction deletes the live value of a key deleted and re-created in the same revision, leaving an empty keyspace after restart

Open
#22,376 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
52.3k
Forks
10.5k
Avg merge
2d 21h
Merged PRs (30d)
43

Description

### Bug report criteria

- [X] This bug report is not security related, security issues should be disclosed privately via [the report form](https://github.com/etcd-io/etcd/security/advisories/new).
- [X] This is not a support request or question, support requests or questions should be raised in the etcd [discussion forums](https://github.com/etcd-io/etcd/discussions).
- [X] You have read the etcd [bug reporting guidelines](https://github.com/etcd-io/etcd/blob/main/Documentation/contributor-guide/reporting_bugs.md).
- [X] Existing open issues along with etcd [frequently asked questions](https://etcd.io/docs/latest/faq) have been checked and this is not a duplicate.

### What happened?

If a key is deleted and re-created **within a single main revision**, and the store is later compacted **at exactly that revision**, compaction deletes the key's live value from the backend while leaving the in-memory index pointing at it.

The next range over that key hits the `range failed to find revision pair` fatal and the process exits. On restart, `restore()` rebuilds the index from a backend that no longer holds the rows, so etcd comes back with an **empty keyspace**, reports itself healthy, and keeps serving. There is no error, no alarm, and no readiness failure — the data is simply gone.

This is a regression introduced by #18274 (`*: keep tombstone if revision == compactAtRev`), which changed the generation-selection condition in `keyIndex.doCompact` from `tomb > atRev` to `tomb >= atRev`. That makes compaction stop at the generation that *ends* with the tombstone and never look at the following generation, which holds the key's live re-creation at a higher sub revision in the same main revision.

I hit this on a production single-member store. The behaviour below is reproduced from scratch against `main` (8b3de3c66).

#### Observed

```
{"level":"fatal","caller":"mvcc/kvstore_txn.go:128","msg":"range failed to find revision pair",
"revision-main":7,"revision-sub":5,"revision-current":7,"range-option-rev":0,
"range-option-limit":0,"key":"Y2ZnLw==","end":"Y2ZnMA==","len-revpairs":3,"len-values":0}
```

Note `range-option-rev: 0` — this is a read at the *latest* revision, not a historical read of a compacted one.

After the process restarts:

```
$ etcdctl get --prefix "" -w json
revision = 7
keys = 0
$ etcdctl endpoint health
127.0.0.1:2379 is healthy: successfully committed proposal: took = 937.584µs
```

### What did you expect to happen?

Within one main revision, a later sub revision supersedes an earlier one — `keyIndex.since()` encodes exactly this rule ("*replace the revision with a new one that has higher sub value, because the original one should not be seen by external*"). A key deleted and re-created in the same revision is therefore **alive** at the end of that revision, and compaction at that revision must retain its put.

Concretely, for a key that existed before the rewrite:

```
gen0: [ ..., 4.0 put, 7.2 tombstone ] <- the DeleteRange
gen1: [ 7.5 put ] <- the Put, same main revision
```

`compact(7)` must keep `7.5`. Today it keeps `7.2` and deletes `7.5` from the backend, while `gen1` survives in the index still referencing `7.5`.

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

#### 1. Client-level (a real server, public API only)

```go
package main

import (
"context"
"fmt"
"log"
"time"

clientv3 "go.etcd.io/etcd/client/v3"
)

func main() {
cli, err := clientv3.New(clientv3.Config{
Endpoints: []string{"127.0.0.1:2379"},
DialTimeout: 5 * time.Second,
})
if err != nil {
log.Fatal(err)
}
defer cli.Close()
ctx := context.Background()

keys := []string{"cfg/a", "cfg/b", "cfg/c"}
for _, k := range keys {
if _, err := cli.Put(ctx, k, "v1"); err != nil {
log.Fatal(err)
}
}

// one transaction: wipe the keyspace, then re-create every key
ops := []clientv3.Op{clientv3.OpDelete("\x00", clientv3.WithFromKey())}
for _, k := range keys {
ops = append(ops, clientv3.OpPut(k, "v2"))
}
resp, err := cli.Txn(ctx).Then(ops...).Commit()
if err != nil {
log.Fatal(err)
}
rev := resp.Header.Revision

if _, err := cli.Compact(ctx, rev, clientv3.WithCompactPhysical()); err != nil {
log.Fatal(err)
}

gr, err := cli.Get(ctx, "cfg/", clientv3.WithPrefix())
if err != nil {
log.Fatalf("get failed: %v", err) // server is gone
}
fmt.Printf("after compaction: %d keys\n", len(gr.Kvs)) // want 3
}
```

Against `main` the server dies on the final `Get`; restart it and the keyspace is empty while `endpoint health` is green.

#### 2. Unit level

```go
func TestCompactRecreatedInSameRev(t *testing.T) {
lg := zaptest.NewLogger(t)
ki := &keyIndex{key: []byte("foo")}
ki.put(lg, 3, 0)
require.NoError(t, ki.tombstone(lg, 4, 0))
ki.put(lg, 4, 1)

available := make(map[Revision]struct{})
ki.compact(lg, 4, available)

// fails on main: available is {4.0}, and the index still points at 4.1
require.Contains(t, available, Revision{Main: 4, Sub: 1})
}
```

#### 3. Store level (the empty-keyspace outcome)

Put a few keys, then in one `storeTxnWrite` call `DeleteRange` over them followed by `Put` of the same keys, compact at that revision, close the store and re-open it over the same backend. Counting keys after the rebuild gives `0` on `main` and the correct count with the generation selection corrected.

### Anything else we need to know?

**How the shape is reachable.** `checkIntervals` in `server/etcdserver/api/v3rpc/key.go` is meant to reject a txn that deletes and puts the same key, but it builds the delete's interval with `adt.NewStringAffineInterval(key, range_end)`. For `range_end == "\x00"` — the idiomatic "delete from here to the end of the keyspace", i.e. `clientv3.WithFromKey()` — that interval is `[key, "\x00")`, which is empty and intersects nothing, so the overlapping puts are not caught. The apply path performs no such check. Measured against `main`:

| delete op in the same txn as `OpPut("cfg/a", …)` | result |
| --- | --- |
| `OpDelete("cfg/", WithPrefix())` | rejected |
| `OpDelete("cfg/a")` | rejected |
| `OpDelete("cfg/", WithRange("cfg0"))` | rejected |
| `OpDelete("cfg/", WithFromKey())` | **accepted** |
| `OpDelete("\x00", WithFromKey())` | **accepted** |
| `OpDelete("a", WithRange("\x00"))` | **accepted** |

So "rewrite the whole keyspace in one transaction" — a `DeleteRange` from `\x00` followed by the new values — is accepted today and produces a tombstone and a put for every key within one main revision.

Whether `checkIntervals` should also be tightened is a separate question. The mvcc layer needs to be correct either way: the apply path does not validate, so the same entry applies unchecked on every follower and on WAL replay, and the deleted-then-recreated shape has well-defined semantics.

**Precondition.** Both a tombstone *and* a put for the same key in the same main revision, compacted at exactly that revision, are required. If the store is empty when the rewrite runs, the DeleteRange tombstones nothing, each key has a single generation, and compaction at the live revision is harmless. Compacting the same data at any *later* revision is also harmless.

**Affected versions.** The regressing commit is on every branch that carries #18274 and its backports:

| branch | first affected release |
| --- | --- |
| release-3.4 | v3.4.34 |
| release-3.5 | v3.5.16 |
| release-3.6 | all |
| release-3.7 | all |
| main | yes |

Verified by building the unit reproducer at `bbdc94181^` (passes) and `bbdc94181` (fails), and by running an equivalent reproducer on `origin/release-3.4` and `origin/release-3.5` (both fail).

**Not to be confused with** #18089 / #18274 (a dropped tombstone at the compaction revision) — this is the opposite side of the same line, a dropped *put*. Any fix has to preserve #18274's behaviour.

I have a fix and tests ready and will open a PR referencing this issue.

### Etcd version (please run commands below)

```console
$ etcd --version
etcd Version: 3.8.0-alpha.0
Git SHA: 8b3de3c66
Go Version: go1.27.0
Go OS/Arch: darwin/arm64
```

Also reproduced on `origin/release-3.4` and `origin/release-3.5`; originally observed on v3.7.1 (`quay.io/coreos/etcd:v3.7.1`).

### Etcd configuration (command line flags or environment variables)

Single member, defaults. The production occurrence used auto periodic compaction; the compaction revision simply has to land on the revision of the rewrite transaction.

```
etcd --data-dir ./data \
--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:2380 --initial-advertise-peer-urls http://127.0.0.1:2380 \
--initial-cluster default=http://127.0.0.1:2380
```

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

```console
$ etcdctl endpoint status -w table
# revision preserved, keyspace empty after the restart
$ etcdctl get --prefix "" --count-only -w json
{"header":{"revision":7},"count":0}
```

`etcd_debugging_mvcc_keys_total` goes to `0` across the restart.

### Relevant log output

```console
{"level":"info","msg":"finished scheduled compaction","compact-revision":7,"took":"11.9ms"}
{"level":"fatal","caller":"mvcc/kvstore_txn.go:128","msg":"range failed to find revision pair","revision-main":7,"revision-sub":5,"revision-current":7,"range-option-rev":0,"range-option-limit":0,"key":"Y2ZnLw==","end":"Y2ZnMA==","len-revpairs":3,"len-values":0}
```

Contributor guide

Open the contributing guide

Research direction

Start with keyIndex.doCompact and the named TestCompactRecreatedInSameRev unit reproducer, then inspect mvcc/kvstore_txn.go around the range failure. Review checkIntervals in server/etcdserver/api/v3rpc/key.go for the reachable transaction shape. Done means the live same-revision put remains available after compaction, the unit test passes, and reopening the store preserves the keys.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
databases, distributed-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.