Skip to content

[Bug]: FileAuditKV.MultiSave is unimplemented, breaking repair segment-storage-layout and silently no-opping restore #520

Description

@lokeshramchand-ctrl

Current Behavior

states/kv/kv_audit.go:

func (c *FileAuditKV) MultiSave(ctx context.Context, keys, values []string) error {
	return errors.New("not implemented")
}

FileAuditKV is the default MetaKV wrapper used whenever birdwatcher successfully opens its audit_*.log file (states/instance.go, GetInstanceState):

file, err := os.OpenFile(name, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644)
if err != nil {
	fmt.Println("failed to open audit.log file!")
	kv = cli
} else {
	kv = metakv.NewFileAuditKV(cli, file)
}

So in the normal case (file opens fine), every write goes through FileAuditKV, and every other method (Save, Remove, RemoveWithPrefix, ...) correctly delegates to the wrapped client after writing its audit record. MultiSave does not: it always returns an error and never calls the underlying client at all.

Two real commands depend on MultiSave and are both broken by this:

  1. repair segment-storage-layout (states/etcd/repair/segment_storage_layout.go) backs up original metadata via MultiSave before applying the fix:
if err := c.client.MultiSave(ctx, backupKeys, originalValues); err != nil {
	return fmt.Errorf("failed to back up original metadata: %w", err)
}

Under default settings this fails immediately with failed to back up original metadata: not implemented, before any repair happens. The command cannot succeed through normal CLI usage.

  1. restore (states/etcd_restore.go) writes each backup batch via MultiSave inside a worker goroutine:
err = cli.MultiSave(ctx, keys, values)
if err != nil {
	fmt.Println(err.Error())
}

The error is only printed, never propagated. Every batch fails, the restore loop finishes, and the CLI can report the restore as done while zero keys were actually written back to etcd. It looks like a successful restore and is actually a complete no-op.

Expected Behavior

FileAuditKV.MultiSave should write an audit record (mirroring the pattern already used in Save) and then delegate to the wrapped client's MultiSave, returning its result, the same way every other method on FileAuditKV behaves.

Steps To Reproduce

  1. Run birdwatcher against a live/dry-run etcd instance so audit_*.log opens successfully (the default path).
  2. Run repair segment-storage-layout ... --run on any segment.
  3. Observe it fails at "failed to back up original metadata: not implemented" every time, regardless of input.

Alternatively:

  1. With audit logging enabled, run restore from a backup file.
  2. Observe the restore reports completion, but etcd contains none of the restored keys.

Environment

Any birdwatcher build with audit logging enabled (default when audit_*.log can be created), repair segment-storage-layout or restore commands.

Anything else?

errors is already imported in this file (github.com/cockroachdb/errors), so the stub was presumably a deliberate placeholder that was never finished before MultiSave-dependent commands (segment-storage-layout repair, batch restore) were added on top of it.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions