[saintstack]

fdbbackup query reports failures as a JSON object on stdout and then returns normally. stopAfter() therefore yields a present Optional, and main() leaves the exit status at FDB_EXIT_SUCCESS.

Every failure path is affected, including "no restorable files set found for specified key ranges" and the catch-all for an arbitrary Error. Automation that checks the exit code — the normal thing for shell automation to trust — reads those failures as successes.

Fix

queryBackup now returns its own exit status through the fstatus future. That future was already declared and already inspected in main() for exactly this purpose, but nothing had ever assigned it.

The six failure paths co_return FDB_EXIT_ERROR; the success path returns FDB_EXIT_SUCCESS.

The fstatus check is also widened so an absent value maps to FDB_EXIT_ERROR. Absent means stopAfter() absorbed a fatal error rather than the command choosing a status — reachable here because the restore-timestamp resolution at the top of queryBackup sits outside the function's try.

Why not just throw

Throwing from the failure paths would produce the right exit code via the existing f check, but stopAfter() would also raise a SevError trace event and print Fatal Error: ... to stderr for an ordinary "no restorable files set" answer. That is the wrong severity for a routine negative result, and it breaks the one useful property this subcommand has: stdout is a single parseable JSON document and nothing else.

Note that submitBackup and submitDBBackup already throw actor_cancelled specifically to reach stopAfter's quiet branch, so the tension with stopAfter's severity is pre-existing.

Testing

• ninja fdbbackup backup_tests clean.

• BackupTests: 21/21, exit 0.

• Failure path, against a container URL that does not resolve:

  $ fdbbackup query -d 'file:///tmp/nope' --query-restore-version 1; echo "exit=$?"
  {"key_ranges_filter":"","destination_container":"file:///tmp/nope","error":"Backup Container URL invalid"}
  exit=1
  

  Previously exit=0.

• The success path returning 0 has not been exercised against a real backup container. It is co_return FDB_EXIT_SUCCESS with FDB_EXIT_SUCCESS == 0.

Caveat

stopAfter leaves its result at the initialized T() for please_reboot, please_reboot_delete, actor_cancelled and local_config_changed. For int that is 0, so a cancelled query still exits 0 and the new absent-value branch does not cover it. This matches the pre-existing behaviour through the f path and does not matter for a one-shot CLI, but it means this change should not be described as "any failure now exits non-zero".

Compatibility

fdbbackup query previously exited 0 on failure and now exits 1. Anything relying on the old behaviour — for example treating a non-zero exit as "the binary itself broke" rather than "the query failed" — will see a change. The JSON on stdout is unchanged in both shape and content.

No fdbbackup query invocation exists in fdb-kubernetes-tests.

[akshay-v-kaushik]

@saintstack LGTM for the most part. I had two thoughts:

1. Should we pin the exit codes with two tests? fdbbackup/tests/ has no query coverage.
2. timeKeeperVersionFromDatetime is the only await outside the try. Should we consider moving it inside would make that path consistent if you think it's worth it

[saintstack]

Should we pin the exit codes with two tests? fdbbackup/tests/ has no query coverage.
timeKeeperVersionFromDatetime is the only await outside the try. Should we consider moving it inside would make that path consistent if you think it's worth it

Good review. Done. Please take another look @akshay-v-kaushik

[akshay-v-kaushik]

LGTM!

[foundationdb-ci]

Result of foundationdb-pr-clang-ide on Linux RHEL 9

• Commit ID: 3f8db96
• Duration 0:35:29
• Result: ✅ SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)

[foundationdb-ci]

Result of foundationdb-pr-macos-m1 on macOS 14.x

• Commit ID: 3f8db96
• Duration 0:38:47
• Result: ✅ SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)

[foundationdb-ci]

Result of foundationdb-pr on Linux RHEL 9

• Commit ID: 3f8db96
• Duration 0:57:51
• Result: ✅ SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)

[foundationdb-ci]

Result of foundationdb-pr-clang on Linux RHEL 9

• Commit ID: 3f8db96
• Duration 1:01:32
• Result: ✅ SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)

[foundationdb-ci]

Result of foundationdb-pr-clang-arm on Linux RHEL 9

• Commit ID: 3f8db96
• Duration 1:05:39
• Result: ✅ SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)

[foundationdb-ci]

Result of foundationdb-pr-cluster-tests on Linux RHEL 9

• Commit ID: 3f8db96
• Duration 1:47:49
• Result: ✅ SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)
• Cluster Test Logs zip file of the test logs (available for 30 days)

[foundationdb-ci]

Result of foundationdb-pr-macos on macOS 14.x

• Commit ID: 3f8db96
• Duration 2:27:16
• Result: ✅ SUCCEEDED
• Error: N/A
• Build Log terminal output (available for 30 days)
• Build Workspace zip file of the working directory (available for 30 days)

[saintstack]

Thanks for the review @akshay-v-kaushik