From 04af804442c4d330dc6f017d1f45495aadc6082a Mon Sep 17 00:00:00 2001 From: chenqiang Date: Wed, 29 Apr 2026 12:45:47 +0800 Subject: [PATCH 1/5] gpbackman: fall back to coordinator data dir for history db, fail loud when missing. Two improvements to how gpbackman locates and opens gpbackup_history.db: 1. When --history-db is empty, fall back (in priority order) to $COORDINATOR_DATA_DIRECTORY and $MASTER_DATA_DIRECTORY before the current directory. These variables are exported by the standard Cloudberry/Greenplum environment scripts, so users running gpbackman from a sourced cluster shell no longer need to repeat --history-db on every invocation. 2. OpenHistoryDB now pre-checks the file with os.Stat and opens the SQLite database with the rw URI mode. A missing file produces a friendly error pointing at --history-db and the env vars, instead of silently creating an empty file that later fails with "no such table: backups" when callers issue queries. The previous cwd-only fallback is preserved as a last resort. Tests cover the new env-var resolution order and the safe-open behaviour. Signed-off-by: chenqiang --- gpbackman/cmd/backup_clean.go | 2 +- gpbackman/cmd/backup_delete.go | 2 +- gpbackman/cmd/backup_info.go | 2 +- gpbackman/cmd/constants.go | 8 +++++ gpbackman/cmd/history_clean.go | 2 +- gpbackman/cmd/report_info.go | 2 +- gpbackman/cmd/root.go | 4 ++- gpbackman/cmd/wrappers.go | 16 +++++++-- gpbackman/cmd/wrappers_test.go | 45 ++++++++++++++++++++++++- gpbackman/gpbckpconfig/utils_db.go | 27 +++++++++++++-- gpbackman/gpbckpconfig/utils_db_test.go | 38 +++++++++++++++++++++ 11 files changed, 137 insertions(+), 11 deletions(-) diff --git a/gpbackman/cmd/backup_clean.go b/gpbackman/cmd/backup_clean.go index 4f5e2179..1ccaf8f8 100644 --- a/gpbackman/cmd/backup_clean.go +++ b/gpbackman/cmd/backup_clean.go @@ -77,7 +77,7 @@ For non local backups the following logic are applied: The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory.`, +If the --history-db option is not specified, the history database is looked for under $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY); if neither variable is set, the current directory is used as a last resort.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) diff --git a/gpbackman/cmd/backup_delete.go b/gpbackman/cmd/backup_delete.go index 14fa2795..202d52b3 100644 --- a/gpbackman/cmd/backup_delete.go +++ b/gpbackman/cmd/backup_delete.go @@ -84,7 +84,7 @@ For non local backups the following logic are applied: The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory.`, +If the --history-db option is not specified, the history database is looked for under $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY); if neither variable is set, the current directory is used as a last resort.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) diff --git a/gpbackman/cmd/backup_info.go b/gpbackman/cmd/backup_info.go index 199311cc..0515e402 100644 --- a/gpbackman/cmd/backup_info.go +++ b/gpbackman/cmd/backup_info.go @@ -98,7 +98,7 @@ To display the "object filtering details" column for all backups without using - The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory.`, +If the --history-db option is not specified, the history database is looked for under $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY); if neither variable is set, the current directory is used as a last resort.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) diff --git a/gpbackman/cmd/constants.go b/gpbackman/cmd/constants.go index 8259b594..14610c5a 100644 --- a/gpbackman/cmd/constants.go +++ b/gpbackman/cmd/constants.go @@ -72,4 +72,12 @@ var ( beforeTimestamp string // Timestamp to delete all backups after. afterTimestamp string + + // historyDBEnvVars lists, in priority order, the environment variables + // inspected when --history-db is not supplied. They are exported by the + // standard Cloudberry/Greenplum cluster environment scripts. + historyDBEnvVars = []string{ + "COORDINATOR_DATA_DIRECTORY", + "MASTER_DATA_DIRECTORY", + } ) diff --git a/gpbackman/cmd/history_clean.go b/gpbackman/cmd/history_clean.go index d5de643a..836991b8 100644 --- a/gpbackman/cmd/history_clean.go +++ b/gpbackman/cmd/history_clean.go @@ -50,7 +50,7 @@ Only --older-than-days or --before-timestamp option must be specified, not both. The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory.`, +If the --history-db option is not specified, the history database is looked for under $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY); if neither variable is set, the current directory is used as a last resort.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) diff --git a/gpbackman/cmd/report_info.go b/gpbackman/cmd/report_info.go index 3ac7428f..168695ae 100644 --- a/gpbackman/cmd/report_info.go +++ b/gpbackman/cmd/report_info.go @@ -78,7 +78,7 @@ It is not necessary to use the --plugin-report-file-path flag for the following The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory.`, +If the --history-db option is not specified, the history database is looked for under $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY); if neither variable is set, the current directory is used as a last resort.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) diff --git a/gpbackman/cmd/root.go b/gpbackman/cmd/root.go index 1c113b37..e8473269 100644 --- a/gpbackman/cmd/root.go +++ b/gpbackman/cmd/root.go @@ -50,7 +50,9 @@ func init() { &rootHistoryDB, historyDBFlagName, "", - "full path to the gpbackup_history.db file", + "full path to the gpbackup_history.db file (if unset, falls back to "+ + "$COORDINATOR_DATA_DIRECTORY/gpbackup_history.db, then "+ + "$MASTER_DATA_DIRECTORY/gpbackup_history.db, then the current directory)", ) rootCmd.PersistentFlags().StringVar( &rootLogFile, diff --git a/gpbackman/cmd/wrappers.go b/gpbackman/cmd/wrappers.go index 79e34f38..be1c5b59 100644 --- a/gpbackman/cmd/wrappers.go +++ b/gpbackman/cmd/wrappers.go @@ -81,12 +81,24 @@ func setLogLevelFile(level string) error { return nil } +// getHistoryDBPath resolves the path to the gpbackup_history.db file. +// When the --history-db flag is empty, fall back (in order) to the +// COORDINATOR_DATA_DIRECTORY and MASTER_DATA_DIRECTORY environment variables +// that the standard Cloudberry/Greenplum environment scripts export, so that +// users running gpbackman from a sourced cluster shell do not need to repeat +// the path on every invocation. As a last resort, return the bare filename +// (resolved against the current working directory), preserving the previous +// behaviour. func getHistoryDBPath(historyDBPath string) string { - var historyDBName = historyDBNameConst if historyDBPath != "" { return historyDBPath } - return historyDBName + for _, envVar := range historyDBEnvVars { + if dir := os.Getenv(envVar); dir != "" { + return filepath.Join(dir, historyDBNameConst) + } + } + return historyDBNameConst } func checkCompatibleFlags(flags *pflag.FlagSet, flagNames ...string) error { diff --git a/gpbackman/cmd/wrappers_test.go b/gpbackman/cmd/wrappers_test.go index e378fbf1..d7e16093 100644 --- a/gpbackman/cmd/wrappers_test.go +++ b/gpbackman/cmd/wrappers_test.go @@ -33,13 +33,56 @@ import ( var _ = Describe("wrappers tests", func() { Describe("getHistoryDBPath", func() { - It("returns default path when input is empty", func() { + // Save and restore env vars so these cases don't leak into the rest + // of the suite when run with --randomize-all. + var savedEnv map[string]string + + BeforeEach(func() { + savedEnv = make(map[string]string, len(historyDBEnvVars)) + for _, name := range historyDBEnvVars { + savedEnv[name] = os.Getenv(name) + os.Unsetenv(name) + } + }) + + AfterEach(func() { + for name, val := range savedEnv { + if val == "" { + os.Unsetenv(name) + } else { + os.Setenv(name, val) + } + } + }) + + It("returns default filename when input is empty and no env vars are set", func() { Expect(getHistoryDBPath("")).To(Equal(historyDBNameConst)) }) It("returns input path when not empty", func() { Expect(getHistoryDBPath("path/to/" + historyDBNameConst)).To(Equal("path/to/" + historyDBNameConst)) }) + + It("falls back to COORDINATOR_DATA_DIRECTORY when input is empty", func() { + os.Setenv("COORDINATOR_DATA_DIRECTORY", "/coord/data") + Expect(getHistoryDBPath("")).To(Equal(filepath.Join("/coord/data", historyDBNameConst))) + }) + + It("falls back to MASTER_DATA_DIRECTORY when COORDINATOR is unset", func() { + os.Setenv("MASTER_DATA_DIRECTORY", "/master/data") + Expect(getHistoryDBPath("")).To(Equal(filepath.Join("/master/data", historyDBNameConst))) + }) + + It("prefers COORDINATOR over MASTER when both are set", func() { + os.Setenv("COORDINATOR_DATA_DIRECTORY", "/coord/data") + os.Setenv("MASTER_DATA_DIRECTORY", "/master/data") + Expect(getHistoryDBPath("")).To(Equal(filepath.Join("/coord/data", historyDBNameConst))) + }) + + It("explicit input wins over env vars", func() { + os.Setenv("COORDINATOR_DATA_DIRECTORY", "/coord/data") + Expect(getHistoryDBPath("/explicit/path.db")).To(Equal("/explicit/path.db")) + }) }) Describe("formatBackupDuration", func() { diff --git a/gpbackman/gpbckpconfig/utils_db.go b/gpbackman/gpbckpconfig/utils_db.go index 304c0907..9cf18df5 100644 --- a/gpbackman/gpbckpconfig/utils_db.go +++ b/gpbackman/gpbckpconfig/utils_db.go @@ -21,15 +21,38 @@ package gpbckpconfig import ( "database/sql" + "errors" "fmt" + "os" "strings" "github.com/apache/cloudberry-backup/history" ) -// OpenHistoryDB opens the history backup database. +// OpenHistoryDB opens an existing gpbackup_history.db SQLite database. +// +// The path is opened with the SQLite "rw" URI mode so that a missing file +// produces a clear error rather than being silently created as an empty +// database (which would later fail with a confusing "no such table: backups" +// when callers issue queries). Existence is also pre-checked with os.Stat to +// surface a friendly error message that points the caller at the relevant +// flag and environment variables. func OpenHistoryDB(historyDBPath string) (*sql.DB, error) { - db, err := sql.Open("sqlite3", historyDBPath) + if _, err := os.Stat(historyDBPath); err != nil { + if errors.Is(err, os.ErrNotExist) { + return nil, fmt.Errorf( + "gpbackup history database file not found: %s. "+ + "Specify the path via --history-db, set "+ + "COORDINATOR_DATA_DIRECTORY (or MASTER_DATA_DIRECTORY), "+ + "or run gpbackman from the directory that contains "+ + "gpbackup_history.db", + historyDBPath, + ) + } + return nil, err + } + // mode=rw opens an existing database for read+write but never creates one. + db, err := sql.Open("sqlite3", "file:"+historyDBPath+"?mode=rw") if err != nil { return nil, err } diff --git a/gpbackman/gpbckpconfig/utils_db_test.go b/gpbackman/gpbckpconfig/utils_db_test.go index 17c6530b..a89e93e9 100644 --- a/gpbackman/gpbckpconfig/utils_db_test.go +++ b/gpbackman/gpbckpconfig/utils_db_test.go @@ -20,7 +20,10 @@ under the License. package gpbckpconfig import ( + "database/sql" "fmt" + "os" + "path/filepath" "github.com/apache/cloudberry-backup/history" . "github.com/onsi/ginkgo/v2" @@ -28,6 +31,41 @@ import ( ) var _ = Describe("utils_db tests", func() { + Describe("OpenHistoryDB", func() { + It("returns a friendly error and does not create a file when the path does not exist", func() { + tempDir := GinkgoT().TempDir() + missing := filepath.Join(tempDir, "does-not-exist.db") + + db, err := OpenHistoryDB(missing) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("not found")) + Expect(err.Error()).To(ContainSubstring("--history-db")) + Expect(db).To(BeNil()) + + // Critical regression: no empty SQLite file must have been created. + _, statErr := os.Stat(missing) + Expect(os.IsNotExist(statErr)).To(BeTrue(), "OpenHistoryDB must not create the file when it is missing") + }) + + It("opens an existing SQLite history database successfully", func() { + tempDir := GinkgoT().TempDir() + path := filepath.Join(tempDir, "gpbackup_history.db") + + // Seed an existing (but empty) SQLite file via the rwc URI mode. + seed, err := sql.Open("sqlite3", "file:"+path+"?mode=rwc") + Expect(err).NotTo(HaveOccurred()) + Expect(seed.Ping()).To(Succeed()) + Expect(seed.Close()).To(Succeed()) + + db, err := OpenHistoryDB(path) + Expect(err).NotTo(HaveOccurred()) + Expect(db).NotTo(BeNil()) + Expect(db.Ping()).To(Succeed()) + Expect(db.Close()).To(Succeed()) + }) + }) + Describe("getBackupNameQuery", func() { It("returns correct query for various flag combinations", func() { tests := []struct { From ef444f4ae2d188c17b478285194b6aab8d649c5d Mon Sep 17 00:00:00 2001 From: chenqiang Date: Wed, 29 Apr 2026 12:45:52 +0800 Subject: [PATCH 2/5] gpbackman: document history-db env-var fallback in COMMANDS.md and README.md. Replace the "current directory" wording everywhere it appeared with the new resolution order: $COORDINATOR_DATA_DIRECTORY, $MASTER_DATA_DIRECTORY, then the current directory. Signed-off-by: chenqiang --- gpbackman/COMMANDS.md | 10 +++++----- gpbackman/README.md | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/gpbackman/COMMANDS.md b/gpbackman/COMMANDS.md index c67947a3..cf82aa8b 100644 --- a/gpbackman/COMMANDS.md +++ b/gpbackman/COMMANDS.md @@ -72,7 +72,7 @@ For non local backups the following logic are applied: The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory. +If the --history-db option is not specified, the history database is looked for under `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`); if neither variable is set, the current directory is used as a last resort. Usage: gpbackman backup-clean [flags] @@ -158,7 +158,7 @@ For non local backups the following logic are applied: The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory. +If the --history-db option is not specified, the history database is looked for under `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`); if neither variable is set, the current directory is used as a last resort. Usage: gpbackman backup-delete [flags] @@ -256,7 +256,7 @@ To display the "object filtering details" column for all backups without using - The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory. +If the --history-db option is not specified, the history database is looked for under `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`); if neither variable is set, the current directory is used as a last resort. Usage: gpbackman backup-info [flags] @@ -455,7 +455,7 @@ Only --older-than-days or --before-timestamp option must be specified, not both. The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory. +If the --history-db option is not specified, the history database is looked for under `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`); if neither variable is set, the current directory is used as a last resort. Usage: gpbackman history-clean [flags] @@ -524,7 +524,7 @@ It is not necessary to use the --plugin-report-file-path flag for the following The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database will be searched in the current directory. +If the --history-db option is not specified, the history database is looked for under `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`); if neither variable is set, the current directory is used as a last resort. Usage: gpbackman report-info [flags] diff --git a/gpbackman/README.md b/gpbackman/README.md index 388ff898..923964ad 100644 --- a/gpbackman/README.md +++ b/gpbackman/README.md @@ -53,7 +53,7 @@ Available Commands: Flags: -h, --help help for gpbackman - --history-db string full path to the gpbackup_history.db file + --history-db string full path to the gpbackup_history.db file (if unset, falls back to $COORDINATOR_DATA_DIRECTORY/gpbackup_history.db, then $MASTER_DATA_DIRECTORY/gpbackup_history.db, then the current directory) --log-file string full path to log file directory, if not specified, the log file will be created in the $HOME/gpAdminLogs directory --log-level-console string level for console logging (error, info, debug, verbose) (default "info") --log-level-file string level for file logging (error, info, debug, verbose) (default "info") From 77e414071d2e0d14de5429c28fa245005e8522d5 Mon Sep 17 00:00:00 2001 From: chenqiang Date: Wed, 29 Apr 2026 15:35:38 +0800 Subject: [PATCH 3/5] gpbackman: gate history-db env-var fallback behind --auto-load-history-db opt-in. Address review feedback on the original PR: * The env-var fallback ($COORDINATOR_DATA_DIRECTORY then $MASTER_DATA_DIRECTORY) is now opt-in via a new persistent flag --auto-load-history-db. The default behaviour (current-directory lookup) is unchanged from upstream main, so destructive subcommands (backup-delete, backup-clean, history-clean) cannot silently target the wrong cluster's history DB on a multi-cluster jump host. * Drop the "Greenplum" mention in the comment for getHistoryDBPath to keep wording aligned with the Cloudberry environment scripts. * Trim the verbose --history-db description in gpbackman/README.md and add a separate --auto-load-history-db line. * Refine the not-found error message in OpenHistoryDB to mention the new flag rather than implying that setting env vars alone helps. * Tests updated to cover the opt-in matrix: env vars are ignored when the flag is off, honoured when on, and an explicit --history-db still wins regardless. The OpenHistoryDB safe-open change (os.Stat pre-check + mode=rw URI to avoid silently creating an empty SQLite file) is preserved as-is. Signed-off-by: chenqiang --- gpbackman/COMMANDS.md | 10 +++++----- gpbackman/README.md | 3 ++- gpbackman/cmd/backup_clean.go | 4 ++-- gpbackman/cmd/backup_delete.go | 4 ++-- gpbackman/cmd/backup_info.go | 4 ++-- gpbackman/cmd/constants.go | 1 + gpbackman/cmd/history_clean.go | 4 ++-- gpbackman/cmd/report_info.go | 4 ++-- gpbackman/cmd/root.go | 21 +++++++++++++------- gpbackman/cmd/wrappers.go | 24 ++++++++++++---------- gpbackman/cmd/wrappers_test.go | 32 ++++++++++++++++++++---------- gpbackman/gpbckpconfig/utils_db.go | 8 ++++---- 12 files changed, 70 insertions(+), 49 deletions(-) diff --git a/gpbackman/COMMANDS.md b/gpbackman/COMMANDS.md index cf82aa8b..7cb15fd0 100644 --- a/gpbackman/COMMANDS.md +++ b/gpbackman/COMMANDS.md @@ -72,7 +72,7 @@ For non local backups the following logic are applied: The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for under `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`); if neither variable is set, the current directory is used as a last resort. +If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`) instead. Usage: gpbackman backup-clean [flags] @@ -158,7 +158,7 @@ For non local backups the following logic are applied: The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for under `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`); if neither variable is set, the current directory is used as a last resort. +If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`) instead. Usage: gpbackman backup-delete [flags] @@ -256,7 +256,7 @@ To display the "object filtering details" column for all backups without using - The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for under `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`); if neither variable is set, the current directory is used as a last resort. +If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`) instead. Usage: gpbackman backup-info [flags] @@ -455,7 +455,7 @@ Only --older-than-days or --before-timestamp option must be specified, not both. The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for under `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`); if neither variable is set, the current directory is used as a last resort. +If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`) instead. Usage: gpbackman history-clean [flags] @@ -524,7 +524,7 @@ It is not necessary to use the --plugin-report-file-path flag for the following The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for under `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`); if neither variable is set, the current directory is used as a last resort. +If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`) instead. Usage: gpbackman report-info [flags] diff --git a/gpbackman/README.md b/gpbackman/README.md index 923964ad..930e570b 100644 --- a/gpbackman/README.md +++ b/gpbackman/README.md @@ -53,7 +53,8 @@ Available Commands: Flags: -h, --help help for gpbackman - --history-db string full path to the gpbackup_history.db file (if unset, falls back to $COORDINATOR_DATA_DIRECTORY/gpbackup_history.db, then $MASTER_DATA_DIRECTORY/gpbackup_history.db, then the current directory) + --auto-load-history-db resolve gpbackup_history.db from $COORDINATOR_DATA_DIRECTORY (or $MASTER_DATA_DIRECTORY) when --history-db is unset + --history-db string full path to the gpbackup_history.db file --log-file string full path to log file directory, if not specified, the log file will be created in the $HOME/gpAdminLogs directory --log-level-console string level for console logging (error, info, debug, verbose) (default "info") --log-level-file string level for file logging (error, info, debug, verbose) (default "info") diff --git a/gpbackman/cmd/backup_clean.go b/gpbackman/cmd/backup_clean.go index 1ccaf8f8..eacd3418 100644 --- a/gpbackman/cmd/backup_clean.go +++ b/gpbackman/cmd/backup_clean.go @@ -77,7 +77,7 @@ For non local backups the following logic are applied: The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for under $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY); if neither variable is set, the current directory is used as a last resort.`, +If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY) instead, pass the --auto-load-history-db flag.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) @@ -205,7 +205,7 @@ func doCleanBackup() { } func cleanBackup() error { - hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB)) + hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB, rootAutoLoadHistoryDB)) if err != nil { gplog.Error("%s", textmsg.ErrorTextUnableActionHistoryDB("open", err)) return err diff --git a/gpbackman/cmd/backup_delete.go b/gpbackman/cmd/backup_delete.go index 202d52b3..2d4b95f8 100644 --- a/gpbackman/cmd/backup_delete.go +++ b/gpbackman/cmd/backup_delete.go @@ -84,7 +84,7 @@ For non local backups the following logic are applied: The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for under $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY); if neither variable is set, the current directory is used as a last resort.`, +If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY) instead, pass the --auto-load-history-db flag.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) @@ -205,7 +205,7 @@ func doDeleteBackup() { } func deleteBackup() error { - hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB)) + hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB, rootAutoLoadHistoryDB)) if err != nil { gplog.Error("%s", textmsg.ErrorTextUnableActionHistoryDB("open", err)) return err diff --git a/gpbackman/cmd/backup_info.go b/gpbackman/cmd/backup_info.go index 0515e402..adb5c118 100644 --- a/gpbackman/cmd/backup_info.go +++ b/gpbackman/cmd/backup_info.go @@ -98,7 +98,7 @@ To display the "object filtering details" column for all backups without using - The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for under $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY); if neither variable is set, the current directory is used as a last resort.`, +If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY) instead, pass the --auto-load-history-db flag.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) @@ -226,7 +226,7 @@ func backupInfo() error { } t := tablewriter.NewWriter(os.Stdout) initTable(t, opts.ShowDetails) - hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB)) + hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB, rootAutoLoadHistoryDB)) if err != nil { gplog.Error("%s", textmsg.ErrorTextUnableActionHistoryDB("open", err)) return err diff --git a/gpbackman/cmd/constants.go b/gpbackman/cmd/constants.go index 14610c5a..7b5b59ed 100644 --- a/gpbackman/cmd/constants.go +++ b/gpbackman/cmd/constants.go @@ -35,6 +35,7 @@ const ( // Flags. historyDBFlagName = "history-db" + autoLoadHistoryDBFlagName = "auto-load-history-db" logFileFlagName = "log-file" logLevelConsoleFlagName = "log-level-console" logLevelFileFlagName = "log-level-file" diff --git a/gpbackman/cmd/history_clean.go b/gpbackman/cmd/history_clean.go index 836991b8..361f4ee0 100644 --- a/gpbackman/cmd/history_clean.go +++ b/gpbackman/cmd/history_clean.go @@ -50,7 +50,7 @@ Only --older-than-days or --before-timestamp option must be specified, not both. The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for under $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY); if neither variable is set, the current directory is used as a last resort.`, +If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY) instead, pass the --auto-load-history-db flag.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) @@ -106,7 +106,7 @@ func doCleanHistory() { } func cleanHistory() error { - hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB)) + hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB, rootAutoLoadHistoryDB)) if err != nil { gplog.Error("%s", textmsg.ErrorTextUnableActionHistoryDB("open", err)) return err diff --git a/gpbackman/cmd/report_info.go b/gpbackman/cmd/report_info.go index 168695ae..042a3da5 100644 --- a/gpbackman/cmd/report_info.go +++ b/gpbackman/cmd/report_info.go @@ -78,7 +78,7 @@ It is not necessary to use the --plugin-report-file-path flag for the following The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for under $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY); if neither variable is set, the current directory is used as a last resort.`, +If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY) instead, pass the --auto-load-history-db flag.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) @@ -174,7 +174,7 @@ func doReportInfo() { } func reportInfo() error { - hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB)) + hDB, err := gpbckpconfig.OpenHistoryDB(getHistoryDBPath(rootHistoryDB, rootAutoLoadHistoryDB)) if err != nil { gplog.Error("%s", textmsg.ErrorTextUnableActionHistoryDB("open", err)) return err diff --git a/gpbackman/cmd/root.go b/gpbackman/cmd/root.go index e8473269..50884771 100644 --- a/gpbackman/cmd/root.go +++ b/gpbackman/cmd/root.go @@ -33,10 +33,11 @@ var version string // Flags for the gpbackman command (rootCmd) var ( - rootHistoryDB string - rootLogFile string - rootLogLevelConsole string - rootLogLevelFile string + rootHistoryDB string + rootAutoLoadHistoryDB bool + rootLogFile string + rootLogLevelConsole string + rootLogLevelFile string ) var rootCmd = &cobra.Command{ @@ -50,9 +51,15 @@ func init() { &rootHistoryDB, historyDBFlagName, "", - "full path to the gpbackup_history.db file (if unset, falls back to "+ - "$COORDINATOR_DATA_DIRECTORY/gpbackup_history.db, then "+ - "$MASTER_DATA_DIRECTORY/gpbackup_history.db, then the current directory)", + "full path to the gpbackup_history.db file", + ) + rootCmd.PersistentFlags().BoolVar( + &rootAutoLoadHistoryDB, + autoLoadHistoryDBFlagName, + false, + "when --history-db is unset, look up gpbackup_history.db under "+ + "$COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY) before "+ + "falling back to the current directory", ) rootCmd.PersistentFlags().StringVar( &rootLogFile, diff --git a/gpbackman/cmd/wrappers.go b/gpbackman/cmd/wrappers.go index be1c5b59..f809ce4c 100644 --- a/gpbackman/cmd/wrappers.go +++ b/gpbackman/cmd/wrappers.go @@ -82,20 +82,22 @@ func setLogLevelFile(level string) error { } // getHistoryDBPath resolves the path to the gpbackup_history.db file. -// When the --history-db flag is empty, fall back (in order) to the -// COORDINATOR_DATA_DIRECTORY and MASTER_DATA_DIRECTORY environment variables -// that the standard Cloudberry/Greenplum environment scripts export, so that -// users running gpbackman from a sourced cluster shell do not need to repeat -// the path on every invocation. As a last resort, return the bare filename -// (resolved against the current working directory), preserving the previous -// behaviour. -func getHistoryDBPath(historyDBPath string) string { +// An explicit --history-db value always wins. Otherwise, when the caller +// asks for auto-load (--auto-load-history-db), look up the file under the +// COORDINATOR_DATA_DIRECTORY / MASTER_DATA_DIRECTORY environment variables +// exported by the standard Cloudberry environment scripts. As a final +// fallback, return the bare filename so it is resolved against the current +// working directory, preserving the original behaviour for the default +// invocation. +func getHistoryDBPath(historyDBPath string, autoLoad bool) string { if historyDBPath != "" { return historyDBPath } - for _, envVar := range historyDBEnvVars { - if dir := os.Getenv(envVar); dir != "" { - return filepath.Join(dir, historyDBNameConst) + if autoLoad { + for _, envVar := range historyDBEnvVars { + if dir := os.Getenv(envVar); dir != "" { + return filepath.Join(dir, historyDBNameConst) + } } } return historyDBNameConst diff --git a/gpbackman/cmd/wrappers_test.go b/gpbackman/cmd/wrappers_test.go index d7e16093..401f2962 100644 --- a/gpbackman/cmd/wrappers_test.go +++ b/gpbackman/cmd/wrappers_test.go @@ -55,33 +55,43 @@ var _ = Describe("wrappers tests", func() { } }) - It("returns default filename when input is empty and no env vars are set", func() { - Expect(getHistoryDBPath("")).To(Equal(historyDBNameConst)) + It("returns default filename when input is empty (auto-load off, no env)", func() { + Expect(getHistoryDBPath("", false)).To(Equal(historyDBNameConst)) }) It("returns input path when not empty", func() { - Expect(getHistoryDBPath("path/to/" + historyDBNameConst)).To(Equal("path/to/" + historyDBNameConst)) + Expect(getHistoryDBPath("path/to/"+historyDBNameConst, false)).To(Equal("path/to/" + historyDBNameConst)) }) - It("falls back to COORDINATOR_DATA_DIRECTORY when input is empty", func() { + It("ignores env vars by default (auto-load off)", func() { os.Setenv("COORDINATOR_DATA_DIRECTORY", "/coord/data") - Expect(getHistoryDBPath("")).To(Equal(filepath.Join("/coord/data", historyDBNameConst))) + os.Setenv("MASTER_DATA_DIRECTORY", "/master/data") + Expect(getHistoryDBPath("", false)).To(Equal(historyDBNameConst)) + }) + + It("falls back to COORDINATOR_DATA_DIRECTORY when auto-load is on", func() { + os.Setenv("COORDINATOR_DATA_DIRECTORY", "/coord/data") + Expect(getHistoryDBPath("", true)).To(Equal(filepath.Join("/coord/data", historyDBNameConst))) }) - It("falls back to MASTER_DATA_DIRECTORY when COORDINATOR is unset", func() { + It("falls back to MASTER_DATA_DIRECTORY when COORDINATOR is unset and auto-load is on", func() { os.Setenv("MASTER_DATA_DIRECTORY", "/master/data") - Expect(getHistoryDBPath("")).To(Equal(filepath.Join("/master/data", historyDBNameConst))) + Expect(getHistoryDBPath("", true)).To(Equal(filepath.Join("/master/data", historyDBNameConst))) }) - It("prefers COORDINATOR over MASTER when both are set", func() { + It("prefers COORDINATOR over MASTER when both are set and auto-load is on", func() { os.Setenv("COORDINATOR_DATA_DIRECTORY", "/coord/data") os.Setenv("MASTER_DATA_DIRECTORY", "/master/data") - Expect(getHistoryDBPath("")).To(Equal(filepath.Join("/coord/data", historyDBNameConst))) + Expect(getHistoryDBPath("", true)).To(Equal(filepath.Join("/coord/data", historyDBNameConst))) + }) + + It("returns the cwd default when auto-load is on but no env vars are set", func() { + Expect(getHistoryDBPath("", true)).To(Equal(historyDBNameConst)) }) - It("explicit input wins over env vars", func() { + It("explicit input wins over env vars even when auto-load is on", func() { os.Setenv("COORDINATOR_DATA_DIRECTORY", "/coord/data") - Expect(getHistoryDBPath("/explicit/path.db")).To(Equal("/explicit/path.db")) + Expect(getHistoryDBPath("/explicit/path.db", true)).To(Equal("/explicit/path.db")) }) }) diff --git a/gpbackman/gpbckpconfig/utils_db.go b/gpbackman/gpbckpconfig/utils_db.go index 9cf18df5..07e307d9 100644 --- a/gpbackman/gpbckpconfig/utils_db.go +++ b/gpbackman/gpbckpconfig/utils_db.go @@ -42,10 +42,10 @@ func OpenHistoryDB(historyDBPath string) (*sql.DB, error) { if errors.Is(err, os.ErrNotExist) { return nil, fmt.Errorf( "gpbackup history database file not found: %s. "+ - "Specify the path via --history-db, set "+ - "COORDINATOR_DATA_DIRECTORY (or MASTER_DATA_DIRECTORY), "+ - "or run gpbackman from the directory that contains "+ - "gpbackup_history.db", + "Specify the path via --history-db, run gpbackman from "+ + "the directory that contains gpbackup_history.db, or "+ + "pass --auto-load-history-db to resolve it from "+ + "$COORDINATOR_DATA_DIRECTORY (or $MASTER_DATA_DIRECTORY)", historyDBPath, ) } From 70097db8fb6b2c6ff249b640775ac88475764530 Mon Sep 17 00:00:00 2001 From: chenqiang Date: Wed, 29 Apr 2026 16:17:57 +0800 Subject: [PATCH 4/5] gpbackman: wrap non-NotExist os.Stat error with path context in OpenHistoryDB. Previously a permission-denied or I/O failure on the history DB path returned the bare os.Stat error, which gave no hint about which operation or path failed. Wrap it with fmt.Errorf using %w so errors.Is/As still work while surfacing "stat history db " in the message, matching the friendly tone of the ErrNotExist branch. --- gpbackman/gpbckpconfig/utils_db.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gpbackman/gpbckpconfig/utils_db.go b/gpbackman/gpbckpconfig/utils_db.go index 07e307d9..6a834010 100644 --- a/gpbackman/gpbckpconfig/utils_db.go +++ b/gpbackman/gpbckpconfig/utils_db.go @@ -49,7 +49,7 @@ func OpenHistoryDB(historyDBPath string) (*sql.DB, error) { historyDBPath, ) } - return nil, err + return nil, fmt.Errorf("stat history db %q: %w", historyDBPath, err) } // mode=rw opens an existing database for read+write but never creates one. db, err := sql.Open("sqlite3", "file:"+historyDBPath+"?mode=rw") From 2d8868937dbded9159c44b92083980efcea4e8e5 Mon Sep 17 00:00:00 2001 From: chenqiang Date: Wed, 29 Apr 2026 20:48:53 +0800 Subject: [PATCH 5/5] gpbackman: drop MASTER_DATA_DIRECTORY from history-db env-var fallback. Cloudberry switched from MASTER_DATA_DIRECTORY to COORDINATOR_DATA_DIRECTORY starting around release 1.5.4, and the standard environment scripts no longer export the legacy variable. Maintaining a fallback for it in gpbackman just adds surface area (constants, help text, docs, tests) for a value that should not exist in a current Cloudberry install. Drop MASTER_DATA_DIRECTORY entirely from the resolution chain, the flag help, the per-command long help strings, README/COMMANDS docs, and the corresponding test cases. The flag description is shortened in root.go to match the concise wording already used in README. The lingering "Cloudberry/Greenplum" comment in constants.go is updated to "Cloudberry" in the same pass. Resolution chain after this change: explicit --history-db -> $COORDINATOR_DATA_DIRECTORY (only when --auto-load-history-db is set) -> current working directory (default) Addresses review feedback from woblerr, tuhaihe, and MisterRaindrop on PR #97. The pre-existing MASTER_DATA_DIRECTORY reference in .github/workflows/cloudberry-backup-ci.yml predates this PR and is unrelated to history-db resolution; it is left for a separate cleanup. --- gpbackman/COMMANDS.md | 10 +++++----- gpbackman/README.md | 2 +- gpbackman/cmd/backup_clean.go | 2 +- gpbackman/cmd/backup_delete.go | 2 +- gpbackman/cmd/backup_info.go | 2 +- gpbackman/cmd/constants.go | 7 +++---- gpbackman/cmd/history_clean.go | 2 +- gpbackman/cmd/report_info.go | 2 +- gpbackman/cmd/root.go | 4 +--- gpbackman/cmd/wrappers.go | 9 ++++----- gpbackman/cmd/wrappers_test.go | 12 ------------ gpbackman/gpbckpconfig/utils_db.go | 2 +- 12 files changed, 20 insertions(+), 36 deletions(-) diff --git a/gpbackman/COMMANDS.md b/gpbackman/COMMANDS.md index 7cb15fd0..d9e43d47 100644 --- a/gpbackman/COMMANDS.md +++ b/gpbackman/COMMANDS.md @@ -72,7 +72,7 @@ For non local backups the following logic are applied: The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`) instead. +If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` instead. Usage: gpbackman backup-clean [flags] @@ -158,7 +158,7 @@ For non local backups the following logic are applied: The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`) instead. +If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` instead. Usage: gpbackman backup-delete [flags] @@ -256,7 +256,7 @@ To display the "object filtering details" column for all backups without using - The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`) instead. +If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` instead. Usage: gpbackman backup-info [flags] @@ -455,7 +455,7 @@ Only --older-than-days or --before-timestamp option must be specified, not both. The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`) instead. +If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` instead. Usage: gpbackman history-clean [flags] @@ -524,7 +524,7 @@ It is not necessary to use the --plugin-report-file-path flag for the following The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` (then `$MASTER_DATA_DIRECTORY`) instead. +If the --history-db option is not specified, the history database is looked for in the current directory. Pass `--auto-load-history-db` to resolve it from `$COORDINATOR_DATA_DIRECTORY` instead. Usage: gpbackman report-info [flags] diff --git a/gpbackman/README.md b/gpbackman/README.md index 930e570b..d9bd9ae8 100644 --- a/gpbackman/README.md +++ b/gpbackman/README.md @@ -53,7 +53,7 @@ Available Commands: Flags: -h, --help help for gpbackman - --auto-load-history-db resolve gpbackup_history.db from $COORDINATOR_DATA_DIRECTORY (or $MASTER_DATA_DIRECTORY) when --history-db is unset + --auto-load-history-db resolve gpbackup_history.db from $COORDINATOR_DATA_DIRECTORY when --history-db is unset --history-db string full path to the gpbackup_history.db file --log-file string full path to log file directory, if not specified, the log file will be created in the $HOME/gpAdminLogs directory --log-level-console string level for console logging (error, info, debug, verbose) (default "info") diff --git a/gpbackman/cmd/backup_clean.go b/gpbackman/cmd/backup_clean.go index eacd3418..46c9548b 100644 --- a/gpbackman/cmd/backup_clean.go +++ b/gpbackman/cmd/backup_clean.go @@ -77,7 +77,7 @@ For non local backups the following logic are applied: The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY) instead, pass the --auto-load-history-db flag.`, +If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY instead, pass the --auto-load-history-db flag.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) diff --git a/gpbackman/cmd/backup_delete.go b/gpbackman/cmd/backup_delete.go index 2d4b95f8..4eb525a5 100644 --- a/gpbackman/cmd/backup_delete.go +++ b/gpbackman/cmd/backup_delete.go @@ -84,7 +84,7 @@ For non local backups the following logic are applied: The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY) instead, pass the --auto-load-history-db flag.`, +If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY instead, pass the --auto-load-history-db flag.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) diff --git a/gpbackman/cmd/backup_info.go b/gpbackman/cmd/backup_info.go index adb5c118..96674b48 100644 --- a/gpbackman/cmd/backup_info.go +++ b/gpbackman/cmd/backup_info.go @@ -98,7 +98,7 @@ To display the "object filtering details" column for all backups without using - The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY) instead, pass the --auto-load-history-db flag.`, +If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY instead, pass the --auto-load-history-db flag.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) diff --git a/gpbackman/cmd/constants.go b/gpbackman/cmd/constants.go index 7b5b59ed..03b13d36 100644 --- a/gpbackman/cmd/constants.go +++ b/gpbackman/cmd/constants.go @@ -74,11 +74,10 @@ var ( // Timestamp to delete all backups after. afterTimestamp string - // historyDBEnvVars lists, in priority order, the environment variables - // inspected when --history-db is not supplied. They are exported by the - // standard Cloudberry/Greenplum cluster environment scripts. + // historyDBEnvVars lists the environment variables inspected when + // --history-db is not supplied. Exported by the standard Cloudberry + // cluster environment scripts. historyDBEnvVars = []string{ "COORDINATOR_DATA_DIRECTORY", - "MASTER_DATA_DIRECTORY", } ) diff --git a/gpbackman/cmd/history_clean.go b/gpbackman/cmd/history_clean.go index 361f4ee0..42366421 100644 --- a/gpbackman/cmd/history_clean.go +++ b/gpbackman/cmd/history_clean.go @@ -50,7 +50,7 @@ Only --older-than-days or --before-timestamp option must be specified, not both. The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY) instead, pass the --auto-load-history-db flag.`, +If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY instead, pass the --auto-load-history-db flag.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) diff --git a/gpbackman/cmd/report_info.go b/gpbackman/cmd/report_info.go index 042a3da5..36bf1207 100644 --- a/gpbackman/cmd/report_info.go +++ b/gpbackman/cmd/report_info.go @@ -78,7 +78,7 @@ It is not necessary to use the --plugin-report-file-path flag for the following The gpbackup_history.db file location can be set using the --history-db option. Can be specified only once. The full path to the file is required. -If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY) instead, pass the --auto-load-history-db flag.`, +If the --history-db option is not specified, the history database is looked for in the current directory. To resolve it from $COORDINATOR_DATA_DIRECTORY instead, pass the --auto-load-history-db flag.`, Args: cobra.NoArgs, Run: func(cmd *cobra.Command, args []string) { doRootFlagValidation(cmd.Flags(), checkFileExistsConst) diff --git a/gpbackman/cmd/root.go b/gpbackman/cmd/root.go index 50884771..79156cea 100644 --- a/gpbackman/cmd/root.go +++ b/gpbackman/cmd/root.go @@ -57,9 +57,7 @@ func init() { &rootAutoLoadHistoryDB, autoLoadHistoryDBFlagName, false, - "when --history-db is unset, look up gpbackup_history.db under "+ - "$COORDINATOR_DATA_DIRECTORY (then $MASTER_DATA_DIRECTORY) before "+ - "falling back to the current directory", + "resolve gpbackup_history.db from $COORDINATOR_DATA_DIRECTORY when --history-db is unset", ) rootCmd.PersistentFlags().StringVar( &rootLogFile, diff --git a/gpbackman/cmd/wrappers.go b/gpbackman/cmd/wrappers.go index f809ce4c..c36a5256 100644 --- a/gpbackman/cmd/wrappers.go +++ b/gpbackman/cmd/wrappers.go @@ -84,11 +84,10 @@ func setLogLevelFile(level string) error { // getHistoryDBPath resolves the path to the gpbackup_history.db file. // An explicit --history-db value always wins. Otherwise, when the caller // asks for auto-load (--auto-load-history-db), look up the file under the -// COORDINATOR_DATA_DIRECTORY / MASTER_DATA_DIRECTORY environment variables -// exported by the standard Cloudberry environment scripts. As a final -// fallback, return the bare filename so it is resolved against the current -// working directory, preserving the original behaviour for the default -// invocation. +// COORDINATOR_DATA_DIRECTORY environment variable exported by the standard +// Cloudberry environment scripts. As a final fallback, return the bare +// filename so it is resolved against the current working directory, +// preserving the original behaviour for the default invocation. func getHistoryDBPath(historyDBPath string, autoLoad bool) string { if historyDBPath != "" { return historyDBPath diff --git a/gpbackman/cmd/wrappers_test.go b/gpbackman/cmd/wrappers_test.go index 401f2962..10c0d0c5 100644 --- a/gpbackman/cmd/wrappers_test.go +++ b/gpbackman/cmd/wrappers_test.go @@ -65,7 +65,6 @@ var _ = Describe("wrappers tests", func() { It("ignores env vars by default (auto-load off)", func() { os.Setenv("COORDINATOR_DATA_DIRECTORY", "/coord/data") - os.Setenv("MASTER_DATA_DIRECTORY", "/master/data") Expect(getHistoryDBPath("", false)).To(Equal(historyDBNameConst)) }) @@ -74,17 +73,6 @@ var _ = Describe("wrappers tests", func() { Expect(getHistoryDBPath("", true)).To(Equal(filepath.Join("/coord/data", historyDBNameConst))) }) - It("falls back to MASTER_DATA_DIRECTORY when COORDINATOR is unset and auto-load is on", func() { - os.Setenv("MASTER_DATA_DIRECTORY", "/master/data") - Expect(getHistoryDBPath("", true)).To(Equal(filepath.Join("/master/data", historyDBNameConst))) - }) - - It("prefers COORDINATOR over MASTER when both are set and auto-load is on", func() { - os.Setenv("COORDINATOR_DATA_DIRECTORY", "/coord/data") - os.Setenv("MASTER_DATA_DIRECTORY", "/master/data") - Expect(getHistoryDBPath("", true)).To(Equal(filepath.Join("/coord/data", historyDBNameConst))) - }) - It("returns the cwd default when auto-load is on but no env vars are set", func() { Expect(getHistoryDBPath("", true)).To(Equal(historyDBNameConst)) }) diff --git a/gpbackman/gpbckpconfig/utils_db.go b/gpbackman/gpbckpconfig/utils_db.go index 6a834010..48cc28d7 100644 --- a/gpbackman/gpbckpconfig/utils_db.go +++ b/gpbackman/gpbckpconfig/utils_db.go @@ -45,7 +45,7 @@ func OpenHistoryDB(historyDBPath string) (*sql.DB, error) { "Specify the path via --history-db, run gpbackman from "+ "the directory that contains gpbackup_history.db, or "+ "pass --auto-load-history-db to resolve it from "+ - "$COORDINATOR_DATA_DIRECTORY (or $MASTER_DATA_DIRECTORY)", + "$COORDINATOR_DATA_DIRECTORY", historyDBPath, ) }