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 7741382a40f0e77739b559075385fc34d311597a Author: chenqiang <[email protected]> AuthorDate: Wed Apr 29 20:48:53 2026 +0800 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, ) } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
