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:58:59 UTC
(cloudberry-backup) 28/40: gpbackman: fall back to coordinator data dir for history db, fail loud when missing.
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 a96766b95fbc95bb5e673a24fb7d469a312174d3
Author: chenqiang <ch...@hashdata.cn>
AuthorDate: Wed Apr 29 12:45:47 2026 +0800
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 <ch...@hashdata.cn>
---
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 {
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org