talmacschen-arch opened a new pull request, #97:
URL: https://github.com/apache/cloudberry-backup/pull/97
Fixes #96
## Motivation
When `gpbackman backup-info` (or any other gpbackman subcommand) is run
without `--history-db`, the current code drops down to a relative
`gpbackup_history.db` resolved against the current directory. Two
problems follow:
1. The cluster's real history DB lives under
`$COORDINATOR_DATA_DIRECTORY/gpbackup_history.db` (or the legacy
`$MASTER_DATA_DIRECTORY` on older installs), which is rarely the
user's shell `cwd`. Operators end up typing the long path on every
invocation despite the relevant env vars already being set by the
standard cluster environment scripts.
2. SQLite's default open mode silently **creates** a new empty database
when the file is missing. The very next query then fails with
`no such table: backups`, which is confusing and leaves an empty
`gpbackup_history.db` litter in `cwd`.
Reproducer (current behaviour on `main`):
```console
$ cd /tmp/empty
$ gpbackman backup-info
[ERROR]:-Unable to read data from history db. Error: no such table: backups
$ ls
gpbackup_history.db # ← silently created, 0 rows, polluted cwd
```
## Changes
* **`gpbackman/cmd/wrappers.go`** — `getHistoryDBPath` resolves the path
in order: `--history-db` → `$COORDINATOR_DATA_DIRECTORY` →
`$MASTER_DATA_DIRECTORY` → bare filename in `cwd` (preserved for
backward compatibility).
* **`gpbackman/gpbckpconfig/utils_db.go`** — `OpenHistoryDB` now
pre-checks the file with `os.Stat` and opens SQLite via the
`file:<path>?mode=rw` URI. Missing files surface a clear error that
references `--history-db` and the env vars; no empty file is ever
created.
* **Help text** updated for the `--history-db` flag and every
command's `Long` description; `gpbackman/COMMANDS.md` and
`gpbackman/README.md` aligned.
* **Tests** added/extended in `wrappers_test.go` (env-var resolution
order, explicit-wins-over-env) and in `utils_db_test.go` (friendly
error + no-file-created regression, happy-path open).
## Backward compatibility
Fully compatible. The old "search the current directory" behaviour is
preserved as the last fallback, so any existing scripts that rely on
running gpbackman from a directory containing `gpbackup_history.db`
continue to work unchanged. Users who currently pass `--history-db`
explicitly are unaffected.
The only user-visible *behaviour* change is that running gpbackman
against a non-existent path now errors immediately with a clear message
instead of producing a misleading "no such table: backups" error after
silently creating an empty file. This is the intended fix.
## How tested
* `go test ./gpbackman/cmd/... ./gpbackman/gpbckpconfig/...
./gpbackman/textmsg/...` — all pass.
* `make build` — five binaries built cleanly.
* Manual smoke test against a real cluster history DB:
* Empty `cwd`, no env → friendly error, no file created. ✅
* `COORDINATOR_DATA_DIRECTORY` pointing at a non-existent path → friendly
error mentioning the resolved path. ✅
* `COORDINATOR_DATA_DIRECTORY` pointing at the coordinator data dir →
backup list rendered without `--history-db`. ✅
## Notes
* DCO `Signed-off-by` included on both commits.
* See #96 for additional discussion / open questions (env-var name choice,
optional `GPBACKMAN_HISTORY_DB` override).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
