Skip to content

Commit

Permalink
etcdutl: fix TotalKey counting in snapshot status
Browse files Browse the repository at this point in the history
- Expose IsTombstone function from mvcc package
- Use IsTombstone to check deletion state on revision keys
- Add more tests

Fixes etcd-io#19253

Signed-off-by: Xiang Ji <johnsmith.jix@gmail.com>
  • Loading branch information
jxustc committed Feb 7, 2025
1 parent 83d2c81 commit 049178d
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 2 deletions.
8 changes: 7 additions & 1 deletion etcdutl/snapshot/v3_snapshot.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,13 @@ func (s *v3Manager) Status(dbPath string) (ds Status, err error) {
if err != nil {
return fmt.Errorf("cannot unmarshal value, key: %q value: %q err: %w", k, v, err)
}
seenKeys[string(kv.Key)] = struct{}{}
key := string(kv.Key)
// refer to https://etcd.io/docs/v3.5/learning/data_model/
if !mvcc.IsTombstone(k) {
seenKeys[key] = struct{}{}
} else {
delete(seenKeys, key)
}
}
return nil
}); err != nil {
Expand Down
79 changes: 78 additions & 1 deletion etcdutl/snapshot/v3_snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,84 @@ func TestSnapshotStatusDuplicateKeys(t *testing.T) {
status, err := NewV3(zap.NewNop()).Status(dbpath)
require.NoError(t, err)
// unique keys: key1, key2
assert.Equal(t, 2, status.TotalKey)
assert.Equalf(t, 2, status.TotalKey, "mismatch on duplicate inserts, expected 2 unique keys (key1/key2)")
}

// TestSnapshotStatusMixedRevisions tests if key count respects latest revision state
func TestSnapshotStatusMixedRevisions(t *testing.T) {
dbpath := createDB(t, func(srv *etcdserver.EtcdServer) {
// key1: create -> put -> delete
key := []byte("key1")
for i := 0; i < 3; i++ {
if i < 2 {
_, err := srv.Put(context.TODO(), &etcdserverpb.PutRequest{Key: key, Value: []byte(strconv.Itoa(i))})
require.NoError(t, err)
} else {
_, err := srv.DeleteRange(context.TODO(), &etcdserverpb.DeleteRangeRequest{Key: key})
require.NoError(t, err)
}
}
})

status, err := NewV3(zap.NewNop()).Status(dbpath)
require.NoError(t, err)
assert.Equalf(t, 0, status.TotalKey, "final state should be tombstone")
}

// TestSnapshotStatusIgnoredTombstones tests if deleted keys are excluded from TotalKey count
func TestSnapshotStatusIgnoredTombstones(t *testing.T) {
dbpath := createDB(t, func(srv *etcdserver.EtcdServer) {
// key1: create -> delete -> re-create -> delete
key := []byte("key1")
for i := 0; i < 2; i++ {
_, err := srv.Put(context.TODO(), &etcdserverpb.PutRequest{Key: key, Value: make([]byte, 1)})
require.NoError(t, err)
_, err = srv.DeleteRange(context.TODO(), &etcdserverpb.DeleteRangeRequest{Key: key})
require.NoError(t, err)
}
})

status, err := NewV3(zap.NewNop()).Status(dbpath)
require.NoError(t, err)
assert.Equalf(t, 0, status.TotalKey, "expected 0 active keys after multi delete cycle")
}

// TestSnapshotStatusRestoredKeys tests if recreated keys after deletion are counted
func TestSnapshotStatusRestoredKeys(t *testing.T) {
dbpath := createDB(t, func(srv *etcdserver.EtcdServer) {
// key1: create -> delete -> re-create -> delete -> re-create
key := []byte("key1")
for i := 0; i < 5; i++ {
if i%2 == 0 {
_, err := srv.Put(context.TODO(), &etcdserverpb.PutRequest{Key: key, Value: make([]byte, 1)})
require.NoError(t, err)
} else {
_, err := srv.DeleteRange(context.TODO(), &etcdserverpb.DeleteRangeRequest{Key: key})
require.NoError(t, err)
}
}
})

status, err := NewV3(zap.NewNop()).Status(dbpath)
require.NoError(t, err)
assert.Equalf(t, 1, status.TotalKey, "recreated key should count as active")
}

// TestSnapshotStatusMixedDeletions tests if only active keys survive after mixed operations
func TestSnapshotStatusMixedDeletions(t *testing.T) {
dbpath := createDB(t, func(srv *etcdserver.EtcdServer) {
// Put("key1") -> Put("key2")-> Delete("key1")
_, err := srv.Put(context.TODO(), &etcdserverpb.PutRequest{Key: []byte("key1"), Value: make([]byte, 1)})
require.NoError(t, err)
_, err = srv.Put(context.TODO(), &etcdserverpb.PutRequest{Key: []byte("key2"), Value: make([]byte, 1)})
require.NoError(t, err)
_, err = srv.DeleteRange(context.TODO(), &etcdserverpb.DeleteRangeRequest{Key: []byte("key1")})
require.NoError(t, err)
})

status, err := NewV3(zap.NewNop()).Status(dbpath)
require.NoError(t, err)
assert.Equalf(t, 1, status.TotalKey, "only remaining valid key should be key2")
}

// insertKeys insert `numKeys` number of keys of `valueSize` size into a running etcd server.
Expand Down
4 changes: 4 additions & 0 deletions server/storage/mvcc/revision.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,3 +120,7 @@ func BytesToBucketKey(bytes []byte) BucketKey {
func isTombstone(b []byte) bool {
return len(b) == markedRevBytesLen && b[markBytePosition] == markTombstone
}

func IsTombstone(b []byte) bool {
return isTombstone(b)
}

0 comments on commit 049178d

Please sign in to comment.