From 7e313adcec02ff7b48f069db2c75c27343bef9f5 Mon Sep 17 00:00:00 2001 From: Ignacio Van Droogenbroeck <64545348+xe-nvdk@users.noreply.github.com> Date: Thu, 1 Oct 2026 16:44:44 -0600 Subject: [PATCH 1/2] feat(backup): show what a backup lacks in list, show, status and the --wait results Arc 26.09.3 reports a backup's incompleteness on every backup endpoint: the listing carries skipped_files, skipped_metadata_files and unaddressable_files; the manifest names up to 32 skipped files in skipped_sample and counts overlong keys in skipped_overlong_keys; the status endpoint names a backup's skipped files as it already did for a restore. arcli ignored all of it and its list help said the listing could not tell whether a backup is incomplete. The client types mirror the fields. "backup list" gains an INCOMPLETE column: "-" when the server reports nothing missing, otherwise the counts the server keeps apart ("3 skipped, 1 metadata, 2 unaddressable"), never summed, because skipped data files are inside the file total, metadata skips are not, and unaddressable files were never inventoried; csv appends the three columns. "backup show" says the counts in words, including how many skips were for a key too long to store and how many outside-root Iceberg warehouse files were skipped, then names the files. "backup status" and the final status of "--wait" list the skipped files the server names, report files a backup could not list, and, for a restore, what the restored backup already lacked and the warehouse files left out. "create --wait" ends its success line with every gap, prints the names, and points at "backup show". "restore" warns before restoring a backup that lacks files for any reason the manifest records. Against an Arc older than 26.09.3 everything reads as none reported. --- docs/releases/v26.09.5.md | 12 ++ internal/client/backup.go | 74 +++++++--- internal/client/backup_incomplete_test.go | 58 ++++++++ internal/commands/backup.go | 151 ++++++++++++++++++-- internal/commands/backup_incomplete_test.go | 151 ++++++++++++++++++++ internal/commands/backup_test.go | 55 ++++++- 6 files changed, 466 insertions(+), 35 deletions(-) create mode 100644 docs/releases/v26.09.5.md create mode 100644 internal/client/backup_incomplete_test.go create mode 100644 internal/commands/backup_incomplete_test.go diff --git a/docs/releases/v26.09.5.md b/docs/releases/v26.09.5.md new file mode 100644 index 0000000..1cf4994 --- /dev/null +++ b/docs/releases/v26.09.5.md @@ -0,0 +1,12 @@ +arcli 26.09.5 shows what a backup lacks, now that Arc 26.09.3 reports it on every backup endpoint. + +### Changed + +- `arcli backup list` gains an `INCOMPLETE` column: `-` when the server reports nothing missing, otherwise the counts the server keeps apart, such as `3 skipped, 1 metadata, 2 unaddressable` (skipped: data files inventoried but not stored; metadata: Iceberg metadata or compaction recovery state that was skipped; unaddressable: files whose key no listing can return, so they were never inventoried). The csv output appends three columns after `total_size_bytes`: `skipped_files`, `skipped_metadata_files`, `unaddressable_files`; scripts that take the last column by position should be updated. `-o json` is unchanged. +- `arcli backup show` names the files: its `INCOMPLETE` line now says how many files of the total and how many metadata files were skipped, how many of those were skipped for a key too long to store, how many files could not be listed, and how many files of an outside-root Iceberg warehouse were skipped, followed by one `skipped:` line per file the manifest names (up to 32) and one `unaddressable:` line per file in that sample. +- `arcli backup status`, and `arcli backup create --wait` and `arcli backup restore --wait` when they print the final status, list the skipped files the server names, report files a backup could not list, and, for a restore, say what the restored backup already lacked when it was taken and how many Iceberg warehouse files were not restored because the node has no outside-root warehouse. `create --wait` ends its success line with every gap the server reports and points at `arcli backup show` for the full breakdown. +- `arcli backup restore` warns before restoring a backup that lacks files for any reason the manifest records, not only skipped data files, and says how many of the skipped files had keys too long to store. + +### Notes + +- Against an Arc older than 26.09.3 every one of these reads as "none reported": `-` in the listing, no extra lines elsewhere. That is not a guarantee of completeness; it is what the server knew. diff --git a/internal/client/backup.go b/internal/client/backup.go index 2bfb3fb..34c46a1 100644 --- a/internal/client/backup.go +++ b/internal/client/backup.go @@ -37,13 +37,23 @@ func (e *BackupBusyError) Error() string { } // BackupSummary is one entry of GET /api/v1/backup/. +// +// The three incompleteness counts arrive from Arc 26.09.3 on and are absent +// (zero) from older servers; each is its own population on the server: +// SkippedFiles are data files inside TotalFiles that were not stored, +// SkippedMetadataFiles are Iceberg metadata or compaction recovery state +// (not inside TotalFiles), UnaddressableFiles were never inventoried because +// no listing can return their key. type BackupSummary struct { - BackupID string `json:"backup_id"` - CreatedAt time.Time `json:"created_at"` - BackupType string `json:"backup_type"` - TotalFiles int64 `json:"total_files"` - TotalBytes int64 `json:"total_size_bytes"` - DatabaseCount int `json:"database_count"` + BackupID string `json:"backup_id"` + CreatedAt time.Time `json:"created_at"` + BackupType string `json:"backup_type"` + TotalFiles int64 `json:"total_files"` + TotalBytes int64 `json:"total_size_bytes"` + DatabaseCount int `json:"database_count"` + SkippedFiles int64 `json:"skipped_files,omitempty"` + SkippedMetadataFiles int64 `json:"skipped_metadata_files,omitempty"` + UnaddressableFiles int64 `json:"unaddressable_files,omitempty"` } // BackupMeasurement / BackupDatabase / BackupManifest mirror @@ -61,19 +71,34 @@ type BackupDatabase struct { SizeBytes int64 `json:"size_bytes"` } +// BackupIcebergWarehouse mirrors the manifest's iceberg_warehouse object: an +// Iceberg warehouse outside the storage root, copied separately. +type BackupIcebergWarehouse struct { + Path string `json:"path"` + FileCount int64 `json:"file_count"` + SizeBytes int64 `json:"size_bytes"` + SkippedFiles int64 `json:"skipped_files,omitempty"` +} + type BackupManifest struct { - Version string `json:"version"` - BackupID string `json:"backup_id"` - CreatedAt time.Time `json:"created_at"` - BackupType string `json:"backup_type"` - Databases []BackupDatabase `json:"databases"` - TotalFiles int64 `json:"total_files"` - TotalSizeBytes int64 `json:"total_size_bytes"` - SkippedFiles int64 `json:"skipped_files,omitempty"` - HasMetadata bool `json:"has_metadata"` - HasIcebergCatalog bool `json:"has_iceberg_catalog,omitempty"` - HasConfig bool `json:"has_config"` - Raw json.RawMessage `json:"-"` + Version string `json:"version"` + BackupID string `json:"backup_id"` + CreatedAt time.Time `json:"created_at"` + BackupType string `json:"backup_type"` + Databases []BackupDatabase `json:"databases"` + TotalFiles int64 `json:"total_files"` + TotalSizeBytes int64 `json:"total_size_bytes"` + SkippedFiles int64 `json:"skipped_files,omitempty"` + SkippedMetadataFiles int64 `json:"skipped_metadata_files,omitempty"` + SkippedSample []string `json:"skipped_sample,omitempty"` + SkippedOverlongKeys int64 `json:"skipped_overlong_keys,omitempty"` + UnaddressableFiles int64 `json:"unaddressable_files,omitempty"` + UnaddressableSample []string `json:"unaddressable_sample,omitempty"` + IcebergWarehouse *BackupIcebergWarehouse `json:"iceberg_warehouse,omitempty"` + HasMetadata bool `json:"has_metadata"` + HasIcebergCatalog bool `json:"has_iceberg_catalog,omitempty"` + HasConfig bool `json:"has_config"` + Raw json.RawMessage `json:"-"` } // BackupProgress is GET /api/v1/backup/status when an operation has run @@ -91,6 +116,19 @@ type BackupProgress struct { StartedAt time.Time `json:"started_at"` CompletedAt *time.Time `json:"completed_at,omitempty"` Error string `json:"error,omitempty"` + // SkippedSample names up to 32 of the skipped files: for a backup (Arc + // 26.09.3+) its skipped data and Iceberg metadata files, for a restore the + // backup objects it could not read. UnaddressableFiles is published for + // both operations; its sample only for a restore. + SkippedSample []string `json:"skipped_sample,omitempty"` + UnaddressableFiles int64 `json:"unaddressable_files,omitempty"` + UnaddressableSample []string `json:"unaddressable_sample,omitempty"` + // Restore only: what the restored backup already lacked when it was taken, + // and warehouse files left out because this node has no outside-root + // Iceberg warehouse. + BackupSkippedFiles int64 `json:"backup_skipped_files,omitempty"` + BackupUnaddressableFiles int64 `json:"backup_unaddressable_files,omitempty"` + IcebergWarehouseFilesSkipped int64 `json:"iceberg_warehouse_files_skipped,omitempty"` } // BackupStatus is the decoded status endpoint: Idle when the server has diff --git a/internal/client/backup_incomplete_test.go b/internal/client/backup_incomplete_test.go new file mode 100644 index 0000000..67b1f1f --- /dev/null +++ b/internal/client/backup_incomplete_test.go @@ -0,0 +1,58 @@ +package client + +import ( + "context" + "net/http" + "strings" + "testing" +) + +// Arc 26.09.3 reports a backup's incompleteness on every endpoint (arc#977); +// the client must decode each count and sample under the server's keys and +// read zero where an older server omits them. +func TestBackup_IncompleteShapes(t *testing.T) { + long := "mydb/cpu/2026/09/17/00/" + strings.Repeat("x", 960) + ".parquet" + cli, _ := newAuthTestClient(t, func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/api/v1/backup/": + _, _ = w.Write([]byte(`{"backups":[{"backup_id":"backup-20260917-000000-aaaaaaaa","created_at":"2026-09-17T00:00:00Z","backup_type":"full","total_files":21,"total_size_bytes":2086,"database_count":1,"skipped_files":3,"skipped_metadata_files":1,"unaddressable_files":2},{"backup_id":"backup-20260916-000000-bbbbbbbb","created_at":"2026-09-16T00:00:00Z","backup_type":"full","total_files":20,"total_size_bytes":2000,"database_count":1}],"count":2}`)) + case "/api/v1/backup/backup-20260917-000000-aaaaaaaa": + _, _ = w.Write([]byte(`{"version":"26.09.3","backup_id":"backup-20260917-000000-aaaaaaaa","created_at":"2026-09-17T00:00:00Z","backup_type":"full","databases":[{"name":"mydb","measurements":[{"name":"cpu","file_count":21,"size_bytes":2086}],"file_count":21,"size_bytes":2086}],"total_files":21,"total_size_bytes":2086,"skipped_files":3,"skipped_metadata_files":1,"skipped_sample":["mydb/cpu/2026/09/17/00/a.parquet","mydb/cpu/2026/09/17/00/b.parquet","` + long + `","mydb/cpu/metadata/00003-5f2c.metadata.json"],"skipped_overlong_keys":1,"unaddressable_files":2,"unaddressable_sample":["mydb/cpu/2026/09/17/00/.hidden.parquet","mydb/cpu/2026/09/17/00/bad key.parquet"],"iceberg_warehouse":{"path":"/srv/wh","file_count":4,"size_bytes":100,"skipped_files":1},"has_metadata":true,"has_config":false}`)) + case "/api/v1/backup/status": + _, _ = w.Write([]byte(`{"operation":"restore","backup_id":"backup-20260917-000000-aaaaaaaa","status":"failed","total_files":17,"processed_files":16,"skipped_files":1,"unaddressable_files":1,"unaddressable_sample":["backup-20260917-000000-aaaaaaaa/data/mydb/cpu/2026/09/17/00/.hidden.parquet"],"skipped_sample":["backup-20260917-000000-aaaaaaaa/data/mydb/cpu/2026/09/17/00/c.parquet"],"missing_files":1,"backup_skipped_files":3,"backup_unaddressable_files":2,"iceberg_warehouse_files_skipped":4,"total_bytes":2086,"processed_bytes":1600,"started_at":"2026-09-17T01:00:00Z","completed_at":"2026-09-17T01:00:02Z","error":"restore incomplete: 1 objects could not be read from backup storage (skipped_sample)"}`)) + } + }) + ctx := context.Background() + list, _, err := cli.ListBackups(ctx) + if err != nil || len(list) != 2 { + t.Fatalf("list=%+v err=%v", list, err) + } + if list[0].SkippedFiles != 3 || list[0].SkippedMetadataFiles != 1 || list[0].UnaddressableFiles != 2 { + t.Errorf("incomplete entry = %+v", list[0]) + } + if list[1].SkippedFiles != 0 || list[1].SkippedMetadataFiles != 0 || list[1].UnaddressableFiles != 0 { + t.Errorf("entry without the keys must read zero: %+v", list[1]) + } + m, err := cli.GetBackup(ctx, "backup-20260917-000000-aaaaaaaa") + if err != nil { + t.Fatal(err) + } + if m.SkippedFiles != 3 || m.SkippedMetadataFiles != 1 || m.SkippedOverlongKeys != 1 || m.UnaddressableFiles != 2 { + t.Errorf("manifest counts = %+v", m) + } + if len(m.SkippedSample) != 4 || m.SkippedSample[2] != long || len(m.UnaddressableSample) != 2 { + t.Errorf("manifest samples = %v / %v", m.SkippedSample, m.UnaddressableSample) + } + if m.IcebergWarehouse == nil || m.IcebergWarehouse.SkippedFiles != 1 || m.IcebergWarehouse.Path != "/srv/wh" { + t.Errorf("iceberg warehouse = %+v", m.IcebergWarehouse) + } + st, err := cli.BackupStatus(ctx) + if err != nil || st.Progress == nil { + t.Fatalf("status=%+v err=%v", st, err) + } + p := st.Progress + if len(p.SkippedSample) != 1 || len(p.UnaddressableSample) != 1 || p.UnaddressableFiles != 1 || + p.BackupSkippedFiles != 3 || p.BackupUnaddressableFiles != 2 || p.IcebergWarehouseFilesSkipped != 4 { + t.Errorf("restore progress = %+v", p) + } +} diff --git a/internal/commands/backup.go b/internal/commands/backup.go index 81802b3..627ba58 100644 --- a/internal/commands/backup.go +++ b/internal/commands/backup.go @@ -152,6 +152,56 @@ func describeProgress(w io.Writer, p *client.BackupProgress) { if p.Error != "" { fmt.Fprintf(w, "error: %s\n", clean(p.Error)) } + // What the counts above do not say: which files, what the restored backup + // already lacked, and warehouse files a restore had nowhere to put. The + // server already names the counts of a failed restore in its error, so + // only the names and the backup's own gaps are added here. + if p.BackupSkippedFiles > 0 || p.BackupUnaddressableFiles > 0 { + fmt.Fprintf(w, "backup had: %s when it was taken\n", describeIncomplete(p.BackupSkippedFiles, 0, p.BackupUnaddressableFiles)) + } + if p.IcebergWarehouseFilesSkipped > 0 { + fmt.Fprintf(w, "iceberg: %d warehouse files not restored (this node has no outside-root warehouse)\n", p.IcebergWarehouseFilesSkipped) + } + writeSample(w, "skipped", p.SkippedSample) + if p.UnaddressableFiles > 0 && len(p.UnaddressableSample) == 0 { + fmt.Fprintf(w, " unaddressable: %d files (names: arcli backup show %s)\n", p.UnaddressableFiles, clean(p.BackupID)) + } + writeSample(w, "unaddressable", p.UnaddressableSample) +} + +// describeIncomplete phrases a backup's incompleteness counts, each in its own +// domain because the server keeps them apart: skipped data files are part of +// the backup's file total, metadata skips (Iceberg metadata, compaction +// recovery state) are not, and unaddressable files were never inventoried at +// all, so the numbers are never summed. Returns "" when all are zero, which is +// also what an Arc older than 26.09.3 reports: none, not necessarily complete. +func describeIncomplete(skipped, metadata, unaddressable int64) string { + var parts []string + if skipped > 0 { + parts = append(parts, fmt.Sprintf("%d skipped", skipped)) + } + if metadata > 0 { + parts = append(parts, fmt.Sprintf("%d metadata", metadata)) + } + if unaddressable > 0 { + parts = append(parts, fmt.Sprintf("%d unaddressable", unaddressable)) + } + return strings.Join(parts, ", ") +} + +// writeSample prints one labelled line per path (up to the server's 32). +func writeSample(w io.Writer, label string, paths []string) { + for _, p := range paths { + fmt.Fprintf(w, " %-14s %s\n", label+":", clean(p)) + } +} + +// plural returns n with the singular or plural noun. +func plural(n int64, singular, pluralForm string) string { + if n == 1 { + return fmt.Sprintf("%d %s", n, singular) + } + return fmt.Sprintf("%d %s", n, pluralForm) } // ---- create ---------------------------------------------------------------- @@ -251,11 +301,23 @@ deletion can run at a time.`, describeProgress(cmd.OutOrStdout(), final) return fmt.Errorf("backup %s %s: %s", clean(id), clean(final.Status), clean(final.Error)) } - fmt.Fprintf(cmd.OutOrStdout(), "Backup %s completed: %d files, %s", clean(id), final.ProcessedFiles, humanBytes(final.ProcessedBytes)) + out := cmd.OutOrStdout() + fmt.Fprintf(out, "Backup %s completed: %d files, %s", clean(id), final.ProcessedFiles, humanBytes(final.ProcessedBytes)) + var gaps []string if final.SkippedFiles > 0 { - fmt.Fprintf(cmd.OutOrStdout(), " (%d files skipped — the backup is incomplete)", final.SkippedFiles) + gaps = append(gaps, fmt.Sprintf("%d files skipped", final.SkippedFiles)) + } + if final.UnaddressableFiles > 0 { + gaps = append(gaps, fmt.Sprintf("%d files could not be listed", final.UnaddressableFiles)) + } + if len(gaps) > 0 { + fmt.Fprintf(out, " (%s — the backup is incomplete)", strings.Join(gaps, ", ")) + } + fmt.Fprintln(out) + if len(gaps) > 0 { + writeSample(out, "skipped", final.SkippedSample) + fmt.Fprintf(out, "see \"arcli backup show %s\" for the full breakdown\n", clean(id)) } - fmt.Fprintln(cmd.OutOrStdout()) return nil }, } @@ -279,10 +341,18 @@ func newBackupListCmd() *cobra.Command { c := &cobra.Command{ Use: "list", Short: "List backups, newest first", - Long: `List backups (GET /api/v1/backup/), newest first. The listing cannot -tell whether a backup is incomplete; "backup show" reports skipped files. + Long: `List backups (GET /api/v1/backup/), newest first. -Output formats: table (default) | json | csv`, +INCOMPLETE says what the backup lacks, as the server counts it (Arc +26.09.3+): "skipped" data files that were inventoried but not stored, +"metadata" files (Iceberg metadata, compaction recovery state) that were +skipped, and "unaddressable" files whose key no listing can return, so +they were never inventoried. "-" means none reported: a complete backup, +or an Arc older than 26.09.3. "backup show" names the files. + +Output formats: table (default) | json | csv. The csv columns +skipped_files, skipped_metadata_files and unaddressable_files follow +total_size_bytes.`, Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, args []string) error { if !validListFormat(outputFormat) { @@ -306,12 +376,12 @@ Output formats: table (default) | json | csv`, if outputFormat == output.FormatCSV { cw := csv.NewWriter(w) if !noHeader { - if err := cw.Write([]string{"backup_id", "created_at", "backup_type", "database_count", "total_files", "total_size_bytes"}); err != nil { + if err := cw.Write([]string{"backup_id", "created_at", "backup_type", "database_count", "total_files", "total_size_bytes", "skipped_files", "skipped_metadata_files", "unaddressable_files"}); err != nil { return err } } for _, b := range list { - if err := cw.Write([]string{b.BackupID, fmtTimeVal(b.CreatedAt, ""), b.BackupType, strconv.Itoa(b.DatabaseCount), strconv.FormatInt(b.TotalFiles, 10), strconv.FormatInt(b.TotalBytes, 10)}); err != nil { + if err := cw.Write([]string{b.BackupID, fmtTimeVal(b.CreatedAt, ""), b.BackupType, strconv.Itoa(b.DatabaseCount), strconv.FormatInt(b.TotalFiles, 10), strconv.FormatInt(b.TotalBytes, 10), strconv.FormatInt(b.SkippedFiles, 10), strconv.FormatInt(b.SkippedMetadataFiles, 10), strconv.FormatInt(b.UnaddressableFiles, 10)}); err != nil { return err } } @@ -324,9 +394,13 @@ Output formats: table (default) | json | csv`, } rows := make([][]string, 0, len(list)) for _, b := range list { - rows = append(rows, []string{clean(b.BackupID), fmtTimeVal(b.CreatedAt, "-"), clean(b.BackupType), strconv.Itoa(b.DatabaseCount), strconv.FormatInt(b.TotalFiles, 10), humanBytes(b.TotalBytes)}) + incomplete := describeIncomplete(b.SkippedFiles, b.SkippedMetadataFiles, b.UnaddressableFiles) + if incomplete == "" { + incomplete = "-" + } + rows = append(rows, []string{clean(b.BackupID), fmtTimeVal(b.CreatedAt, "-"), clean(b.BackupType), strconv.Itoa(b.DatabaseCount), strconv.FormatInt(b.TotalFiles, 10), humanBytes(b.TotalBytes), incomplete}) } - headers := []string{"ID", "CREATED", "TYPE", "DATABASES", "FILES", "SIZE"} + headers := []string{"ID", "CREATED", "TYPE", "DATABASES", "FILES", "SIZE", "INCOMPLETE"} if noHeader { headers = nil } @@ -382,8 +456,10 @@ func writeManifest(w io.Writer, m *client.BackupManifest) error { fmt.Fprintf(w, "created: %s\n", fmtTimeVal(m.CreatedAt, "-")) fmt.Fprintf(w, "type: %s (server %s)\n", clean(m.BackupType), clean(m.Version)) fmt.Fprintf(w, "contents: %d files, %s, metadata %t, config %t\n", m.TotalFiles, humanBytes(m.TotalSizeBytes), m.HasMetadata, m.HasConfig) - if m.SkippedFiles > 0 { - fmt.Fprintf(w, "INCOMPLETE: %d files were skipped while backing up\n", m.SkippedFiles) + if line := describeManifestIncomplete(m); line != "" { + fmt.Fprintf(w, "INCOMPLETE: %s\n", line) + writeSample(w, "skipped", m.SkippedSample) + writeSample(w, "unaddressable", m.UnaddressableSample) } if len(m.Databases) == 0 { _, err := fmt.Fprintln(w, "(no databases)") @@ -402,6 +478,39 @@ func writeManifest(w io.Writer, m *client.BackupManifest) error { return output.Table(w, []string{"DATABASE", "MEASUREMENT", "FILES", "SIZE"}, rows) } +// describeManifestIncomplete phrases a manifest's incompleteness, or returns +// "" for a backup that reports none. The skipped clause counts data files +// against the backup's file total and metadata files apart (they are not in +// that total); the overlong clause spans both, as the server counts it. +func describeManifestIncomplete(m *client.BackupManifest) string { + var clauses []string + var skipped []string + if m.SkippedFiles > 0 { + skipped = append(skipped, fmt.Sprintf("%d of %d files", m.SkippedFiles, m.TotalFiles)) + } + if m.SkippedMetadataFiles > 0 { + skipped = append(skipped, plural(m.SkippedMetadataFiles, "metadata file", "metadata files")) + } + if len(skipped) > 0 { + verb := "were" + if m.SkippedFiles+m.SkippedMetadataFiles == 1 { + verb = "was" + } + clause := strings.Join(skipped, " and ") + " " + verb + " skipped while backing up" + if m.SkippedOverlongKeys > 0 { + clause += fmt.Sprintf(" (%d for a key too long to store)", m.SkippedOverlongKeys) + } + clauses = append(clauses, clause) + } + if m.UnaddressableFiles > 0 { + clauses = append(clauses, plural(m.UnaddressableFiles, "file", "files")+" could not be listed (unaddressable)") + } + if m.IcebergWarehouse != nil && m.IcebergWarehouse.SkippedFiles > 0 { + clauses = append(clauses, plural(m.IcebergWarehouse.SkippedFiles, "Iceberg warehouse file was", "Iceberg warehouse files were")+" skipped") + } + return strings.Join(clauses, "; ") +} + func newBackupStatusCmd() *cobra.Command { var ( f connFlags @@ -564,8 +673,22 @@ to 2h).`, return fmt.Errorf("backup %s does not contain the server config; drop --with-config", clean(id)) } stderr := cmd.ErrOrStderr() - if m.SkippedFiles > 0 { - fmt.Fprintf(stderr, "warning: backup %s is incomplete (%d files were skipped when it was taken)\n", clean(id), m.SkippedFiles) + if m.SkippedFiles > 0 || m.SkippedMetadataFiles > 0 || m.UnaddressableFiles > 0 { + var gaps []string + if m.SkippedFiles > 0 { + g := fmt.Sprintf("%d files were skipped when it was taken", m.SkippedFiles) + if m.SkippedOverlongKeys > 0 { + g += fmt.Sprintf(", %d of them for keys too long to store", m.SkippedOverlongKeys) + } + gaps = append(gaps, g) + } + if m.SkippedMetadataFiles > 0 { + gaps = append(gaps, fmt.Sprintf("%d metadata files were skipped", m.SkippedMetadataFiles)) + } + if m.UnaddressableFiles > 0 { + gaps = append(gaps, fmt.Sprintf("%d files could not be listed", m.UnaddressableFiles)) + } + fmt.Fprintf(stderr, "warning: backup %s is incomplete (%s)\n", clean(id), strings.Join(gaps, "; ")) } dbs := make([]string, 0, len(m.Databases)) for _, db := range m.Databases { diff --git a/internal/commands/backup_incomplete_test.go b/internal/commands/backup_incomplete_test.go new file mode 100644 index 0000000..0c84c95 --- /dev/null +++ b/internal/commands/backup_incomplete_test.go @@ -0,0 +1,151 @@ +package commands + +import ( + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" +) + +// Fixtures in the shapes Arc 26.09.3 (arc#977) produces: a backup with 21 +// data files of which 3 were skipped (1 for an overlong key), 1 metadata file +// skipped, 2 unaddressable files; its completed backup status; and a failed +// restore of it. This file uses only symbols that exist before the arcli +// change, so it compiles against the old renderings and fails there. +const incompleteID = "backup-20260917-000000-aaaaaaaa" + +func newIncompleteBackupServer(t *testing.T) *httptest.Server { + t.Helper() + long := "mydb/cpu/2026/09/17/00/" + strings.Repeat("x", 960) + ".parquet" + var restored atomic.Bool + mux := http.NewServeMux() + mux.HandleFunc("/health", func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"status":"ok","time":"t","uptime":"1s"}`)) + }) + mux.HandleFunc("/api/v1/backup/", func(w http.ResponseWriter, r *http.Request) { + rest := strings.TrimPrefix(r.URL.Path, "/api/v1/backup/") + switch { + case rest == "" && r.Method == http.MethodGet: + _, _ = w.Write([]byte(`{"backups":[{"backup_id":"` + incompleteID + `","created_at":"2026-09-17T00:00:00Z","backup_type":"full","total_files":21,"total_size_bytes":2086,"database_count":1,"skipped_files":3,"skipped_metadata_files":1,"unaddressable_files":2},{"backup_id":"backup-20260916-000000-bbbbbbbb","created_at":"2026-09-16T00:00:00Z","backup_type":"full","total_files":20,"total_size_bytes":2000,"database_count":1}],"count":2}`)) + case rest == "status": + if restored.Load() { + _, _ = w.Write([]byte(`{"operation":"restore","backup_id":"` + incompleteID + `","status":"failed","total_files":17,"processed_files":16,"skipped_files":1,"unaddressable_files":1,"unaddressable_sample":["` + incompleteID + `/data/mydb/cpu/2026/09/17/00/.hidden.parquet"],"skipped_sample":["` + incompleteID + `/data/mydb/cpu/2026/09/17/00/c.parquet"],"missing_files":1,"backup_skipped_files":3,"backup_unaddressable_files":2,"iceberg_warehouse_files_skipped":4,"total_bytes":2086,"processed_bytes":1600,"started_at":"2026-09-17T01:00:00Z","completed_at":"2026-09-17T01:00:02Z","error":"restore incomplete: 1 objects could not be read from backup storage (skipped_sample); the files that could be restored are in place"}`)) + return + } + _, _ = w.Write([]byte(`{"operation":"backup","backup_id":"` + incompleteID + `","status":"completed","total_files":23,"processed_files":19,"skipped_files":4,"unaddressable_files":2,"skipped_sample":["mydb/cpu/2026/09/17/00/a.parquet","mydb/cpu/2026/09/17/00/b.parquet","` + long + `","mydb/cpu/metadata/00003-5f2c.metadata.json"],"total_bytes":2086,"processed_bytes":1790,"started_at":"2026-09-17T00:00:00Z","completed_at":"2026-09-17T00:00:03Z"}`)) + case rest == "restore" && r.Method == http.MethodPost: + restored.Store(true) + w.WriteHeader(202) + _, _ = w.Write([]byte(`{"backup_id":"` + incompleteID + `","message":"Restore started","status":"running"}`)) + case rest == incompleteID && r.Method == http.MethodGet: + _, _ = w.Write([]byte(`{"version":"26.09.3","backup_id":"` + incompleteID + `","created_at":"2026-09-17T00:00:00Z","backup_type":"full","databases":[{"name":"mydb","measurements":[{"name":"cpu","file_count":21,"size_bytes":2086}],"file_count":21,"size_bytes":2086}],"total_files":21,"total_size_bytes":2086,"skipped_files":3,"skipped_metadata_files":1,"skipped_sample":["mydb/cpu/2026/09/17/00/a.parquet","mydb/cpu/2026/09/17/00/b.parquet","` + long + `","mydb/cpu/metadata/00003-5f2c.metadata.json"],"skipped_overlong_keys":1,"unaddressable_files":2,"unaddressable_sample":["mydb/cpu/2026/09/17/00/.hidden.parquet","mydb/cpu/2026/09/17/00/bad key.parquet"],"iceberg_warehouse":{"path":"/srv/wh","file_count":4,"size_bytes":100,"skipped_files":1},"has_metadata":true,"has_config":false}`)) + default: + w.WriteHeader(404) + _, _ = w.Write([]byte(`{"error":"Backup not found"}`)) + } + }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + return srv +} + +func TestBackup_IncompleteIsVisible(t *testing.T) { + fastPolls(t) + srv := newIncompleteBackupServer(t) + writeTestConfig(t, srv.URL, "tok") + + // list: the table names the three populations apart and never sums them; + // the complete entry reads "-". + out, _, err := execCmd(t, newBackupListCmd()) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{"INCOMPLETE", "3 skipped, 1 metadata, 2 unaddressable", "│ -"} { + if !strings.Contains(out, want) { + t.Errorf("list table lacks %q:\n%s", want, out) + } + } + out, _, err = execCmd(t, newBackupListCmd(), "-o", "csv") + if err != nil { + t.Fatal(err) + } + lines := strings.Split(strings.TrimSpace(out), "\n") + if len(lines) != 3 || lines[0] != "backup_id,created_at,backup_type,database_count,total_files,total_size_bytes,skipped_files,skipped_metadata_files,unaddressable_files" { + t.Errorf("csv header = %q", lines[0]) + } + if len(lines) == 3 && (!strings.HasSuffix(lines[1], ",21,2086,3,1,2") || !strings.HasSuffix(lines[2], ",20,2000,0,0,0")) { + t.Errorf("csv rows = %q", lines[1:]) + } + out, _, err = execCmd(t, newBackupListCmd(), "-o", "csv", "--no-header") + if err != nil || strings.Contains(out, "backup_id,") || !strings.Contains(out, ",3,1,2") { + t.Errorf("csv --no-header: err=%v out=%q", err, out) + } + + // show: the counts in words, each in its own domain, then the names. + out, _, err = execCmd(t, newBackupShowCmd(), incompleteID) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{ + "INCOMPLETE: 3 of 21 files and 1 metadata file were skipped while backing up (1 for a key too long to store); 2 files could not be listed (unaddressable); 1 Iceberg warehouse file was skipped\n", + " skipped: mydb/cpu/2026/09/17/00/a.parquet\n", + " skipped: mydb/cpu/metadata/00003-5f2c.metadata.json\n", + " unaddressable: mydb/cpu/2026/09/17/00/bad key.parquet\n", + } { + if !strings.Contains(out, want) { + t.Errorf("show lacks %q:\n%s", want, out) + } + } + if strings.Count(out, " skipped:") != 4 || strings.Count(out, " unaddressable:") != 2 { + t.Errorf("show sample line counts wrong:\n%s", out) + } + + // status of the completed backup: the names, and the unaddressable + // count whose names only the manifest has. + out, _, err = execCmd(t, newBackupStatusCmd()) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{ + "files: 19 / 23 (4 skipped)\n", + " skipped: mydb/cpu/2026/09/17/00/b.parquet\n", + " unaddressable: 2 files (names: arcli backup show " + incompleteID + ")\n", + } { + if !strings.Contains(out, want) { + t.Errorf("backup status lacks %q:\n%s", want, out) + } + } + + // restore: the pre-flight warning covers every gap the manifest records. + out, stderr, err := execCmd(t, newBackupRestoreCmd(), incompleteID, "--yes") + if err != nil { + t.Fatalf("restore: %v (stderr %s)", err, stderr) + } + if !strings.Contains(stderr, "warning: backup "+incompleteID+" is incomplete (3 files were skipped when it was taken, 1 of them for keys too long to store; 1 metadata files were skipped; 2 files could not be listed)\n") { + t.Errorf("restore warning missing or wrong:\n%s", stderr) + } + if !strings.Contains(out, "Restore of "+incompleteID+" started") { + t.Errorf("restore out = %q", out) + } + + // status of the failed restore: the server's error already carries the + // counts; arcli adds the names, what the backup lacked, and the + // warehouse files left out. + out, _, err = execCmd(t, newBackupStatusCmd()) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{ + "status: failed\n", + "error: restore incomplete: 1 objects could not be read", + "backup had: 3 skipped, 2 unaddressable when it was taken\n", + "iceberg: 4 warehouse files not restored (this node has no outside-root warehouse)\n", + " skipped: " + incompleteID + "/data/mydb/cpu/2026/09/17/00/c.parquet\n", + " unaddressable: " + incompleteID + "/data/mydb/cpu/2026/09/17/00/.hidden.parquet\n", + } { + if !strings.Contains(out, want) { + t.Errorf("restore status lacks %q:\n%s", want, out) + } + } +} diff --git a/internal/commands/backup_test.go b/internal/commands/backup_test.go index f910760..1b7d128 100644 --- a/internal/commands/backup_test.go +++ b/internal/commands/backup_test.go @@ -214,6 +214,9 @@ type fakeBackupServer struct { publishDelay time.Duration counter int32 srv *httptest.Server + // incomplete makes every backup report one skipped and one unaddressable + // file on the status, the manifest and the listing (Arc 26.09.3 shapes). + incomplete bool } func newFakeBackupServer(t *testing.T) *fakeBackupServer { @@ -224,7 +227,11 @@ func newFakeBackupServer(t *testing.T) *fakeBackupServer { _, _ = w.Write([]byte(`{"status":"ok","time":"t","uptime":"1s"}`)) }) manifest := func(id string) string { - return `{"version":"dev","backup_id":"` + id + `","created_at":"2026-09-07T20:00:00Z","backup_type":"full","databases":[{"name":"smoke","measurements":[{"name":"cpu","file_count":1,"size_bytes":1064}],"file_count":1,"size_bytes":1064}],"total_files":1,"total_size_bytes":1064,"has_metadata":true,"has_config":false}` + gaps := "" + if f.incomplete { + gaps = `"skipped_files":1,"skipped_sample":["smoke/cpu/2026/09/07/20/gone.parquet"],"unaddressable_files":1,"unaddressable_sample":["smoke/.hidden.parquet"],` + } + return `{"version":"dev","backup_id":"` + id + `","created_at":"2026-09-07T20:00:00Z","backup_type":"full","databases":[{"name":"smoke","measurements":[{"name":"cpu","file_count":1,"size_bytes":1064}],"file_count":1,"size_bytes":1064}],"total_files":1,"total_size_bytes":1064,` + gaps + `"has_metadata":true,"has_config":false}` } publish := func(op, id string) { time.Sleep(f.publishDelay) @@ -234,7 +241,11 @@ func newFakeBackupServer(t *testing.T) *fakeBackupServer { f.mu.Unlock() time.Sleep(f.publishDelay) f.mu.Lock() - f.status = `{"operation":"` + op + `","backup_id":"` + id + `","status":"completed","total_files":1,"processed_files":1,"skipped_files":0,"total_bytes":1064,"processed_bytes":1064,"started_at":"` + started + `","completed_at":"` + time.Now().UTC().Format(time.RFC3339Nano) + `"}` + done := `"skipped_files":0,` + if f.incomplete && op == "backup" { + done = `"skipped_files":1,"unaddressable_files":1,"skipped_sample":["smoke/cpu/2026/09/07/20/gone.parquet"],` + } + f.status = `{"operation":"` + op + `","backup_id":"` + id + `","status":"completed","total_files":1,"processed_files":1,` + done + `"total_bytes":1064,"processed_bytes":1064,"started_at":"` + started + `","completed_at":"` + time.Now().UTC().Format(time.RFC3339Nano) + `"}` if op == "backup" { f.backups[id] = manifest(id) } @@ -259,7 +270,11 @@ func newFakeBackupServer(t *testing.T) *fakeBackupServer { case rest == "" && r.Method == http.MethodGet: list := []string{} for id := range f.backups { - list = append(list, `{"backup_id":"`+id+`","created_at":"2026-09-07T20:00:00Z","backup_type":"full","total_files":1,"total_size_bytes":1064,"database_count":1}`) + gaps := "" + if f.incomplete { + gaps = `,"skipped_files":1,"unaddressable_files":1` + } + list = append(list, `{"backup_id":"`+id+`","created_at":"2026-09-07T20:00:00Z","backup_type":"full","total_files":1,"total_size_bytes":1064,"database_count":1`+gaps+`}`) } _, _ = w.Write([]byte(`{"backups":[` + strings.Join(list, ",") + `],"count":` + itoa(int64(len(list))) + `}`)) case rest == "status": @@ -453,3 +468,37 @@ func TestBackup_DisabledServer(t *testing.T) { t.Errorf("err = %v", err) } } + +// A backup the server reports as incomplete: create --wait says so on its +// success line and names the files; list and show carry the gaps too. +func TestBackup_CreateWaitReportsIncomplete(t *testing.T) { + fastPolls(t) + f := newFakeBackupServer(t) + f.incomplete = true + writeTestConfig(t, f.srv.URL, "tok") + out, _, err := execCmd(t, newBackupCreateCmd(), "--wait", "--wait-timeout", "10s") + if err != nil { + t.Fatal(err) + } + for _, want := range []string{ + "completed: 1 files, 1.0 KiB (1 files skipped, 1 files could not be listed — the backup is incomplete)\n", + " skipped: smoke/cpu/2026/09/07/20/gone.parquet\n", + "see \"arcli backup show backup-20260907-200001-00000001\" for the full breakdown\n", + } { + if !strings.Contains(out, want) { + t.Errorf("create --wait output lacks %q:\n%s", want, out) + } + } + out, _, err = execCmd(t, newBackupListCmd()) + if err != nil || !strings.Contains(out, "1 skipped, 1 unaddressable") { + t.Errorf("list: err=%v out=%s", err, out) + } + out, _, err = execCmd(t, newBackupShowCmd(), "backup-20260907-200001-00000001") + if err != nil || !strings.Contains(out, "INCOMPLETE: 1 of 1 files was skipped while backing up; 1 file could not be listed (unaddressable)\n skipped: smoke/cpu/2026/09/07/20/gone.parquet\n unaddressable: smoke/.hidden.parquet\n") { + t.Errorf("show: err=%v out=%s", err, out) + } + out, _, err = execCmd(t, newBackupStatusCmd()) + if err != nil || !strings.Contains(out, " skipped: smoke/cpu/2026/09/07/20/gone.parquet\n unaddressable: 1 files (names: arcli backup show backup-20260907-200001-00000001)\n") { + t.Errorf("status: err=%v out=%s", err, out) + } +} From 7e83d3cfabf543a1e6532a60ac00d6b809d1308d Mon Sep 17 00:00:00 2001 From: Ignacio Van Droogenbroeck <64545348+xe-nvdk@users.noreply.github.com> Date: Thu, 1 Oct 2026 16:58:01 -0600 Subject: [PATCH 2/2] feat(backup): point unaddressable names at the log until a manifest exists; restore warning in show's shape Review fix-up. A backup publishes its unaddressable count before copying but names the files only in its manifest, which exists once it has completed; while it runs, or after it failed, "backup status" now sends the operator to the server log instead of to a "backup show" that would answer not found. The restore pre-flight warning takes the same shape as "backup show": the overlong-key clause hangs on the joined skipped sentence, because the server counts overlong keys across data and metadata files alike, so a metadata-only overlong skip is no longer dropped or charged to the data files; plurals are right; and a backup whose only gap is outside-root Iceberg warehouse files warns too, as the release note already claimed. A completed "restore --wait" now says what the restored backup lacked and how many warehouse files were not restored, the one gap no earlier surface can show when the node has Iceberg off. The "backup show" hint after "create --wait" moves to stderr with backticks like the file's other hints; "N of M files" falls back to "N files" for a manifest from before the data/metadata split, where N can exceed M. Tests cover json passthrough, the metadata-only singular case, and the failed and running backup status hint. --- docs/releases/v26.09.5.md | 4 +- internal/commands/backup.go | 109 +++++++++++++++----- internal/commands/backup_incomplete_test.go | 66 ++++++++++-- internal/commands/backup_test.go | 6 +- 4 files changed, 146 insertions(+), 39 deletions(-) diff --git a/docs/releases/v26.09.5.md b/docs/releases/v26.09.5.md index 1cf4994..a5fe8de 100644 --- a/docs/releases/v26.09.5.md +++ b/docs/releases/v26.09.5.md @@ -4,8 +4,8 @@ arcli 26.09.5 shows what a backup lacks, now that Arc 26.09.3 reports it on ever - `arcli backup list` gains an `INCOMPLETE` column: `-` when the server reports nothing missing, otherwise the counts the server keeps apart, such as `3 skipped, 1 metadata, 2 unaddressable` (skipped: data files inventoried but not stored; metadata: Iceberg metadata or compaction recovery state that was skipped; unaddressable: files whose key no listing can return, so they were never inventoried). The csv output appends three columns after `total_size_bytes`: `skipped_files`, `skipped_metadata_files`, `unaddressable_files`; scripts that take the last column by position should be updated. `-o json` is unchanged. - `arcli backup show` names the files: its `INCOMPLETE` line now says how many files of the total and how many metadata files were skipped, how many of those were skipped for a key too long to store, how many files could not be listed, and how many files of an outside-root Iceberg warehouse were skipped, followed by one `skipped:` line per file the manifest names (up to 32) and one `unaddressable:` line per file in that sample. -- `arcli backup status`, and `arcli backup create --wait` and `arcli backup restore --wait` when they print the final status, list the skipped files the server names, report files a backup could not list, and, for a restore, say what the restored backup already lacked when it was taken and how many Iceberg warehouse files were not restored because the node has no outside-root warehouse. `create --wait` ends its success line with every gap the server reports and points at `arcli backup show` for the full breakdown. -- `arcli backup restore` warns before restoring a backup that lacks files for any reason the manifest records, not only skipped data files, and says how many of the skipped files had keys too long to store. +- `arcli backup status`, and the final status that `arcli backup create --wait` and `arcli backup restore --wait` print, list the skipped files the server names and report files a backup could not list (their names are in the manifest once the backup has completed, otherwise in the server log). For a restore, failed or completed, they say what the restored backup already lacked when it was taken and how many Iceberg warehouse files were not restored because the node has no outside-root warehouse. `create --wait` ends its success line with every gap the server reports, prints the names, and points at `arcli backup show` for the full breakdown. +- `arcli backup restore` warns before restoring a backup that lacks files for any reason the manifest records (skipped data or metadata files, files that could not be listed, outside-root Iceberg warehouse files), not only skipped data files, and says how many of the skipped files had keys too long to store. ### Notes diff --git a/internal/commands/backup.go b/internal/commands/backup.go index 627ba58..1828d19 100644 --- a/internal/commands/backup.go +++ b/internal/commands/backup.go @@ -156,17 +156,50 @@ func describeProgress(w io.Writer, p *client.BackupProgress) { // already lacked, and warehouse files a restore had nowhere to put. The // server already names the counts of a failed restore in its error, so // only the names and the backup's own gaps are added here. + writeRestoreGaps(w, p) + writeSample(w, "skipped", p.SkippedSample) + if p.UnaddressableFiles > 0 && len(p.UnaddressableSample) == 0 { + // A backup publishes the count before copying but names the files + // only in its manifest, which exists once it has completed; while it + // runs, or after it failed, the names are in the server log. + if p.Operation == "backup" && p.Status == "completed" { + fmt.Fprintf(w, " unaddressable: %d files (names: arcli backup show %s)\n", p.UnaddressableFiles, clean(p.BackupID)) + } else { + fmt.Fprintf(w, " unaddressable: %d files (names in the server log)\n", p.UnaddressableFiles) + } + } + writeSample(w, "unaddressable", p.UnaddressableSample) +} + +// writeRestoreGaps prints what a restore's progress says about gaps that are +// not the restore's own doing: what the restored backup already lacked when it +// was taken, and warehouse files this node had nowhere to put. Printed for a +// failed and a completed restore alike, since the latter is the only place the +// warehouse gap shows when Iceberg is off on the node. +func writeRestoreGaps(w io.Writer, p *client.BackupProgress) { if p.BackupSkippedFiles > 0 || p.BackupUnaddressableFiles > 0 { fmt.Fprintf(w, "backup had: %s when it was taken\n", describeIncomplete(p.BackupSkippedFiles, 0, p.BackupUnaddressableFiles)) } if p.IcebergWarehouseFilesSkipped > 0 { fmt.Fprintf(w, "iceberg: %d warehouse files not restored (this node has no outside-root warehouse)\n", p.IcebergWarehouseFilesSkipped) } - writeSample(w, "skipped", p.SkippedSample) - if p.UnaddressableFiles > 0 && len(p.UnaddressableSample) == 0 { - fmt.Fprintf(w, " unaddressable: %d files (names: arcli backup show %s)\n", p.UnaddressableFiles, clean(p.BackupID)) +} + +// overlongClause phrases how many skips were for a destination key over the +// storage limit, the permanent cause fixed by renaming the file. +func overlongClause(n int64) string { + if n == 1 { + return "1 for a key too long to store" } - writeSample(w, "unaddressable", p.UnaddressableSample) + return fmt.Sprintf("%d for keys too long to store", n) +} + +// wasWere is the verb for n skipped files. +func wasWere(n int64) string { + if n == 1 { + return "was" + } + return "were" } // describeIncomplete phrases a backup's incompleteness counts, each in its own @@ -316,7 +349,7 @@ deletion can run at a time.`, fmt.Fprintln(out) if len(gaps) > 0 { writeSample(out, "skipped", final.SkippedSample) - fmt.Fprintf(out, "see \"arcli backup show %s\" for the full breakdown\n", clean(id)) + fmt.Fprintf(cmd.ErrOrStderr(), "see `arcli backup show %s` for the full breakdown\n", clean(id)) } return nil }, @@ -486,19 +519,22 @@ func describeManifestIncomplete(m *client.BackupManifest) string { var clauses []string var skipped []string if m.SkippedFiles > 0 { - skipped = append(skipped, fmt.Sprintf("%d of %d files", m.SkippedFiles, m.TotalFiles)) + // A manifest written before the data/metadata split folded metadata + // skips into skipped_files, so the count can exceed the total; then + // "N of M" would read as nonsense. + if m.SkippedFiles > m.TotalFiles { + skipped = append(skipped, plural(m.SkippedFiles, "file", "files")) + } else { + skipped = append(skipped, fmt.Sprintf("%d of %d files", m.SkippedFiles, m.TotalFiles)) + } } if m.SkippedMetadataFiles > 0 { skipped = append(skipped, plural(m.SkippedMetadataFiles, "metadata file", "metadata files")) } if len(skipped) > 0 { - verb := "were" - if m.SkippedFiles+m.SkippedMetadataFiles == 1 { - verb = "was" - } - clause := strings.Join(skipped, " and ") + " " + verb + " skipped while backing up" + clause := strings.Join(skipped, " and ") + " " + wasWere(m.SkippedFiles+m.SkippedMetadataFiles) + " skipped while backing up" if m.SkippedOverlongKeys > 0 { - clause += fmt.Sprintf(" (%d for a key too long to store)", m.SkippedOverlongKeys) + clause += " (" + overlongClause(m.SkippedOverlongKeys) + ")" } clauses = append(clauses, clause) } @@ -511,6 +547,36 @@ func describeManifestIncomplete(m *client.BackupManifest) string { return strings.Join(clauses, "; ") } +// describeRestoreGaps phrases what a backup lacked when it was taken, for the +// warning before a restore: the same shape as describeManifestIncomplete, with +// the overlong clause on the joined skipped sentence because the server counts +// overlong keys across data and metadata files alike. "" when the manifest +// records no gap. +func describeRestoreGaps(m *client.BackupManifest) string { + var clauses []string + var skipped []string + if m.SkippedFiles > 0 { + skipped = append(skipped, plural(m.SkippedFiles, "file", "files")) + } + if m.SkippedMetadataFiles > 0 { + skipped = append(skipped, plural(m.SkippedMetadataFiles, "metadata file", "metadata files")) + } + if len(skipped) > 0 { + clause := strings.Join(skipped, " and ") + " " + wasWere(m.SkippedFiles+m.SkippedMetadataFiles) + " skipped when it was taken" + if m.SkippedOverlongKeys > 0 { + clause += ", " + strings.Replace(overlongClause(m.SkippedOverlongKeys), " for ", " of them for ", 1) + } + clauses = append(clauses, clause) + } + if m.UnaddressableFiles > 0 { + clauses = append(clauses, plural(m.UnaddressableFiles, "file", "files")+" could not be listed") + } + if m.IcebergWarehouse != nil && m.IcebergWarehouse.SkippedFiles > 0 { + clauses = append(clauses, plural(m.IcebergWarehouse.SkippedFiles, "Iceberg warehouse file was", "Iceberg warehouse files were")+" skipped") + } + return strings.Join(clauses, "; ") +} + func newBackupStatusCmd() *cobra.Command { var ( f connFlags @@ -673,22 +739,8 @@ to 2h).`, return fmt.Errorf("backup %s does not contain the server config; drop --with-config", clean(id)) } stderr := cmd.ErrOrStderr() - if m.SkippedFiles > 0 || m.SkippedMetadataFiles > 0 || m.UnaddressableFiles > 0 { - var gaps []string - if m.SkippedFiles > 0 { - g := fmt.Sprintf("%d files were skipped when it was taken", m.SkippedFiles) - if m.SkippedOverlongKeys > 0 { - g += fmt.Sprintf(", %d of them for keys too long to store", m.SkippedOverlongKeys) - } - gaps = append(gaps, g) - } - if m.SkippedMetadataFiles > 0 { - gaps = append(gaps, fmt.Sprintf("%d metadata files were skipped", m.SkippedMetadataFiles)) - } - if m.UnaddressableFiles > 0 { - gaps = append(gaps, fmt.Sprintf("%d files could not be listed", m.UnaddressableFiles)) - } - fmt.Fprintf(stderr, "warning: backup %s is incomplete (%s)\n", clean(id), strings.Join(gaps, "; ")) + if gaps := describeRestoreGaps(m); gaps != "" { + fmt.Fprintf(stderr, "warning: backup %s is incomplete (%s)\n", clean(id), gaps) } dbs := make([]string, 0, len(m.Databases)) for _, db := range m.Databases { @@ -781,6 +833,7 @@ to 2h).`, } } else if final.Status == "completed" { fmt.Fprintf(cmd.OutOrStdout(), "Restore of %s completed: %d files, %s written\n", clean(id), final.ProcessedFiles, humanBytes(final.ProcessedBytes)) + writeRestoreGaps(cmd.OutOrStdout(), final) } else { describeProgress(cmd.OutOrStdout(), final) } diff --git a/internal/commands/backup_incomplete_test.go b/internal/commands/backup_incomplete_test.go index 0c84c95..8fb2990 100644 --- a/internal/commands/backup_incomplete_test.go +++ b/internal/commands/backup_incomplete_test.go @@ -13,7 +13,10 @@ import ( // skipped, 2 unaddressable files; its completed backup status; and a failed // restore of it. This file uses only symbols that exist before the arcli // change, so it compiles against the old renderings and fails there. -const incompleteID = "backup-20260917-000000-aaaaaaaa" +const ( + incompleteID = "backup-20260917-000000-aaaaaaaa" + metadataOnlyID = "backup-20260915-000000-cccccccc" +) func newIncompleteBackupServer(t *testing.T) *httptest.Server { t.Helper() @@ -27,10 +30,10 @@ func newIncompleteBackupServer(t *testing.T) *httptest.Server { rest := strings.TrimPrefix(r.URL.Path, "/api/v1/backup/") switch { case rest == "" && r.Method == http.MethodGet: - _, _ = w.Write([]byte(`{"backups":[{"backup_id":"` + incompleteID + `","created_at":"2026-09-17T00:00:00Z","backup_type":"full","total_files":21,"total_size_bytes":2086,"database_count":1,"skipped_files":3,"skipped_metadata_files":1,"unaddressable_files":2},{"backup_id":"backup-20260916-000000-bbbbbbbb","created_at":"2026-09-16T00:00:00Z","backup_type":"full","total_files":20,"total_size_bytes":2000,"database_count":1}],"count":2}`)) + _, _ = w.Write([]byte(`{"backups":[{"backup_id":"` + incompleteID + `","created_at":"2026-09-17T00:00:00Z","backup_type":"full","total_files":21,"total_size_bytes":2086,"database_count":1,"skipped_files":3,"skipped_metadata_files":1,"unaddressable_files":2},{"backup_id":"backup-20260916-000000-bbbbbbbb","created_at":"2026-09-16T00:00:00Z","backup_type":"full","total_files":20,"total_size_bytes":2000,"database_count":1},{"backup_id":"` + metadataOnlyID + `","created_at":"2026-09-15T00:00:00Z","backup_type":"full","total_files":20,"total_size_bytes":2000,"database_count":1,"skipped_metadata_files":1}],"count":3}`)) case rest == "status": if restored.Load() { - _, _ = w.Write([]byte(`{"operation":"restore","backup_id":"` + incompleteID + `","status":"failed","total_files":17,"processed_files":16,"skipped_files":1,"unaddressable_files":1,"unaddressable_sample":["` + incompleteID + `/data/mydb/cpu/2026/09/17/00/.hidden.parquet"],"skipped_sample":["` + incompleteID + `/data/mydb/cpu/2026/09/17/00/c.parquet"],"missing_files":1,"backup_skipped_files":3,"backup_unaddressable_files":2,"iceberg_warehouse_files_skipped":4,"total_bytes":2086,"processed_bytes":1600,"started_at":"2026-09-17T01:00:00Z","completed_at":"2026-09-17T01:00:02Z","error":"restore incomplete: 1 objects could not be read from backup storage (skipped_sample); the files that could be restored are in place"}`)) + _, _ = w.Write([]byte(`{"operation":"restore","backup_id":"` + incompleteID + `","status":"failed","total_files":17,"processed_files":16,"skipped_files":1,"unaddressable_files":1,"unaddressable_sample":["` + incompleteID + `/data/mydb/cpu/2026/09/17/00/.hidden.parquet"],"skipped_sample":["` + incompleteID + `/data/mydb/cpu/2026/09/17/00/c.parquet"],"missing_files":1,"backup_skipped_files":3,"backup_unaddressable_files":2,"iceberg_warehouse_files_skipped":4,"total_bytes":2086,"processed_bytes":1600,"started_at":"2026-09-17T01:00:00Z","completed_at":"2026-09-17T01:00:02Z","error":"restore incomplete: 1 objects could not be read from backup storage (skipped_sample); 1 data files are in backup storage under names no listing returns (unaddressable_sample; rename them and re-run); 1 data files the backup inventoried are absent from backup storage (missing_files); the files that could be restored are in place"}`)) return } _, _ = w.Write([]byte(`{"operation":"backup","backup_id":"` + incompleteID + `","status":"completed","total_files":23,"processed_files":19,"skipped_files":4,"unaddressable_files":2,"skipped_sample":["mydb/cpu/2026/09/17/00/a.parquet","mydb/cpu/2026/09/17/00/b.parquet","` + long + `","mydb/cpu/metadata/00003-5f2c.metadata.json"],"total_bytes":2086,"processed_bytes":1790,"started_at":"2026-09-17T00:00:00Z","completed_at":"2026-09-17T00:00:03Z"}`)) @@ -38,6 +41,8 @@ func newIncompleteBackupServer(t *testing.T) *httptest.Server { restored.Store(true) w.WriteHeader(202) _, _ = w.Write([]byte(`{"backup_id":"` + incompleteID + `","message":"Restore started","status":"running"}`)) + case rest == metadataOnlyID && r.Method == http.MethodGet: + _, _ = w.Write([]byte(`{"version":"26.09.3","backup_id":"` + metadataOnlyID + `","created_at":"2026-09-15T00:00:00Z","backup_type":"full","databases":[{"name":"mydb","measurements":[{"name":"cpu","file_count":20,"size_bytes":2000}],"file_count":20,"size_bytes":2000}],"total_files":20,"total_size_bytes":2000,"skipped_metadata_files":1,"skipped_sample":["mydb/cpu/metadata/00004-aa11.metadata.json"],"skipped_overlong_keys":1,"has_metadata":true,"has_config":false}`)) case rest == incompleteID && r.Method == http.MethodGet: _, _ = w.Write([]byte(`{"version":"26.09.3","backup_id":"` + incompleteID + `","created_at":"2026-09-17T00:00:00Z","backup_type":"full","databases":[{"name":"mydb","measurements":[{"name":"cpu","file_count":21,"size_bytes":2086}],"file_count":21,"size_bytes":2086}],"total_files":21,"total_size_bytes":2086,"skipped_files":3,"skipped_metadata_files":1,"skipped_sample":["mydb/cpu/2026/09/17/00/a.parquet","mydb/cpu/2026/09/17/00/b.parquet","` + long + `","mydb/cpu/metadata/00003-5f2c.metadata.json"],"skipped_overlong_keys":1,"unaddressable_files":2,"unaddressable_sample":["mydb/cpu/2026/09/17/00/.hidden.parquet","mydb/cpu/2026/09/17/00/bad key.parquet"],"iceberg_warehouse":{"path":"/srv/wh","file_count":4,"size_bytes":100,"skipped_files":1},"has_metadata":true,"has_config":false}`)) default: @@ -71,16 +76,32 @@ func TestBackup_IncompleteIsVisible(t *testing.T) { t.Fatal(err) } lines := strings.Split(strings.TrimSpace(out), "\n") - if len(lines) != 3 || lines[0] != "backup_id,created_at,backup_type,database_count,total_files,total_size_bytes,skipped_files,skipped_metadata_files,unaddressable_files" { - t.Errorf("csv header = %q", lines[0]) + if len(lines) != 4 || lines[0] != "backup_id,created_at,backup_type,database_count,total_files,total_size_bytes,skipped_files,skipped_metadata_files,unaddressable_files" { + t.Errorf("csv header = %q (%d lines)", lines[0], len(lines)) } - if len(lines) == 3 && (!strings.HasSuffix(lines[1], ",21,2086,3,1,2") || !strings.HasSuffix(lines[2], ",20,2000,0,0,0")) { + if len(lines) == 4 && (!strings.HasSuffix(lines[1], ",21,2086,3,1,2") || !strings.HasSuffix(lines[2], ",20,2000,0,0,0") || !strings.HasSuffix(lines[3], ",20,2000,0,1,0")) { t.Errorf("csv rows = %q", lines[1:]) } out, _, err = execCmd(t, newBackupListCmd(), "-o", "csv", "--no-header") if err != nil || strings.Contains(out, "backup_id,") || !strings.Contains(out, ",3,1,2") { t.Errorf("csv --no-header: err=%v out=%q", err, out) } + // json is the server's body, untouched. + out, _, err = execCmd(t, newBackupListCmd(), "-o", "json") + if err != nil || !strings.Contains(out, `"skipped_metadata_files": 1`) || strings.Contains(out, "INCOMPLETE") || strings.Contains(out, "2 unaddressable") { + t.Errorf("json passthrough: err=%v out=%s", err, out) + } + // A backup whose only gap is one metadata file, skipped for an overlong + // key: singular wording, and the overlong clause survives without any + // data-file skip to hang on. + out, _, err = execCmd(t, newBackupListCmd()) + if err != nil || !strings.Contains(out, "│ 1 metadata") { + t.Errorf("metadata-only list cell: err=%v out=%s", err, out) + } + out, _, err = execCmd(t, newBackupShowCmd(), metadataOnlyID) + if err != nil || !strings.Contains(out, "INCOMPLETE: 1 metadata file was skipped while backing up (1 for a key too long to store)\n skipped: mydb/cpu/metadata/00004-aa11.metadata.json\n") { + t.Errorf("metadata-only show: err=%v out=%s", err, out) + } // show: the counts in words, each in its own domain, then the names. out, _, err = execCmd(t, newBackupShowCmd(), incompleteID) @@ -122,7 +143,7 @@ func TestBackup_IncompleteIsVisible(t *testing.T) { if err != nil { t.Fatalf("restore: %v (stderr %s)", err, stderr) } - if !strings.Contains(stderr, "warning: backup "+incompleteID+" is incomplete (3 files were skipped when it was taken, 1 of them for keys too long to store; 1 metadata files were skipped; 2 files could not be listed)\n") { + if !strings.Contains(stderr, "warning: backup "+incompleteID+" is incomplete (3 files and 1 metadata file were skipped when it was taken, 1 of them for a key too long to store; 2 files could not be listed; 1 Iceberg warehouse file was skipped)\n") { t.Errorf("restore warning missing or wrong:\n%s", stderr) } if !strings.Contains(out, "Restore of "+incompleteID+" started") { @@ -148,4 +169,35 @@ func TestBackup_IncompleteIsVisible(t *testing.T) { t.Errorf("restore status lacks %q:\n%s", want, out) } } + + // The metadata-only backup's pre-flight warning: singular wording, and + // the overlong clause survives without a data-file skip to hang on. (Runs + // last: the fake server's status is the failed restore from here on, so + // this restore is "accepted but not yet published", which is fine.) + _, stderr0, err := execCmd(t, newBackupRestoreCmd(), metadataOnlyID, "--yes") + if err != nil || !strings.Contains(stderr0, "warning: backup "+metadataOnlyID+" is incomplete (1 metadata file was skipped when it was taken, 1 of them for a key too long to store)\n") { + t.Errorf("metadata-only restore warning: err=%v stderr=%s", err, stderr0) + } +} + +// While a backup runs, or after it failed, no manifest exists, so the hint for +// unaddressable names must point at the server log, not at "backup show". +func TestBackup_StatusFailedBackupNamesAreInTheLog(t *testing.T) { + fastPolls(t) + for _, status := range []string{"failed", "running"} { + mux := http.NewServeMux() + mux.HandleFunc("/api/v1/backup/status", func(w http.ResponseWriter, r *http.Request) { + _, _ = w.Write([]byte(`{"operation":"backup","backup_id":"` + incompleteID + `","status":"` + status + `","total_files":24,"processed_files":19,"skipped_files":5,"unaddressable_files":2,"skipped_sample":["smoke/m1/a.parquet"],"total_bytes":2086,"processed_bytes":1600,"started_at":"2026-09-17T00:00:00Z","error":"backup failed: 5 of 24 files skipped (>10%): 5 could not be read at copy time (check source storage)"}`)) + }) + srv := httptest.NewServer(mux) + writeTestConfig(t, srv.URL, "tok") + out, _, err := execCmd(t, newBackupStatusCmd()) + srv.Close() + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out, " skipped: smoke/m1/a.parquet\n unaddressable: 2 files (names in the server log)\n") || strings.Contains(out, "backup show") { + t.Errorf("%s backup status: %s", status, out) + } + } } diff --git a/internal/commands/backup_test.go b/internal/commands/backup_test.go index 1b7d128..9674202 100644 --- a/internal/commands/backup_test.go +++ b/internal/commands/backup_test.go @@ -476,19 +476,21 @@ func TestBackup_CreateWaitReportsIncomplete(t *testing.T) { f := newFakeBackupServer(t) f.incomplete = true writeTestConfig(t, f.srv.URL, "tok") - out, _, err := execCmd(t, newBackupCreateCmd(), "--wait", "--wait-timeout", "10s") + out, stderr, err := execCmd(t, newBackupCreateCmd(), "--wait", "--wait-timeout", "10s") if err != nil { t.Fatal(err) } for _, want := range []string{ "completed: 1 files, 1.0 KiB (1 files skipped, 1 files could not be listed — the backup is incomplete)\n", " skipped: smoke/cpu/2026/09/07/20/gone.parquet\n", - "see \"arcli backup show backup-20260907-200001-00000001\" for the full breakdown\n", } { if !strings.Contains(out, want) { t.Errorf("create --wait output lacks %q:\n%s", want, out) } } + if !strings.Contains(stderr, "see `arcli backup show backup-20260907-200001-00000001` for the full breakdown\n") { + t.Errorf("create --wait stderr lacks the show hint:\n%s", stderr) + } out, _, err = execCmd(t, newBackupListCmd()) if err != nil || !strings.Contains(out, "1 skipped, 1 unaddressable") { t.Errorf("list: err=%v out=%s", err, out)