You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudberry.apache.org by dj...@apache.org on 2026/04/30 01:16:41 UTC
(cloudberry-backup) 03/05: gpbackman: gate history-db env-var fallback behind --auto-load-history-db opt-in.
This is an automated email from the ASF dual-hosted git repository.
tuhaihe pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/cloudberry-backup.git
commit de5a244925cada1ba94f2084336b9a830d3858d3
Author: chenqiang <ch...@hashdata.cn>
AuthorDate: Wed Apr 29 15:35:38 2026 +0800
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 <ch...@hashdata.cn>
---
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,
)
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org