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/09/02 01:59:03 UTC

(cloudberry-backup) 32/40: gpbackman: drop MASTER_DATA_DIRECTORY from history-db env-var fallback.

This is an automated email from the ASF dual-hosted git repository.

tuhaihe pushed a commit to branch REL_2_STABLE
in repository https://gitbox.apache.org/repos/asf/cloudberry-backup.git

commit 92b995f735748ccb57b890af273d69f7cef70dae
Author: chenqiang <ch...@hashdata.cn>
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: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org