From b886c71b22d584cf47d70ba476b1586c44210ec9 Mon Sep 17 00:00:00 2001 From: woblerr Date: Tue, 28 Jul 2026 16:33:19 +0000 Subject: [PATCH 01/19] Add gpbackman coordinator discovery helpers. --- gpbackman/gpbckpconfig/cluster.go | 63 +++++- gpbackman/gpbckpconfig/cluster_test.go | 257 +++++++++++++++++++++++++ 2 files changed, 319 insertions(+), 1 deletion(-) create mode 100644 gpbackman/gpbckpconfig/cluster_test.go diff --git a/gpbackman/gpbckpconfig/cluster.go b/gpbackman/gpbckpconfig/cluster.go index 93f6f7c8..0e03ca7d 100644 --- a/gpbackman/gpbckpconfig/cluster.go +++ b/gpbackman/gpbckpconfig/cluster.go @@ -35,6 +35,20 @@ type SegmentConfig struct { DataDir string } +// StandbyCoordinator stores the standby coordinator connection target. +type StandbyCoordinator struct { + Hostname string `db:"hostname"` + DataDir string `db:"datadir"` +} + +const ( + defaultClusterDatabase = "postgres" + primaryCoordinatorDataDirSQL = "SELECT datadir FROM gp_segment_configuration WHERE content = -1 AND role = 'p' AND status = 'u';" + upStandbyCoordinatorSQL = "SELECT hostname, datadir FROM gp_segment_configuration WHERE content = -1 AND role = 'm' AND status = 'u';" +) + +var connectLocalCluster = sqlx.Connect + // NewClusterLocalClusterConn creates a new connection to the local postgres database. // Returns an error if the connection could not be established. func NewClusterLocalClusterConn(dbName string) (*sqlx.DB, error) { @@ -55,7 +69,46 @@ func NewClusterLocalClusterConn(dbName string) (*sqlx.DB, error) { port = 5432 } connStr := fmt.Sprintf("postgres://%s@%s:%d/%s?sslmode=disable&connect_timeout=60", username, host, port, dbName) - return sqlx.Connect("postgres", connStr) + return connectLocalCluster("postgres", connStr) +} + +// NewClusterLocalClusterDefaultConn creates a local cluster connection using PGDATABASE or postgres. +func NewClusterLocalClusterDefaultConn() (*sqlx.DB, error) { + dbName := operating.System.Getenv("PGDATABASE") + if dbName == "" { + dbName = defaultClusterDatabase + } + return NewClusterLocalClusterConn(dbName) +} + +// GetPrimaryCoordinatorDataDir returns the up primary coordinator data directory. +func GetPrimaryCoordinatorDataDir() (string, error) { + db, err := NewClusterLocalClusterDefaultConn() + if err != nil { + return "", err + } + defer db.Close() + return QueryPrimaryCoordinatorDataDir(db) +} + +// QueryPrimaryCoordinatorDataDir queries the up primary coordinator data directory. +func QueryPrimaryCoordinatorDataDir(conn *sqlx.DB) (string, error) { + return ExecuteQueryLocalClusterConn[string](conn, primaryCoordinatorDataDirSQL) +} + +// GetUpStandbyCoordinator returns the up standby coordinator from the local cluster catalog. +func GetUpStandbyCoordinator() (StandbyCoordinator, error) { + db, err := NewClusterLocalClusterDefaultConn() + if err != nil { + return StandbyCoordinator{}, err + } + defer db.Close() + return QueryUpStandbyCoordinator(db) +} + +// QueryUpStandbyCoordinator queries the up standby coordinator from the local cluster catalog. +func QueryUpStandbyCoordinator(conn *sqlx.DB) (StandbyCoordinator, error) { + return ExecuteQueryLocalClusterConn[StandbyCoordinator](conn, upStandbyCoordinatorSQL) } // ExecuteQueryLocalClusterConn executes a query on the local cluster connection and returns the result. @@ -72,6 +125,7 @@ func NewClusterLocalClusterConn(dbName string) (*sqlx.DB, error) { // The function supports the following types for T: // - string: The result will be a single string value. // - []SegmentConfig: The result will be a slice of SegmentConfig structs. +// - StandbyCoordinator: The result will be a standby coordinator struct. // // If the type T is not supported, the function returns an error indicating the unsupported type. func ExecuteQueryLocalClusterConn[T any](conn *sqlx.DB, query string) (T, error) { @@ -91,6 +145,13 @@ func ExecuteQueryLocalClusterConn[T any](conn *sqlx.DB, query string) (T, error) return result, err } result = any(segConfigs).(T) + case StandbyCoordinator: + var standbyCoordinator StandbyCoordinator + err := conn.Get(&standbyCoordinator, query) + if err != nil { + return result, err + } + result = any(standbyCoordinator).(T) default: return result, fmt.Errorf("unsupported type") } diff --git a/gpbackman/gpbckpconfig/cluster_test.go b/gpbackman/gpbckpconfig/cluster_test.go new file mode 100644 index 00000000..3e786f8e --- /dev/null +++ b/gpbackman/gpbckpconfig/cluster_test.go @@ -0,0 +1,257 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +*/ + +package gpbckpconfig + +import ( + "database/sql" + "errors" + "os" + "regexp" + + "github.com/DATA-DOG/go-sqlmock" + "github.com/jmoiron/sqlx" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +type savedEnvValue struct { + value string + ok bool +} + +var _ = Describe("cluster tests", func() { + var ( + originalConnect func(string, string) (*sqlx.DB, error) + savedEnv map[string]savedEnvValue + ) + + BeforeEach(func() { + originalConnect = connectLocalCluster + savedEnv = saveClusterEnv("PGDATABASE", "PGUSER", "PGHOST", "PGPORT") + }) + + AfterEach(func() { + connectLocalCluster = originalConnect + restoreClusterEnv(savedEnv) + }) + + Describe("NewClusterLocalClusterDefaultConn", func() { + It("uses PGDATABASE when it is set", func() { + setClusterEnv(map[string]string{ + "PGDATABASE": "template1", + "PGUSER": "backup_user", + "PGHOST": "coordinator", + "PGPORT": "15432", + }) + sqlDB, mock, err := sqlmock.New() + Expect(err).NotTo(HaveOccurred()) + defer sqlDB.Close() + var gotDriver string + var gotConnStr string + connectLocalCluster = func(driverName, dataSourceName string) (*sqlx.DB, error) { + gotDriver = driverName + gotConnStr = dataSourceName + return sqlx.NewDb(sqlDB, "sqlmock"), nil + } + + db, err := NewClusterLocalClusterDefaultConn() + + Expect(err).NotTo(HaveOccurred()) + mock.ExpectClose() + Expect(db.Close()).To(Succeed()) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + Expect(gotDriver).To(Equal("postgres")) + Expect(gotConnStr).To(Equal("postgres://backup_user@coordinator:15432/template1?sslmode=disable&connect_timeout=60")) + }) + + It("falls back to postgres when PGDATABASE is not set", func() { + setClusterEnv(map[string]string{ + "PGUSER": "backup_user", + "PGHOST": "coordinator", + "PGPORT": "15432", + }) + sqlDB, mock, err := sqlmock.New() + Expect(err).NotTo(HaveOccurred()) + defer sqlDB.Close() + var gotConnStr string + connectLocalCluster = func(driverName, dataSourceName string) (*sqlx.DB, error) { + gotConnStr = dataSourceName + return sqlx.NewDb(sqlDB, "sqlmock"), nil + } + + db, err := NewClusterLocalClusterDefaultConn() + + Expect(err).NotTo(HaveOccurred()) + mock.ExpectClose() + Expect(db.Close()).To(Succeed()) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + Expect(gotConnStr).To(Equal("postgres://backup_user@coordinator:15432/postgres?sslmode=disable&connect_timeout=60")) + }) + + It("returns connection errors", func() { + setClusterEnv(map[string]string{ + "PGUSER": "backup_user", + "PGHOST": "coordinator", + "PGPORT": "15432", + }) + connectErr := errors.New("connection failed") + connectLocalCluster = func(driverName, dataSourceName string) (*sqlx.DB, error) { + return nil, connectErr + } + + db, err := NewClusterLocalClusterDefaultConn() + + Expect(db).To(BeNil()) + Expect(err).To(MatchError(connectErr)) + }) + }) + + Describe("QueryPrimaryCoordinatorDataDir", func() { + It("returns the primary coordinator data directory", func() { + db, mock := newClusterSQLMock() + defer db.Close() + mock.ExpectQuery(regexp.QuoteMeta(primaryCoordinatorDataDirSQL)). + WillReturnRows(sqlmock.NewRows([]string{"datadir"}).AddRow("/data/primary")) + + dataDir, err := QueryPrimaryCoordinatorDataDir(db) + + Expect(err).NotTo(HaveOccurred()) + Expect(dataDir).To(Equal("/data/primary")) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + }) + + It("returns query errors", func() { + db, mock := newClusterSQLMock() + defer db.Close() + queryErr := errors.New("query failed") + mock.ExpectQuery(regexp.QuoteMeta(primaryCoordinatorDataDirSQL)).WillReturnError(queryErr) + + dataDir, err := QueryPrimaryCoordinatorDataDir(db) + + Expect(dataDir).To(BeEmpty()) + Expect(err).To(MatchError(queryErr)) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + }) + }) + + Describe("QueryUpStandbyCoordinator", func() { + It("returns the up standby coordinator", func() { + db, mock := newClusterSQLMock() + defer db.Close() + mock.ExpectQuery(regexp.QuoteMeta(upStandbyCoordinatorSQL)). + WillReturnRows(sqlmock.NewRows([]string{"hostname", "datadir"}).AddRow("standby-host", "/data/standby")) + + standbyCoordinator, err := QueryUpStandbyCoordinator(db) + + Expect(err).NotTo(HaveOccurred()) + Expect(standbyCoordinator).To(Equal(StandbyCoordinator{ + Hostname: "standby-host", + DataDir: "/data/standby", + })) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + }) + + It("returns sql.ErrNoRows when no up standby is present", func() { + db, mock := newClusterSQLMock() + defer db.Close() + mock.ExpectQuery(regexp.QuoteMeta(upStandbyCoordinatorSQL)). + WillReturnRows(sqlmock.NewRows([]string{"hostname", "datadir"})) + + standbyCoordinator, err := QueryUpStandbyCoordinator(db) + + Expect(standbyCoordinator).To(Equal(StandbyCoordinator{})) + Expect(errors.Is(err, sql.ErrNoRows)).To(BeTrue()) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + }) + + It("returns query errors", func() { + db, mock := newClusterSQLMock() + defer db.Close() + queryErr := errors.New("query failed") + mock.ExpectQuery(regexp.QuoteMeta(upStandbyCoordinatorSQL)).WillReturnError(queryErr) + + standbyCoordinator, err := QueryUpStandbyCoordinator(db) + + Expect(standbyCoordinator).To(Equal(StandbyCoordinator{})) + Expect(err).To(MatchError(queryErr)) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + }) + }) + + Describe("GetPrimaryCoordinatorDataDir", func() { + It("returns connection errors", func() { + connectErr := errors.New("connection failed") + connectLocalCluster = func(driverName, dataSourceName string) (*sqlx.DB, error) { + return nil, connectErr + } + + dataDir, err := GetPrimaryCoordinatorDataDir() + + Expect(dataDir).To(BeEmpty()) + Expect(err).To(MatchError(connectErr)) + }) + }) + + Describe("GetUpStandbyCoordinator", func() { + It("returns connection errors", func() { + connectErr := errors.New("connection failed") + connectLocalCluster = func(driverName, dataSourceName string) (*sqlx.DB, error) { + return nil, connectErr + } + + standbyCoordinator, err := GetUpStandbyCoordinator() + + Expect(standbyCoordinator).To(Equal(StandbyCoordinator{})) + Expect(err).To(MatchError(connectErr)) + }) + }) +}) + +func newClusterSQLMock() (*sqlx.DB, sqlmock.Sqlmock) { + sqlDB, mock, err := sqlmock.New() + Expect(err).NotTo(HaveOccurred()) + return sqlx.NewDb(sqlDB, "sqlmock"), mock +} + +func saveClusterEnv(names ...string) map[string]savedEnvValue { + saved := make(map[string]savedEnvValue, len(names)) + for _, name := range names { + value, ok := os.LookupEnv(name) + saved[name] = savedEnvValue{value: value, ok: ok} + _ = os.Unsetenv(name) + } + return saved +} + +func restoreClusterEnv(saved map[string]savedEnvValue) { + for name, envValue := range saved { + if envValue.ok { + _ = os.Setenv(name, envValue.value) + continue + } + _ = os.Unsetenv(name) + } +} + +func setClusterEnv(values map[string]string) { + for name, value := range values { + _ = os.Setenv(name, value) + } +} From bc05be9ff91c6dc8d6d0bcf2ee5841872c46228b Mon Sep 17 00:00:00 2001 From: woblerr Date: Tue, 28 Jul 2026 16:45:14 +0000 Subject: [PATCH 02/19] Add gpbackup standby history sync. --- backup/backup.go | 4 + backup/history_standby_sync.go | 412 ++++++++++++++++++++++++++++ backup/history_standby_sync_test.go | 403 +++++++++++++++++++++++++++ options/flag.go | 82 +++--- options/flag_test.go | 19 ++ 5 files changed, 880 insertions(+), 40 deletions(-) create mode 100644 backup/history_standby_sync.go create mode 100644 backup/history_standby_sync_test.go diff --git a/backup/backup.go b/backup/backup.go index 9e12918d..c0032746 100644 --- a/backup/backup.go +++ b/backup/backup.go @@ -509,6 +509,7 @@ func DoCleanup(backupFailed bool) { // failure; in either case, update the end time to the actual value. Between our signal handler and recovering // panics, there should be no way for gpbackup to exit that leaves the entry in the initial status. + historyUpdated := false if !MustGetFlagBool(options.NO_HISTORY) { var statusString string if backupFailed { @@ -525,9 +526,12 @@ func DoCleanup(backupFailed bool) { historyDB.Close() if err != nil { gplog.Error("Unable to update history database. Error: %v", err) + } else { + historyUpdated = true } } } + syncBackupHistoryToStandbyAfterCleanup(backupFailed, historyUpdated) err := backupLockFile.Unlock() if err != nil && backupLockFile != "" { diff --git a/backup/history_standby_sync.go b/backup/history_standby_sync.go new file mode 100644 index 00000000..9b46b9bc --- /dev/null +++ b/backup/history_standby_sync.go @@ -0,0 +1,412 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +*/ + +package backup + +import ( + "database/sql" + "errors" + "fmt" + "net/url" + "os" + "os/exec" + "path/filepath" + "strings" + + "github.com/apache/cloudberry-backup/options" + "github.com/apache/cloudberry-go-libs/gplog" + "github.com/apache/cloudberry-go-libs/operating" + _ "github.com/mattn/go-sqlite3" + "github.com/nightlyone/lockfile" +) + +const ( + backupHistoryDBName = "gpbackup_history.db" + backupHistoryStandbySyncSSHOptions = "ssh -o StrictHostKeyChecking=no -o ConnectTimeout=30" + backupHistoryStandbySyncTempDirPattern = "gpbackup-history-standby-sync-*" + backupHistoryStandbySyncStandbySQL = "SELECT hostname, datadir FROM gp_segment_configuration WHERE content = -1 AND role = 'm' AND status = 'u';" +) + +type backupHistoryStandbySyncTarget struct { + sourceDBPath string + standbyHost string + standbyDataDir string + standbyHistoryDBPath string +} + +type backupHistoryStandbySyncStandby struct { + Hostname string `db:"hostname"` + DataDir string `db:"datadir"` +} + +type backupHistoryStandbySyncCommand interface { + CombinedOutput() ([]byte, error) +} + +var ( + backupHistoryStandbySync = syncBackupHistoryToStandby + + backupHistoryStandbySyncCommandExec = func(name string, args ...string) backupHistoryStandbySyncCommand { + return exec.Command(name, args...) + } + backupHistoryStandbySyncMkdirTemp = os.MkdirTemp + backupHistoryStandbySyncRemoveAll = os.RemoveAll + backupHistoryStandbySyncCurrentUser = func() (string, error) { + currentUser, err := operating.System.CurrentUser() + if err != nil { + return "", err + } + return currentUser.Username, nil + } +) + +func syncBackupHistoryToStandbyBestEffort(disabled bool) (string, error) { + if disabled { + skipReason := "disabled by --" + options.NO_HISTORY_SYNC_STANDBY + gplog.Info("Skipping history db sync to standby coordinator: %s", skipReason) + return skipReason, nil + } + + skipReason, err := backupHistoryStandbySync() + if err != nil { + gplog.Warn("History db sync to standby coordinator failed; standby history may be stale: %v", err) + return "", err + } + if skipReason != "" { + gplog.Debug("Skipping history db sync to standby coordinator: %s", skipReason) + } + return skipReason, nil +} + +func syncBackupHistoryToStandbyAfterCleanup(backupFailed bool, historyUpdated bool) { + if backupFailed || !historyUpdated || MustGetFlagBool(options.NO_HISTORY) { + return + } + _, _ = syncBackupHistoryToStandbyBestEffort(MustGetFlagBool(options.NO_HISTORY_SYNC_STANDBY)) +} + +func syncBackupHistoryToStandby() (string, error) { + sourceDBPath, sourceInfo, err := canonicalBackupHistoryStandbySyncSource(globalFPInfo.GetBackupHistoryDatabasePath()) + if err != nil { + return "", err + } + + target, skipReason, err := discoverBackupHistoryStandbySyncTarget(sourceDBPath) + if err != nil { + return "", err + } + if skipReason != "" { + return skipReason, nil + } + + userName, err := backupHistoryStandbySyncCurrentUser() + if err != nil { + return "", fmt.Errorf("resolve current OS user for standby history sync: %w", err) + } + + err = withBackupHistoryStandbySyncLock(sourceDBPath, func() error { + return withBackupHistoryStandbySyncSnapshot(sourceDBPath, sourceInfo.Mode().Perm(), func(snapshotPath string) error { + return syncBackupHistoryStandbySnapshot(target, userName, snapshotPath) + }) + }) + if err != nil { + return "", err + } + return "", nil +} + +func canonicalBackupHistoryStandbySyncSource(sourceDBPath string) (string, os.FileInfo, error) { + absoluteSourceDBPath, err := filepath.Abs(filepath.Clean(sourceDBPath)) + if err != nil { + return "", nil, fmt.Errorf("resolve absolute source history db path for standby sync: %w", err) + } + canonicalSourceDBPath, err := filepath.EvalSymlinks(absoluteSourceDBPath) + if err != nil { + return "", nil, fmt.Errorf("resolve canonical source history db path for standby sync: %w", err) + } + sourceInfo, err := os.Stat(canonicalSourceDBPath) + if err != nil { + return "", nil, fmt.Errorf("stat source history db for standby sync: %w", err) + } + if !sourceInfo.Mode().IsRegular() { + return "", nil, fmt.Errorf("source history db for standby sync is not a regular file: %s", canonicalSourceDBPath) + } + return canonicalSourceDBPath, sourceInfo, nil +} + +func discoverBackupHistoryStandbySyncTarget(sourceDBPath string) (*backupHistoryStandbySyncTarget, string, error) { + standby, err := queryBackupHistoryStandbySyncStandby() + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, "no up standby coordinator found", nil + } + return nil, "", fmt.Errorf("query up standby coordinator for standby history sync discovery: %w", err) + } + target := &backupHistoryStandbySyncTarget{ + sourceDBPath: sourceDBPath, + standbyHost: standby.Hostname, + standbyDataDir: standby.DataDir, + standbyHistoryDBPath: filepath.Join(standby.DataDir, backupHistoryDBName), + } + gplog.Debug("Discovered standby history sync target: source=%s standby=%s:%s", target.sourceDBPath, target.standbyHost, target.standbyHistoryDBPath) + return target, "", nil +} + +func queryBackupHistoryStandbySyncStandby() (backupHistoryStandbySyncStandby, error) { + var standby backupHistoryStandbySyncStandby + if connectionPool == nil { + return standby, errors.New("connection pool is not initialized") + } + err := connectionPool.Get(&standby, backupHistoryStandbySyncStandbySQL) + return standby, err +} + +func withBackupHistoryStandbySyncLock(sourceDBPath string, syncFn func() error) error { + lockPath := backupHistoryStandbySyncLockPath(sourceDBPath) + sourceLock, err := lockfile.New(lockPath) + if err != nil { + return fmt.Errorf("create standby history sync lock %s: %w", lockPath, err) + } + if err := sourceLock.TryLock(); err != nil { + return fmt.Errorf("lock standby history sync source %s: %w", sourceDBPath, err) + } + + syncErr := syncFn() + unlockErr := sourceLock.Unlock() + if syncErr != nil { + if unlockErr != nil { + return fmt.Errorf("%w; additionally failed to release standby history sync lock %s: %v", syncErr, lockPath, unlockErr) + } + return syncErr + } + if unlockErr != nil { + return fmt.Errorf("release standby history sync lock %s: %w", lockPath, unlockErr) + } + return nil +} + +func backupHistoryStandbySyncLockPath(sourceDBPath string) string { + return sourceDBPath + ".sync.lock" +} + +func withBackupHistoryStandbySyncSnapshot(sourceDBPath string, sourceMode os.FileMode, syncFn func(string) error) error { + snapshotPath, tempDir, err := createBackupHistoryStandbySyncSnapshot(sourceDBPath, sourceMode) + if tempDir != "" { + defer cleanupBackupHistoryStandbySyncTempDir(tempDir) + } + if err != nil { + return err + } + return syncFn(snapshotPath) +} + +func createBackupHistoryStandbySyncSnapshot(sourceDBPath string, sourceMode os.FileMode) (string, string, error) { + tempDir, err := backupHistoryStandbySyncMkdirTemp("", backupHistoryStandbySyncTempDirPattern) + if err != nil { + return "", "", fmt.Errorf("create local standby history sync temp directory: %w", err) + } + snapshotPath := filepath.Join(tempDir, backupHistoryDBName) + if err := vacuumBackupHistoryStandbySyncSnapshot(sourceDBPath, snapshotPath); err != nil { + cleanupBackupHistoryStandbySyncTempDir(tempDir) + return "", "", err + } + if err := os.Chmod(snapshotPath, sourceMode); err != nil { + cleanupBackupHistoryStandbySyncTempDir(tempDir) + return "", "", fmt.Errorf("set standby history sync snapshot permissions from source history db: %w", err) + } + if err := validateBackupHistoryStandbySyncSnapshot(snapshotPath); err != nil { + cleanupBackupHistoryStandbySyncTempDir(tempDir) + return "", "", err + } + return snapshotPath, tempDir, nil +} + +func vacuumBackupHistoryStandbySyncSnapshot(sourceDBPath, snapshotPath string) error { + sourceDB, err := sql.Open("sqlite3", backupHistoryStandbySyncSQLiteURI(sourceDBPath, "ro")) + if err != nil { + return fmt.Errorf("open source history db for standby sync snapshot: %w", err) + } + defer sourceDB.Close() + + if _, err := sourceDB.Exec("VACUUM main INTO ?", snapshotPath); err != nil { + return fmt.Errorf("create standby history sync snapshot with VACUUM INTO: %w", err) + } + return nil +} + +func validateBackupHistoryStandbySyncSnapshot(snapshotPath string) error { + results, err := runBackupHistoryStandbySyncQuickCheck(snapshotPath) + if err != nil { + return err + } + if len(results) != 1 || results[0] != "ok" { + return fmt.Errorf("validate standby history sync snapshot quick_check: expected single ok result, got %v", results) + } + return nil +} + +func runBackupHistoryStandbySyncQuickCheck(snapshotPath string) ([]string, error) { + snapshotDB, err := sql.Open("sqlite3", backupHistoryStandbySyncSQLiteURI(snapshotPath, "ro")) + if err != nil { + return nil, fmt.Errorf("open standby history sync snapshot read-only: %w", err) + } + defer snapshotDB.Close() + + rows, err := snapshotDB.Query("PRAGMA quick_check") + if err != nil { + return nil, fmt.Errorf("run PRAGMA quick_check on standby history sync snapshot: %w", err) + } + defer rows.Close() + + results := make([]string, 0) + for rows.Next() { + var result string + if err := rows.Scan(&result); err != nil { + return nil, fmt.Errorf("scan PRAGMA quick_check result for standby history sync snapshot: %w", err) + } + results = append(results, result) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("read PRAGMA quick_check results for standby history sync snapshot: %w", err) + } + return results, nil +} + +func cleanupBackupHistoryStandbySyncTempDir(tempDir string) { + if err := backupHistoryStandbySyncRemoveAll(tempDir); err != nil && !errors.Is(err, os.ErrNotExist) { + gplog.Debug("Unable to remove local standby history sync temp directory %s: %v", tempDir, err) + } +} + +func syncBackupHistoryStandbySnapshot(target *backupHistoryStandbySyncTarget, userName, snapshotPath string) error { + remoteTempPath := newBackupHistoryStandbySyncRemoteTempPath(target.standbyDataDir, snapshotPath) + if err := rsyncBackupHistoryStandbySyncSnapshot(snapshotPath, target.standbyHost, userName, remoteTempPath); err != nil { + return cleanupBackupHistoryStandbySyncRemoteTempAfterError(err, target.standbyHost, userName, remoteTempPath) + } + if err := installBackupHistoryStandbySyncSnapshot(target, userName, remoteTempPath); err != nil { + return cleanupBackupHistoryStandbySyncRemoteTempAfterError(err, target.standbyHost, userName, remoteTempPath) + } + return nil +} + +func newBackupHistoryStandbySyncRemoteTempPath(standbyDataDir, snapshotPath string) string { + return filepath.Join(standbyDataDir, fmt.Sprintf(".%s.%s.tmp", backupHistoryDBName, filepath.Base(filepath.Dir(snapshotPath)))) +} + +func rsyncBackupHistoryStandbySyncSnapshot(snapshotPath, standbyHost, userName, remoteTempPath string) error { + args := buildBackupHistoryStandbySyncRsyncArgs(snapshotPath, standbyHost, userName, remoteTempPath) + gplog.Debug("Transfer history db snapshot to standby coordinator: %s -> %s:%s", snapshotPath, standbyHost, remoteTempPath) + output, err := backupHistoryStandbySyncCommandExec("rsync", args...).CombinedOutput() + if err != nil { + return fmt.Errorf("rsync standby history snapshot to %s:%s failed: %w%s", standbyHost, remoteTempPath, err, formatBackupHistoryStandbySyncCommandOutput(output)) + } + return nil +} + +func buildBackupHistoryStandbySyncRsyncArgs(snapshotPath, standbyHost, userName, remoteTempPath string) []string { + return []string{ + "-p", + "-e", + backupHistoryStandbySyncSSHOptions, + "--", + snapshotPath, + fmt.Sprintf("%s@%s:%s", userName, standbyHost, shellQuoteBackupHistoryStandbySyncPath(remoteTempPath)), + } +} + +func installBackupHistoryStandbySyncSnapshot(target *backupHistoryStandbySyncTarget, userName, remoteTempPath string) error { + command := buildBackupHistoryStandbySyncRemoteInstallCommand(remoteTempPath, target.standbyHistoryDBPath) + gplog.Debug("Install history db snapshot on standby coordinator: %s:%s", target.standbyHost, target.standbyHistoryDBPath) + output, err := runBackupHistoryStandbySyncSSHCommand(command, target.standbyHost, userName) + if err != nil { + return fmt.Errorf("install standby history snapshot on %s:%s failed: %w%s", target.standbyHost, target.standbyHistoryDBPath, err, formatBackupHistoryStandbySyncCommandOutput(output)) + } + return nil +} + +func buildBackupHistoryStandbySyncRemoteInstallCommand(remoteTempPath, standbyHistoryDBPath string) string { + quotedTempPath := shellQuoteBackupHistoryStandbySyncPath(remoteTempPath) + quotedHistoryDBPath := shellQuoteBackupHistoryStandbySyncPath(standbyHistoryDBPath) + return fmt.Sprintf( + "test -f %s && if test -e %s; then chown --reference=%s -- %s && chmod --reference=%s -- %s; fi && mv -f -- %s %s", + quotedTempPath, + quotedHistoryDBPath, + quotedHistoryDBPath, + quotedTempPath, + quotedHistoryDBPath, + quotedTempPath, + quotedTempPath, + quotedHistoryDBPath, + ) +} + +func cleanupBackupHistoryStandbySyncRemoteTempAfterError(primaryErr error, standbyHost, userName, remoteTempPath string) error { + if cleanupErr := cleanupBackupHistoryStandbySyncRemoteTemp(standbyHost, userName, remoteTempPath); cleanupErr != nil { + return fmt.Errorf("%w; additionally failed to clean up remote temp file: %v", primaryErr, cleanupErr) + } + return primaryErr +} + +func cleanupBackupHistoryStandbySyncRemoteTemp(standbyHost, userName, remoteTempPath string) error { + command := buildBackupHistoryStandbySyncRemoteCleanupCommand(remoteTempPath) + gplog.Debug("Clean up remote standby history sync temp file: %s:%s", standbyHost, remoteTempPath) + output, err := runBackupHistoryStandbySyncSSHCommand(command, standbyHost, userName) + if err != nil { + return fmt.Errorf("clean up remote standby history sync temp file %s:%s failed: %w%s", standbyHost, remoteTempPath, err, formatBackupHistoryStandbySyncCommandOutput(output)) + } + return nil +} + +func buildBackupHistoryStandbySyncRemoteCleanupCommand(remoteTempPath string) string { + return fmt.Sprintf("rm -f -- %s", shellQuoteBackupHistoryStandbySyncPath(remoteTempPath)) +} + +func runBackupHistoryStandbySyncSSHCommand(remoteCommand, standbyHost, userName string) ([]byte, error) { + return backupHistoryStandbySyncCommandExec( + "ssh", + "-o", + "StrictHostKeyChecking=no", + "-o", + "ConnectTimeout=30", + fmt.Sprintf("%s@%s", userName, standbyHost), + remoteCommand, + ).CombinedOutput() +} + +func backupHistoryStandbySyncSQLiteURI(dbPath, mode string) string { + query := url.Values{} + query.Set("mode", mode) + dbURI := url.URL{Scheme: "file", Path: dbPath, RawQuery: query.Encode()} + return dbURI.String() +} + +func shellQuoteBackupHistoryStandbySyncPath(value string) string { + if value == "" { + return "''" + } + return "'" + strings.ReplaceAll(value, "'", "'\"'\"'") + "'" +} + +func formatBackupHistoryStandbySyncCommandOutput(output []byte) string { + trimmedOutput := strings.TrimSpace(string(output)) + if trimmedOutput == "" { + return "" + } + return ": " + trimmedOutput +} diff --git a/backup/history_standby_sync_test.go b/backup/history_standby_sync_test.go new file mode 100644 index 00000000..3f4fcd12 --- /dev/null +++ b/backup/history_standby_sync_test.go @@ -0,0 +1,403 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +*/ + +package backup + +import ( + "database/sql" + "errors" + "fmt" + "os" + "path/filepath" + "regexp" + + "github.com/DATA-DOG/go-sqlmock" + backupfilepath "github.com/apache/cloudberry-backup/filepath" + "github.com/apache/cloudberry-backup/options" + "github.com/apache/cloudberry-go-libs/dbconn" + "github.com/apache/cloudberry-go-libs/testhelper" + "github.com/jmoiron/sqlx" + "github.com/nightlyone/lockfile" + "github.com/spf13/pflag" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +type backupHistoryStandbySyncCommandCall struct { + name string + args []string +} + +type backupHistoryStandbySyncCommandResponse struct { + output []byte + err error +} + +type backupHistoryStandbySyncFakeCommand struct { + output []byte + err error +} + +func (c backupHistoryStandbySyncFakeCommand) CombinedOutput() ([]byte, error) { + return c.output, c.err +} + +var _ = Describe("backup history standby sync", func() { + var originalSync func() (string, error) + + BeforeEach(func() { + testhelper.SetupTestLogger() + cmdFlags = pflag.NewFlagSet("gpbackup", pflag.ContinueOnError) + options.SetBackupFlagDefaults(cmdFlags) + globalFPInfo = backupfilepath.FilePathInfo{} + connectionPool = nil + originalSync = backupHistoryStandbySync + backupHistoryStandbySync = syncBackupHistoryToStandby + backupHistoryStandbySyncCommandExec = func(name string, args ...string) backupHistoryStandbySyncCommand { + return backupHistoryStandbySyncFakeCommand{} + } + backupHistoryStandbySyncMkdirTemp = os.MkdirTemp + backupHistoryStandbySyncRemoveAll = os.RemoveAll + backupHistoryStandbySyncCurrentUser = func() (string, error) { + return "gpadmin", nil + } + }) + + AfterEach(func() { + backupHistoryStandbySync = originalSync + backupHistoryStandbySyncCommandExec = func(name string, args ...string) backupHistoryStandbySyncCommand { + return backupHistoryStandbySyncFakeCommand{} + } + backupHistoryStandbySyncMkdirTemp = os.MkdirTemp + backupHistoryStandbySyncRemoveAll = os.RemoveAll + backupHistoryStandbySyncCurrentUser = func() (string, error) { + return "gpadmin", nil + } + if connectionPool != nil { + connectionPool.Close() + } + }) + + It("creates a verified snapshot with source permissions", func() { + tmpDir := GinkgoT().TempDir() + sourcePath := filepath.Join(tmpDir, backupHistoryDBName) + createBackupHistoryStandbySyncSQLiteDB(sourcePath) + Expect(os.Chmod(sourcePath, 0o640)).To(Succeed()) + + snapshotPath, tempDir, err := createBackupHistoryStandbySyncSnapshot(sourcePath, 0o640) + Expect(err).ToNot(HaveOccurred()) + defer cleanupBackupHistoryStandbySyncTempDir(tempDir) + + Expect(snapshotPath).To(Equal(filepath.Join(tempDir, backupHistoryDBName))) + snapshotInfo, err := os.Stat(snapshotPath) + Expect(err).ToNot(HaveOccurred()) + Expect(snapshotInfo.Mode().Perm()).To(Equal(os.FileMode(0o640))) + + snapshotDB, err := sql.Open("sqlite3", backupHistoryStandbySyncSQLiteURI(snapshotPath, "ro")) + Expect(err).ToNot(HaveOccurred()) + defer snapshotDB.Close() + var value string + Expect(snapshotDB.QueryRow("SELECT value FROM sync_test WHERE id = 1").Scan(&value)).To(Succeed()) + Expect(value).To(Equal("present")) + Expect(validateBackupHistoryStandbySyncSnapshot(snapshotPath)).To(Succeed()) + }) + + It("rejects corrupted SQLite sources before transport", func() { + tmpDir := GinkgoT().TempDir() + sourcePath := filepath.Join(tmpDir, backupHistoryDBName) + Expect(os.WriteFile(sourcePath, []byte("not sqlite"), 0o600)).To(Succeed()) + + _, tempDir, err := createBackupHistoryStandbySyncSnapshot(sourcePath, 0o600) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("VACUUM INTO")) + Expect(tempDir).To(BeEmpty()) + }) + + It("canonicalizes symlink sources and builds the shared lock path from the canonical source", func() { + tmpDir := GinkgoT().TempDir() + realDir := filepath.Join(tmpDir, "real") + linkDir := filepath.Join(tmpDir, "link") + Expect(os.Mkdir(realDir, 0o700)).To(Succeed()) + Expect(os.Mkdir(linkDir, 0o700)).To(Succeed()) + realSourcePath := filepath.Join(realDir, backupHistoryDBName) + createBackupHistoryStandbySyncSQLiteDB(realSourcePath) + linkSourcePath := filepath.Join(linkDir, backupHistoryDBName) + Expect(os.Symlink(realSourcePath, linkSourcePath)).To(Succeed()) + + canonicalSourcePath, _, err := canonicalBackupHistoryStandbySyncSource(linkSourcePath) + Expect(err).ToNot(HaveOccurred()) + Expect(canonicalSourcePath).To(Equal(realSourcePath)) + Expect(backupHistoryStandbySyncLockPath(canonicalSourcePath)).To(Equal(realSourcePath + ".sync.lock")) + }) + + It("skips when no up standby coordinator exists", func() { + tmpDir := GinkgoT().TempDir() + sourcePath := filepath.Join(tmpDir, backupHistoryDBName) + createBackupHistoryStandbySyncSQLiteDB(sourcePath) + globalFPInfo = backupfilepath.FilePathInfo{SegDirMap: map[int]string{-1: tmpDir}} + mock := setupBackupHistoryStandbySyncConnection() + mock.ExpectQuery(regexp.QuoteMeta(backupHistoryStandbySyncStandbySQL)).WillReturnError(sql.ErrNoRows) + + commandCalls := setBackupHistoryStandbySyncCommands(nil) + skipReason, err := syncBackupHistoryToStandby() + + Expect(err).ToNot(HaveOccurred()) + Expect(skipReason).To(Equal("no up standby coordinator found")) + Expect(*commandCalls).To(BeEmpty()) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + }) + + It("orchestrates discovery, snapshot, rsync transport, atomic install, and local cleanup", func() { + tmpDir := GinkgoT().TempDir() + primaryDataDir := filepath.Join(tmpDir, "primary") + standbyDataDir := filepath.Join(tmpDir, "standby data") + snapshotDir := filepath.Join(tmpDir, "snapshot dir") + Expect(os.Mkdir(primaryDataDir, 0o700)).To(Succeed()) + Expect(os.Mkdir(standbyDataDir, 0o700)).To(Succeed()) + sourcePath := filepath.Join(primaryDataDir, backupHistoryDBName) + createBackupHistoryStandbySyncSQLiteDB(sourcePath) + globalFPInfo = backupfilepath.FilePathInfo{SegDirMap: map[int]string{-1: primaryDataDir}} + mock := setupBackupHistoryStandbySyncConnection() + expectBackupHistoryStandbySyncStandby(mock, "sdw-standby", standbyDataDir) + backupHistoryStandbySyncMkdirTemp = func(dir, pattern string) (string, error) { + Expect(dir).To(Equal("")) + Expect(pattern).To(Equal(backupHistoryStandbySyncTempDirPattern)) + Expect(os.Mkdir(snapshotDir, 0o700)).To(Succeed()) + return snapshotDir, nil + } + + commandCalls := setBackupHistoryStandbySyncCommands([]backupHistoryStandbySyncCommandResponse{{}, {}}) + skipReason, err := syncBackupHistoryToStandby() + + Expect(err).ToNot(HaveOccurred()) + Expect(skipReason).To(BeEmpty()) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + Expect(*commandCalls).To(HaveLen(2)) + snapshotPath := filepath.Join(snapshotDir, backupHistoryDBName) + remoteTempPath := newBackupHistoryStandbySyncRemoteTempPath(standbyDataDir, snapshotPath) + Expect((*commandCalls)[0].name).To(Equal("rsync")) + Expect((*commandCalls)[0].args).To(Equal(buildBackupHistoryStandbySyncRsyncArgs(snapshotPath, "sdw-standby", "gpadmin", remoteTempPath))) + Expect((*commandCalls)[1].name).To(Equal("ssh")) + Expect((*commandCalls)[1].args).To(Equal([]string{ + "-o", + "StrictHostKeyChecking=no", + "-o", + "ConnectTimeout=30", + "gpadmin@sdw-standby", + buildBackupHistoryStandbySyncRemoteInstallCommand(remoteTempPath, filepath.Join(standbyDataDir, backupHistoryDBName)), + })) + _, err = os.Stat(snapshotDir) + Expect(errors.Is(err, os.ErrNotExist)).To(BeTrue()) + }) + + It("releases the source lock after transport errors", func() { + tmpDir := GinkgoT().TempDir() + primaryDataDir := filepath.Join(tmpDir, "primary") + standbyDataDir := filepath.Join(tmpDir, "standby") + Expect(os.Mkdir(primaryDataDir, 0o700)).To(Succeed()) + sourcePath := filepath.Join(primaryDataDir, backupHistoryDBName) + createBackupHistoryStandbySyncSQLiteDB(sourcePath) + globalFPInfo = backupfilepath.FilePathInfo{SegDirMap: map[int]string{-1: primaryDataDir}} + mock := setupBackupHistoryStandbySyncConnection() + expectBackupHistoryStandbySyncStandby(mock, "sdw-standby", standbyDataDir) + setBackupHistoryStandbySyncCommands([]backupHistoryStandbySyncCommandResponse{ + {output: []byte("rsync failed"), err: errors.New("exit status 1")}, + {}, + }) + + _, err := syncBackupHistoryToStandby() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("rsync standby history snapshot")) + Expect(err.Error()).To(ContainSubstring("rsync failed")) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + + sourceLock, err := lockfile.New(backupHistoryStandbySyncLockPath(sourcePath)) + Expect(err).ToNot(HaveOccurred()) + Expect(sourceLock.TryLock()).To(Succeed()) + Expect(sourceLock.Unlock()).To(Succeed()) + }) + + It("returns lock contention as an error without creating a snapshot", func() { + tmpDir := GinkgoT().TempDir() + primaryDataDir := filepath.Join(tmpDir, "primary") + standbyDataDir := filepath.Join(tmpDir, "standby") + Expect(os.Mkdir(primaryDataDir, 0o700)).To(Succeed()) + sourcePath := filepath.Join(primaryDataDir, backupHistoryDBName) + createBackupHistoryStandbySyncSQLiteDB(sourcePath) + globalFPInfo = backupfilepath.FilePathInfo{SegDirMap: map[int]string{-1: primaryDataDir}} + mock := setupBackupHistoryStandbySyncConnection() + expectBackupHistoryStandbySyncStandby(mock, "sdw-standby", standbyDataDir) + lockPath := backupHistoryStandbySyncLockPath(sourcePath) + Expect(os.WriteFile(lockPath, []byte(fmt.Sprintf("%d\n", os.Getppid())), 0o600)).To(Succeed()) + defer os.Remove(lockPath) + mkdirTempCalls := 0 + backupHistoryStandbySyncMkdirTemp = func(dir, pattern string) (string, error) { + mkdirTempCalls++ + return "", errors.New("snapshot should not be created") + } + + _, err := syncBackupHistoryToStandby() + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("lock standby history sync source")) + Expect(mkdirTempCalls).To(Equal(0)) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + }) + + It("quotes remote shell paths in rsync, install, and cleanup commands", func() { + remoteTempPath := "/data dir/standby's/.gpbackup_history.db.tmp" + destPath := "/data dir/standby's/gpbackup_history.db" + + Expect(buildBackupHistoryStandbySyncRsyncArgs("/tmp/snapshot", "sdw-standby", "gpadmin", remoteTempPath)).To(Equal([]string{ + "-p", + "-e", + backupHistoryStandbySyncSSHOptions, + "--", + "/tmp/snapshot", + "gpadmin@sdw-standby:" + shellQuoteBackupHistoryStandbySyncPath(remoteTempPath), + })) + installCommand := buildBackupHistoryStandbySyncRemoteInstallCommand(remoteTempPath, destPath) + Expect(installCommand).To(ContainSubstring("test -f " + shellQuoteBackupHistoryStandbySyncPath(remoteTempPath))) + Expect(installCommand).To(ContainSubstring("chown --reference=" + shellQuoteBackupHistoryStandbySyncPath(destPath) + " -- " + shellQuoteBackupHistoryStandbySyncPath(remoteTempPath))) + Expect(installCommand).To(ContainSubstring("chmod --reference=" + shellQuoteBackupHistoryStandbySyncPath(destPath) + " -- " + shellQuoteBackupHistoryStandbySyncPath(remoteTempPath))) + Expect(installCommand).To(ContainSubstring("mv -f -- " + shellQuoteBackupHistoryStandbySyncPath(remoteTempPath) + " " + shellQuoteBackupHistoryStandbySyncPath(destPath))) + Expect(buildBackupHistoryStandbySyncRemoteCleanupCommand(remoteTempPath)).To(Equal("rm -f -- " + shellQuoteBackupHistoryStandbySyncPath(remoteTempPath))) + }) + + It("chains remote cleanup errors onto the primary transport error", func() { + commandCalls := setBackupHistoryStandbySyncCommands([]backupHistoryStandbySyncCommandResponse{ + {output: []byte("cleanup failed"), err: errors.New("exit status 255")}, + }) + primaryErr := errors.New("install failed") + + err := cleanupBackupHistoryStandbySyncRemoteTempAfterError(primaryErr, "sdw-standby", "gpadmin", "/standby/.tmp") + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("install failed")) + Expect(err.Error()).To(ContainSubstring("additionally failed to clean up remote temp file")) + Expect(err.Error()).To(ContainSubstring("cleanup failed")) + Expect(*commandCalls).To(HaveLen(1)) + Expect((*commandCalls)[0].name).To(Equal("ssh")) + }) + + It("logs disabled automatic sync without invoking discovery", func() { + stdout, _, _ := testhelper.SetupTestLogger() + syncCalls := 0 + backupHistoryStandbySync = func() (string, error) { + syncCalls++ + return "", errors.New("sync should not run") + } + + skipReason, err := syncBackupHistoryToStandbyBestEffort(true) + + Expect(err).ToNot(HaveOccurred()) + Expect(skipReason).To(Equal("disabled by --" + options.NO_HISTORY_SYNC_STANDBY)) + Expect(syncCalls).To(Equal(0)) + Expect(string(stdout.Contents())).To(ContainSubstring("Skipping history db sync to standby coordinator: disabled by --" + options.NO_HISTORY_SYNC_STANDBY)) + }) + + It("warns automatic sync failures without exiting", func() { + stdout, _, _ := testhelper.SetupTestLogger() + backupHistoryStandbySync = func() (string, error) { + return "", errors.New("transport failed") + } + + _, err := syncBackupHistoryToStandbyBestEffort(false) + + Expect(err).To(HaveOccurred()) + Expect(string(stdout.Contents())).To(ContainSubstring("History db sync to standby coordinator failed; standby history may be stale: transport failed")) + }) + + It("runs automatic sync only after successful cleanup history update for successful backups", func() { + calls := 0 + disabledValues := make([]bool, 0) + originalBestEffort := backupHistoryStandbySync + backupHistoryStandbySync = func() (string, error) { + calls++ + return "", nil + } + defer func() { + backupHistoryStandbySync = originalBestEffort + }() + backupHistoryStandbySyncCommandExec = func(name string, args ...string) backupHistoryStandbySyncCommand { + return backupHistoryStandbySyncFakeCommand{} + } + + syncBackupHistoryToStandbyAfterCleanup(false, true) + Expect(calls).To(Equal(1)) + + syncBackupHistoryToStandbyAfterCleanup(true, true) + syncBackupHistoryToStandbyAfterCleanup(false, false) + Expect(cmdFlags.Set(options.NO_HISTORY, "true")).To(Succeed()) + syncBackupHistoryToStandbyAfterCleanup(false, true) + Expect(calls).To(Equal(1)) + + Expect(cmdFlags.Set(options.NO_HISTORY, "false")).To(Succeed()) + Expect(cmdFlags.Set(options.NO_HISTORY_SYNC_STANDBY, "true")).To(Succeed()) + backupHistoryStandbySync = func() (string, error) { + calls++ + disabledValues = append(disabledValues, true) + return "", nil + } + syncBackupHistoryToStandbyAfterCleanup(false, true) + Expect(calls).To(Equal(1)) + Expect(disabledValues).To(BeEmpty()) + }) +}) + +func createBackupHistoryStandbySyncSQLiteDB(path string) { + db, err := sql.Open("sqlite3", backupHistoryStandbySyncSQLiteURI(path, "rwc")) + Expect(err).ToNot(HaveOccurred()) + defer db.Close() + + _, err = db.Exec("CREATE TABLE sync_test (id INTEGER PRIMARY KEY, value TEXT)") + Expect(err).ToNot(HaveOccurred()) + _, err = db.Exec("INSERT INTO sync_test (value) VALUES ('present')") + Expect(err).ToNot(HaveOccurred()) +} + +func setupBackupHistoryStandbySyncConnection() sqlmock.Sqlmock { + sqlDB, mock, err := sqlmock.New() + Expect(err).ToNot(HaveOccurred()) + connectionPool = &dbconn.DBConn{ + ConnPool: []*sqlx.DB{sqlx.NewDb(sqlDB, "sqlmock")}, + NumConns: 1, + Tx: []*sqlx.Tx{nil}, + } + return mock +} + +func expectBackupHistoryStandbySyncStandby(mock sqlmock.Sqlmock, host, dataDir string) { + mock.ExpectQuery(regexp.QuoteMeta(backupHistoryStandbySyncStandbySQL)). + WillReturnRows(sqlmock.NewRows([]string{"hostname", "datadir"}).AddRow(host, dataDir)) +} + +func setBackupHistoryStandbySyncCommands(responses []backupHistoryStandbySyncCommandResponse) *[]backupHistoryStandbySyncCommandCall { + calls := make([]backupHistoryStandbySyncCommandCall, 0) + backupHistoryStandbySyncCommandExec = func(name string, args ...string) backupHistoryStandbySyncCommand { + calls = append(calls, backupHistoryStandbySyncCommandCall{name: name, args: append([]string{}, args...)}) + response := backupHistoryStandbySyncCommandResponse{} + if len(calls) <= len(responses) { + response = responses[len(calls)-1] + } + return backupHistoryStandbySyncFakeCommand{output: response.output, err: response.err} + } + return &calls +} diff --git a/options/flag.go b/options/flag.go index 3618a259..f5e2083b 100644 --- a/options/flag.go +++ b/options/flag.go @@ -14,46 +14,47 @@ import ( ) const ( - BACKUP_DIR = "backup-dir" - COMPRESSION_TYPE = "compression-type" - COMPRESSION_LEVEL = "compression-level" - DATA_ONLY = "data-only" - DBNAME = "dbname" - DEBUG = "debug" - EXCLUDE_RELATION = "exclude-table" - EXCLUDE_RELATION_FILE = "exclude-table-file" - EXCLUDE_SCHEMA = "exclude-schema" - EXCLUDE_SCHEMA_FILE = "exclude-schema-file" - FROM_TIMESTAMP = "from-timestamp" - INCLUDE_RELATION = "include-table" - INCLUDE_RELATION_FILE = "include-table-file" - INCLUDE_SCHEMA = "include-schema" - INCLUDE_SCHEMA_FILE = "include-schema-file" - INCREMENTAL = "incremental" - JOBS = "jobs" - LEAF_PARTITION_DATA = "leaf-partition-data" - METADATA_ONLY = "metadata-only" - NO_COMPRESSION = "no-compression" - NO_HISTORY = "no-history" - PLUGIN_CONFIG = "plugin-config" - QUIET = "quiet" - SINGLE_DATA_FILE = "single-data-file" - COPY_QUEUE_SIZE = "copy-queue-size" - VERBOSE = "verbose" - WITH_STATS = "with-stats" - CREATE_DB = "create-db" - ON_ERROR_CONTINUE = "on-error-continue" - REDIRECT_DB = "redirect-db" - RUN_ANALYZE = "run-analyze" - SINGLE_BACKUP_DIR = "single-backup-dir" - TIMESTAMP = "timestamp" - WITH_GLOBALS = "with-globals" - REDIRECT_SCHEMA = "redirect-schema" - TRUNCATE_TABLE = "truncate-table" - WITHOUT_GLOBALS = "without-globals" - RESIZE_CLUSTER = "resize-cluster" - NO_INHERITS = "no-inherits" - REPORT_DIR = "report-dir" + BACKUP_DIR = "backup-dir" + COMPRESSION_TYPE = "compression-type" + COMPRESSION_LEVEL = "compression-level" + DATA_ONLY = "data-only" + DBNAME = "dbname" + DEBUG = "debug" + EXCLUDE_RELATION = "exclude-table" + EXCLUDE_RELATION_FILE = "exclude-table-file" + EXCLUDE_SCHEMA = "exclude-schema" + EXCLUDE_SCHEMA_FILE = "exclude-schema-file" + FROM_TIMESTAMP = "from-timestamp" + INCLUDE_RELATION = "include-table" + INCLUDE_RELATION_FILE = "include-table-file" + INCLUDE_SCHEMA = "include-schema" + INCLUDE_SCHEMA_FILE = "include-schema-file" + INCREMENTAL = "incremental" + JOBS = "jobs" + LEAF_PARTITION_DATA = "leaf-partition-data" + METADATA_ONLY = "metadata-only" + NO_COMPRESSION = "no-compression" + NO_HISTORY = "no-history" + NO_HISTORY_SYNC_STANDBY = "no-history-sync-standby" + PLUGIN_CONFIG = "plugin-config" + QUIET = "quiet" + SINGLE_DATA_FILE = "single-data-file" + COPY_QUEUE_SIZE = "copy-queue-size" + VERBOSE = "verbose" + WITH_STATS = "with-stats" + CREATE_DB = "create-db" + ON_ERROR_CONTINUE = "on-error-continue" + REDIRECT_DB = "redirect-db" + RUN_ANALYZE = "run-analyze" + SINGLE_BACKUP_DIR = "single-backup-dir" + TIMESTAMP = "timestamp" + WITH_GLOBALS = "with-globals" + REDIRECT_SCHEMA = "redirect-schema" + TRUNCATE_TABLE = "truncate-table" + WITHOUT_GLOBALS = "without-globals" + RESIZE_CLUSTER = "resize-cluster" + NO_INHERITS = "no-inherits" + REPORT_DIR = "report-dir" ) func SetBackupFlagDefaults(flagSet *pflag.FlagSet) { @@ -79,6 +80,7 @@ func SetBackupFlagDefaults(flagSet *pflag.FlagSet) { flagSet.Bool(METADATA_ONLY, false, "Only back up metadata, do not back up data") flagSet.Bool(NO_COMPRESSION, false, "Skip compression of data files") flagSet.Bool(NO_HISTORY, false, "Do not write a backup entry to the gpbackup_history database") + flagSet.Bool(NO_HISTORY_SYNC_STANDBY, false, "Do not sync gpbackup_history.db to the standby coordinator") flagSet.String(PLUGIN_CONFIG, "", "The configuration file to use for a plugin") flagSet.Bool("version", false, "Print version number and exit") flagSet.Bool(QUIET, false, "Suppress non-warning, non-error log messages") diff --git a/options/flag_test.go b/options/flag_test.go index b9ecb277..456f52d4 100644 --- a/options/flag_test.go +++ b/options/flag_test.go @@ -57,5 +57,24 @@ var _ = Describe("utils/flag tests", func() { Expect(result).To(Equal([]string{"-s", "some_argument"})) }) }) + Context("SetBackupFlagDefaults", func() { + It("registers no-history-sync-standby for gpbackup with a false default", func() { + flagSet := pflag.NewFlagSet("gpbackup", pflag.ContinueOnError) + options.SetBackupFlagDefaults(flagSet) + + flag := flagSet.Lookup(options.NO_HISTORY_SYNC_STANDBY) + Expect(flag).ToNot(BeNil()) + value, err := flagSet.GetBool(options.NO_HISTORY_SYNC_STANDBY) + Expect(err).ToNot(HaveOccurred()) + Expect(value).To(BeFalse()) + }) + + It("does not register no-history-sync-standby for gprestore", func() { + flagSet := pflag.NewFlagSet("gprestore", pflag.ContinueOnError) + options.SetRestoreFlagDefaults(flagSet) + + Expect(flagSet.Lookup(options.NO_HISTORY_SYNC_STANDBY)).To(BeNil()) + }) + }) }) }) From f45e497cc53955a4ddce854e52c2ab2d909456f9 Mon Sep 17 00:00:00 2001 From: woblerr Date: Tue, 28 Jul 2026 16:54:51 +0000 Subject: [PATCH 03/19] Add gpbackman standby history sync engine. --- gpbackman/cmd/history_standby_sync.go | 463 +++++++++++++++++ gpbackman/cmd/history_standby_sync_test.go | 552 +++++++++++++++++++++ gpbackman/cmd/wrappers.go | 17 + gpbackman/cmd/wrappers_test.go | 67 +++ 4 files changed, 1099 insertions(+) create mode 100644 gpbackman/cmd/history_standby_sync.go create mode 100644 gpbackman/cmd/history_standby_sync_test.go diff --git a/gpbackman/cmd/history_standby_sync.go b/gpbackman/cmd/history_standby_sync.go new file mode 100644 index 00000000..8e5301c6 --- /dev/null +++ b/gpbackman/cmd/history_standby_sync.go @@ -0,0 +1,463 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +*/ + +package cmd + +import ( + "database/sql" + "errors" + "fmt" + "net/url" + "os" + "path/filepath" + "strings" + "time" + + "github.com/apache/cloudberry-backup/gpbackman/gpbckpconfig" + "github.com/apache/cloudberry-go-libs/gplog" + "github.com/apache/cloudberry-go-libs/operating" + "github.com/jmoiron/sqlx" + _ "github.com/mattn/go-sqlite3" + "github.com/nightlyone/lockfile" +) + +const ( + noHistorySyncStandbyFlagName = "no-history-sync-standby" + historyStandbySyncSSHOptions = "ssh -o StrictHostKeyChecking=no -o ConnectTimeout=30" + historyStandbySyncTempDirPattern = "gpbackman-history-standby-sync-%s-%d-*" +) + +type historyStandbySyncResult struct { + skipReason string + err error +} + +type historyStandbySyncTarget struct { + sourceDBPath string + sourceMode os.FileMode + standbyHost string + standbyDataDir string + standbyHistoryDBPath string +} + +var ( + historyStandbySync = syncHistoryStandby + historyStandbySyncOpenClusterConn = gpbckpconfig.NewClusterLocalClusterDefaultConn + historyStandbySyncMkdirTemp = os.MkdirTemp + historyStandbySyncRemoveAll = os.RemoveAll + historyStandbySyncNow = time.Now + historyStandbySyncPID = os.Getpid + historyStandbySyncCurrentUser = func() (string, error) { + currentUser, err := operating.System.CurrentUser() + if err != nil { + return "", err + } + return currentUser.Username, nil + } + historyStandbySyncRunSSHCommand = runHistoryStandbySyncSSHCommand +) + +func syncHistoryStandbyBestEffort(disabled bool) historyStandbySyncResult { + if disabled { + result := historyStandbySyncResult{skipReason: "disabled by --" + noHistorySyncStandbyFlagName} + gplog.Info("Skipping history db sync to standby coordinator: %s", result.skipReason) + return result + } + + result := historyStandbySync() + if result.err != nil { + gplog.Warn("History db sync to standby coordinator failed; standby history may be stale: %v", result.err) + return result + } + if result.skipReason != "" { + gplog.Debug("Skipping history db sync to standby coordinator: %s", result.skipReason) + } + return result +} + +func syncHistoryStandbyStrict() error { + result := historyStandbySync() + if result.err != nil { + return result.err + } + if result.skipReason != "" { + return fmt.Errorf("history db sync to standby coordinator skipped: %s", result.skipReason) + } + return nil +} + +func syncHistoryStandby() historyStandbySyncResult { + sourceDBPath, skipReason := getHistoryStandbySyncSourceDBPath() + if skipReason != "" { + return historyStandbySyncResult{skipReason: skipReason} + } + + target, skipReason, err := discoverHistoryStandbySyncTarget(sourceDBPath) + if err != nil { + return historyStandbySyncResult{err: err} + } + if skipReason != "" { + return historyStandbySyncResult{skipReason: skipReason} + } + + userName, err := historyStandbySyncCurrentUser() + if err != nil { + return historyStandbySyncResult{err: fmt.Errorf("resolve current OS user for standby history sync: %w", err)} + } + + gplog.Info("Sync history db to standby coordinator: %s", target.sourceDBPath) + err = withHistoryStandbySyncLock(target.sourceDBPath, func() error { + return withHistoryStandbySyncSnapshot(target.sourceDBPath, target.sourceMode, func(snapshotPath string) error { + return syncHistoryStandbySnapshotToStandby(target, userName, snapshotPath) + }) + }) + if err != nil { + return historyStandbySyncResult{err: err} + } + gplog.Info("History db sync to standby coordinator succeeded: %s:%s", target.standbyHost, target.standbyHistoryDBPath) + return historyStandbySyncResult{} +} + +func getHistoryStandbySyncSourceDBPath() (string, string) { + sourceDBPath := getHistoryDBPath(rootHistoryDB, rootAutoLoadHistoryDB) + if rootHistoryDB == "" && !rootAutoLoadHistoryDB { + return sourceDBPath, "using default working-directory history db" + } + if rootHistoryDB == "" && rootAutoLoadHistoryDB && sourceDBPath == historyDBNameConst { + return sourceDBPath, "--auto-load-history-db did not resolve the cluster history db" + } + return sourceDBPath, "" +} + +func discoverHistoryStandbySyncTarget(sourceDBPath string) (*historyStandbySyncTarget, string, error) { + db, err := historyStandbySyncOpenClusterConn() + if err != nil { + return nil, "", fmt.Errorf("connect to local cluster for standby history sync discovery: %w", err) + } + defer db.Close() + + primaryDataDir, err := queryHistoryStandbySyncPrimaryDataDir(db) + if err != nil { + return nil, "", fmt.Errorf("query primary coordinator datadir for standby history sync discovery: %w", err) + } + + canonicalSourceDBPath, sourceInfo, err := canonicalHistoryStandbySyncSource(sourceDBPath) + if err != nil { + return nil, "", err + } + canonicalPrimaryHistoryDBPath, err := canonicalHistoryStandbySyncPath(filepath.Join(primaryDataDir, historyDBNameConst)) + if err != nil { + return nil, "", fmt.Errorf("resolve canonical primary history db path for standby sync: %w", err) + } + if canonicalSourceDBPath != canonicalPrimaryHistoryDBPath { + return nil, fmt.Sprintf("source history db %s is not cluster history db %s", canonicalSourceDBPath, canonicalPrimaryHistoryDBPath), nil + } + + standbyConfig, err := queryHistoryStandbySyncStandby(db) + if err != nil { + if errors.Is(err, sql.ErrNoRows) { + return nil, "no up standby coordinator found", nil + } + return nil, "", fmt.Errorf("query up standby coordinator for standby history sync discovery: %w", err) + } + + target := &historyStandbySyncTarget{ + sourceDBPath: canonicalSourceDBPath, + sourceMode: sourceInfo.Mode().Perm(), + standbyHost: standbyConfig.Hostname, + standbyDataDir: standbyConfig.DataDir, + standbyHistoryDBPath: filepath.Join(standbyConfig.DataDir, historyDBNameConst), + } + gplog.Debug("Discovered standby history sync target: source=%s standby=%s:%s", target.sourceDBPath, target.standbyHost, target.standbyHistoryDBPath) + return target, "", nil +} + +func queryHistoryStandbySyncPrimaryDataDir(db *sqlx.DB) (string, error) { + return gpbckpconfig.QueryPrimaryCoordinatorDataDir(db) +} + +func queryHistoryStandbySyncStandby(db *sqlx.DB) (gpbckpconfig.StandbyCoordinator, error) { + return gpbckpconfig.QueryUpStandbyCoordinator(db) +} + +func canonicalHistoryStandbySyncSource(sourceDBPath string) (string, os.FileInfo, error) { + canonicalSourceDBPath, err := canonicalHistoryStandbySyncPath(sourceDBPath) + if err != nil { + return "", nil, fmt.Errorf("resolve canonical source history db path for standby sync: %w", err) + } + sourceInfo, err := os.Stat(canonicalSourceDBPath) + if err != nil { + return "", nil, fmt.Errorf("stat source history db for standby sync: %w", err) + } + if !sourceInfo.Mode().IsRegular() { + return "", nil, fmt.Errorf("source history db for standby sync is not a regular file: %s", canonicalSourceDBPath) + } + return canonicalSourceDBPath, sourceInfo, nil +} + +func canonicalHistoryStandbySyncPath(path string) (string, error) { + absolutePath, err := filepath.Abs(filepath.Clean(path)) + if err != nil { + return "", err + } + canonicalPath, err := filepath.EvalSymlinks(absolutePath) + if err != nil { + return "", err + } + return filepath.Clean(canonicalPath), nil +} + +func withHistoryStandbySyncLock(sourceDBPath string, syncFn func() error) error { + lockPath := historyStandbySyncLockPath(sourceDBPath) + sourceLock, err := lockfile.New(lockPath) + if err != nil { + return fmt.Errorf("create standby history sync lock %s: %w", lockPath, err) + } + if err := sourceLock.TryLock(); err != nil { + return fmt.Errorf("lock standby history sync source %s: %w", sourceDBPath, err) + } + + syncErr := syncFn() + unlockErr := sourceLock.Unlock() + if syncErr != nil { + if unlockErr != nil { + return fmt.Errorf("%w; additionally failed to release standby history sync lock %s: %v", syncErr, lockPath, unlockErr) + } + return syncErr + } + if unlockErr != nil { + return fmt.Errorf("release standby history sync lock %s: %w", lockPath, unlockErr) + } + return nil +} + +func historyStandbySyncLockPath(sourceDBPath string) string { + return sourceDBPath + ".sync.lock" +} + +func withHistoryStandbySyncSnapshot(sourceDBPath string, sourceMode os.FileMode, syncFn func(string) error) error { + snapshotPath, tempDir, err := createHistoryStandbySyncSnapshot(sourceDBPath, sourceMode) + if tempDir != "" { + defer cleanupHistoryStandbySyncTempDir(tempDir) + } + if err != nil { + return err + } + return syncFn(snapshotPath) +} + +func createHistoryStandbySyncSnapshot(sourceDBPath string, sourceMode os.FileMode) (string, string, error) { + tempDirPattern := fmt.Sprintf( + historyStandbySyncTempDirPattern, + historyStandbySyncNow().UTC().Format("20060102150405"), + historyStandbySyncPID(), + ) + tempDir, err := historyStandbySyncMkdirTemp("", tempDirPattern) + if err != nil { + return "", "", fmt.Errorf("create local standby history sync temp directory: %w", err) + } + snapshotPath := filepath.Join(tempDir, historyDBNameConst) + if err := vacuumHistoryStandbySyncSnapshot(sourceDBPath, snapshotPath); err != nil { + cleanupHistoryStandbySyncTempDir(tempDir) + return "", "", err + } + if err := os.Chmod(snapshotPath, sourceMode); err != nil { + cleanupHistoryStandbySyncTempDir(tempDir) + return "", "", fmt.Errorf("set standby history sync snapshot permissions from source history db: %w", err) + } + if err := validateHistoryStandbySyncSnapshot(snapshotPath); err != nil { + cleanupHistoryStandbySyncTempDir(tempDir) + return "", "", err + } + return snapshotPath, tempDir, nil +} + +func vacuumHistoryStandbySyncSnapshot(sourceDBPath, snapshotPath string) error { + sourceDB, err := sql.Open("sqlite3", historyStandbySyncSQLiteURI(sourceDBPath, "ro")) + if err != nil { + return fmt.Errorf("open source history db for standby sync snapshot: %w", err) + } + defer sourceDB.Close() + + if _, err := sourceDB.Exec("VACUUM main INTO ?", snapshotPath); err != nil { + return fmt.Errorf("create standby history sync snapshot with VACUUM INTO: %w", err) + } + return nil +} + +func validateHistoryStandbySyncSnapshot(snapshotPath string) error { + results, err := runHistoryStandbySyncQuickCheck(snapshotPath) + if err != nil { + return err + } + if len(results) != 1 || results[0] != "ok" { + return fmt.Errorf("validate standby history sync snapshot quick_check: expected single ok result, got %v", results) + } + return nil +} + +func runHistoryStandbySyncQuickCheck(snapshotPath string) ([]string, error) { + snapshotDB, err := sql.Open("sqlite3", historyStandbySyncSQLiteURI(snapshotPath, "ro")) + if err != nil { + return nil, fmt.Errorf("open standby history sync snapshot read-only: %w", err) + } + defer snapshotDB.Close() + + rows, err := snapshotDB.Query("PRAGMA quick_check") + if err != nil { + return nil, fmt.Errorf("run PRAGMA quick_check on standby history sync snapshot: %w", err) + } + defer rows.Close() + + results := make([]string, 0) + for rows.Next() { + var result string + if err := rows.Scan(&result); err != nil { + return nil, fmt.Errorf("scan PRAGMA quick_check result for standby history sync snapshot: %w", err) + } + results = append(results, result) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("read PRAGMA quick_check results for standby history sync snapshot: %w", err) + } + return results, nil +} + +func cleanupHistoryStandbySyncTempDir(tempDir string) { + if err := historyStandbySyncRemoveAll(tempDir); err != nil && !errors.Is(err, os.ErrNotExist) { + gplog.Debug("Unable to remove local standby history sync temp directory %s: %v", tempDir, err) + } +} + +func syncHistoryStandbySnapshotToStandby(target *historyStandbySyncTarget, userName, snapshotPath string) error { + remoteTempPath := newHistoryStandbySyncRemoteTempPath(target.standbyDataDir, snapshotPath) + if err := rsyncHistoryStandbySyncSnapshot(snapshotPath, target.standbyHost, userName, remoteTempPath); err != nil { + return cleanupHistoryStandbySyncRemoteTempAfterError(err, target.standbyHost, userName, remoteTempPath) + } + if err := installHistoryStandbySyncSnapshotOnStandby(target, userName, remoteTempPath); err != nil { + return cleanupHistoryStandbySyncRemoteTempAfterError(err, target.standbyHost, userName, remoteTempPath) + } + return nil +} + +func newHistoryStandbySyncRemoteTempPath(standbyDataDir, snapshotPath string) string { + return filepath.Join(standbyDataDir, fmt.Sprintf(".%s.%s.tmp", historyDBNameConst, filepath.Base(filepath.Dir(snapshotPath)))) +} + +func rsyncHistoryStandbySyncSnapshot(snapshotPath, standbyHost, userName, remoteTempPath string) error { + args := buildHistoryStandbySyncRsyncArgs(snapshotPath, standbyHost, userName, remoteTempPath) + gplog.Debug("Transfer history db snapshot to standby coordinator: %s -> %s:%s", snapshotPath, standbyHost, remoteTempPath) + output, err := execCombinedOutputCommand("rsync", args...).CombinedOutput() + if err != nil { + return fmt.Errorf("rsync standby history snapshot to %s:%s failed: %w%s", standbyHost, remoteTempPath, err, formatHistoryStandbySyncCommandOutput(output)) + } + return nil +} + +func buildHistoryStandbySyncRsyncArgs(snapshotPath, standbyHost, userName, remoteTempPath string) []string { + return []string{ + "-p", + "-e", + historyStandbySyncSSHOptions, + "--", + snapshotPath, + fmt.Sprintf("%s@%s:%s", userName, standbyHost, shellQuoteHistoryStandbySyncPath(remoteTempPath)), + } +} + +func installHistoryStandbySyncSnapshotOnStandby(target *historyStandbySyncTarget, userName, remoteTempPath string) error { + command := buildHistoryStandbySyncRemoteInstallCommand(remoteTempPath, target.standbyHistoryDBPath) + gplog.Debug("Install history db snapshot on standby coordinator: %s:%s", target.standbyHost, target.standbyHistoryDBPath) + output, err := historyStandbySyncRunSSHCommand(command, target.standbyHost, userName) + if err != nil { + return fmt.Errorf("install standby history snapshot on %s:%s failed: %w%s", target.standbyHost, target.standbyHistoryDBPath, err, formatHistoryStandbySyncCommandOutput(output)) + } + return nil +} + +func buildHistoryStandbySyncRemoteInstallCommand(remoteTempPath, standbyHistoryDBPath string) string { + quotedTempPath := shellQuoteHistoryStandbySyncPath(remoteTempPath) + quotedHistoryDBPath := shellQuoteHistoryStandbySyncPath(standbyHistoryDBPath) + return fmt.Sprintf( + "test -f %s && if test -e %s; then chown --reference=%s -- %s && chmod --reference=%s -- %s; fi && mv -f -- %s %s", + quotedTempPath, + quotedHistoryDBPath, + quotedHistoryDBPath, + quotedTempPath, + quotedHistoryDBPath, + quotedTempPath, + quotedTempPath, + quotedHistoryDBPath, + ) +} + +func cleanupHistoryStandbySyncRemoteTempAfterError(primaryErr error, standbyHost, userName, remoteTempPath string) error { + if cleanupErr := cleanupHistoryStandbySyncRemoteTemp(standbyHost, userName, remoteTempPath); cleanupErr != nil { + return fmt.Errorf("%w; additionally failed to clean up remote temp file: %v", primaryErr, cleanupErr) + } + return primaryErr +} + +func cleanupHistoryStandbySyncRemoteTemp(standbyHost, userName, remoteTempPath string) error { + command := buildHistoryStandbySyncRemoteCleanupCommand(remoteTempPath) + gplog.Debug("Clean up remote standby history sync temp file: %s:%s", standbyHost, remoteTempPath) + output, err := historyStandbySyncRunSSHCommand(command, standbyHost, userName) + if err != nil { + return fmt.Errorf("clean up remote standby history sync temp file %s:%s failed: %w%s", standbyHost, remoteTempPath, err, formatHistoryStandbySyncCommandOutput(output)) + } + return nil +} + +func buildHistoryStandbySyncRemoteCleanupCommand(remoteTempPath string) string { + return fmt.Sprintf("rm -f -- %s", shellQuoteHistoryStandbySyncPath(remoteTempPath)) +} + +func runHistoryStandbySyncSSHCommand(remoteCommand, standbyHost, userName string) ([]byte, error) { + return execCombinedOutputCommand( + "ssh", + "-o", + "StrictHostKeyChecking=no", + "-o", + "ConnectTimeout=30", + fmt.Sprintf("%s@%s", userName, standbyHost), + remoteCommand, + ).CombinedOutput() +} + +func historyStandbySyncSQLiteURI(dbPath, mode string) string { + query := url.Values{} + query.Set("mode", mode) + dbURI := url.URL{Scheme: "file", Path: dbPath, RawQuery: query.Encode()} + return dbURI.String() +} + +func shellQuoteHistoryStandbySyncPath(value string) string { + if value == "" { + return "''" + } + return "'" + strings.ReplaceAll(value, "'", "'\"'\"'") + "'" +} + +func formatHistoryStandbySyncCommandOutput(output []byte) string { + trimmedOutput := strings.TrimSpace(string(output)) + if trimmedOutput == "" { + return "" + } + return ": " + trimmedOutput +} diff --git a/gpbackman/cmd/history_standby_sync_test.go b/gpbackman/cmd/history_standby_sync_test.go new file mode 100644 index 00000000..4289391b --- /dev/null +++ b/gpbackman/cmd/history_standby_sync_test.go @@ -0,0 +1,552 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +*/ + +package cmd + +import ( + "database/sql" + "errors" + "fmt" + "os" + "path/filepath" + "regexp" + "time" + + "github.com/DATA-DOG/go-sqlmock" + "github.com/apache/cloudberry-go-libs/testhelper" + "github.com/jmoiron/sqlx" + "github.com/nightlyone/lockfile" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +const ( + historyStandbySyncPrimarySQL = "SELECT datadir FROM gp_segment_configuration WHERE content = -1 AND role = 'p' AND status = 'u';" + historyStandbySyncStandbySQL = "SELECT hostname, datadir FROM gp_segment_configuration WHERE content = -1 AND role = 'm' AND status = 'u';" +) + +type historyStandbySyncCommandCall struct { + name string + args []string +} + +type historyStandbySyncCommandResponse struct { + output []byte + err error +} + +type historyStandbySyncFakeCommand struct { + output []byte + err error +} + +func (c historyStandbySyncFakeCommand) CombinedOutput() ([]byte, error) { + return c.output, c.err +} + +var _ = Describe("history standby sync", func() { + var ( + originalHistoryStandbySync func() historyStandbySyncResult + originalOpenClusterConn func() (*sqlx.DB, error) + originalMkdirTemp func(string, string) (string, error) + originalRemoveAll func(string) error + originalNow func() time.Time + originalPID func() int + originalCurrentUser func() (string, error) + originalRunSSHCommand func(string, string, string) ([]byte, error) + originalExecCombinedOutputCommand func(string, ...string) combinedOutputCommand + savedRootHistoryDB string + savedRootAutoLoadHistoryDB bool + savedHistoryStandbySyncEnvironment map[string]string + savedHistoryStandbySyncEnvironmentPresent map[string]bool + ) + + BeforeEach(func() { + testhelper.SetupTestLogger() + originalHistoryStandbySync = historyStandbySync + originalOpenClusterConn = historyStandbySyncOpenClusterConn + originalMkdirTemp = historyStandbySyncMkdirTemp + originalRemoveAll = historyStandbySyncRemoveAll + originalNow = historyStandbySyncNow + originalPID = historyStandbySyncPID + originalCurrentUser = historyStandbySyncCurrentUser + originalRunSSHCommand = historyStandbySyncRunSSHCommand + originalExecCombinedOutputCommand = execCombinedOutputCommand + savedRootHistoryDB = rootHistoryDB + savedRootAutoLoadHistoryDB = rootAutoLoadHistoryDB + savedHistoryStandbySyncEnvironment = make(map[string]string) + savedHistoryStandbySyncEnvironmentPresent = make(map[string]bool) + for _, name := range append(historyDBEnvVars, "PGDATABASE") { + value, ok := os.LookupEnv(name) + savedHistoryStandbySyncEnvironment[name] = value + savedHistoryStandbySyncEnvironmentPresent[name] = ok + Expect(os.Unsetenv(name)).To(Succeed()) + } + + rootHistoryDB = "" + rootAutoLoadHistoryDB = false + historyStandbySync = syncHistoryStandby + historyStandbySyncOpenClusterConn = func() (*sqlx.DB, error) { + return nil, errors.New("cluster connection was not expected") + } + historyStandbySyncMkdirTemp = os.MkdirTemp + historyStandbySyncRemoveAll = os.RemoveAll + historyStandbySyncNow = func() time.Time { + return time.Date(2026, 7, 28, 16, 0, 0, 0, time.UTC) + } + historyStandbySyncPID = func() int { + return 4242 + } + historyStandbySyncCurrentUser = func() (string, error) { + return "gpadmin", nil + } + historyStandbySyncRunSSHCommand = runHistoryStandbySyncSSHCommand + execCombinedOutputCommand = func(name string, args ...string) combinedOutputCommand { + return historyStandbySyncFakeCommand{} + } + }) + + AfterEach(func() { + historyStandbySync = originalHistoryStandbySync + historyStandbySyncOpenClusterConn = originalOpenClusterConn + historyStandbySyncMkdirTemp = originalMkdirTemp + historyStandbySyncRemoveAll = originalRemoveAll + historyStandbySyncNow = originalNow + historyStandbySyncPID = originalPID + historyStandbySyncCurrentUser = originalCurrentUser + historyStandbySyncRunSSHCommand = originalRunSSHCommand + execCombinedOutputCommand = originalExecCombinedOutputCommand + rootHistoryDB = savedRootHistoryDB + rootAutoLoadHistoryDB = savedRootAutoLoadHistoryDB + for name, value := range savedHistoryStandbySyncEnvironment { + if savedHistoryStandbySyncEnvironmentPresent[name] { + Expect(os.Setenv(name, value)).To(Succeed()) + } else { + Expect(os.Unsetenv(name)).To(Succeed()) + } + } + }) + + It("skips default and unresolved auto-loaded history db sources before discovery", func() { + sourceDBPath, skipReason := getHistoryStandbySyncSourceDBPath() + Expect(sourceDBPath).To(Equal(historyDBNameConst)) + Expect(skipReason).To(Equal("using default working-directory history db")) + + rootAutoLoadHistoryDB = true + sourceDBPath, skipReason = getHistoryStandbySyncSourceDBPath() + Expect(sourceDBPath).To(Equal(historyDBNameConst)) + Expect(skipReason).To(Equal("--auto-load-history-db did not resolve the cluster history db")) + }) + + It("uses auto-load history db path resolved from coordinator data directory", func() { + rootAutoLoadHistoryDB = true + Expect(os.Setenv("COORDINATOR_DATA_DIRECTORY", "/coordinator/data")).To(Succeed()) + + sourceDBPath, skipReason := getHistoryStandbySyncSourceDBPath() + + Expect(sourceDBPath).To(Equal(filepath.Join("/coordinator/data", historyDBNameConst))) + Expect(skipReason).To(BeEmpty()) + }) + + It("creates a verified snapshot with source contents and permissions", func() { + tmpDir := GinkgoT().TempDir() + sourceDBPath := filepath.Join(tmpDir, historyDBNameConst) + createHistoryStandbySyncSQLiteDB(sourceDBPath) + Expect(os.Chmod(sourceDBPath, 0o640)).To(Succeed()) + + snapshotPath, tempDir, err := createHistoryStandbySyncSnapshot(sourceDBPath, 0o640) + Expect(err).ToNot(HaveOccurred()) + defer cleanupHistoryStandbySyncTempDir(tempDir) + + Expect(snapshotPath).To(Equal(filepath.Join(tempDir, historyDBNameConst))) + snapshotInfo, err := os.Stat(snapshotPath) + Expect(err).ToNot(HaveOccurred()) + Expect(snapshotInfo.Mode().Perm()).To(Equal(os.FileMode(0o640))) + + snapshotDB, err := sql.Open("sqlite3", historyStandbySyncSQLiteURI(snapshotPath, "ro")) + Expect(err).ToNot(HaveOccurred()) + defer snapshotDB.Close() + var value string + Expect(snapshotDB.QueryRow("SELECT value FROM sync_test WHERE id = 1").Scan(&value)).To(Succeed()) + Expect(value).To(Equal("present")) + Expect(validateHistoryStandbySyncSnapshot(snapshotPath)).To(Succeed()) + }) + + It("rejects corrupted SQLite sources before transport and removes the temp directory", func() { + tmpDir := GinkgoT().TempDir() + sourceDBPath := filepath.Join(tmpDir, historyDBNameConst) + Expect(os.WriteFile(sourceDBPath, []byte("not sqlite"), 0o600)).To(Succeed()) + snapshotDir := filepath.Join(tmpDir, "snapshot") + historyStandbySyncMkdirTemp = func(dir, pattern string) (string, error) { + Expect(os.Mkdir(snapshotDir, 0o700)).To(Succeed()) + return snapshotDir, nil + } + + _, tempDir, err := createHistoryStandbySyncSnapshot(sourceDBPath, 0o600) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("VACUUM INTO")) + Expect(tempDir).To(BeEmpty()) + _, statErr := os.Stat(snapshotDir) + Expect(errors.Is(statErr, os.ErrNotExist)).To(BeTrue()) + }) + + It("canonicalizes symlink sources and uses the shared lock path suffix", func() { + tmpDir := GinkgoT().TempDir() + primaryDataDir := filepath.Join(tmpDir, "primary") + Expect(os.Mkdir(primaryDataDir, 0o700)).To(Succeed()) + realSourceDBPath := filepath.Join(primaryDataDir, historyDBNameConst) + createHistoryStandbySyncSQLiteDB(realSourceDBPath) + linkSourceDBPath := filepath.Join(tmpDir, "history-link.db") + Expect(os.Symlink(realSourceDBPath, linkSourceDBPath)).To(Succeed()) + + canonicalSourceDBPath, _, err := canonicalHistoryStandbySyncSource(linkSourceDBPath) + + Expect(err).ToNot(HaveOccurred()) + Expect(canonicalSourceDBPath).To(Equal(realSourceDBPath)) + Expect(historyStandbySyncLockPath(canonicalSourceDBPath)).To(Equal(realSourceDBPath + ".sync.lock")) + }) + + It("rejects custom history db paths after primary datadir discovery", func() { + tmpDir := GinkgoT().TempDir() + primaryDataDir := filepath.Join(tmpDir, "primary") + customDataDir := filepath.Join(tmpDir, "custom") + Expect(os.Mkdir(primaryDataDir, 0o700)).To(Succeed()) + Expect(os.Mkdir(customDataDir, 0o700)).To(Succeed()) + createHistoryStandbySyncSQLiteDB(filepath.Join(primaryDataDir, historyDBNameConst)) + customSourceDBPath := filepath.Join(customDataDir, historyDBNameConst) + createHistoryStandbySyncSQLiteDB(customSourceDBPath) + mock := setupHistoryStandbySyncClusterConn(primaryDataDir, "", nil, false) + + target, skipReason, err := discoverHistoryStandbySyncTarget(customSourceDBPath) + + Expect(err).ToNot(HaveOccurred()) + Expect(target).To(BeNil()) + Expect(skipReason).To(ContainSubstring("is not cluster history db")) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + }) + + It("rejects non-regular source history db files", func() { + tmpDir := GinkgoT().TempDir() + primaryDataDir := filepath.Join(tmpDir, "primary") + sourceDBPath := filepath.Join(primaryDataDir, historyDBNameConst) + Expect(os.MkdirAll(sourceDBPath, 0o700)).To(Succeed()) + mock := setupHistoryStandbySyncClusterConn(primaryDataDir, "", nil, false) + + target, skipReason, err := discoverHistoryStandbySyncTarget(sourceDBPath) + + Expect(target).To(BeNil()) + Expect(skipReason).To(BeEmpty()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("not a regular file")) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + }) + + It("skips when no up standby coordinator exists", func() { + tmpDir := GinkgoT().TempDir() + primaryDataDir := filepath.Join(tmpDir, "primary") + Expect(os.Mkdir(primaryDataDir, 0o700)).To(Succeed()) + sourceDBPath := filepath.Join(primaryDataDir, historyDBNameConst) + createHistoryStandbySyncSQLiteDB(sourceDBPath) + rootHistoryDB = sourceDBPath + mock := setupHistoryStandbySyncClusterConn(primaryDataDir, "", sql.ErrNoRows, true) + commandCalls := setHistoryStandbySyncCommands(nil) + + result := syncHistoryStandby() + + Expect(result.err).ToNot(HaveOccurred()) + Expect(result.skipReason).To(Equal("no up standby coordinator found")) + Expect(*commandCalls).To(BeEmpty()) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + }) + + It("orchestrates discovery, lock, snapshot, rsync transport, atomic install, and local cleanup", func() { + tmpDir := GinkgoT().TempDir() + primaryDataDir := filepath.Join(tmpDir, "primary") + standbyDataDir := filepath.Join(tmpDir, "standby data") + snapshotDir := filepath.Join(tmpDir, "snapshot dir") + Expect(os.Mkdir(primaryDataDir, 0o700)).To(Succeed()) + sourceDBPath := filepath.Join(primaryDataDir, historyDBNameConst) + createHistoryStandbySyncSQLiteDB(sourceDBPath) + rootHistoryDB = sourceDBPath + mock := setupHistoryStandbySyncClusterConn(primaryDataDir, standbyDataDir, nil, true) + historyStandbySyncMkdirTemp = func(dir, pattern string) (string, error) { + Expect(dir).To(Equal("")) + Expect(pattern).To(Equal("gpbackman-history-standby-sync-20260728160000-4242-*")) + Expect(os.Mkdir(snapshotDir, 0o700)).To(Succeed()) + return snapshotDir, nil + } + commandCalls := setHistoryStandbySyncCommands([]historyStandbySyncCommandResponse{{}, {}}) + + result := syncHistoryStandby() + + Expect(result.err).ToNot(HaveOccurred()) + Expect(result.skipReason).To(BeEmpty()) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + Expect(*commandCalls).To(HaveLen(2)) + snapshotPath := filepath.Join(snapshotDir, historyDBNameConst) + remoteTempPath := newHistoryStandbySyncRemoteTempPath(standbyDataDir, snapshotPath) + Expect((*commandCalls)[0].name).To(Equal("rsync")) + Expect((*commandCalls)[0].args).To(Equal(buildHistoryStandbySyncRsyncArgs(snapshotPath, "sdw-standby", "gpadmin", remoteTempPath))) + Expect((*commandCalls)[1].name).To(Equal("ssh")) + Expect((*commandCalls)[1].args).To(Equal([]string{ + "-o", + "StrictHostKeyChecking=no", + "-o", + "ConnectTimeout=30", + "gpadmin@sdw-standby", + buildHistoryStandbySyncRemoteInstallCommand(remoteTempPath, filepath.Join(standbyDataDir, historyDBNameConst)), + })) + _, err := os.Stat(snapshotDir) + Expect(errors.Is(err, os.ErrNotExist)).To(BeTrue()) + }) + + It("uses the default cluster discovery connection for PGDATABASE resolution", func() { + tmpDir := GinkgoT().TempDir() + primaryDataDir := filepath.Join(tmpDir, "primary") + Expect(os.Mkdir(primaryDataDir, 0o700)).To(Succeed()) + sourceDBPath := filepath.Join(primaryDataDir, historyDBNameConst) + createHistoryStandbySyncSQLiteDB(sourceDBPath) + rootHistoryDB = sourceDBPath + Expect(os.Setenv("PGDATABASE", "template1")).To(Succeed()) + openCalls := 0 + mock := setupHistoryStandbySyncClusterConnWithHook(primaryDataDir, filepath.Join(tmpDir, "standby"), nil, true, func(db *sqlx.DB) (*sqlx.DB, error) { + openCalls++ + Expect(os.Getenv("PGDATABASE")).To(Equal("template1")) + return db, nil + }) + setHistoryStandbySyncCommands([]historyStandbySyncCommandResponse{{}, {}}) + + result := syncHistoryStandby() + + Expect(result.err).ToNot(HaveOccurred()) + Expect(openCalls).To(Equal(1)) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + }) + + It("releases the source lock after transport errors", func() { + tmpDir := GinkgoT().TempDir() + primaryDataDir := filepath.Join(tmpDir, "primary") + standbyDataDir := filepath.Join(tmpDir, "standby") + Expect(os.Mkdir(primaryDataDir, 0o700)).To(Succeed()) + sourceDBPath := filepath.Join(primaryDataDir, historyDBNameConst) + createHistoryStandbySyncSQLiteDB(sourceDBPath) + rootHistoryDB = sourceDBPath + mock := setupHistoryStandbySyncClusterConn(primaryDataDir, standbyDataDir, nil, true) + setHistoryStandbySyncCommands([]historyStandbySyncCommandResponse{ + {output: []byte("rsync failed"), err: errors.New("exit status 1")}, + {}, + }) + + result := syncHistoryStandby() + + Expect(result.err).To(HaveOccurred()) + Expect(result.err.Error()).To(ContainSubstring("rsync standby history snapshot")) + Expect(result.err.Error()).To(ContainSubstring("rsync failed")) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + + sourceLock, err := lockfile.New(historyStandbySyncLockPath(sourceDBPath)) + Expect(err).ToNot(HaveOccurred()) + Expect(sourceLock.TryLock()).To(Succeed()) + Expect(sourceLock.Unlock()).To(Succeed()) + }) + + It("returns lock contention as an error without creating a snapshot", func() { + tmpDir := GinkgoT().TempDir() + primaryDataDir := filepath.Join(tmpDir, "primary") + standbyDataDir := filepath.Join(tmpDir, "standby") + Expect(os.Mkdir(primaryDataDir, 0o700)).To(Succeed()) + sourceDBPath := filepath.Join(primaryDataDir, historyDBNameConst) + createHistoryStandbySyncSQLiteDB(sourceDBPath) + rootHistoryDB = sourceDBPath + mock := setupHistoryStandbySyncClusterConn(primaryDataDir, standbyDataDir, nil, true) + lockPath := historyStandbySyncLockPath(sourceDBPath) + Expect(os.WriteFile(lockPath, []byte(fmt.Sprintf("%d\n", os.Getppid())), 0o600)).To(Succeed()) + defer os.Remove(lockPath) + mkdirTempCalls := 0 + historyStandbySyncMkdirTemp = func(dir, pattern string) (string, error) { + mkdirTempCalls++ + return "", errors.New("snapshot should not be created") + } + + result := syncHistoryStandby() + + Expect(result.err).To(HaveOccurred()) + Expect(result.err.Error()).To(ContainSubstring("lock standby history sync source")) + Expect(mkdirTempCalls).To(Equal(0)) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + }) + + It("quotes remote shell paths in rsync, install, and cleanup commands", func() { + remoteTempPath := "/data dir/standby's/.gpbackup_history.db.tmp" + destPath := "/data dir/standby's/gpbackup_history.db" + + Expect(buildHistoryStandbySyncRsyncArgs("/tmp/snapshot", "sdw-standby", "gpadmin", remoteTempPath)).To(Equal([]string{ + "-p", + "-e", + historyStandbySyncSSHOptions, + "--", + "/tmp/snapshot", + "gpadmin@sdw-standby:" + shellQuoteHistoryStandbySyncPath(remoteTempPath), + })) + installCommand := buildHistoryStandbySyncRemoteInstallCommand(remoteTempPath, destPath) + Expect(installCommand).To(ContainSubstring("test -f " + shellQuoteHistoryStandbySyncPath(remoteTempPath))) + Expect(installCommand).To(ContainSubstring("chown --reference=" + shellQuoteHistoryStandbySyncPath(destPath) + " -- " + shellQuoteHistoryStandbySyncPath(remoteTempPath))) + Expect(installCommand).To(ContainSubstring("chmod --reference=" + shellQuoteHistoryStandbySyncPath(destPath) + " -- " + shellQuoteHistoryStandbySyncPath(remoteTempPath))) + Expect(installCommand).To(ContainSubstring("mv -f -- " + shellQuoteHistoryStandbySyncPath(remoteTempPath) + " " + shellQuoteHistoryStandbySyncPath(destPath))) + Expect(buildHistoryStandbySyncRemoteCleanupCommand(remoteTempPath)).To(Equal("rm -f -- " + shellQuoteHistoryStandbySyncPath(remoteTempPath))) + }) + + It("cleans the concrete remote temp file after install errors", func() { + tmpDir := GinkgoT().TempDir() + primaryDataDir := filepath.Join(tmpDir, "primary") + standbyDataDir := filepath.Join(tmpDir, "standby") + Expect(os.Mkdir(primaryDataDir, 0o700)).To(Succeed()) + sourceDBPath := filepath.Join(primaryDataDir, historyDBNameConst) + createHistoryStandbySyncSQLiteDB(sourceDBPath) + rootHistoryDB = sourceDBPath + mock := setupHistoryStandbySyncClusterConn(primaryDataDir, standbyDataDir, nil, true) + commandCalls := setHistoryStandbySyncCommands([]historyStandbySyncCommandResponse{ + {}, + {output: []byte("install failed"), err: errors.New("exit status 1")}, + {}, + }) + + result := syncHistoryStandby() + + Expect(result.err).To(HaveOccurred()) + Expect(result.err.Error()).To(ContainSubstring("install standby history snapshot")) + Expect(result.err.Error()).To(ContainSubstring("install failed")) + Expect(*commandCalls).To(HaveLen(3)) + Expect((*commandCalls)[2].name).To(Equal("ssh")) + Expect((*commandCalls)[2].args[len((*commandCalls)[2].args)-1]).To(HavePrefix("rm -f -- ")) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + }) + + It("chains remote cleanup errors onto the primary transport error", func() { + historyStandbySyncRunSSHCommand = func(remoteCommand, standbyHost, userName string) ([]byte, error) { + Expect(remoteCommand).To(Equal(buildHistoryStandbySyncRemoteCleanupCommand("/standby/.tmp"))) + Expect(standbyHost).To(Equal("sdw-standby")) + Expect(userName).To(Equal("gpadmin")) + return []byte("cleanup failed"), errors.New("exit status 255") + } + primaryErr := errors.New("install failed") + + err := cleanupHistoryStandbySyncRemoteTempAfterError(primaryErr, "sdw-standby", "gpadmin", "/standby/.tmp") + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("install failed")) + Expect(err.Error()).To(ContainSubstring("additionally failed to clean up remote temp file")) + Expect(err.Error()).To(ContainSubstring("cleanup failed")) + }) + + It("keeps automatic sync best-effort while strict sync treats skips as errors", func() { + stdout, _, _ := testhelper.SetupTestLogger() + syncCalls := 0 + historyStandbySync = func() historyStandbySyncResult { + syncCalls++ + return historyStandbySyncResult{err: errors.New("transport failed")} + } + + result := syncHistoryStandbyBestEffort(false) + Expect(result.err).To(HaveOccurred()) + Expect(syncCalls).To(Equal(1)) + Expect(string(stdout.Contents())).To(ContainSubstring("History db sync to standby coordinator failed; standby history may be stale: transport failed")) + + historyStandbySync = func() historyStandbySyncResult { + return historyStandbySyncResult{skipReason: "no up standby coordinator found"} + } + err := syncHistoryStandbyStrict() + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("history db sync to standby coordinator skipped: no up standby coordinator found")) + }) + + It("skips disabled automatic sync without invoking discovery", func() { + syncCalls := 0 + historyStandbySync = func() historyStandbySyncResult { + syncCalls++ + return historyStandbySyncResult{err: errors.New("sync should not run")} + } + + result := syncHistoryStandbyBestEffort(true) + + Expect(result.err).ToNot(HaveOccurred()) + Expect(result.skipReason).To(Equal("disabled by --" + noHistorySyncStandbyFlagName)) + Expect(syncCalls).To(Equal(0)) + }) +}) + +func createHistoryStandbySyncSQLiteDB(path string) { + Expect(os.MkdirAll(filepath.Dir(path), 0o700)).To(Succeed()) + db, err := sql.Open("sqlite3", historyStandbySyncSQLiteURI(path, "rwc")) + Expect(err).ToNot(HaveOccurred()) + defer db.Close() + + _, err = db.Exec("CREATE TABLE sync_test (id INTEGER PRIMARY KEY, value TEXT)") + Expect(err).ToNot(HaveOccurred()) + _, err = db.Exec("INSERT INTO sync_test (value) VALUES ('present')") + Expect(err).ToNot(HaveOccurred()) +} + +func setupHistoryStandbySyncClusterConn(primaryDataDir, standbyDataDir string, standbyErr error, expectStandby bool) sqlmock.Sqlmock { + return setupHistoryStandbySyncClusterConnWithHook(primaryDataDir, standbyDataDir, standbyErr, expectStandby, func(db *sqlx.DB) (*sqlx.DB, error) { + return db, nil + }) +} + +func setupHistoryStandbySyncClusterConnWithHook( + primaryDataDir string, + standbyDataDir string, + standbyErr error, + expectStandby bool, + hook func(*sqlx.DB) (*sqlx.DB, error), +) sqlmock.Sqlmock { + sqlDB, mock, err := sqlmock.New() + Expect(err).ToNot(HaveOccurred()) + db := sqlx.NewDb(sqlDB, "sqlmock") + mock.ExpectQuery(regexp.QuoteMeta(historyStandbySyncPrimarySQL)). + WillReturnRows(sqlmock.NewRows([]string{"datadir"}).AddRow(primaryDataDir)) + if expectStandby { + if standbyErr != nil { + mock.ExpectQuery(regexp.QuoteMeta(historyStandbySyncStandbySQL)).WillReturnError(standbyErr) + } else { + mock.ExpectQuery(regexp.QuoteMeta(historyStandbySyncStandbySQL)). + WillReturnRows(sqlmock.NewRows([]string{"hostname", "datadir"}).AddRow("sdw-standby", standbyDataDir)) + } + } + mock.ExpectClose() + historyStandbySyncOpenClusterConn = func() (*sqlx.DB, error) { + return hook(db) + } + return mock +} + +func setHistoryStandbySyncCommands(responses []historyStandbySyncCommandResponse) *[]historyStandbySyncCommandCall { + calls := make([]historyStandbySyncCommandCall, 0) + execCombinedOutputCommand = func(name string, args ...string) combinedOutputCommand { + calls = append(calls, historyStandbySyncCommandCall{name: name, args: append([]string{}, args...)}) + response := historyStandbySyncCommandResponse{} + if len(calls) <= len(responses) { + response = responses[len(calls)-1] + } + return historyStandbySyncFakeCommand{output: response.output, err: response.err} + } + return &calls +} diff --git a/gpbackman/cmd/wrappers.go b/gpbackman/cmd/wrappers.go index c36a5256..e5540e30 100644 --- a/gpbackman/cmd/wrappers.go +++ b/gpbackman/cmd/wrappers.go @@ -23,6 +23,7 @@ import ( "database/sql" "fmt" "os" + "os/exec" "path/filepath" "strings" @@ -36,6 +37,14 @@ import ( var execOSExit = os.Exit +type combinedOutputCommand interface { + CombinedOutput() ([]byte, error) +} + +var execCombinedOutputCommand = func(name string, args ...string) combinedOutputCommand { + return exec.Command(name, args...) +} + func logHeadersDebug() { gplog.Debug("Start %s version %s", commandName, getVersion()) gplog.Debug("Use console log level: %s", rootLogLevelConsole) @@ -122,6 +131,14 @@ func formatBackupDuration(value float64) string { return fmt.Sprintf("%02d:%02d:%02d", hours, minutes, seconds) } +func runHistoryMutationWithStandbySync(work func() error, disabled bool) { + if err := work(); err != nil { + execOSExit(exitErrorCode) + return + } + _ = syncHistoryStandbyBestEffort(disabled) +} + // The backup can be used in one of the cases for local and plugin backups: // - backup is active // - backup is not active, but the --force flag is set. diff --git a/gpbackman/cmd/wrappers_test.go b/gpbackman/cmd/wrappers_test.go index 10c0d0c5..a9165b2d 100644 --- a/gpbackman/cmd/wrappers_test.go +++ b/gpbackman/cmd/wrappers_test.go @@ -20,6 +20,7 @@ under the License. package cmd import ( + "errors" "fmt" "os" "path/filepath" @@ -100,6 +101,72 @@ var _ = Describe("wrappers tests", func() { }) }) + Describe("runHistoryMutationWithStandbySync", func() { + var ( + originalHistoryStandbySync func() historyStandbySyncResult + originalExecOSExit func(int) + ) + + BeforeEach(func() { + originalHistoryStandbySync = historyStandbySync + originalExecOSExit = execOSExit + }) + + AfterEach(func() { + historyStandbySync = originalHistoryStandbySync + execOSExit = originalExecOSExit + }) + + It("runs standby sync after successful work returns", func() { + calls := make([]string, 0) + historyStandbySync = func() historyStandbySyncResult { + calls = append(calls, "sync") + return historyStandbySyncResult{} + } + + runHistoryMutationWithStandbySync(func() error { + calls = append(calls, "work") + return nil + }, false) + + Expect(calls).To(Equal([]string{"work", "sync"})) + }) + + It("does not run standby sync after work errors", func() { + syncCalls := 0 + exitCalls := 0 + historyStandbySync = func() historyStandbySyncResult { + syncCalls++ + return historyStandbySyncResult{} + } + execOSExit = func(code int) { + exitCalls++ + Expect(code).To(Equal(exitErrorCode)) + } + + runHistoryMutationWithStandbySync(func() error { + return errors.New("work failed") + }, false) + + Expect(syncCalls).To(Equal(0)) + Expect(exitCalls).To(Equal(1)) + }) + + It("honors the disabled automatic policy after successful work", func() { + syncCalls := 0 + historyStandbySync = func() historyStandbySyncResult { + syncCalls++ + return historyStandbySyncResult{} + } + + runHistoryMutationWithStandbySync(func() error { + return nil + }, true) + + Expect(syncCalls).To(Equal(0)) + }) + }) + Describe("checkCompatibleFlags", func() { It("does not return error when no flags changed", func() { flags := pflag.NewFlagSet("test", pflag.ContinueOnError) From 8d3f00b324e345a7966566e12a0d3afb6fdd7da2 Mon Sep 17 00:00:00 2001 From: woblerr Date: Tue, 28 Jul 2026 17:04:39 +0000 Subject: [PATCH 04/19] Add gpbackman history sync command and mutation hooks. --- gpbackman/cmd/backup_clean.go | 26 +-- gpbackman/cmd/backup_delete.go | 26 +-- gpbackman/cmd/constants.go | 1 + gpbackman/cmd/history_clean.go | 16 +- gpbackman/cmd/history_standby_sync.go | 14 +- gpbackman/cmd/history_sync.go | 54 ++++++ gpbackman/cmd/history_sync_test.go | 254 ++++++++++++++++++++++++++ gpbackman/cmd/wrappers.go | 2 +- gpbackman/cmd/wrappers_test.go | 34 ++++ gpbackman/textmsg/error.go | 8 + gpbackman/textmsg/error_test.go | 12 ++ gpbackman/textmsg/info.go | 12 ++ gpbackman/textmsg/info_test.go | 3 + gpbackman/textmsg/warn.go | 4 + gpbackman/textmsg/warn_test.go | 12 ++ 15 files changed, 442 insertions(+), 36 deletions(-) create mode 100644 gpbackman/cmd/history_sync.go create mode 100644 gpbackman/cmd/history_sync_test.go diff --git a/gpbackman/cmd/backup_clean.go b/gpbackman/cmd/backup_clean.go index 46c9548b..5a7eed3c 100644 --- a/gpbackman/cmd/backup_clean.go +++ b/gpbackman/cmd/backup_clean.go @@ -34,13 +34,14 @@ import ( // Flags for the gpbackman backup-clean command (backupCleanCmd) var ( - backupCleanBeforeTimestamp string - backupCleanAfterTimestamp string - backupCleanPluginConfigFile string - backupCleanBackupDir string - backupCleanOlderThanDays uint - backupCleanParallelProcesses int - backupCleanCascade bool + backupCleanBeforeTimestamp string + backupCleanAfterTimestamp string + backupCleanPluginConfigFile string + backupCleanBackupDir string + backupCleanOlderThanDays uint + backupCleanParallelProcesses int + backupCleanCascade bool + backupCleanNoHistorySyncStandby bool ) var backupCleanCmd = &cobra.Command{ @@ -130,6 +131,12 @@ func init() { 1, "the number of parallel processes to delete local backups", ) + backupCleanCmd.Flags().BoolVar( + &backupCleanNoHistorySyncStandby, + noHistorySyncStandbyFlagName, + false, + "skip automatic gpbackup_history.db sync to standby coordinator after this command", + ) backupCleanCmd.MarkFlagsMutuallyExclusive(beforeTimestampFlagName, olderThanDaysFlagName, afterTimestampFlagName) } @@ -198,10 +205,7 @@ func doCleanBackupFlagValidation(flags *pflag.FlagSet) { func doCleanBackup() { logHeadersDebug() - err := cleanBackup() - if err != nil { - execOSExit(exitErrorCode) - } + runHistoryMutationWithStandbySync(cleanBackup, backupCleanNoHistorySyncStandby) } func cleanBackup() error { diff --git a/gpbackman/cmd/backup_delete.go b/gpbackman/cmd/backup_delete.go index 4eb525a5..46f29b48 100644 --- a/gpbackman/cmd/backup_delete.go +++ b/gpbackman/cmd/backup_delete.go @@ -41,13 +41,14 @@ import ( // Flags for the gpbackman backup-delete command (backupDeleteCmd) var ( - backupDeleteTimestamp []string - backupDeletePluginConfigFile string - backupDeleteBackupDir string - backupDeleteCascade bool - backupDeleteForce bool - backupDeleteIgnoreErrors bool - backupDeleteParallelProcesses int + backupDeleteTimestamp []string + backupDeletePluginConfigFile string + backupDeleteBackupDir string + backupDeleteCascade bool + backupDeleteForce bool + backupDeleteIgnoreErrors bool + backupDeleteNoHistorySyncStandby bool + backupDeleteParallelProcesses int ) var backupDeleteCmd = &cobra.Command{ Use: "backup-delete", @@ -139,6 +140,12 @@ func init() { false, "ignore errors when deleting backups", ) + backupDeleteCmd.Flags().BoolVar( + &backupDeleteNoHistorySyncStandby, + noHistorySyncStandbyFlagName, + false, + "skip automatic gpbackup_history.db sync to standby coordinator after this command", + ) _ = backupDeleteCmd.MarkPersistentFlagRequired(timestampFlagName) } @@ -198,10 +205,7 @@ func doDeleteBackupFlagValidation(flags *pflag.FlagSet) { func doDeleteBackup() { logHeadersDebug() - err := deleteBackup() - if err != nil { - execOSExit(exitErrorCode) - } + runHistoryMutationWithStandbySync(deleteBackup, backupDeleteNoHistorySyncStandby) } func deleteBackup() error { diff --git a/gpbackman/cmd/constants.go b/gpbackman/cmd/constants.go index 03b13d36..99b8f756 100644 --- a/gpbackman/cmd/constants.go +++ b/gpbackman/cmd/constants.go @@ -56,6 +56,7 @@ const ( backupDirFlagName = "backup-dir" parallelProcessesFlagName = "parallel-processes" ignoreErrorsFlagName = "ignore-errors" + noHistorySyncStandbyFlagName = "no-history-sync-standby" detailFlagName = "detail" exitErrorCode = 1 diff --git a/gpbackman/cmd/history_clean.go b/gpbackman/cmd/history_clean.go index 42366421..f5ea6489 100644 --- a/gpbackman/cmd/history_clean.go +++ b/gpbackman/cmd/history_clean.go @@ -32,8 +32,9 @@ import ( // Flags for the gpbackman history-clean command (historyCleanCmd) var ( - historyCleanBeforeTimestamp string - historyCleanOlderThanDays uint + historyCleanBeforeTimestamp string + historyCleanOlderThanDays uint + historyCleanNoHistorySyncStandby bool ) var historyCleanCmd = &cobra.Command{ @@ -73,6 +74,12 @@ func init() { "", "delete information about backups older than the given timestamp", ) + historyCleanCmd.Flags().BoolVar( + &historyCleanNoHistorySyncStandby, + noHistorySyncStandbyFlagName, + false, + "skip automatic gpbackup_history.db sync to standby coordinator after this command", + ) historyCleanCmd.MarkFlagsMutuallyExclusive(beforeTimestampFlagName, olderThanDaysFlagName) } @@ -99,10 +106,7 @@ func doCleanHistoryFlagValidation(flags *pflag.FlagSet) { func doCleanHistory() { logHeadersDebug() - err := cleanHistory() - if err != nil { - execOSExit(exitErrorCode) - } + runHistoryMutationWithStandbySync(cleanHistory, historyCleanNoHistorySyncStandby) } func cleanHistory() error { diff --git a/gpbackman/cmd/history_standby_sync.go b/gpbackman/cmd/history_standby_sync.go index 8e5301c6..5a86043e 100644 --- a/gpbackman/cmd/history_standby_sync.go +++ b/gpbackman/cmd/history_standby_sync.go @@ -30,6 +30,7 @@ import ( "time" "github.com/apache/cloudberry-backup/gpbackman/gpbckpconfig" + "github.com/apache/cloudberry-backup/gpbackman/textmsg" "github.com/apache/cloudberry-go-libs/gplog" "github.com/apache/cloudberry-go-libs/operating" "github.com/jmoiron/sqlx" @@ -38,7 +39,6 @@ import ( ) const ( - noHistorySyncStandbyFlagName = "no-history-sync-standby" historyStandbySyncSSHOptions = "ssh -o StrictHostKeyChecking=no -o ConnectTimeout=30" historyStandbySyncTempDirPattern = "gpbackman-history-standby-sync-%s-%d-*" ) @@ -76,17 +76,17 @@ var ( func syncHistoryStandbyBestEffort(disabled bool) historyStandbySyncResult { if disabled { result := historyStandbySyncResult{skipReason: "disabled by --" + noHistorySyncStandbyFlagName} - gplog.Info("Skipping history db sync to standby coordinator: %s", result.skipReason) + gplog.Info("%s", textmsg.InfoTextHistoryStandbySyncSkip(result.skipReason)) return result } result := historyStandbySync() if result.err != nil { - gplog.Warn("History db sync to standby coordinator failed; standby history may be stale: %v", result.err) + gplog.Warn("%s", textmsg.WarnTextHistoryStandbySyncFailed(result.err)) return result } if result.skipReason != "" { - gplog.Debug("Skipping history db sync to standby coordinator: %s", result.skipReason) + gplog.Debug("%s", textmsg.InfoTextHistoryStandbySyncSkip(result.skipReason)) } return result } @@ -97,7 +97,7 @@ func syncHistoryStandbyStrict() error { return result.err } if result.skipReason != "" { - return fmt.Errorf("history db sync to standby coordinator skipped: %s", result.skipReason) + return textmsg.ErrorHistoryStandbySyncSkippedError(result.skipReason) } return nil } @@ -121,7 +121,7 @@ func syncHistoryStandby() historyStandbySyncResult { return historyStandbySyncResult{err: fmt.Errorf("resolve current OS user for standby history sync: %w", err)} } - gplog.Info("Sync history db to standby coordinator: %s", target.sourceDBPath) + gplog.Info("%s", textmsg.InfoTextHistoryStandbySyncStart(target.sourceDBPath)) err = withHistoryStandbySyncLock(target.sourceDBPath, func() error { return withHistoryStandbySyncSnapshot(target.sourceDBPath, target.sourceMode, func(snapshotPath string) error { return syncHistoryStandbySnapshotToStandby(target, userName, snapshotPath) @@ -130,7 +130,7 @@ func syncHistoryStandby() historyStandbySyncResult { if err != nil { return historyStandbySyncResult{err: err} } - gplog.Info("History db sync to standby coordinator succeeded: %s:%s", target.standbyHost, target.standbyHistoryDBPath) + gplog.Info("%s", textmsg.InfoTextHistoryStandbySyncSuccess(target.standbyHost, target.standbyHistoryDBPath)) return historyStandbySyncResult{} } diff --git a/gpbackman/cmd/history_sync.go b/gpbackman/cmd/history_sync.go new file mode 100644 index 00000000..2efcd399 --- /dev/null +++ b/gpbackman/cmd/history_sync.go @@ -0,0 +1,54 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +*/ + +package cmd + +import ( + "github.com/apache/cloudberry-go-libs/gplog" + "github.com/spf13/cobra" + + "github.com/apache/cloudberry-backup/gpbackman/textmsg" +) + +var historySyncCmd = &cobra.Command{ + Use: "history-sync", + Short: "Sync the history database to the standby coordinator", + Long: `Sync the gpbackup_history.db file to the standby coordinator. + +The command uses the cluster history database from --history-db, or from +$COORDINATOR_DATA_DIRECTORY when --auto-load-history-db is set. It succeeds +only after the standby file is replaced atomically with a verified snapshot.`, + Args: cobra.NoArgs, + Run: func(cmd *cobra.Command, args []string) { + doRootFlagValidation(cmd.Flags(), checkFileExistsConst) + doHistorySync() + }, +} + +func init() { + rootCmd.AddCommand(historySyncCmd) +} + +func doHistorySync() { + logHeadersDebug() + if err := syncHistoryStandbyStrict(); err != nil { + gplog.Error("%s", textmsg.ErrorTextUnableSyncHistoryDBToStandby(err)) + execOSExit(exitErrorCode) + } +} diff --git a/gpbackman/cmd/history_sync_test.go b/gpbackman/cmd/history_sync_test.go new file mode 100644 index 00000000..2c7f6910 --- /dev/null +++ b/gpbackman/cmd/history_sync_test.go @@ -0,0 +1,254 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +*/ + +package cmd + +import ( + "bytes" + "errors" + "os" + + "github.com/apache/cloudberry-go-libs/testhelper" + "github.com/spf13/cobra" + "github.com/spf13/pflag" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("history sync command", func() { + Describe("command registration", func() { + AfterEach(func() { + rootCmd.SetOut(os.Stdout) + rootCmd.SetErr(os.Stderr) + }) + + It("shows history-sync in root help without exposing the automatic disable flag", func() { + var output bytes.Buffer + rootCmd.SetOut(&output) + rootCmd.SetErr(&output) + + Expect(rootCmd.Help()).To(Succeed()) + + help := output.String() + Expect(help).To(ContainSubstring("history-sync")) + Expect(help).ToNot(ContainSubstring(noHistorySyncStandbyFlagName)) + }) + + It("registers no-history-sync-standby only on mutation commands", func() { + mutationCommands := map[string]bool{ + "backup-delete": true, + "backup-clean": true, + "history-clean": true, + } + + for _, command := range rootCmd.Commands() { + flag := command.Flags().Lookup(noHistorySyncStandbyFlagName) + if mutationCommands[command.Name()] { + Expect(flag).ToNot(BeNil(), command.Name()) + Expect(flag.DefValue).To(Equal("false"), command.Name()) + continue + } + Expect(flag).To(BeNil(), command.Name()) + } + Expect(rootCmd.PersistentFlags().Lookup(noHistorySyncStandbyFlagName)).To(BeNil()) + }) + + It("keeps history-sync strict with inherited global flags and no local flags", func() { + Expect(commandByName("history-sync")).To(Equal(historySyncCmd)) + Expect(historySyncCmd.Args(historySyncCmd, []string{"unexpected"})).To(HaveOccurred()) + Expect(flagNames(historySyncCmd.LocalFlags())).To(BeEmpty()) + Expect(historySyncCmd.Flags().Lookup(noHistorySyncStandbyFlagName)).To(BeNil()) + for _, flagName := range []string{ + historyDBFlagName, + autoLoadHistoryDBFlagName, + logFileFlagName, + logLevelConsoleFlagName, + logLevelFileFlagName, + } { + Expect(historySyncCmd.Flag(flagName)).ToNot(BeNil(), flagName) + } + }) + }) + + Describe("strict history sync execution", func() { + var ( + originalHistoryStandbySync func() historyStandbySyncResult + originalExecOSExit func(int) + savedRootHistoryDB string + savedRootAutoLoadHistoryDB bool + savedPGPassword string + savedPGPasswordPresent bool + exitCodes []int + ) + + BeforeEach(func() { + testhelper.SetupTestLogger() + originalHistoryStandbySync = historyStandbySync + originalExecOSExit = execOSExit + savedRootHistoryDB = rootHistoryDB + savedRootAutoLoadHistoryDB = rootAutoLoadHistoryDB + savedPGPassword, savedPGPasswordPresent = os.LookupEnv("PGPASSWORD") + Expect(os.Setenv("PGPASSWORD", "do-not-log-this-password")).To(Succeed()) + exitCodes = make([]int, 0) + execOSExit = func(code int) { + exitCodes = append(exitCodes, code) + } + rootHistoryDB = "" + rootAutoLoadHistoryDB = false + }) + + AfterEach(func() { + historyStandbySync = originalHistoryStandbySync + execOSExit = originalExecOSExit + rootHistoryDB = savedRootHistoryDB + rootAutoLoadHistoryDB = savedRootAutoLoadHistoryDB + if savedPGPasswordPresent { + Expect(os.Setenv("PGPASSWORD", savedPGPassword)).To(Succeed()) + } else { + Expect(os.Unsetenv("PGPASSWORD")).To(Succeed()) + } + }) + + It("exits zero only when strict sync succeeds", func() { + historyStandbySync = func() historyStandbySyncResult { + return historyStandbySyncResult{} + } + + doHistorySync() + + Expect(exitCodes).To(BeEmpty()) + }) + + It("treats the default working-directory source as a strict error", func() { + stdout, stderr, _ := testhelper.SetupTestLogger() + historyStandbySync = syncHistoryStandby + + doHistorySync() + + logOutput := string(stdout.Contents()) + string(stderr.Contents()) + Expect(exitCodes).To(Equal([]int{exitErrorCode})) + Expect(logOutput).To(ContainSubstring("Unable to sync history db to standby coordinator")) + Expect(logOutput).To(ContainSubstring("using default working-directory history db")) + Expect(logOutput).ToNot(ContainSubstring("do-not-log-this-password")) + }) + + It("exits with one for strict skip and stage errors", func() { + tests := []struct { + name string + result historyStandbySyncResult + want string + }{ + { + name: "custom source", + result: historyStandbySyncResult{skipReason: "source history db /custom/gpbackup_history.db is not cluster history db /primary/gpbackup_history.db"}, + want: "source history db /custom/gpbackup_history.db is not cluster history db /primary/gpbackup_history.db", + }, + { + name: "no standby", + result: historyStandbySyncResult{skipReason: "no up standby coordinator found"}, + want: "no up standby coordinator found", + }, + { + name: "busy lock", + result: historyStandbySyncResult{err: errors.New("lock standby history sync source /primary/gpbackup_history.db: already locked")}, + want: "lock standby history sync source /primary/gpbackup_history.db", + }, + { + name: "stage error", + result: historyStandbySyncResult{err: errors.New("validate standby history sync snapshot quick_check failed")}, + want: "validate standby history sync snapshot quick_check failed", + }, + } + + for _, tt := range tests { + stdout, stderr, _ := testhelper.SetupTestLogger() + exitCodes = make([]int, 0) + result := tt.result + historyStandbySync = func() historyStandbySyncResult { + return result + } + + doHistorySync() + + logOutput := string(stdout.Contents()) + string(stderr.Contents()) + Expect(exitCodes).To(Equal([]int{exitErrorCode}), tt.name) + Expect(logOutput).To(ContainSubstring("Unable to sync history db to standby coordinator"), tt.name) + Expect(logOutput).To(ContainSubstring(tt.want), tt.name) + Expect(logOutput).ToNot(ContainSubstring("do-not-log-this-password"), tt.name) + } + }) + }) + + Describe("mutation command automatic sync hooks", func() { + var ( + originalRunHistoryMutationWithStandbySync func(func() error, bool) + savedBackupDeleteNoHistorySyncStandby bool + savedBackupCleanNoHistorySyncStandby bool + savedHistoryCleanNoHistorySyncStandby bool + ) + + BeforeEach(func() { + originalRunHistoryMutationWithStandbySync = runHistoryMutationWithStandbySync + savedBackupDeleteNoHistorySyncStandby = backupDeleteNoHistorySyncStandby + savedBackupCleanNoHistorySyncStandby = backupCleanNoHistorySyncStandby + savedHistoryCleanNoHistorySyncStandby = historyCleanNoHistorySyncStandby + }) + + AfterEach(func() { + runHistoryMutationWithStandbySync = originalRunHistoryMutationWithStandbySync + backupDeleteNoHistorySyncStandby = savedBackupDeleteNoHistorySyncStandby + backupCleanNoHistorySyncStandby = savedBackupCleanNoHistorySyncStandby + historyCleanNoHistorySyncStandby = savedHistoryCleanNoHistorySyncStandby + }) + + It("wraps all mutation commands and passes their disable flag values", func() { + disabledValues := make([]bool, 0) + runHistoryMutationWithStandbySync = func(work func() error, disabled bool) { + disabledValues = append(disabledValues, disabled) + } + backupDeleteNoHistorySyncStandby = true + backupCleanNoHistorySyncStandby = false + historyCleanNoHistorySyncStandby = true + + doDeleteBackup() + doCleanBackup() + doCleanHistory() + + Expect(disabledValues).To(Equal([]bool{true, false, true})) + }) + }) +}) + +func commandByName(name string) *cobra.Command { + for _, command := range rootCmd.Commands() { + if command.Name() == name { + return command + } + } + return nil +} + +func flagNames(flags *pflag.FlagSet) []string { + names := make([]string, 0) + flags.VisitAll(func(flag *pflag.Flag) { + names = append(names, flag.Name) + }) + return names +} diff --git a/gpbackman/cmd/wrappers.go b/gpbackman/cmd/wrappers.go index e5540e30..a3c66425 100644 --- a/gpbackman/cmd/wrappers.go +++ b/gpbackman/cmd/wrappers.go @@ -131,7 +131,7 @@ func formatBackupDuration(value float64) string { return fmt.Sprintf("%02d:%02d:%02d", hours, minutes, seconds) } -func runHistoryMutationWithStandbySync(work func() error, disabled bool) { +var runHistoryMutationWithStandbySync = func(work func() error, disabled bool) { if err := work(); err != nil { execOSExit(exitErrorCode) return diff --git a/gpbackman/cmd/wrappers_test.go b/gpbackman/cmd/wrappers_test.go index a9165b2d..12342ae5 100644 --- a/gpbackman/cmd/wrappers_test.go +++ b/gpbackman/cmd/wrappers_test.go @@ -132,6 +132,24 @@ var _ = Describe("wrappers tests", func() { Expect(calls).To(Equal([]string{"work", "sync"})) }) + It("runs standby sync after deferred work cleanup completes", func() { + calls := make([]string, 0) + historyStandbySync = func() historyStandbySyncResult { + calls = append(calls, "sync") + return historyStandbySyncResult{} + } + + runHistoryMutationWithStandbySync(func() error { + calls = append(calls, "work") + defer func() { + calls = append(calls, "close") + }() + return nil + }, false) + + Expect(calls).To(Equal([]string{"work", "close", "sync"})) + }) + It("does not run standby sync after work errors", func() { syncCalls := 0 exitCalls := 0 @@ -152,6 +170,22 @@ var _ = Describe("wrappers tests", func() { Expect(exitCalls).To(Equal(1)) }) + It("keeps the command exit code successful when automatic sync fails", func() { + exitCalls := 0 + historyStandbySync = func() historyStandbySyncResult { + return historyStandbySyncResult{err: errors.New("transport failed")} + } + execOSExit = func(code int) { + exitCalls++ + } + + runHistoryMutationWithStandbySync(func() error { + return nil + }, false) + + Expect(exitCalls).To(Equal(0)) + }) + It("honors the disabled automatic policy after successful work", func() { syncCalls := 0 historyStandbySync = func() historyStandbySyncResult { diff --git a/gpbackman/textmsg/error.go b/gpbackman/textmsg/error.go index 46599331..fc6475e3 100644 --- a/gpbackman/textmsg/error.go +++ b/gpbackman/textmsg/error.go @@ -147,6 +147,10 @@ func ErrorTextUnableCleanDB(err error) string { return fmt.Sprintf("Unable to clean db. Error: %v", err) } +func ErrorTextUnableSyncHistoryDBToStandby(err error) string { + return fmt.Sprintf("Unable to sync history db to standby coordinator. Error: %v", err) +} + func ErrorTextUnableDeletePluginBackup(backupName string, err error) string { return fmt.Sprintf("Unable to delete plugin backup %s. Error: %v", backupName, err) } @@ -259,6 +263,10 @@ func ErrorInvalidInputValueError(value string) error { return fmt.Errorf("invalid input value: %s", value) } +func ErrorHistoryStandbySyncSkippedError(reason string) error { + return fmt.Errorf("history db sync to standby coordinator skipped: %s", reason) +} + // Error that is returned when backup has specific delete status. func ErrorSetBackupDeleteStatus(backupName, status string) error { diff --git a/gpbackman/textmsg/error_test.go b/gpbackman/textmsg/error_test.go index b022f504..cd86e482 100644 --- a/gpbackman/textmsg/error_test.go +++ b/gpbackman/textmsg/error_test.go @@ -42,6 +42,7 @@ var _ = Describe("error tests", func() { {"ErrorTextUnableCheckPath", ErrorTextUnableCheckPath, "Unable to check path. Error: test error"}, {"ErrorTextUnableDeleteLocalBackup", ErrorTextUnableDeleteLocalBackup, "Unable to delete local backup. Error: test error"}, {"ErrorTextUnableCleanDB", ErrorTextUnableCleanDB, "Unable to clean db. Error: test error"}, + {"ErrorTextUnableSyncHistoryDBToStandby", ErrorTextUnableSyncHistoryDBToStandby, "Unable to sync history db to standby coordinator. Error: test error"}, } for _, tt := range tests { Expect(tt.function(testError)).To(Equal(tt.want), tt.name) @@ -161,4 +162,15 @@ var _ = Describe("error tests", func() { } }) }) + + Describe("history standby sync errors", func() { + It("returns accepted text without environment values", func() { + err := ErrorHistoryStandbySyncSkippedError("no up standby coordinator found") + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("history db sync to standby coordinator skipped: no up standby coordinator found")) + Expect(err.Error()).ToNot(ContainSubstring("PGPASSWORD")) + Expect(err.Error()).ToNot(ContainSubstring("secret")) + }) + }) }) diff --git a/gpbackman/textmsg/info.go b/gpbackman/textmsg/info.go index dafe1408..f8a1f7e5 100644 --- a/gpbackman/textmsg/info.go +++ b/gpbackman/textmsg/info.go @@ -71,3 +71,15 @@ func InfoTextSegmentPrefix(segPrefix string) string { func InfoTextNothingToDo() string { return "Nothing to do" } + +func InfoTextHistoryStandbySyncStart(sourceDBPath string) string { + return fmt.Sprintf("Sync history db to standby coordinator: %s", sourceDBPath) +} + +func InfoTextHistoryStandbySyncSuccess(standbyHost, standbyHistoryDBPath string) string { + return fmt.Sprintf("History db sync to standby coordinator succeeded: %s:%s", standbyHost, standbyHistoryDBPath) +} + +func InfoTextHistoryStandbySyncSkip(reason string) string { + return fmt.Sprintf("Skipping history db sync to standby coordinator: %s", reason) +} diff --git a/gpbackman/textmsg/info_test.go b/gpbackman/textmsg/info_test.go index 34d7c5c1..a7438d32 100644 --- a/gpbackman/textmsg/info_test.go +++ b/gpbackman/textmsg/info_test.go @@ -38,6 +38,8 @@ var _ = Describe("info tests", func() { {"InfoTextBackupAlreadyDeleted", "TestBackup", InfoTextBackupAlreadyDeleted, "Backup TestBackup has already been deleted"}, {"InfoTextBackupDirPath", "/test/path", InfoTextBackupDirPath, "Path to backup directory: /test/path"}, {"InfoTextSegmentPrefix", "TestValue", InfoTextSegmentPrefix, "Segment Prefix: TestValue"}, + {"InfoTextHistoryStandbySyncStart", "/data/gpbackup_history.db", InfoTextHistoryStandbySyncStart, "Sync history db to standby coordinator: /data/gpbackup_history.db"}, + {"InfoTextHistoryStandbySyncSkip", "no up standby coordinator found", InfoTextHistoryStandbySyncSkip, "Skipping history db sync to standby coordinator: no up standby coordinator found"}, } for _, tt := range tests { Expect(tt.function(tt.value)).To(Equal(tt.want), tt.name) @@ -55,6 +57,7 @@ var _ = Describe("info tests", func() { want string }{ {"InfoTextBackupStatus", "TestBackup", "In Progress", InfoTextBackupStatus, "Backup TestBackup has status: In Progress"}, + {"InfoTextHistoryStandbySyncSuccess", "sdw-standby", "/standby/gpbackup_history.db", InfoTextHistoryStandbySyncSuccess, "History db sync to standby coordinator succeeded: sdw-standby:/standby/gpbackup_history.db"}, } for _, tt := range tests { Expect(tt.function(tt.value1, tt.value2)).To(Equal(tt.want), tt.name) diff --git a/gpbackman/textmsg/warn.go b/gpbackman/textmsg/warn.go index 97dc366e..eb90663c 100644 --- a/gpbackman/textmsg/warn.go +++ b/gpbackman/textmsg/warn.go @@ -24,3 +24,7 @@ import "fmt" func WarnTextBackupUnableGetReport(backupName string) string { return fmt.Sprintf("Unable to get report for backup %s. Check if backup is active", backupName) } + +func WarnTextHistoryStandbySyncFailed(err error) string { + return fmt.Sprintf("History db sync to standby coordinator failed; standby history may be stale: %v", err) +} diff --git a/gpbackman/textmsg/warn_test.go b/gpbackman/textmsg/warn_test.go index 775d9c51..50077e5e 100644 --- a/gpbackman/textmsg/warn_test.go +++ b/gpbackman/textmsg/warn_test.go @@ -20,6 +20,8 @@ under the License. package textmsg import ( + "errors" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -40,4 +42,14 @@ var _ = Describe("warn tests", func() { } }) }) + + Describe("warn text functions with error only", func() { + It("returns correct warn text without environment values", func() { + text := WarnTextHistoryStandbySyncFailed(errors.New("transport failed")) + + Expect(text).To(Equal("History db sync to standby coordinator failed; standby history may be stale: transport failed")) + Expect(text).ToNot(ContainSubstring("PGPASSWORD")) + Expect(text).ToNot(ContainSubstring("secret")) + }) + }) }) From b3fb53cc83b4f7923b918721634945a53d0b4890 Mon Sep 17 00:00:00 2001 From: woblerr Date: Wed, 29 Jul 2026 13:55:01 +0000 Subject: [PATCH 05/19] Add standby history sync end-to-end coverage. --- end_to_end/history_standby_sync_test.go | 215 ++++++++++++++++++++++++ 1 file changed, 215 insertions(+) create mode 100644 end_to_end/history_standby_sync_test.go diff --git a/end_to_end/history_standby_sync_test.go b/end_to_end/history_standby_sync_test.go new file mode 100644 index 00000000..20bbe639 --- /dev/null +++ b/end_to_end/history_standby_sync_test.go @@ -0,0 +1,215 @@ +/* +Licensed to the Apache Software Foundation (ASF) under one +or more contributor license agreements. See the NOTICE file +distributed with this work for additional information +regarding copyright ownership. The ASF licenses this file +to you under the Apache License, Version 2.0 (the +"License"); you may not use this file except in compliance +with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, +software distributed under the License is distributed on an +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +KIND, either express or implied. See the License for the +specific language governing permissions and limitations +under the License. +*/ + +package end_to_end_test + +import ( + "bytes" + "database/sql" + "fmt" + "net/url" + "os" + "os/exec" + stdpath "path/filepath" + "strings" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +const upStandbyCoordinatorQuery = ` + SELECT hostname, datadir + FROM gp_segment_configuration + WHERE content = -1 AND role = 'm' AND status = 'u'` + +type standbyCoordinatorTarget struct { + Hostname string `db:"hostname"` + DataDir string `db:"datadir"` +} + +type historyLogicalRow struct { + Timestamp string + Status string + DateDeleted string +} + +func discoverUpStandbyCoordinator() standbyCoordinatorTarget { + var targets []standbyCoordinatorTarget + err := backupConn.Select(&targets, upStandbyCoordinatorQuery) + Expect(err).ToNot(HaveOccurred()) + if len(targets) == 0 { + Skip("standby history sync requires an up standby coordinator") + } + Expect(targets).To(HaveLen(1), "expected exactly one up standby coordinator") + return targets[0] +} + +func quoteRemoteShellPath(value string) string { + return "'" + strings.ReplaceAll(value, "'", "'\"'\"'") + "'" +} + +func copyStandbyHistoryDB(target standbyCoordinatorTarget) string { + tempDir, err := os.MkdirTemp("", "gpbackup-history-standby-e2e-") + Expect(err).ToNot(HaveOccurred()) + DeferCleanup(func() { + Expect(os.RemoveAll(tempDir)).To(Succeed()) + }) + + localPath := stdpath.Join(tempDir, "gpbackup_history.db") + localFile, err := os.OpenFile(localPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0600) + Expect(err).ToNot(HaveOccurred()) + + remotePath := stdpath.Join(target.DataDir, "gpbackup_history.db") + remoteCommand := fmt.Sprintf("cat -- %s", quoteRemoteShellPath(remotePath)) + command := exec.Command( + "ssh", + "-o", "StrictHostKeyChecking=no", + "-o", "ConnectTimeout=30", + target.Hostname, + remoteCommand, + ) + command.Stdout = localFile + var stderr bytes.Buffer + command.Stderr = &stderr + + runErr := command.Run() + closeErr := localFile.Close() + Expect(runErr).ToNot(HaveOccurred(), "copy %s:%s: %s", target.Hostname, remotePath, strings.TrimSpace(stderr.String())) + Expect(closeErr).ToNot(HaveOccurred()) + return localPath +} + +func readHistoryLogicalRows(historyDBPath string) []historyLogicalRow { + dsn := (&url.URL{Scheme: "file", Path: historyDBPath}).String() + "?mode=ro" + db, err := sql.Open("sqlite3", dsn) + Expect(err).ToNot(HaveOccurred()) + defer db.Close() + + var quickCheck string + err = db.QueryRow("PRAGMA quick_check").Scan(&quickCheck) + Expect(err).ToNot(HaveOccurred()) + Expect(quickCheck).To(Equal("ok")) + + rows, err := db.Query(` + SELECT timestamp, status, date_deleted + FROM backups + ORDER BY timestamp`) + Expect(err).ToNot(HaveOccurred()) + defer rows.Close() + + logicalRows := make([]historyLogicalRow, 0) + for rows.Next() { + var row historyLogicalRow + Expect(rows.Scan(&row.Timestamp, &row.Status, &row.DateDeleted)).To(Succeed()) + logicalRows = append(logicalRows, row) + } + Expect(rows.Err()).ToNot(HaveOccurred()) + return logicalRows +} + +func findHistoryLogicalRow(rows []historyLogicalRow, timestamp string) historyLogicalRow { + for _, row := range rows { + if row.Timestamp == timestamp { + return row + } + } + Fail(fmt.Sprintf("history row %s was not found", timestamp)) + return historyLogicalRow{} +} + +var _ = Describe("history database standby sync", func() { + var ( + primaryHistoryDB string + standbyTarget standbyCoordinatorTarget + ) + + BeforeEach(func() { + if useOldBackupVersion { + Skip("standby history sync is not applicable in old backup version mode") + } + end_to_end_setup() + standbyTarget = discoverUpStandbyCoordinator() + primaryHistoryDB = getHistoryDBPathForCluster() + }) + + AfterEach(func() { + end_to_end_teardown() + }) + + It("keeps standby history logically consistent across automatic, disabled, explicit, and mutation sync", func() { + baselineOutput := gpbackup( + gpbackupPath, + backupHelperPath, + "--backup-dir", backupDir, + ) + baselineTimestamp := getBackupTimestamp(string(baselineOutput)) + Expect(baselineTimestamp).ToNot(BeEmpty()) + + primaryBaseline := readHistoryLogicalRows(primaryHistoryDB) + standbyBaseline := readHistoryLogicalRows(copyStandbyHistoryDB(standbyTarget)) + Expect(standbyBaseline).To(Equal(primaryBaseline)) + Expect(findHistoryLogicalRow(standbyBaseline, baselineTimestamp).Status).To(Equal("Success")) + + disabledOutput := gpbackup( + gpbackupPath, + backupHelperPath, + "--backup-dir", backupDir, + "--no-history-sync-standby", + ) + disabledTimestamp := getBackupTimestamp(string(disabledOutput)) + Expect(disabledTimestamp).ToNot(BeEmpty()) + + primaryAfterDisabledSync := readHistoryLogicalRows(primaryHistoryDB) + Expect(findHistoryLogicalRow(primaryAfterDisabledSync, disabledTimestamp).Status).To(Equal("Success")) + standbyAfterDisabledSync := readHistoryLogicalRows(copyStandbyHistoryDB(standbyTarget)) + Expect(standbyAfterDisabledSync).To(Equal(standbyBaseline)) + + historySyncCommand := exec.Command( + gpbackmanPath, + "history-sync", + "--auto-load-history-db", + ) + historySyncCommand.Env = append( + os.Environ(), + fmt.Sprintf("COORDINATOR_DATA_DIRECTORY=%s", stdpath.Dir(primaryHistoryDB)), + ) + mustRunCommand(historySyncCommand) + + primaryAfterExplicitSync := readHistoryLogicalRows(primaryHistoryDB) + standbyAfterExplicitSync := readHistoryLogicalRows(copyStandbyHistoryDB(standbyTarget)) + Expect(standbyAfterExplicitSync).To(Equal(primaryAfterExplicitSync)) + Expect(findHistoryLogicalRow(standbyAfterExplicitSync, disabledTimestamp).Status).To(Equal("Success")) + + gpbackman( + "backup-delete", + "--history-db", primaryHistoryDB, + "--timestamp", baselineTimestamp, + "--backup-dir", backupDir, + ) + + primaryAfterDelete := readHistoryLogicalRows(primaryHistoryDB) + deletedRow := findHistoryLogicalRow(primaryAfterDelete, baselineTimestamp) + Expect(deletedRow.DateDeleted).ToNot(BeEmpty()) + Expect(deletedRow.DateDeleted).ToNot(Equal("In progress")) + + standbyAfterDelete := readHistoryLogicalRows(copyStandbyHistoryDB(standbyTarget)) + Expect(standbyAfterDelete).To(Equal(primaryAfterDelete)) + Expect(findHistoryLogicalRow(standbyAfterDelete, baselineTimestamp).DateDeleted).To(Equal(deletedRow.DateDeleted)) + }) +}) From a77e6a6ae355677d1335aa0a0c374d4c623b50f2 Mon Sep 17 00:00:00 2001 From: woblerr Date: Wed, 29 Jul 2026 14:00:17 +0000 Subject: [PATCH 06/19] Document standby history database synchronization. --- README.md | 47 ++++++++++++++- gpbackman/COMMANDS.md | 134 +++++++++++++++++++++++++++++++++++++----- gpbackman/README.md | 56 +++++++++++++++++- 3 files changed, 218 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index eb2fafaa..d5da4d70 100644 --- a/README.md +++ b/README.md @@ -89,6 +89,51 @@ gprestore --timestamp Run `--help` with either command for a complete list of options. +### Standby history database synchronization + +After a successful backup, `gpbackup` automatically copies a consistent +snapshot of the coordinator's `gpbackup_history.db` to an up standby +coordinator. Synchronization starts only after the final `Success` history row +has been written and the local SQLite connection has been closed. + +This synchronization is best effort. If no up standby exists, synchronization +is skipped. If discovery, snapshot creation, transfer, or installation fails, +`gpbackup` logs a warning but keeps the successful backup exit status. A +failed or terminated backup is not synchronized. Synchronization also does not +run when `--no-history` is used or when the final history update fails. Use +`--no-history-sync-standby` to keep writing local history while disabling +standby synchronization for one backup: + +```bash +gpbackup --dbname --no-history-sync-standby +``` + +The synchronization process: + +1. Takes a non-waiting lock next to the canonical source database. +2. Creates a consistent SQLite snapshot with `VACUUM INTO` and accepts it only + when `PRAGMA quick_check` returns `ok`. +3. Transfers the snapshot with `rsync -p` to a unique temporary file in the + standby coordinator data directory. +4. Preserves the existing standby file's owner, group, and mode when it + exists, then atomically renames the temporary file to + `gpbackup_history.db`. + +The host running `gpbackup` must have `ssh` and `rsync`, and the current OS +user must have non-interactive SSH access to the standby host. That user must +be able to create files in the standby coordinator data directory and preserve +the destination file's ownership and permissions. The cluster must expose an +up standby in `gp_segment_configuration`. + +The atomic rename prevents readers from observing a partially copied database, +but it is not a failover coordination mechanism. A coordinator role change +during synchronization can race with discovery and installation. Processes +that already have the old standby database open continue reading that old +inode until they close and reopen it. + +For automatic synchronization after history maintenance and for the strict +manual command, see [gpBackMan history synchronization](./gpbackman/README.md#standby-history-database-synchronization). + ## Additional tools This repository also includes the following tools: @@ -196,4 +241,4 @@ the [LICENSE](./LICENSE). ## Acknowledgment Thanks to all the Greenplum Backup contributors, more details in its [GitHub -page](https://github.com/greenplum-db/gpbackup-archive). \ No newline at end of file +page](https://github.com/greenplum-db/gpbackup-archive). diff --git a/gpbackman/COMMANDS.md b/gpbackman/COMMANDS.md index d9e43d47..34f8db4f 100644 --- a/gpbackman/COMMANDS.md +++ b/gpbackman/COMMANDS.md @@ -31,17 +31,38 @@ - [Examples](#examples-3) - [Delete information about deleted backups from history database older than n days](#delete-information-about-deleted-backups-from-history-database-older-than-n-days) - [Delete information about deleted backups from history database older than timestamp](#delete-information-about-deleted-backups-from-history-database-older-than-timestamp) -- [Display the report for a specific backup (`report-info`)](#display-the-report-for-a-specific-backup-report-info) +- [Sync the history database to the standby coordinator (`history-sync`)](#sync-the-history-database-to-the-standby-coordinator-history-sync) - [Examples](#examples-4) +- [Display the report for a specific backup (`report-info`)](#display-the-report-for-a-specific-backup-report-info) + - [Examples](#examples-5) - [Display the backup report from local storage](#display-the-backup-report-from-local-storage) - [Display the backup report using storage plugin](#display-the-backup-report-using-storage-plugin) +`backup-clean`, `backup-delete`, and `history-clean` automatically attempt to +synchronize the cluster `gpbackup_history.db` to an up standby after a +successful command, including a successful no-op. This is best effort: skips +and failures do not change an otherwise successful command exit status. +For `backup-delete --ignore-errors`, synchronization still runs when the +command returns its existing successful result after recording deletion +errors. +Add `--no-history-sync-standby` to any of these three commands to disable the +attempt. Read-only commands do not synchronize history. + +Automatic synchronization requires the history database to resolve to +`/gpbackup_history.db`. Use +`--auto-load-history-db` to resolve that path from +`$COORDINATOR_DATA_DIRECTORY`, or pass the cluster database explicitly with +`--history-db`. A symlink is accepted when its canonical target is the cluster +database; custom and default working-directory databases are not synchronized. +See [`history-sync`](#sync-the-history-database-to-the-standby-coordinator-history-sync) +for strict behavior and operational requirements. + # Delete all existing backups older than the specified time condition (`backup-clean`) Available options for `backup-clean` command and their description: ```bash ./gpbackman backup-clean -h -elete all existing backups older than the specified time condition. +Delete all existing backups older than the specified time condition. To delete backup sets older than the given timestamp, use the --before-timestamp option. To delete backup sets older than the given number of days, use the --older-than-day option. @@ -72,7 +93,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` instead. +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. Usage: gpbackman backup-clean [flags] @@ -83,11 +104,13 @@ Flags: --before-timestamp string delete backup sets older than the given timestamp --cascade delete all dependent backups -h, --help help for backup-clean + --no-history-sync-standby skip automatic gpbackup_history.db sync to standby coordinator after this command --older-than-days uint delete backup sets older than the given number of days --parallel-processes int the number of parallel processes to delete local backups (default 1) --plugin-config string the full path to plugin config file Global Flags: + --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") @@ -97,16 +120,16 @@ Global Flags: ## Examples ### Delete all backups from local storage older than the specified time condition -Delete specific backup : +Delete backups older than a timestamp: ```bash ./gpbackman backup-clean \ --before-timestamp 20240701100000 \ --cascade ``` -Delete specific backup with specifying the number of parallel processes: +Delete backups older than a number of days with multiple parallel processes: ```bash -./gpbackman backup-delete \ +./gpbackman backup-clean \ --older-than-days 7 \ --parallel-processes 5 ``` @@ -158,7 +181,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` instead. +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. Usage: gpbackman backup-delete [flags] @@ -169,11 +192,13 @@ Flags: --force try to delete, even if the backup already mark as deleted -h, --help help for backup-delete --ignore-errors ignore errors when deleting backups + --no-history-sync-standby skip automatic gpbackup_history.db sync to standby coordinator after this command --parallel-processes int the number of parallel processes to delete local backups (default 1) --plugin-config string the full path to plugin config file --timestamp stringArray the backup timestamp for deleting, could be specified multiple times Global Flags: + --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") @@ -256,7 +281,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` instead. +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. Usage: gpbackman backup-info [flags] @@ -273,6 +298,7 @@ Flags: --type string backup type filter (full, incremental, data-only, metadata-only) Global Flags: + --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") @@ -455,7 +481,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` instead. +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. Usage: gpbackman history-clean [flags] @@ -463,9 +489,11 @@ Usage: Flags: --before-timestamp string delete information about backups older than the given timestamp -h, --help help for history-clean + --no-history-sync-standby skip automatic gpbackup_history.db sync to standby coordinator after this command --older-than-days uint delete information about backups older than the given number of days Global Flags: + --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") @@ -477,14 +505,87 @@ Global Flags: Delete information about deleted backups from history database older than 7 days: ```bash ./gpbackman history-clean \ - --older-than-days 7 \ + --older-than-days 7 ``` ### Delete information about deleted backups from history database older than timestamp Delete information about deleted backups from history database older than timestamp `20240101100000`: ```bash ./gpbackman history-clean \ - --before-timestamp 20240101100000 \ + --before-timestamp 20240101100000 +``` + +# Sync the history database to the standby coordinator (`history-sync`) + +`history-sync` performs a strict, explicit synchronization. It returns exit +status 0 only after a verified SQLite snapshot has been atomically installed +as `gpbackup_history.db` on an up standby coordinator. Unlike automatic +synchronization, a normal skip is an error: no up standby, an ineligible +source, an unresolved auto-loaded source, or a busy sync lock causes exit +status 1, as does any discovery, snapshot, transfer, or installation failure. + +The source must canonically resolve to +`/gpbackup_history.db`. A symlink to that +file is accepted. A custom `--history-db` path and the default +working-directory database are rejected. Use `--auto-load-history-db` to +resolve the source from `$COORDINATOR_DATA_DIRECTORY`, or pass the cluster +database explicitly with `--history-db`. + +The command uses the current OS user for SSH. The primary host must have +`ssh` and `rsync`; the user must have non-interactive access to the standby, +permission to create the source `.sync.lock`, and permission to write the +standby data directory and preserve the existing destination owner, group, and +mode. An up standby must be visible in `gp_segment_configuration`. + +The source is locked without waiting. gpBackMan creates a consistent snapshot +with SQLite `VACUUM INTO`, preserves its mode, and requires a single `ok` +result from `PRAGMA quick_check`. It transfers that snapshot with `rsync -p` +to a unique temporary path, copies the existing destination metadata when +present, and atomically renames the snapshot into place. Failed transfers and +installs remove only their own temporary file. + +Atomic replacement prevents a partial database from becoming visible, but it +does not coordinate a concurrent failover. A coordinator role change can race +with discovery and installation. A process with the previous database already +open continues reading the old inode until it closes and reopens the file. + +Available options for `history-sync` command and their description: + +```bash +./gpbackman history-sync -h +Sync the gpbackup_history.db file to the standby coordinator. + +The command uses the cluster history database from --history-db, or from +$COORDINATOR_DATA_DIRECTORY when --auto-load-history-db is set. It succeeds +only after the standby file is replaced atomically with a verified snapshot. + +Usage: + gpbackman history-sync [flags] + +Flags: + -h, --help help for history-sync + +Global Flags: + --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") + --log-level-file string level for file logging (error, info, debug, verbose) (default "info") +``` + +## Examples + +Resolve the cluster history database from the coordinator environment: + +```bash +./gpbackman history-sync --auto-load-history-db +``` + +Synchronize an explicitly selected cluster history database: + +```bash +./gpbackman history-sync \ + --history-db "$COORDINATOR_DATA_DIRECTORY/gpbackup_history.db" ``` # Display the report for a specific backup (`report-info`) @@ -492,7 +593,7 @@ Delete information about deleted backups from history database older than timest Available options for `report-info` command and their description: ```bash -./gpbackman.go report-info -h +./gpbackman report-info -h Display the report for a specific backup. The --timestamp option must be specified. @@ -524,7 +625,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` instead. +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. Usage: gpbackman report-info [flags] @@ -537,6 +638,7 @@ Flags: --timestamp string the backup timestamp for report displaying Global Flags: + --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") @@ -546,10 +648,10 @@ Global Flags: ## Examples ### Display the backup report from local storage -With specifying backup directory path: +Without specifying a backup directory path: ```bash ./gpbackman report-info \ - --timestamp 20230809232817 \ + --timestamp 20230809232817 --backup-dir /some/path ``` @@ -570,7 +672,7 @@ For `gpbackup_s3_plugin`: For other plugins: ```bash -./gpbackman report-infodoc \ +./gpbackman report-info \ --timestamp 20230725101959 \ --plugin-config /tmp/gpbackup_plugin_config.yaml \ --plugin-report-file-path /some/path/to/report diff --git a/gpbackman/README.md b/gpbackman/README.md index d9bd9ae8..49de8ebd 100644 --- a/gpbackman/README.md +++ b/gpbackman/README.md @@ -29,6 +29,7 @@ The utility works with `gpbackup_history.db` SQLite history database format. * delete existing backups from local storage or using storage plugins; * delete all existing backups from local storage or using storage plugins older than the specified time condition; * clean deleted backups from the history database; +* synchronize the cluster history database to the standby coordinator. ## Commands ### Introduction @@ -49,11 +50,12 @@ Available Commands: completion Generate the autocompletion script for the specified shell help Help about any command history-clean Clean deleted backups from the history database + history-sync Sync the history database to the standby coordinator report-info Display the report for a specific backup Flags: - -h, --help help for gpbackman --auto-load-history-db resolve gpbackup_history.db from $COORDINATOR_DATA_DIRECTORY when --history-db is unset + -h, --help help for gpbackman --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") @@ -70,9 +72,59 @@ Description of each command: * [Delete a specific existing backup (`backup-delete`)](./COMMANDS.md#delete-a-specific-existing-backup-backup-delete) * [Display information about backups (`backup-info`)](./COMMANDS.md#display-information-about-backups-backup-info) * [Clean deleted backups from the history database (`history-clean`)](./COMMANDS.md#clean-deleted-backups-from-the-history-database-history-clean) +* [Sync the history database to the standby coordinator (`history-sync`)](./COMMANDS.md#sync-the-history-database-to-the-standby-coordinator-history-sync) * [Display the report for a specific backup (`report-info`)](./COMMANDS.md#display-the-report-for-a-specific-backup-report-info) +## Standby history database synchronization + +`backup-delete`, `backup-clean`, and `history-clean` automatically attempt to +synchronize `gpbackup_history.db` after the command has successfully finished +and closed the database. Successful no-op commands also attempt +synchronization. For `backup-delete --ignore-errors`, synchronization runs +when the command completes with its existing successful result, including when +individual deletion errors were recorded. + +Automatic synchronization is best effort. A normal skip, such as no up +standby or an ineligible history database, does not fail the mutation command. +A discovery, snapshot, transfer, or installation error is logged as a warning +without changing an otherwise successful exit status. Use the command-local +`--no-history-sync-standby` flag to disable the attempt for one of these three +commands. The flag is not available on read-only commands or `history-sync`. + +`history-sync` provides strict synchronization: + +```bash +./gpbackman history-sync --auto-load-history-db +``` + +It exits successfully only after a verified snapshot has been installed +atomically on an up standby. An ineligible source, no up standby, a busy sync +lock, or any discovery, snapshot, transfer, or installation error causes a +nonzero exit. + +The source must resolve to +`/gpbackup_history.db`. An explicit +`--history-db` path is accepted only when its canonical path is that cluster +database; a symlink to that file is accepted. A custom database is not +synchronized. With `--auto-load-history-db`, gpBackMan resolves the file from +`$COORDINATOR_DATA_DIRECTORY`. Without either option, gpBackMan uses the +working-directory database for normal command behavior, but standby +synchronization skips it; therefore a strict `history-sync` call fails. +`--history-db` takes precedence when both global options are present. + +Synchronization requires an up standby coordinator, `ssh` and `rsync` on the +host running gpBackMan, and non-interactive SSH access for the current OS user. +The user must be able to read the primary history database, create its adjacent +`.sync.lock`, write temporary files in the standby coordinator data directory, +and preserve the existing standby file's owner, group, and mode. + +gpBackMan creates and validates a consistent SQLite snapshot before transfer, +copies it to a unique standby temporary path, and atomically renames it into +place. Readers never see a partial copy. This does not coordinate a concurrent +coordinator failover: a role change can race with discovery and installation, +and a process that already has the old database open continues using that old +inode until it reopens the file. + ## About gpBackMan is part of the Apache Cloudberry Backup (Incubating) toolset. It is based on the original [gpbackman](https://github.com/woblerr/gpbackman) project. - From 129a0e72a5877cd26d2a3510be17cdfdbb94cd0c Mon Sep 17 00:00:00 2001 From: woblerr Date: Wed, 29 Jul 2026 14:15:03 +0000 Subject: [PATCH 07/19] Tighten standby history sync validation. --- backup/history_standby_sync.go | 12 +++++++++--- backup/history_standby_sync_test.go | 8 ++++++++ gpbackman/cmd/history_standby_sync.go | 16 ++++++++++++---- gpbackman/gpbckpconfig/cluster.go | 8 ++++++-- 4 files changed, 35 insertions(+), 9 deletions(-) diff --git a/backup/history_standby_sync.go b/backup/history_standby_sync.go index 9b46b9bc..36520cd3 100644 --- a/backup/history_standby_sync.go +++ b/backup/history_standby_sync.go @@ -242,7 +242,9 @@ func vacuumBackupHistoryStandbySyncSnapshot(sourceDBPath, snapshotPath string) e if err != nil { return fmt.Errorf("open source history db for standby sync snapshot: %w", err) } - defer sourceDB.Close() + defer func() { + _ = sourceDB.Close() + }() if _, err := sourceDB.Exec("VACUUM main INTO ?", snapshotPath); err != nil { return fmt.Errorf("create standby history sync snapshot with VACUUM INTO: %w", err) @@ -266,13 +268,17 @@ func runBackupHistoryStandbySyncQuickCheck(snapshotPath string) ([]string, error if err != nil { return nil, fmt.Errorf("open standby history sync snapshot read-only: %w", err) } - defer snapshotDB.Close() + defer func() { + _ = snapshotDB.Close() + }() rows, err := snapshotDB.Query("PRAGMA quick_check") if err != nil { return nil, fmt.Errorf("run PRAGMA quick_check on standby history sync snapshot: %w", err) } - defer rows.Close() + defer func() { + _ = rows.Close() + }() results := make([]string, 0) for rows.Next() { diff --git a/backup/history_standby_sync_test.go b/backup/history_standby_sync_test.go index 3f4fcd12..85aaa5fb 100644 --- a/backup/history_standby_sync_test.go +++ b/backup/history_standby_sync_test.go @@ -147,6 +147,14 @@ var _ = Describe("backup history standby sync", func() { Expect(backupHistoryStandbySyncLockPath(canonicalSourcePath)).To(Equal(realSourcePath + ".sync.lock")) }) + It("rejects a non-regular source", func() { + sourcePath := GinkgoT().TempDir() + + _, _, err := canonicalBackupHistoryStandbySyncSource(sourcePath) + + Expect(err).To(MatchError(ContainSubstring("is not a regular file"))) + }) + It("skips when no up standby coordinator exists", func() { tmpDir := GinkgoT().TempDir() sourcePath := filepath.Join(tmpDir, backupHistoryDBName) diff --git a/gpbackman/cmd/history_standby_sync.go b/gpbackman/cmd/history_standby_sync.go index 5a86043e..f9523ecd 100644 --- a/gpbackman/cmd/history_standby_sync.go +++ b/gpbackman/cmd/history_standby_sync.go @@ -150,7 +150,9 @@ func discoverHistoryStandbySyncTarget(sourceDBPath string) (*historyStandbySyncT if err != nil { return nil, "", fmt.Errorf("connect to local cluster for standby history sync discovery: %w", err) } - defer db.Close() + defer func() { + _ = db.Close() + }() primaryDataDir, err := queryHistoryStandbySyncPrimaryDataDir(db) if err != nil { @@ -293,7 +295,9 @@ func vacuumHistoryStandbySyncSnapshot(sourceDBPath, snapshotPath string) error { if err != nil { return fmt.Errorf("open source history db for standby sync snapshot: %w", err) } - defer sourceDB.Close() + defer func() { + _ = sourceDB.Close() + }() if _, err := sourceDB.Exec("VACUUM main INTO ?", snapshotPath); err != nil { return fmt.Errorf("create standby history sync snapshot with VACUUM INTO: %w", err) @@ -317,13 +321,17 @@ func runHistoryStandbySyncQuickCheck(snapshotPath string) ([]string, error) { if err != nil { return nil, fmt.Errorf("open standby history sync snapshot read-only: %w", err) } - defer snapshotDB.Close() + defer func() { + _ = snapshotDB.Close() + }() rows, err := snapshotDB.Query("PRAGMA quick_check") if err != nil { return nil, fmt.Errorf("run PRAGMA quick_check on standby history sync snapshot: %w", err) } - defer rows.Close() + defer func() { + _ = rows.Close() + }() results := make([]string, 0) for rows.Next() { diff --git a/gpbackman/gpbckpconfig/cluster.go b/gpbackman/gpbckpconfig/cluster.go index 0e03ca7d..a889380e 100644 --- a/gpbackman/gpbckpconfig/cluster.go +++ b/gpbackman/gpbckpconfig/cluster.go @@ -87,7 +87,9 @@ func GetPrimaryCoordinatorDataDir() (string, error) { if err != nil { return "", err } - defer db.Close() + defer func() { + _ = db.Close() + }() return QueryPrimaryCoordinatorDataDir(db) } @@ -102,7 +104,9 @@ func GetUpStandbyCoordinator() (StandbyCoordinator, error) { if err != nil { return StandbyCoordinator{}, err } - defer db.Close() + defer func() { + _ = db.Close() + }() return QueryUpStandbyCoordinator(db) } From 85f1d81869c98ef4e6d3c1e6964b8cf347674292 Mon Sep 17 00:00:00 2001 From: woblerr Date: Wed, 5 Aug 2026 17:03:46 +0300 Subject: [PATCH 08/19] Log standby history sync cleanup errors. Keep the primary operation result while making database and row close failures visible. --- backup/history_standby_sync.go | 12 +++++++++--- gpbackman/cmd/history_standby_sync.go | 16 ++++++++++++---- gpbackman/gpbckpconfig/cluster.go | 9 +++++++-- 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/backup/history_standby_sync.go b/backup/history_standby_sync.go index 36520cd3..b9c1b91f 100644 --- a/backup/history_standby_sync.go +++ b/backup/history_standby_sync.go @@ -243,7 +243,9 @@ func vacuumBackupHistoryStandbySyncSnapshot(sourceDBPath, snapshotPath string) e return fmt.Errorf("open source history db for standby sync snapshot: %w", err) } defer func() { - _ = sourceDB.Close() + if closeErr := sourceDB.Close(); closeErr != nil { + gplog.Error("Unable to close source history db for standby sync snapshot: %v", closeErr) + } }() if _, err := sourceDB.Exec("VACUUM main INTO ?", snapshotPath); err != nil { @@ -269,7 +271,9 @@ func runBackupHistoryStandbySyncQuickCheck(snapshotPath string) ([]string, error return nil, fmt.Errorf("open standby history sync snapshot read-only: %w", err) } defer func() { - _ = snapshotDB.Close() + if closeErr := snapshotDB.Close(); closeErr != nil { + gplog.Error("Unable to close standby history sync snapshot: %v", closeErr) + } }() rows, err := snapshotDB.Query("PRAGMA quick_check") @@ -277,7 +281,9 @@ func runBackupHistoryStandbySyncQuickCheck(snapshotPath string) ([]string, error return nil, fmt.Errorf("run PRAGMA quick_check on standby history sync snapshot: %w", err) } defer func() { - _ = rows.Close() + if closeErr := rows.Close(); closeErr != nil { + gplog.Error("Unable to close standby history sync quick_check rows: %v", closeErr) + } }() results := make([]string, 0) diff --git a/gpbackman/cmd/history_standby_sync.go b/gpbackman/cmd/history_standby_sync.go index f9523ecd..c5c29ac8 100644 --- a/gpbackman/cmd/history_standby_sync.go +++ b/gpbackman/cmd/history_standby_sync.go @@ -151,7 +151,9 @@ func discoverHistoryStandbySyncTarget(sourceDBPath string) (*historyStandbySyncT return nil, "", fmt.Errorf("connect to local cluster for standby history sync discovery: %w", err) } defer func() { - _ = db.Close() + if closeErr := db.Close(); closeErr != nil { + gplog.Error("Unable to close local cluster connection for standby history sync discovery: %v", closeErr) + } }() primaryDataDir, err := queryHistoryStandbySyncPrimaryDataDir(db) @@ -296,7 +298,9 @@ func vacuumHistoryStandbySyncSnapshot(sourceDBPath, snapshotPath string) error { return fmt.Errorf("open source history db for standby sync snapshot: %w", err) } defer func() { - _ = sourceDB.Close() + if closeErr := sourceDB.Close(); closeErr != nil { + gplog.Error("Unable to close source history db for standby sync snapshot: %v", closeErr) + } }() if _, err := sourceDB.Exec("VACUUM main INTO ?", snapshotPath); err != nil { @@ -322,7 +326,9 @@ func runHistoryStandbySyncQuickCheck(snapshotPath string) ([]string, error) { return nil, fmt.Errorf("open standby history sync snapshot read-only: %w", err) } defer func() { - _ = snapshotDB.Close() + if closeErr := snapshotDB.Close(); closeErr != nil { + gplog.Error("Unable to close standby history sync snapshot: %v", closeErr) + } }() rows, err := snapshotDB.Query("PRAGMA quick_check") @@ -330,7 +336,9 @@ func runHistoryStandbySyncQuickCheck(snapshotPath string) ([]string, error) { return nil, fmt.Errorf("run PRAGMA quick_check on standby history sync snapshot: %w", err) } defer func() { - _ = rows.Close() + if closeErr := rows.Close(); closeErr != nil { + gplog.Error("Unable to close standby history sync quick_check rows: %v", closeErr) + } }() results := make([]string, 0) diff --git a/gpbackman/gpbckpconfig/cluster.go b/gpbackman/gpbckpconfig/cluster.go index a889380e..fcea6693 100644 --- a/gpbackman/gpbckpconfig/cluster.go +++ b/gpbackman/gpbckpconfig/cluster.go @@ -24,6 +24,7 @@ import ( "strconv" "github.com/apache/cloudberry-backup/gpbackman/textmsg" + "github.com/apache/cloudberry-go-libs/gplog" "github.com/apache/cloudberry-go-libs/operating" "github.com/jmoiron/sqlx" _ "github.com/lib/pq" @@ -88,7 +89,9 @@ func GetPrimaryCoordinatorDataDir() (string, error) { return "", err } defer func() { - _ = db.Close() + if closeErr := db.Close(); closeErr != nil { + gplog.Error("Unable to close local cluster connection: %v", closeErr) + } }() return QueryPrimaryCoordinatorDataDir(db) } @@ -105,7 +108,9 @@ func GetUpStandbyCoordinator() (StandbyCoordinator, error) { return StandbyCoordinator{}, err } defer func() { - _ = db.Close() + if closeErr := db.Close(); closeErr != nil { + gplog.Error("Unable to close local cluster connection: %v", closeErr) + } }() return QueryUpStandbyCoordinator(db) } From 83a1914f47a68a66b20da02a858368e4800aa7af Mon Sep 17 00:00:00 2001 From: woblerr Date: Wed, 5 Aug 2026 23:30:19 +0300 Subject: [PATCH 09/19] Simplify the handling of history synchronization errors in standby mode. Return connection, SQLite, and local cleanup failures to the policy boundary without changing best-effort exit status. --- backup/history_standby_sync.go | 40 +++++---- backup/history_standby_sync_test.go | 76 +++++++++++++++- gpbackman/cmd/history_standby_sync.go | 46 +++++----- gpbackman/cmd/history_standby_sync_test.go | 100 ++++++++++++++++++++- gpbackman/gpbckpconfig/cluster.go | 29 ------ gpbackman/gpbckpconfig/cluster_test.go | 27 ------ 6 files changed, 218 insertions(+), 100 deletions(-) diff --git a/backup/history_standby_sync.go b/backup/history_standby_sync.go index b9c1b91f..64c584e0 100644 --- a/backup/history_standby_sync.go +++ b/backup/history_standby_sync.go @@ -65,6 +65,7 @@ var ( backupHistoryStandbySyncCommandExec = func(name string, args ...string) backupHistoryStandbySyncCommand { return exec.Command(name, args...) } + backupHistoryStandbySyncOpenSQLite = sql.Open backupHistoryStandbySyncMkdirTemp = os.MkdirTemp backupHistoryStandbySyncRemoveAll = os.RemoveAll backupHistoryStandbySyncCurrentUser = func() (string, error) { @@ -205,10 +206,12 @@ func backupHistoryStandbySyncLockPath(sourceDBPath string) string { return sourceDBPath + ".sync.lock" } -func withBackupHistoryStandbySyncSnapshot(sourceDBPath string, sourceMode os.FileMode, syncFn func(string) error) error { +func withBackupHistoryStandbySyncSnapshot(sourceDBPath string, sourceMode os.FileMode, syncFn func(string) error) (retErr error) { snapshotPath, tempDir, err := createBackupHistoryStandbySyncSnapshot(sourceDBPath, sourceMode) if tempDir != "" { - defer cleanupBackupHistoryStandbySyncTempDir(tempDir) + defer func() { + retErr = errors.Join(retErr, cleanupBackupHistoryStandbySyncTempDir(tempDir)) + }() } if err != nil { return err @@ -223,28 +226,28 @@ func createBackupHistoryStandbySyncSnapshot(sourceDBPath string, sourceMode os.F } snapshotPath := filepath.Join(tempDir, backupHistoryDBName) if err := vacuumBackupHistoryStandbySyncSnapshot(sourceDBPath, snapshotPath); err != nil { - cleanupBackupHistoryStandbySyncTempDir(tempDir) - return "", "", err + return "", "", errors.Join(err, cleanupBackupHistoryStandbySyncTempDir(tempDir)) } if err := os.Chmod(snapshotPath, sourceMode); err != nil { - cleanupBackupHistoryStandbySyncTempDir(tempDir) - return "", "", fmt.Errorf("set standby history sync snapshot permissions from source history db: %w", err) + return "", "", errors.Join( + fmt.Errorf("set standby history sync snapshot permissions from source history db: %w", err), + cleanupBackupHistoryStandbySyncTempDir(tempDir), + ) } if err := validateBackupHistoryStandbySyncSnapshot(snapshotPath); err != nil { - cleanupBackupHistoryStandbySyncTempDir(tempDir) - return "", "", err + return "", "", errors.Join(err, cleanupBackupHistoryStandbySyncTempDir(tempDir)) } return snapshotPath, tempDir, nil } -func vacuumBackupHistoryStandbySyncSnapshot(sourceDBPath, snapshotPath string) error { - sourceDB, err := sql.Open("sqlite3", backupHistoryStandbySyncSQLiteURI(sourceDBPath, "ro")) +func vacuumBackupHistoryStandbySyncSnapshot(sourceDBPath, snapshotPath string) (retErr error) { + sourceDB, err := backupHistoryStandbySyncOpenSQLite("sqlite3", backupHistoryStandbySyncSQLiteURI(sourceDBPath, "ro")) if err != nil { return fmt.Errorf("open source history db for standby sync snapshot: %w", err) } defer func() { if closeErr := sourceDB.Close(); closeErr != nil { - gplog.Error("Unable to close source history db for standby sync snapshot: %v", closeErr) + retErr = errors.Join(retErr, fmt.Errorf("close source history db for standby sync snapshot: %w", closeErr)) } }() @@ -265,14 +268,14 @@ func validateBackupHistoryStandbySyncSnapshot(snapshotPath string) error { return nil } -func runBackupHistoryStandbySyncQuickCheck(snapshotPath string) ([]string, error) { - snapshotDB, err := sql.Open("sqlite3", backupHistoryStandbySyncSQLiteURI(snapshotPath, "ro")) +func runBackupHistoryStandbySyncQuickCheck(snapshotPath string) (results []string, retErr error) { + snapshotDB, err := backupHistoryStandbySyncOpenSQLite("sqlite3", backupHistoryStandbySyncSQLiteURI(snapshotPath, "ro")) if err != nil { return nil, fmt.Errorf("open standby history sync snapshot read-only: %w", err) } defer func() { if closeErr := snapshotDB.Close(); closeErr != nil { - gplog.Error("Unable to close standby history sync snapshot: %v", closeErr) + retErr = errors.Join(retErr, fmt.Errorf("close standby history sync snapshot: %w", closeErr)) } }() @@ -282,11 +285,11 @@ func runBackupHistoryStandbySyncQuickCheck(snapshotPath string) ([]string, error } defer func() { if closeErr := rows.Close(); closeErr != nil { - gplog.Error("Unable to close standby history sync quick_check rows: %v", closeErr) + retErr = errors.Join(retErr, fmt.Errorf("close standby history sync quick_check rows: %w", closeErr)) } }() - results := make([]string, 0) + results = make([]string, 0) for rows.Next() { var result string if err := rows.Scan(&result); err != nil { @@ -300,10 +303,11 @@ func runBackupHistoryStandbySyncQuickCheck(snapshotPath string) ([]string, error return results, nil } -func cleanupBackupHistoryStandbySyncTempDir(tempDir string) { +func cleanupBackupHistoryStandbySyncTempDir(tempDir string) error { if err := backupHistoryStandbySyncRemoveAll(tempDir); err != nil && !errors.Is(err, os.ErrNotExist) { - gplog.Debug("Unable to remove local standby history sync temp directory %s: %v", tempDir, err) + return fmt.Errorf("remove local standby history sync temp directory %s: %w", tempDir, err) } + return nil } func syncBackupHistoryStandbySnapshot(target *backupHistoryStandbySyncTarget, userName, snapshotPath string) error { diff --git a/backup/history_standby_sync_test.go b/backup/history_standby_sync_test.go index 85aaa5fb..00d87727 100644 --- a/backup/history_standby_sync_test.go +++ b/backup/history_standby_sync_test.go @@ -31,6 +31,7 @@ import ( backupfilepath "github.com/apache/cloudberry-backup/filepath" "github.com/apache/cloudberry-backup/options" "github.com/apache/cloudberry-go-libs/dbconn" + "github.com/apache/cloudberry-go-libs/gplog" "github.com/apache/cloudberry-go-libs/testhelper" "github.com/jmoiron/sqlx" "github.com/nightlyone/lockfile" @@ -60,7 +61,10 @@ func (c backupHistoryStandbySyncFakeCommand) CombinedOutput() ([]byte, error) { } var _ = Describe("backup history standby sync", func() { - var originalSync func() (string, error) + var ( + originalSync func() (string, error) + originalOpenSQLite func(string, string) (*sql.DB, error) + ) BeforeEach(func() { testhelper.SetupTestLogger() @@ -69,7 +73,9 @@ var _ = Describe("backup history standby sync", func() { globalFPInfo = backupfilepath.FilePathInfo{} connectionPool = nil originalSync = backupHistoryStandbySync + originalOpenSQLite = backupHistoryStandbySyncOpenSQLite backupHistoryStandbySync = syncBackupHistoryToStandby + backupHistoryStandbySyncOpenSQLite = sql.Open backupHistoryStandbySyncCommandExec = func(name string, args ...string) backupHistoryStandbySyncCommand { return backupHistoryStandbySyncFakeCommand{} } @@ -82,6 +88,7 @@ var _ = Describe("backup history standby sync", func() { AfterEach(func() { backupHistoryStandbySync = originalSync + backupHistoryStandbySyncOpenSQLite = originalOpenSQLite backupHistoryStandbySyncCommandExec = func(name string, args ...string) backupHistoryStandbySyncCommand { return backupHistoryStandbySyncFakeCommand{} } @@ -130,6 +137,69 @@ var _ = Describe("backup history standby sync", func() { Expect(tempDir).To(BeEmpty()) }) + It("returns SQLite close errors without changing the gpbackup error code", func() { + sqlDB, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherEqual)) + Expect(err).ToNot(HaveOccurred()) + closeErr := errors.New("close failed") + mock.ExpectExec("VACUUM main INTO ?"). + WithArgs("/tmp/snapshot.db"). + WillReturnResult(sqlmock.NewResult(0, 0)) + mock.ExpectClose().WillReturnError(closeErr) + backupHistoryStandbySyncOpenSQLite = func(driverName, dataSourceName string) (*sql.DB, error) { + Expect(driverName).To(Equal("sqlite3")) + return sqlDB, nil + } + originalErrorCode := gplog.GetErrorCode() + DeferCleanup(gplog.SetErrorCode, originalErrorCode) + gplog.SetErrorCode(0) + + err = vacuumBackupHistoryStandbySyncSnapshot("/tmp/source.db", "/tmp/snapshot.db") + + Expect(errors.Is(err, closeErr)).To(BeTrue()) + Expect(err.Error()).To(ContainSubstring("close source history db for standby sync snapshot")) + Expect(gplog.GetErrorCode()).To(Equal(0)) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + }) + + It("returns local cleanup errors after success and joins them with sync errors", func() { + sourcePath := filepath.Join(GinkgoT().TempDir(), backupHistoryDBName) + createBackupHistoryStandbySyncSQLiteDB(sourcePath) + cleanupErr := errors.New("cleanup failed") + backupHistoryStandbySyncMkdirTemp = func(dir, pattern string) (string, error) { + return GinkgoT().TempDir(), nil + } + backupHistoryStandbySyncRemoveAll = func(path string) error { + return cleanupErr + } + + err := withBackupHistoryStandbySyncSnapshot(sourcePath, 0o600, func(string) error { + return nil + }) + Expect(errors.Is(err, cleanupErr)).To(BeTrue()) + + syncErr := errors.New("sync failed") + err = withBackupHistoryStandbySyncSnapshot(sourcePath, 0o600, func(string) error { + return syncErr + }) + Expect(errors.Is(err, syncErr)).To(BeTrue()) + Expect(errors.Is(err, cleanupErr)).To(BeTrue()) + }) + + It("joins snapshot creation and local cleanup errors", func() { + sourcePath := filepath.Join(GinkgoT().TempDir(), backupHistoryDBName) + Expect(os.WriteFile(sourcePath, []byte("not sqlite"), 0o600)).To(Succeed()) + cleanupErr := errors.New("cleanup failed") + backupHistoryStandbySyncRemoveAll = func(path string) error { + return cleanupErr + } + + _, tempDir, err := createBackupHistoryStandbySyncSnapshot(sourcePath, 0o600) + + Expect(tempDir).To(BeEmpty()) + Expect(err.Error()).To(ContainSubstring("VACUUM INTO")) + Expect(errors.Is(err, cleanupErr)).To(BeTrue()) + }) + It("canonicalizes symlink sources and builds the shared lock path from the canonical source", func() { tmpDir := GinkgoT().TempDir() realDir := filepath.Join(tmpDir, "real") @@ -323,6 +393,9 @@ var _ = Describe("backup history standby sync", func() { It("warns automatic sync failures without exiting", func() { stdout, _, _ := testhelper.SetupTestLogger() + originalErrorCode := gplog.GetErrorCode() + DeferCleanup(gplog.SetErrorCode, originalErrorCode) + gplog.SetErrorCode(0) backupHistoryStandbySync = func() (string, error) { return "", errors.New("transport failed") } @@ -331,6 +404,7 @@ var _ = Describe("backup history standby sync", func() { Expect(err).To(HaveOccurred()) Expect(string(stdout.Contents())).To(ContainSubstring("History db sync to standby coordinator failed; standby history may be stale: transport failed")) + Expect(gplog.GetErrorCode()).To(Equal(0)) }) It("runs automatic sync only after successful cleanup history update for successful backups", func() { diff --git a/gpbackman/cmd/history_standby_sync.go b/gpbackman/cmd/history_standby_sync.go index c5c29ac8..71017ca0 100644 --- a/gpbackman/cmd/history_standby_sync.go +++ b/gpbackman/cmd/history_standby_sync.go @@ -59,6 +59,7 @@ type historyStandbySyncTarget struct { var ( historyStandbySync = syncHistoryStandby historyStandbySyncOpenClusterConn = gpbckpconfig.NewClusterLocalClusterDefaultConn + historyStandbySyncOpenSQLite = sql.Open historyStandbySyncMkdirTemp = os.MkdirTemp historyStandbySyncRemoveAll = os.RemoveAll historyStandbySyncNow = time.Now @@ -145,14 +146,14 @@ func getHistoryStandbySyncSourceDBPath() (string, string) { return sourceDBPath, "" } -func discoverHistoryStandbySyncTarget(sourceDBPath string) (*historyStandbySyncTarget, string, error) { +func discoverHistoryStandbySyncTarget(sourceDBPath string) (target *historyStandbySyncTarget, skipReason string, retErr error) { db, err := historyStandbySyncOpenClusterConn() if err != nil { return nil, "", fmt.Errorf("connect to local cluster for standby history sync discovery: %w", err) } defer func() { if closeErr := db.Close(); closeErr != nil { - gplog.Error("Unable to close local cluster connection for standby history sync discovery: %v", closeErr) + retErr = errors.Join(retErr, fmt.Errorf("close local cluster connection for standby history sync discovery: %w", closeErr)) } }() @@ -181,7 +182,7 @@ func discoverHistoryStandbySyncTarget(sourceDBPath string) (*historyStandbySyncT return nil, "", fmt.Errorf("query up standby coordinator for standby history sync discovery: %w", err) } - target := &historyStandbySyncTarget{ + target = &historyStandbySyncTarget{ sourceDBPath: canonicalSourceDBPath, sourceMode: sourceInfo.Mode().Perm(), standbyHost: standbyConfig.Hostname, @@ -255,10 +256,12 @@ func historyStandbySyncLockPath(sourceDBPath string) string { return sourceDBPath + ".sync.lock" } -func withHistoryStandbySyncSnapshot(sourceDBPath string, sourceMode os.FileMode, syncFn func(string) error) error { +func withHistoryStandbySyncSnapshot(sourceDBPath string, sourceMode os.FileMode, syncFn func(string) error) (retErr error) { snapshotPath, tempDir, err := createHistoryStandbySyncSnapshot(sourceDBPath, sourceMode) if tempDir != "" { - defer cleanupHistoryStandbySyncTempDir(tempDir) + defer func() { + retErr = errors.Join(retErr, cleanupHistoryStandbySyncTempDir(tempDir)) + }() } if err != nil { return err @@ -278,28 +281,28 @@ func createHistoryStandbySyncSnapshot(sourceDBPath string, sourceMode os.FileMod } snapshotPath := filepath.Join(tempDir, historyDBNameConst) if err := vacuumHistoryStandbySyncSnapshot(sourceDBPath, snapshotPath); err != nil { - cleanupHistoryStandbySyncTempDir(tempDir) - return "", "", err + return "", "", errors.Join(err, cleanupHistoryStandbySyncTempDir(tempDir)) } if err := os.Chmod(snapshotPath, sourceMode); err != nil { - cleanupHistoryStandbySyncTempDir(tempDir) - return "", "", fmt.Errorf("set standby history sync snapshot permissions from source history db: %w", err) + return "", "", errors.Join( + fmt.Errorf("set standby history sync snapshot permissions from source history db: %w", err), + cleanupHistoryStandbySyncTempDir(tempDir), + ) } if err := validateHistoryStandbySyncSnapshot(snapshotPath); err != nil { - cleanupHistoryStandbySyncTempDir(tempDir) - return "", "", err + return "", "", errors.Join(err, cleanupHistoryStandbySyncTempDir(tempDir)) } return snapshotPath, tempDir, nil } -func vacuumHistoryStandbySyncSnapshot(sourceDBPath, snapshotPath string) error { - sourceDB, err := sql.Open("sqlite3", historyStandbySyncSQLiteURI(sourceDBPath, "ro")) +func vacuumHistoryStandbySyncSnapshot(sourceDBPath, snapshotPath string) (retErr error) { + sourceDB, err := historyStandbySyncOpenSQLite("sqlite3", historyStandbySyncSQLiteURI(sourceDBPath, "ro")) if err != nil { return fmt.Errorf("open source history db for standby sync snapshot: %w", err) } defer func() { if closeErr := sourceDB.Close(); closeErr != nil { - gplog.Error("Unable to close source history db for standby sync snapshot: %v", closeErr) + retErr = errors.Join(retErr, fmt.Errorf("close source history db for standby sync snapshot: %w", closeErr)) } }() @@ -320,14 +323,14 @@ func validateHistoryStandbySyncSnapshot(snapshotPath string) error { return nil } -func runHistoryStandbySyncQuickCheck(snapshotPath string) ([]string, error) { - snapshotDB, err := sql.Open("sqlite3", historyStandbySyncSQLiteURI(snapshotPath, "ro")) +func runHistoryStandbySyncQuickCheck(snapshotPath string) (results []string, retErr error) { + snapshotDB, err := historyStandbySyncOpenSQLite("sqlite3", historyStandbySyncSQLiteURI(snapshotPath, "ro")) if err != nil { return nil, fmt.Errorf("open standby history sync snapshot read-only: %w", err) } defer func() { if closeErr := snapshotDB.Close(); closeErr != nil { - gplog.Error("Unable to close standby history sync snapshot: %v", closeErr) + retErr = errors.Join(retErr, fmt.Errorf("close standby history sync snapshot: %w", closeErr)) } }() @@ -337,11 +340,11 @@ func runHistoryStandbySyncQuickCheck(snapshotPath string) ([]string, error) { } defer func() { if closeErr := rows.Close(); closeErr != nil { - gplog.Error("Unable to close standby history sync quick_check rows: %v", closeErr) + retErr = errors.Join(retErr, fmt.Errorf("close standby history sync quick_check rows: %w", closeErr)) } }() - results := make([]string, 0) + results = make([]string, 0) for rows.Next() { var result string if err := rows.Scan(&result); err != nil { @@ -355,10 +358,11 @@ func runHistoryStandbySyncQuickCheck(snapshotPath string) ([]string, error) { return results, nil } -func cleanupHistoryStandbySyncTempDir(tempDir string) { +func cleanupHistoryStandbySyncTempDir(tempDir string) error { if err := historyStandbySyncRemoveAll(tempDir); err != nil && !errors.Is(err, os.ErrNotExist) { - gplog.Debug("Unable to remove local standby history sync temp directory %s: %v", tempDir, err) + return fmt.Errorf("remove local standby history sync temp directory %s: %w", tempDir, err) } + return nil } func syncHistoryStandbySnapshotToStandby(target *historyStandbySyncTarget, userName, snapshotPath string) error { diff --git a/gpbackman/cmd/history_standby_sync_test.go b/gpbackman/cmd/history_standby_sync_test.go index 4289391b..14e5dd1b 100644 --- a/gpbackman/cmd/history_standby_sync_test.go +++ b/gpbackman/cmd/history_standby_sync_test.go @@ -65,6 +65,7 @@ var _ = Describe("history standby sync", func() { var ( originalHistoryStandbySync func() historyStandbySyncResult originalOpenClusterConn func() (*sqlx.DB, error) + originalOpenSQLite func(string, string) (*sql.DB, error) originalMkdirTemp func(string, string) (string, error) originalRemoveAll func(string) error originalNow func() time.Time @@ -82,6 +83,7 @@ var _ = Describe("history standby sync", func() { testhelper.SetupTestLogger() originalHistoryStandbySync = historyStandbySync originalOpenClusterConn = historyStandbySyncOpenClusterConn + originalOpenSQLite = historyStandbySyncOpenSQLite originalMkdirTemp = historyStandbySyncMkdirTemp originalRemoveAll = historyStandbySyncRemoveAll originalNow = historyStandbySyncNow @@ -106,6 +108,7 @@ var _ = Describe("history standby sync", func() { historyStandbySyncOpenClusterConn = func() (*sqlx.DB, error) { return nil, errors.New("cluster connection was not expected") } + historyStandbySyncOpenSQLite = sql.Open historyStandbySyncMkdirTemp = os.MkdirTemp historyStandbySyncRemoveAll = os.RemoveAll historyStandbySyncNow = func() time.Time { @@ -126,6 +129,7 @@ var _ = Describe("history standby sync", func() { AfterEach(func() { historyStandbySync = originalHistoryStandbySync historyStandbySyncOpenClusterConn = originalOpenClusterConn + historyStandbySyncOpenSQLite = originalOpenSQLite historyStandbySyncMkdirTemp = originalMkdirTemp historyStandbySyncRemoveAll = originalRemoveAll historyStandbySyncNow = originalNow @@ -208,6 +212,90 @@ var _ = Describe("history standby sync", func() { Expect(errors.Is(statErr, os.ErrNotExist)).To(BeTrue()) }) + It("returns SQLite close errors from snapshot validation", func() { + sqlDB, mock, err := sqlmock.New(sqlmock.QueryMatcherOption(sqlmock.QueryMatcherEqual)) + Expect(err).ToNot(HaveOccurred()) + closeErr := errors.New("close failed") + mock.ExpectQuery("PRAGMA quick_check"). + WillReturnRows(sqlmock.NewRows([]string{"quick_check"}).AddRow("ok")) + mock.ExpectClose().WillReturnError(closeErr) + historyStandbySyncOpenSQLite = func(driverName, dataSourceName string) (*sql.DB, error) { + Expect(driverName).To(Equal("sqlite3")) + return sqlDB, nil + } + + results, err := runHistoryStandbySyncQuickCheck("/tmp/snapshot.db") + + Expect(results).To(Equal([]string{"ok"})) + Expect(errors.Is(err, closeErr)).To(BeTrue()) + Expect(err.Error()).To(ContainSubstring("close standby history sync snapshot")) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + }) + + It("returns local cleanup errors after success and joins them with sync errors", func() { + sourceDBPath := filepath.Join(GinkgoT().TempDir(), historyDBNameConst) + createHistoryStandbySyncSQLiteDB(sourceDBPath) + cleanupErr := errors.New("cleanup failed") + historyStandbySyncMkdirTemp = func(dir, pattern string) (string, error) { + return GinkgoT().TempDir(), nil + } + historyStandbySyncRemoveAll = func(path string) error { + return cleanupErr + } + + err := withHistoryStandbySyncSnapshot(sourceDBPath, 0o600, func(string) error { + return nil + }) + Expect(errors.Is(err, cleanupErr)).To(BeTrue()) + + syncErr := errors.New("sync failed") + err = withHistoryStandbySyncSnapshot(sourceDBPath, 0o600, func(string) error { + return syncErr + }) + Expect(errors.Is(err, syncErr)).To(BeTrue()) + Expect(errors.Is(err, cleanupErr)).To(BeTrue()) + }) + + It("joins snapshot creation and local cleanup errors", func() { + sourceDBPath := filepath.Join(GinkgoT().TempDir(), historyDBNameConst) + Expect(os.WriteFile(sourceDBPath, []byte("not sqlite"), 0o600)).To(Succeed()) + cleanupErr := errors.New("cleanup failed") + historyStandbySyncRemoveAll = func(path string) error { + return cleanupErr + } + + _, tempDir, err := createHistoryStandbySyncSnapshot(sourceDBPath, 0o600) + + Expect(tempDir).To(BeEmpty()) + Expect(err.Error()).To(ContainSubstring("VACUUM INTO")) + Expect(errors.Is(err, cleanupErr)).To(BeTrue()) + }) + + It("returns discovery connection close errors", func() { + primaryDataDir := GinkgoT().TempDir() + sourceDBPath := filepath.Join(primaryDataDir, historyDBNameConst) + createHistoryStandbySyncSQLiteDB(sourceDBPath) + sqlDB, mock, err := sqlmock.New() + Expect(err).ToNot(HaveOccurred()) + mock.ExpectQuery(regexp.QuoteMeta(historyStandbySyncPrimarySQL)). + WillReturnRows(sqlmock.NewRows([]string{"datadir"}).AddRow(primaryDataDir)) + mock.ExpectQuery(regexp.QuoteMeta(historyStandbySyncStandbySQL)). + WillReturnRows(sqlmock.NewRows([]string{"hostname", "datadir"}).AddRow("sdw-standby", "/data/standby")) + closeErr := errors.New("close failed") + mock.ExpectClose().WillReturnError(closeErr) + historyStandbySyncOpenClusterConn = func() (*sqlx.DB, error) { + return sqlx.NewDb(sqlDB, "sqlmock"), nil + } + + target, skipReason, err := discoverHistoryStandbySyncTarget(sourceDBPath) + + Expect(target).ToNot(BeNil()) + Expect(skipReason).To(BeEmpty()) + Expect(errors.Is(err, closeErr)).To(BeTrue()) + Expect(err.Error()).To(ContainSubstring("close local cluster connection for standby history sync discovery")) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + }) + It("canonicalizes symlink sources and uses the shared lock path suffix", func() { tmpDir := GinkgoT().TempDir() primaryDataDir := filepath.Join(tmpDir, "primary") @@ -460,20 +548,24 @@ var _ = Describe("history standby sync", func() { It("keeps automatic sync best-effort while strict sync treats skips as errors", func() { stdout, _, _ := testhelper.SetupTestLogger() syncCalls := 0 + cleanupErr := errors.New("remove local standby history sync temp directory: cleanup failed") historyStandbySync = func() historyStandbySyncResult { syncCalls++ - return historyStandbySyncResult{err: errors.New("transport failed")} + return historyStandbySyncResult{err: cleanupErr} } result := syncHistoryStandbyBestEffort(false) - Expect(result.err).To(HaveOccurred()) + Expect(errors.Is(result.err, cleanupErr)).To(BeTrue()) Expect(syncCalls).To(Equal(1)) - Expect(string(stdout.Contents())).To(ContainSubstring("History db sync to standby coordinator failed; standby history may be stale: transport failed")) + Expect(string(stdout.Contents())).To(ContainSubstring("History db sync to standby coordinator failed; standby history may be stale: remove local standby history sync temp directory: cleanup failed")) + + err := syncHistoryStandbyStrict() + Expect(errors.Is(err, cleanupErr)).To(BeTrue()) historyStandbySync = func() historyStandbySyncResult { return historyStandbySyncResult{skipReason: "no up standby coordinator found"} } - err := syncHistoryStandbyStrict() + err = syncHistoryStandbyStrict() Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("history db sync to standby coordinator skipped: no up standby coordinator found")) }) diff --git a/gpbackman/gpbckpconfig/cluster.go b/gpbackman/gpbckpconfig/cluster.go index fcea6693..b664da20 100644 --- a/gpbackman/gpbckpconfig/cluster.go +++ b/gpbackman/gpbckpconfig/cluster.go @@ -24,7 +24,6 @@ import ( "strconv" "github.com/apache/cloudberry-backup/gpbackman/textmsg" - "github.com/apache/cloudberry-go-libs/gplog" "github.com/apache/cloudberry-go-libs/operating" "github.com/jmoiron/sqlx" _ "github.com/lib/pq" @@ -82,39 +81,11 @@ func NewClusterLocalClusterDefaultConn() (*sqlx.DB, error) { return NewClusterLocalClusterConn(dbName) } -// GetPrimaryCoordinatorDataDir returns the up primary coordinator data directory. -func GetPrimaryCoordinatorDataDir() (string, error) { - db, err := NewClusterLocalClusterDefaultConn() - if err != nil { - return "", err - } - defer func() { - if closeErr := db.Close(); closeErr != nil { - gplog.Error("Unable to close local cluster connection: %v", closeErr) - } - }() - return QueryPrimaryCoordinatorDataDir(db) -} - // QueryPrimaryCoordinatorDataDir queries the up primary coordinator data directory. func QueryPrimaryCoordinatorDataDir(conn *sqlx.DB) (string, error) { return ExecuteQueryLocalClusterConn[string](conn, primaryCoordinatorDataDirSQL) } -// GetUpStandbyCoordinator returns the up standby coordinator from the local cluster catalog. -func GetUpStandbyCoordinator() (StandbyCoordinator, error) { - db, err := NewClusterLocalClusterDefaultConn() - if err != nil { - return StandbyCoordinator{}, err - } - defer func() { - if closeErr := db.Close(); closeErr != nil { - gplog.Error("Unable to close local cluster connection: %v", closeErr) - } - }() - return QueryUpStandbyCoordinator(db) -} - // QueryUpStandbyCoordinator queries the up standby coordinator from the local cluster catalog. func QueryUpStandbyCoordinator(conn *sqlx.DB) (StandbyCoordinator, error) { return ExecuteQueryLocalClusterConn[StandbyCoordinator](conn, upStandbyCoordinatorSQL) diff --git a/gpbackman/gpbckpconfig/cluster_test.go b/gpbackman/gpbckpconfig/cluster_test.go index 3e786f8e..bf6954b2 100644 --- a/gpbackman/gpbckpconfig/cluster_test.go +++ b/gpbackman/gpbckpconfig/cluster_test.go @@ -195,33 +195,6 @@ var _ = Describe("cluster tests", func() { }) }) - Describe("GetPrimaryCoordinatorDataDir", func() { - It("returns connection errors", func() { - connectErr := errors.New("connection failed") - connectLocalCluster = func(driverName, dataSourceName string) (*sqlx.DB, error) { - return nil, connectErr - } - - dataDir, err := GetPrimaryCoordinatorDataDir() - - Expect(dataDir).To(BeEmpty()) - Expect(err).To(MatchError(connectErr)) - }) - }) - - Describe("GetUpStandbyCoordinator", func() { - It("returns connection errors", func() { - connectErr := errors.New("connection failed") - connectLocalCluster = func(driverName, dataSourceName string) (*sqlx.DB, error) { - return nil, connectErr - } - - standbyCoordinator, err := GetUpStandbyCoordinator() - - Expect(standbyCoordinator).To(Equal(StandbyCoordinator{})) - Expect(err).To(MatchError(connectErr)) - }) - }) }) func newClusterSQLMock() (*sqlx.DB, sqlmock.Sqlmock) { From ffa9e39a0ba522532877268816b346a504b726a1 Mon Sep 17 00:00:00 2001 From: woblerr Date: Wed, 5 Aug 2026 23:30:55 +0300 Subject: [PATCH 10/19] Isolate standby history sync end-to-end coverage. Disable automatic sync in shared helpers and restore the original standby history after the dedicated scenario. --- end_to_end/end_to_end_suite_test.go | 42 +++++++++++++++++ end_to_end/history_standby_sync_test.go | 61 ++++++++++++++++++++++++- 2 files changed, 101 insertions(+), 2 deletions(-) diff --git a/end_to_end/end_to_end_suite_test.go b/end_to_end/end_to_end_suite_test.go index cbbfc90f..75d60a32 100644 --- a/end_to_end/end_to_end_suite_test.go +++ b/end_to_end/end_to_end_suite_test.go @@ -90,17 +90,37 @@ func init() { * to allow checking its output. */ func gpbackup(gpbackupPath string, backupHelperPath string, args ...string) []byte { + return runGpbackup(gpbackupPath, backupHelperPath, true, args...) +} + +func gpbackupWithHistoryStandbySync(gpbackupPath string, backupHelperPath string, args ...string) []byte { + return runGpbackup(gpbackupPath, backupHelperPath, false, args...) +} + +func runGpbackup(gpbackupPath string, backupHelperPath string, disableHistoryStandbySync bool, args ...string) []byte { if useOldBackupVersion { _ = os.Chdir("..") command := exec.Command("make", "install", fmt.Sprintf("helper_path=%s", backupHelperPath)) mustRunCommand(command) _ = os.Chdir("end_to_end") } + if disableHistoryStandbySync && !useOldBackupVersion && !hasCommandArgument(args, "--no-history-sync-standby") { + args = append(args, "--no-history-sync-standby") + } args = append([]string{"--verbose", "--dbname", "testdb"}, args...) command := exec.Command(gpbackupPath, args...) return mustRunCommand(command) } +func hasCommandArgument(args []string, expected string) bool { + for _, arg := range args { + if arg == expected { + return true + } + } + return false +} + func gprestore(gprestorePath string, restoreHelperPath string, timestamp string, args ...string) []byte { if useOldBackupVersion { _ = os.Chdir("..") @@ -469,15 +489,37 @@ func moveSegmentBackupFiles(tarBaseName string, extractDirectory string, isMulti // gpbackman helpers func gpbackman(args ...string) []byte { + return runGpbackman(true, args...) +} + +func gpbackmanWithHistoryStandbySync(args ...string) []byte { + return runGpbackman(false, args...) +} + +func runGpbackman(disableHistoryStandbySync bool, args ...string) []byte { + args = gpbackmanArgsWithHistoryStandbySyncPolicy(disableHistoryStandbySync, args) command := exec.Command(gpbackmanPath, args...) return mustRunCommand(command) } func gpbackmanWithError(args ...string) ([]byte, error) { + args = gpbackmanArgsWithHistoryStandbySyncPolicy(true, args) command := exec.Command(gpbackmanPath, args...) return command.CombinedOutput() } +func gpbackmanArgsWithHistoryStandbySyncPolicy(disabled bool, args []string) []string { + if !disabled || len(args) == 0 || hasCommandArgument(args, "--no-history-sync-standby") { + return args + } + switch args[0] { + case "backup-delete", "backup-clean", "history-clean": + return append(args, "--no-history-sync-standby") + default: + return args + } +} + func getHistoryDBPathForCluster() string { mdd := backupCluster.GetDirForContent(-1) return path.Join(mdd, "gpbackup_history.db") diff --git a/end_to_end/history_standby_sync_test.go b/end_to_end/history_standby_sync_test.go index 20bbe639..794847c3 100644 --- a/end_to_end/history_standby_sync_test.go +++ b/end_to_end/history_standby_sync_test.go @@ -28,6 +28,7 @@ import ( "os/exec" stdpath "path/filepath" "strings" + "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -95,6 +96,61 @@ func copyStandbyHistoryDB(target standbyCoordinatorTarget) string { return localPath } +func preserveStandbyHistoryDB(target standbyCoordinatorTarget) { + remotePath := stdpath.Join(target.DataDir, "gpbackup_history.db") + savedPath := fmt.Sprintf("%s.end-to-end-%d-%d", remotePath, os.Getpid(), time.Now().UnixNano()) + quotedRemotePath := quoteRemoteShellPath(remotePath) + quotedSavedPath := quoteRemoteShellPath(savedPath) + saveCommand := fmt.Sprintf( + "if test -f %s; then test ! -e %s && test ! -L %s && cp -p -- %s %s && printf present; elif test ! -e %s && test ! -L %s; then printf absent; else exit 1; fi", + quotedRemotePath, + quotedSavedPath, + quotedSavedPath, + quotedRemotePath, + quotedSavedPath, + quotedRemotePath, + quotedRemotePath, + ) + state := strings.TrimSpace(string(runStandbyHistorySSHCommand(target, saveCommand))) + Expect(state).To(Or(Equal("present"), Equal("absent"))) + + DeferCleanup(func() { + var restoreCommand string + if state == "present" { + restoreCommand = fmt.Sprintf( + "test -f %s && test ! -d %s && mv -f -- %s %s", + quotedSavedPath, + quotedRemotePath, + quotedSavedPath, + quotedRemotePath, + ) + } else { + restoreCommand = fmt.Sprintf( + "test ! -d %s && rm -f -- %s %s", + quotedRemotePath, + quotedRemotePath, + quotedSavedPath, + ) + } + runStandbyHistorySSHCommand(target, restoreCommand) + }) +} + +func runStandbyHistorySSHCommand(target standbyCoordinatorTarget, remoteCommand string) []byte { + command := exec.Command( + "ssh", + "-o", "StrictHostKeyChecking=no", + "-o", "ConnectTimeout=30", + target.Hostname, + remoteCommand, + ) + var stderr bytes.Buffer + command.Stderr = &stderr + output, err := command.Output() + Expect(err).ToNot(HaveOccurred(), "run standby history command on %s: %s", target.Hostname, strings.TrimSpace(stderr.String())) + return output +} + func readHistoryLogicalRows(historyDBPath string) []historyLogicalRow { dsn := (&url.URL{Scheme: "file", Path: historyDBPath}).String() + "?mode=ro" db, err := sql.Open("sqlite3", dsn) @@ -145,6 +201,7 @@ var _ = Describe("history database standby sync", func() { } end_to_end_setup() standbyTarget = discoverUpStandbyCoordinator() + preserveStandbyHistoryDB(standbyTarget) primaryHistoryDB = getHistoryDBPathForCluster() }) @@ -153,7 +210,7 @@ var _ = Describe("history database standby sync", func() { }) It("keeps standby history logically consistent across automatic, disabled, explicit, and mutation sync", func() { - baselineOutput := gpbackup( + baselineOutput := gpbackupWithHistoryStandbySync( gpbackupPath, backupHelperPath, "--backup-dir", backupDir, @@ -196,7 +253,7 @@ var _ = Describe("history database standby sync", func() { Expect(standbyAfterExplicitSync).To(Equal(primaryAfterExplicitSync)) Expect(findHistoryLogicalRow(standbyAfterExplicitSync, disabledTimestamp).Status).To(Equal("Success")) - gpbackman( + gpbackmanWithHistoryStandbySync( "backup-delete", "--history-db", primaryHistoryDB, "--timestamp", baselineTimestamp, From 2d3d2a3b1aaacc3cedb24e83410487ab6461fac6 Mon Sep 17 00:00:00 2001 From: woblerr Date: Wed, 5 Aug 2026 23:48:09 +0300 Subject: [PATCH 11/19] Simplify standby history synchronization documentation. --- gpbackman/COMMANDS.md | 70 ++++++++++--------------------------------- gpbackman/README.md | 67 ++++++++++------------------------------- 2 files changed, 31 insertions(+), 106 deletions(-) diff --git a/gpbackman/COMMANDS.md b/gpbackman/COMMANDS.md index 34f8db4f..fd3ef667 100644 --- a/gpbackman/COMMANDS.md +++ b/gpbackman/COMMANDS.md @@ -17,6 +17,7 @@ under the License. --> +- [Standby history DB sync](#standby-history-db-sync) - [Delete all existing backups older than the specified time condition (`backup-clean`)](#delete-all-existing-backups-older-than-the-specified-time-condition-backup-clean) - [Examples](#examples) - [Delete all backups from local storage older than the specified time condition](#delete-all-backups-from-local-storage-older-than-the-specified-time-condition) @@ -38,24 +39,15 @@ - [Display the backup report from local storage](#display-the-backup-report-from-local-storage) - [Display the backup report using storage plugin](#display-the-backup-report-using-storage-plugin) -`backup-clean`, `backup-delete`, and `history-clean` automatically attempt to -synchronize the cluster `gpbackup_history.db` to an up standby after a -successful command, including a successful no-op. This is best effort: skips -and failures do not change an otherwise successful command exit status. -For `backup-delete --ignore-errors`, synchronization still runs when the -command returns its existing successful result after recording deletion -errors. -Add `--no-history-sync-standby` to any of these three commands to disable the -attempt. Read-only commands do not synchronize history. - -Automatic synchronization requires the history database to resolve to -`/gpbackup_history.db`. Use -`--auto-load-history-db` to resolve that path from -`$COORDINATOR_DATA_DIRECTORY`, or pass the cluster database explicitly with -`--history-db`. A symlink is accepted when its canonical target is the cluster -database; custom and default working-directory databases are not synchronized. -See [`history-sync`](#sync-the-history-database-to-the-standby-coordinator-history-sync) -for strict behavior and operational requirements. +# Standby history DB sync + +The explicit `history-sync` command synchronizes the cluster `gpbackup_history.db` to an up standby coordinator. It does not have successful skips: an unavailable standby, an ineligible source, or a discovery, snapshot, validation, SSH, rsync, or cleanup error is reported as an error and returns a non-zero exit status. + +The source must resolve to the cluster history database at `/gpbackup_history.db`. Select it with `--history-db`, or use `--auto-load-history-db` when `$COORDINATOR_DATA_DIRECTORY` points to the primary coordinator data directory. Custom history databases and the default working-directory database are not eligible for explicit synchronization. + +After a successful `backup-delete`, `backup-clean`, or `history-clean`, gpBackMan also attempts this synchronization automatically. Automatic sync is best-effort: `--no-history-sync-standby` produces an info-level skip, while no up standby and ineligible sources are debug-only skips; sync failures are warnings and do not change the successful primary command result. Read-only commands do not trigger automatic sync. + +Only `gpbackup_history.db` is synchronized. Report files, backup data, and other backup artifacts are not synchronized. # Delete all existing backups older than the specified time condition (`backup-clean`) @@ -517,38 +509,6 @@ Delete information about deleted backups from history database older than timest # Sync the history database to the standby coordinator (`history-sync`) -`history-sync` performs a strict, explicit synchronization. It returns exit -status 0 only after a verified SQLite snapshot has been atomically installed -as `gpbackup_history.db` on an up standby coordinator. Unlike automatic -synchronization, a normal skip is an error: no up standby, an ineligible -source, an unresolved auto-loaded source, or a busy sync lock causes exit -status 1, as does any discovery, snapshot, transfer, or installation failure. - -The source must canonically resolve to -`/gpbackup_history.db`. A symlink to that -file is accepted. A custom `--history-db` path and the default -working-directory database are rejected. Use `--auto-load-history-db` to -resolve the source from `$COORDINATOR_DATA_DIRECTORY`, or pass the cluster -database explicitly with `--history-db`. - -The command uses the current OS user for SSH. The primary host must have -`ssh` and `rsync`; the user must have non-interactive access to the standby, -permission to create the source `.sync.lock`, and permission to write the -standby data directory and preserve the existing destination owner, group, and -mode. An up standby must be visible in `gp_segment_configuration`. - -The source is locked without waiting. gpBackMan creates a consistent snapshot -with SQLite `VACUUM INTO`, preserves its mode, and requires a single `ok` -result from `PRAGMA quick_check`. It transfers that snapshot with `rsync -p` -to a unique temporary path, copies the existing destination metadata when -present, and atomically renames the snapshot into place. Failed transfers and -installs remove only their own temporary file. - -Atomic replacement prevents a partial database from becoming visible, but it -does not coordinate a concurrent failover. A coordinator role change can race -with discovery and installation. A process with the previous database already -open continues reading the old inode until it closes and reopens the file. - Available options for `history-sync` command and their description: ```bash @@ -575,17 +535,17 @@ Global Flags: ## Examples -Resolve the cluster history database from the coordinator environment: +Synchronize an explicitly selected cluster history database: ```bash -./gpbackman history-sync --auto-load-history-db +./gpbackman history-sync \ + --history-db "$COORDINATOR_DATA_DIRECTORY/gpbackup_history.db" ``` -Synchronize an explicitly selected cluster history database: +Resolve the cluster history database from the coordinator environment and synchronize it: ```bash -./gpbackman history-sync \ - --history-db "$COORDINATOR_DATA_DIRECTORY/gpbackup_history.db" +./gpbackman history-sync --auto-load-history-db ``` # Display the report for a specific backup (`report-info`) diff --git a/gpbackman/README.md b/gpbackman/README.md index 49de8ebd..861307c7 100644 --- a/gpbackman/README.md +++ b/gpbackman/README.md @@ -29,7 +29,8 @@ The utility works with `gpbackup_history.db` SQLite history database format. * delete existing backups from local storage or using storage plugins; * delete all existing backups from local storage or using storage plugins older than the specified time condition; * clean deleted backups from the history database; -* synchronize the cluster history database to the standby coordinator. +* manually synchronize the cluster `gpbackup_history.db` to the standby coordinator; +* automatically synchronize the cluster `gpbackup_history.db` after successful backup deletion and history cleanup. ## Commands ### Introduction @@ -65,6 +66,20 @@ Flags: Use "gpbackman [command] --help" for more information about a command. ``` +### Standby history DB sync + +Run `history-sync` to explicitly synchronize the cluster `gpbackup_history.db` to an up standby coordinator. The source must resolve to `/gpbackup_history.db`; a custom database or the default working-directory database is not eligible. Explicit sync treats every non-sync outcome as an error and exits non-zero. + +For the usual cluster setup, resolve the source from the coordinator data directory: + +```bash +./gpbackman history-sync --auto-load-history-db +``` + +After a successful `backup-delete`, `backup-clean`, or `history-clean`, gpBackMan also attempts the same synchronization automatically. Automatic sync is best-effort: ineligible source paths and no standby are debug-only skips, while sync failures are warnings and do not change the successful primary command result. Pass `--no-history-sync-standby` to those mutation commands to disable automatic sync. + +Only `gpbackup_history.db` is synchronized. Report files, backup data, and other backup artifacts are not synchronized. + ### Detail info about commands Description of each command: @@ -75,56 +90,6 @@ Description of each command: * [Sync the history database to the standby coordinator (`history-sync`)](./COMMANDS.md#sync-the-history-database-to-the-standby-coordinator-history-sync) * [Display the report for a specific backup (`report-info`)](./COMMANDS.md#display-the-report-for-a-specific-backup-report-info) -## Standby history database synchronization - -`backup-delete`, `backup-clean`, and `history-clean` automatically attempt to -synchronize `gpbackup_history.db` after the command has successfully finished -and closed the database. Successful no-op commands also attempt -synchronization. For `backup-delete --ignore-errors`, synchronization runs -when the command completes with its existing successful result, including when -individual deletion errors were recorded. - -Automatic synchronization is best effort. A normal skip, such as no up -standby or an ineligible history database, does not fail the mutation command. -A discovery, snapshot, transfer, or installation error is logged as a warning -without changing an otherwise successful exit status. Use the command-local -`--no-history-sync-standby` flag to disable the attempt for one of these three -commands. The flag is not available on read-only commands or `history-sync`. - -`history-sync` provides strict synchronization: - -```bash -./gpbackman history-sync --auto-load-history-db -``` - -It exits successfully only after a verified snapshot has been installed -atomically on an up standby. An ineligible source, no up standby, a busy sync -lock, or any discovery, snapshot, transfer, or installation error causes a -nonzero exit. - -The source must resolve to -`/gpbackup_history.db`. An explicit -`--history-db` path is accepted only when its canonical path is that cluster -database; a symlink to that file is accepted. A custom database is not -synchronized. With `--auto-load-history-db`, gpBackMan resolves the file from -`$COORDINATOR_DATA_DIRECTORY`. Without either option, gpBackMan uses the -working-directory database for normal command behavior, but standby -synchronization skips it; therefore a strict `history-sync` call fails. -`--history-db` takes precedence when both global options are present. - -Synchronization requires an up standby coordinator, `ssh` and `rsync` on the -host running gpBackMan, and non-interactive SSH access for the current OS user. -The user must be able to read the primary history database, create its adjacent -`.sync.lock`, write temporary files in the standby coordinator data directory, -and preserve the existing standby file's owner, group, and mode. - -gpBackMan creates and validates a consistent SQLite snapshot before transfer, -copies it to a unique standby temporary path, and atomically renames it into -place. Readers never see a partial copy. This does not coordinate a concurrent -coordinator failover: a role change can race with discovery and installation, -and a process that already has the old database open continues using that old -inode until it reopens the file. - ## About gpBackMan is part of the Apache Cloudberry Backup (Incubating) toolset. It is based on the original [gpbackman](https://github.com/woblerr/gpbackman) project. From 1f5b35f3a92b1f6e82b2cb4f4a004e25a34e609a Mon Sep 17 00:00:00 2001 From: woblerr Date: Thu, 6 Aug 2026 00:01:45 +0300 Subject: [PATCH 12/19] Fix standby history sync tests on macOS. --- backup/history_standby_sync_test.go | 5 +++++ gpbackman/cmd/history_standby_sync_test.go | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/backup/history_standby_sync_test.go b/backup/history_standby_sync_test.go index 00d87727..a4cb5255 100644 --- a/backup/history_standby_sync_test.go +++ b/backup/history_standby_sync_test.go @@ -206,6 +206,11 @@ var _ = Describe("backup history standby sync", func() { linkDir := filepath.Join(tmpDir, "link") Expect(os.Mkdir(realDir, 0o700)).To(Succeed()) Expect(os.Mkdir(linkDir, 0o700)).To(Succeed()) + // Resolve any symlinks in the OS temp dir itself (e.g. macOS /var -> /private/var) + // so the expected path matches the canonicalization performed by the code under test. + canonicalRealDir, err := filepath.EvalSymlinks(realDir) + Expect(err).ToNot(HaveOccurred()) + realDir = canonicalRealDir realSourcePath := filepath.Join(realDir, backupHistoryDBName) createBackupHistoryStandbySyncSQLiteDB(realSourcePath) linkSourcePath := filepath.Join(linkDir, backupHistoryDBName) diff --git a/gpbackman/cmd/history_standby_sync_test.go b/gpbackman/cmd/history_standby_sync_test.go index 14e5dd1b..574baab8 100644 --- a/gpbackman/cmd/history_standby_sync_test.go +++ b/gpbackman/cmd/history_standby_sync_test.go @@ -300,6 +300,11 @@ var _ = Describe("history standby sync", func() { tmpDir := GinkgoT().TempDir() primaryDataDir := filepath.Join(tmpDir, "primary") Expect(os.Mkdir(primaryDataDir, 0o700)).To(Succeed()) + // Resolve any symlinks in the OS temp dir itself (e.g. macOS /var -> /private/var) + // so the expected path matches the canonicalization performed by the code under test. + canonicalPrimaryDataDir, err := filepath.EvalSymlinks(primaryDataDir) + Expect(err).ToNot(HaveOccurred()) + primaryDataDir = canonicalPrimaryDataDir realSourceDBPath := filepath.Join(primaryDataDir, historyDBNameConst) createHistoryStandbySyncSQLiteDB(realSourceDBPath) linkSourceDBPath := filepath.Join(tmpDir, "history-link.db") From 90f9b58f36255382bf9de91b8a678d9db8b935e0 Mon Sep 17 00:00:00 2001 From: woblerr Date: Thu, 6 Aug 2026 00:02:07 +0300 Subject: [PATCH 13/19] Fix gpbackman report-info examples. --- gpbackman/COMMANDS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gpbackman/COMMANDS.md b/gpbackman/COMMANDS.md index fd3ef667..4a031c90 100644 --- a/gpbackman/COMMANDS.md +++ b/gpbackman/COMMANDS.md @@ -612,13 +612,13 @@ Without specifying a backup directory path: ```bash ./gpbackman report-info \ --timestamp 20230809232817 - --backup-dir /some/path ``` With specifying backup directory path: ```bash ./gpbackman report-info \ --timestamp 20230809232817 \ + --backup-dir /some/path ``` ### Display the backup report using storage plugin From 6df82f087b9828602f89342f84fe459bfce0286a Mon Sep 17 00:00:00 2001 From: woblerr Date: Thu, 6 Aug 2026 00:10:00 +0300 Subject: [PATCH 14/19] Fix link in gpBackMan docs. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index d5da4d70..b01d4861 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,7 @@ that already have the old standby database open continue reading that old inode until they close and reopen it. For automatic synchronization after history maintenance and for the strict -manual command, see [gpBackMan history synchronization](./gpbackman/README.md#standby-history-database-synchronization). +manual command, see [gpBackMan history synchronization](./gpbackman/README.md#standby-history-db-sync). ## Additional tools From a6af82651be8d9a3a05d984f27ea41a0f7ee4263 Mon Sep 17 00:00:00 2001 From: woblerr Date: Thu, 6 Aug 2026 15:22:36 +0300 Subject: [PATCH 15/19] Fix standby history sync rsync destination. Pass remote paths directly to rsync while retaining shell quoting for SSH install and cleanup commands. --- backup/history_standby_sync.go | 2 +- backup/history_standby_sync_test.go | 4 ++-- gpbackman/cmd/history_standby_sync.go | 2 +- gpbackman/cmd/history_standby_sync_test.go | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/backup/history_standby_sync.go b/backup/history_standby_sync.go index 64c584e0..184f7253 100644 --- a/backup/history_standby_sync.go +++ b/backup/history_standby_sync.go @@ -342,7 +342,7 @@ func buildBackupHistoryStandbySyncRsyncArgs(snapshotPath, standbyHost, userName, backupHistoryStandbySyncSSHOptions, "--", snapshotPath, - fmt.Sprintf("%s@%s:%s", userName, standbyHost, shellQuoteBackupHistoryStandbySyncPath(remoteTempPath)), + fmt.Sprintf("%s@%s:%s", userName, standbyHost, remoteTempPath), } } diff --git a/backup/history_standby_sync_test.go b/backup/history_standby_sync_test.go index a4cb5255..32e4c0ad 100644 --- a/backup/history_standby_sync_test.go +++ b/backup/history_standby_sync_test.go @@ -344,7 +344,7 @@ var _ = Describe("backup history standby sync", func() { Expect(mock.ExpectationsWereMet()).To(Succeed()) }) - It("quotes remote shell paths in rsync, install, and cleanup commands", func() { + It("passes rsync paths as arguments and quotes remote shell paths", func() { remoteTempPath := "/data dir/standby's/.gpbackup_history.db.tmp" destPath := "/data dir/standby's/gpbackup_history.db" @@ -354,7 +354,7 @@ var _ = Describe("backup history standby sync", func() { backupHistoryStandbySyncSSHOptions, "--", "/tmp/snapshot", - "gpadmin@sdw-standby:" + shellQuoteBackupHistoryStandbySyncPath(remoteTempPath), + "gpadmin@sdw-standby:" + remoteTempPath, })) installCommand := buildBackupHistoryStandbySyncRemoteInstallCommand(remoteTempPath, destPath) Expect(installCommand).To(ContainSubstring("test -f " + shellQuoteBackupHistoryStandbySyncPath(remoteTempPath))) diff --git a/gpbackman/cmd/history_standby_sync.go b/gpbackman/cmd/history_standby_sync.go index 71017ca0..fa334d14 100644 --- a/gpbackman/cmd/history_standby_sync.go +++ b/gpbackman/cmd/history_standby_sync.go @@ -397,7 +397,7 @@ func buildHistoryStandbySyncRsyncArgs(snapshotPath, standbyHost, userName, remot historyStandbySyncSSHOptions, "--", snapshotPath, - fmt.Sprintf("%s@%s:%s", userName, standbyHost, shellQuoteHistoryStandbySyncPath(remoteTempPath)), + fmt.Sprintf("%s@%s:%s", userName, standbyHost, remoteTempPath), } } diff --git a/gpbackman/cmd/history_standby_sync_test.go b/gpbackman/cmd/history_standby_sync_test.go index 574baab8..b9b05fbd 100644 --- a/gpbackman/cmd/history_standby_sync_test.go +++ b/gpbackman/cmd/history_standby_sync_test.go @@ -487,7 +487,7 @@ var _ = Describe("history standby sync", func() { Expect(mock.ExpectationsWereMet()).To(Succeed()) }) - It("quotes remote shell paths in rsync, install, and cleanup commands", func() { + It("passes rsync paths as arguments and quotes remote shell paths", func() { remoteTempPath := "/data dir/standby's/.gpbackup_history.db.tmp" destPath := "/data dir/standby's/gpbackup_history.db" @@ -497,7 +497,7 @@ var _ = Describe("history standby sync", func() { historyStandbySyncSSHOptions, "--", "/tmp/snapshot", - "gpadmin@sdw-standby:" + shellQuoteHistoryStandbySyncPath(remoteTempPath), + "gpadmin@sdw-standby:" + remoteTempPath, })) installCommand := buildHistoryStandbySyncRemoteInstallCommand(remoteTempPath, destPath) Expect(installCommand).To(ContainSubstring("test -f " + shellQuoteHistoryStandbySyncPath(remoteTempPath))) From d11a60665a4f9482b1b3a9f28093d34e6746e20b Mon Sep 17 00:00:00 2001 From: woblerr Date: Tue, 11 Aug 2026 00:21:18 +0300 Subject: [PATCH 16/19] Add configurable standby history sync timeout. Expose --history-sync-standby-timeout as integer seconds in gpbackup and sync-capable gpBackMan commands. Use int to match existing CLI conventions, default to 300 seconds, and cap values at one day to catch accidental settings without excluding slow transfers. Create one context deadline after SQLite snapshot validation and share its remaining budget across rsync and remote install. This keeps discovery and local snapshot work outside the limit and prevents each transport stage from restarting the timeout. Run rsync and ssh with CommandContext and BatchMode so stalled processes are terminated without waiting for interactive authentication. Use an independent 120-second context for failure cleanup so remote temporary files can still be removed after the transport deadline expires while cleanup remains bounded. Preserve both primary and cleanup errors and keep automatic synchronization best-effort while history-sync stays strict. --- README.md | 8 ++ backup/history_standby_sync.go | 72 ++++++++++----- backup/history_standby_sync_test.go | 99 +++++++++++++++++--- backup/validate.go | 4 + backup/validate_test.go | 4 + gpbackman/COMMANDS.md | 8 +- gpbackman/README.md | 9 ++ gpbackman/cmd/backup_clean.go | 6 ++ gpbackman/cmd/backup_delete.go | 6 ++ gpbackman/cmd/constants.go | 52 +++++------ gpbackman/cmd/history_clean.go | 6 ++ gpbackman/cmd/history_standby_sync.go | 75 +++++++++++----- gpbackman/cmd/history_standby_sync_test.go | 100 +++++++++++++++++++-- gpbackman/cmd/history_sync.go | 6 ++ gpbackman/cmd/history_sync_test.go | 70 ++++++++++++++- gpbackman/cmd/root.go | 11 +++ gpbackman/cmd/wrappers.go | 5 +- options/flag.go | 85 +++++++++--------- options/flag_test.go | 34 +++++++ 19 files changed, 524 insertions(+), 136 deletions(-) diff --git a/README.md b/README.md index b01d4861..1557fc23 100644 --- a/README.md +++ b/README.md @@ -108,6 +108,14 @@ standby synchronization for one backup: gpbackup --dbname --no-history-sync-standby ``` +Configure the sync timeout with `--history-sync-standby-timeout SECONDS`. The +default is 300 seconds; the supported range is 1 to 86400 seconds. The timeout +is one shared budget for `rsync` and remote install. It starts after snapshot +validation. Standby discovery and SQLite snapshot creation and validation +(`VACUUM INTO` and `PRAGMA quick_check`) are outside this budget. If a +transport step fails, remote cleanup of the temporary file uses its own fixed +120-second timeout, independent of `--history-sync-standby-timeout`. + The synchronization process: 1. Takes a non-waiting lock next to the canonical source database. diff --git a/backup/history_standby_sync.go b/backup/history_standby_sync.go index 184f7253..2ad27fc2 100644 --- a/backup/history_standby_sync.go +++ b/backup/history_standby_sync.go @@ -20,6 +20,7 @@ under the License. package backup import ( + "context" "database/sql" "errors" "fmt" @@ -28,6 +29,7 @@ import ( "os/exec" "path/filepath" "strings" + "time" "github.com/apache/cloudberry-backup/options" "github.com/apache/cloudberry-go-libs/gplog" @@ -38,9 +40,15 @@ import ( const ( backupHistoryDBName = "gpbackup_history.db" - backupHistoryStandbySyncSSHOptions = "ssh -o StrictHostKeyChecking=no -o ConnectTimeout=30" + // Leave enough time for the 30-second SSH connection timeout and remote removal + // while keeping failure cleanup bounded. + backupHistoryStandbySyncCleanupTimeout = 120 * time.Second + backupHistoryStandbySyncSSHOptions = "ssh -o BatchMode=yes -o StrictHostKeyChecking=no -o ConnectTimeout=30" backupHistoryStandbySyncTempDirPattern = "gpbackup-history-standby-sync-*" backupHistoryStandbySyncStandbySQL = "SELECT hostname, datadir FROM gp_segment_configuration WHERE content = -1 AND role = 'm' AND status = 'u';" + // Cap the timeout at one day to catch accidentally oversized CLI values. + // A longer transport deadline is not meaningful for standby history synchronization. + maxHistorySyncStandbyTimeoutSeconds = int(24 * time.Hour / time.Second) ) type backupHistoryStandbySyncTarget struct { @@ -62,13 +70,14 @@ type backupHistoryStandbySyncCommand interface { var ( backupHistoryStandbySync = syncBackupHistoryToStandby - backupHistoryStandbySyncCommandExec = func(name string, args ...string) backupHistoryStandbySyncCommand { - return exec.Command(name, args...) + backupHistoryStandbySyncCommandExec = func(ctx context.Context, name string, args ...string) backupHistoryStandbySyncCommand { + return exec.CommandContext(ctx, name, args...) } - backupHistoryStandbySyncOpenSQLite = sql.Open - backupHistoryStandbySyncMkdirTemp = os.MkdirTemp - backupHistoryStandbySyncRemoveAll = os.RemoveAll - backupHistoryStandbySyncCurrentUser = func() (string, error) { + backupHistoryStandbySyncContextWithTimeout = context.WithTimeout + backupHistoryStandbySyncOpenSQLite = sql.Open + backupHistoryStandbySyncMkdirTemp = os.MkdirTemp + backupHistoryStandbySyncRemoveAll = os.RemoveAll + backupHistoryStandbySyncCurrentUser = func() (string, error) { currentUser, err := operating.System.CurrentUser() if err != nil { return "", err @@ -123,7 +132,15 @@ func syncBackupHistoryToStandby() (string, error) { err = withBackupHistoryStandbySyncLock(sourceDBPath, func() error { return withBackupHistoryStandbySyncSnapshot(sourceDBPath, sourceInfo.Mode().Perm(), func(snapshotPath string) error { - return syncBackupHistoryStandbySnapshot(target, userName, snapshotPath) + timeoutSeconds := MustGetFlagInt(options.HISTORY_SYNC_STANDBY_TIMEOUT) + ctx, cancel := backupHistoryStandbySyncContextWithTimeout(context.Background(), time.Duration(timeoutSeconds)*time.Second) + defer cancel() + + transportErr := syncBackupHistoryStandbySnapshot(ctx, target, userName, snapshotPath) + if transportErr != nil && errors.Is(ctx.Err(), context.DeadlineExceeded) { + return fmt.Errorf("standby history sync transport timed out after %d seconds: %w", timeoutSeconds, transportErr) + } + return transportErr }) }) if err != nil { @@ -310,12 +327,12 @@ func cleanupBackupHistoryStandbySyncTempDir(tempDir string) error { return nil } -func syncBackupHistoryStandbySnapshot(target *backupHistoryStandbySyncTarget, userName, snapshotPath string) error { +func syncBackupHistoryStandbySnapshot(ctx context.Context, target *backupHistoryStandbySyncTarget, userName, snapshotPath string) error { remoteTempPath := newBackupHistoryStandbySyncRemoteTempPath(target.standbyDataDir, snapshotPath) - if err := rsyncBackupHistoryStandbySyncSnapshot(snapshotPath, target.standbyHost, userName, remoteTempPath); err != nil { + if err := rsyncBackupHistoryStandbySyncSnapshot(ctx, snapshotPath, target.standbyHost, userName, remoteTempPath); err != nil { return cleanupBackupHistoryStandbySyncRemoteTempAfterError(err, target.standbyHost, userName, remoteTempPath) } - if err := installBackupHistoryStandbySyncSnapshot(target, userName, remoteTempPath); err != nil { + if err := installBackupHistoryStandbySyncSnapshot(ctx, target, userName, remoteTempPath); err != nil { return cleanupBackupHistoryStandbySyncRemoteTempAfterError(err, target.standbyHost, userName, remoteTempPath) } return nil @@ -325,10 +342,13 @@ func newBackupHistoryStandbySyncRemoteTempPath(standbyDataDir, snapshotPath stri return filepath.Join(standbyDataDir, fmt.Sprintf(".%s.%s.tmp", backupHistoryDBName, filepath.Base(filepath.Dir(snapshotPath)))) } -func rsyncBackupHistoryStandbySyncSnapshot(snapshotPath, standbyHost, userName, remoteTempPath string) error { +func rsyncBackupHistoryStandbySyncSnapshot(ctx context.Context, snapshotPath, standbyHost, userName, remoteTempPath string) error { args := buildBackupHistoryStandbySyncRsyncArgs(snapshotPath, standbyHost, userName, remoteTempPath) gplog.Debug("Transfer history db snapshot to standby coordinator: %s -> %s:%s", snapshotPath, standbyHost, remoteTempPath) - output, err := backupHistoryStandbySyncCommandExec("rsync", args...).CombinedOutput() + output, err := backupHistoryStandbySyncCommandExec(ctx, "rsync", args...).CombinedOutput() + if ctxErr := ctx.Err(); ctxErr != nil { + err = ctxErr + } if err != nil { return fmt.Errorf("rsync standby history snapshot to %s:%s failed: %w%s", standbyHost, remoteTempPath, err, formatBackupHistoryStandbySyncCommandOutput(output)) } @@ -346,10 +366,10 @@ func buildBackupHistoryStandbySyncRsyncArgs(snapshotPath, standbyHost, userName, } } -func installBackupHistoryStandbySyncSnapshot(target *backupHistoryStandbySyncTarget, userName, remoteTempPath string) error { +func installBackupHistoryStandbySyncSnapshot(ctx context.Context, target *backupHistoryStandbySyncTarget, userName, remoteTempPath string) error { command := buildBackupHistoryStandbySyncRemoteInstallCommand(remoteTempPath, target.standbyHistoryDBPath) gplog.Debug("Install history db snapshot on standby coordinator: %s:%s", target.standbyHost, target.standbyHistoryDBPath) - output, err := runBackupHistoryStandbySyncSSHCommand(command, target.standbyHost, userName) + output, err := runBackupHistoryStandbySyncSSHCommand(ctx, command, target.standbyHost, userName) if err != nil { return fmt.Errorf("install standby history snapshot on %s:%s failed: %w%s", target.standbyHost, target.standbyHistoryDBPath, err, formatBackupHistoryStandbySyncCommandOutput(output)) } @@ -373,16 +393,19 @@ func buildBackupHistoryStandbySyncRemoteInstallCommand(remoteTempPath, standbyHi } func cleanupBackupHistoryStandbySyncRemoteTempAfterError(primaryErr error, standbyHost, userName, remoteTempPath string) error { - if cleanupErr := cleanupBackupHistoryStandbySyncRemoteTemp(standbyHost, userName, remoteTempPath); cleanupErr != nil { - return fmt.Errorf("%w; additionally failed to clean up remote temp file: %v", primaryErr, cleanupErr) + cleanupCtx, cancel := context.WithTimeout(context.Background(), backupHistoryStandbySyncCleanupTimeout) + defer cancel() + + if cleanupErr := cleanupBackupHistoryStandbySyncRemoteTemp(cleanupCtx, standbyHost, userName, remoteTempPath); cleanupErr != nil { + return fmt.Errorf("%w; additionally failed to clean up remote temp file: %w", primaryErr, cleanupErr) } return primaryErr } -func cleanupBackupHistoryStandbySyncRemoteTemp(standbyHost, userName, remoteTempPath string) error { +func cleanupBackupHistoryStandbySyncRemoteTemp(ctx context.Context, standbyHost, userName, remoteTempPath string) error { command := buildBackupHistoryStandbySyncRemoteCleanupCommand(remoteTempPath) gplog.Debug("Clean up remote standby history sync temp file: %s:%s", standbyHost, remoteTempPath) - output, err := runBackupHistoryStandbySyncSSHCommand(command, standbyHost, userName) + output, err := runBackupHistoryStandbySyncSSHCommand(ctx, command, standbyHost, userName) if err != nil { return fmt.Errorf("clean up remote standby history sync temp file %s:%s failed: %w%s", standbyHost, remoteTempPath, err, formatBackupHistoryStandbySyncCommandOutput(output)) } @@ -393,16 +416,23 @@ func buildBackupHistoryStandbySyncRemoteCleanupCommand(remoteTempPath string) st return fmt.Sprintf("rm -f -- %s", shellQuoteBackupHistoryStandbySyncPath(remoteTempPath)) } -func runBackupHistoryStandbySyncSSHCommand(remoteCommand, standbyHost, userName string) ([]byte, error) { - return backupHistoryStandbySyncCommandExec( +func runBackupHistoryStandbySyncSSHCommand(ctx context.Context, remoteCommand, standbyHost, userName string) ([]byte, error) { + output, err := backupHistoryStandbySyncCommandExec( + ctx, "ssh", "-o", + "BatchMode=yes", + "-o", "StrictHostKeyChecking=no", "-o", "ConnectTimeout=30", fmt.Sprintf("%s@%s", userName, standbyHost), remoteCommand, ).CombinedOutput() + if ctxErr := ctx.Err(); ctxErr != nil { + return output, ctxErr + } + return output, err } func backupHistoryStandbySyncSQLiteURI(dbPath, mode string) string { diff --git a/backup/history_standby_sync_test.go b/backup/history_standby_sync_test.go index 32e4c0ad..3ee5e2cd 100644 --- a/backup/history_standby_sync_test.go +++ b/backup/history_standby_sync_test.go @@ -20,12 +20,14 @@ under the License. package backup import ( + "context" "database/sql" "errors" "fmt" "os" "path/filepath" "regexp" + "time" "github.com/DATA-DOG/go-sqlmock" backupfilepath "github.com/apache/cloudberry-backup/filepath" @@ -42,8 +44,12 @@ import ( ) type backupHistoryStandbySyncCommandCall struct { - name string - args []string + ctx context.Context + ctxErr error + deadline time.Time + hasDeadline bool + name string + args []string } type backupHistoryStandbySyncCommandResponse struct { @@ -62,8 +68,9 @@ func (c backupHistoryStandbySyncFakeCommand) CombinedOutput() ([]byte, error) { var _ = Describe("backup history standby sync", func() { var ( - originalSync func() (string, error) - originalOpenSQLite func(string, string) (*sql.DB, error) + originalSync func() (string, error) + originalOpenSQLite func(string, string) (*sql.DB, error) + originalContextWithTimeout func(context.Context, time.Duration) (context.Context, context.CancelFunc) ) BeforeEach(func() { @@ -74,9 +81,11 @@ var _ = Describe("backup history standby sync", func() { connectionPool = nil originalSync = backupHistoryStandbySync originalOpenSQLite = backupHistoryStandbySyncOpenSQLite + originalContextWithTimeout = backupHistoryStandbySyncContextWithTimeout backupHistoryStandbySync = syncBackupHistoryToStandby backupHistoryStandbySyncOpenSQLite = sql.Open - backupHistoryStandbySyncCommandExec = func(name string, args ...string) backupHistoryStandbySyncCommand { + backupHistoryStandbySyncContextWithTimeout = context.WithTimeout + backupHistoryStandbySyncCommandExec = func(ctx context.Context, name string, args ...string) backupHistoryStandbySyncCommand { return backupHistoryStandbySyncFakeCommand{} } backupHistoryStandbySyncMkdirTemp = os.MkdirTemp @@ -89,7 +98,8 @@ var _ = Describe("backup history standby sync", func() { AfterEach(func() { backupHistoryStandbySync = originalSync backupHistoryStandbySyncOpenSQLite = originalOpenSQLite - backupHistoryStandbySyncCommandExec = func(name string, args ...string) backupHistoryStandbySyncCommand { + backupHistoryStandbySyncContextWithTimeout = originalContextWithTimeout + backupHistoryStandbySyncCommandExec = func(ctx context.Context, name string, args ...string) backupHistoryStandbySyncCommand { return backupHistoryStandbySyncFakeCommand{} } backupHistoryStandbySyncMkdirTemp = os.MkdirTemp @@ -267,7 +277,10 @@ var _ = Describe("backup history standby sync", func() { } commandCalls := setBackupHistoryStandbySyncCommands([]backupHistoryStandbySyncCommandResponse{{}, {}}) + start := time.Now() + Expect(cmdFlags.Set(options.HISTORY_SYNC_STANDBY_TIMEOUT, "600")).To(Succeed()) skipReason, err := syncBackupHistoryToStandby() + finished := time.Now() Expect(err).ToNot(HaveOccurred()) Expect(skipReason).To(BeEmpty()) @@ -278,7 +291,14 @@ var _ = Describe("backup history standby sync", func() { Expect((*commandCalls)[0].name).To(Equal("rsync")) Expect((*commandCalls)[0].args).To(Equal(buildBackupHistoryStandbySyncRsyncArgs(snapshotPath, "sdw-standby", "gpadmin", remoteTempPath))) Expect((*commandCalls)[1].name).To(Equal("ssh")) + Expect((*commandCalls)[1].ctx).To(BeIdenticalTo((*commandCalls)[0].ctx)) + deadline, ok := (*commandCalls)[0].ctx.Deadline() + Expect(ok).To(BeTrue()) + Expect(deadline).To(BeTemporally(">=", start.Add(600*time.Second))) + Expect(deadline).To(BeTemporally("<=", finished.Add(600*time.Second))) Expect((*commandCalls)[1].args).To(Equal([]string{ + "-o", + "BatchMode=yes", "-o", "StrictHostKeyChecking=no", "-o", @@ -290,6 +310,41 @@ var _ = Describe("backup history standby sync", func() { Expect(errors.Is(err, os.ErrNotExist)).To(BeTrue()) }) + It("returns the rsync stage, configured seconds, and DeadlineExceeded without waiting", func() { + tmpDir := GinkgoT().TempDir() + primaryDataDir := filepath.Join(tmpDir, "primary") + standbyDataDir := filepath.Join(tmpDir, "standby") + Expect(os.Mkdir(primaryDataDir, 0o700)).To(Succeed()) + sourcePath := filepath.Join(primaryDataDir, backupHistoryDBName) + createBackupHistoryStandbySyncSQLiteDB(sourcePath) + globalFPInfo = backupfilepath.FilePathInfo{SegDirMap: map[int]string{-1: primaryDataDir}} + mock := setupBackupHistoryStandbySyncConnection() + expectBackupHistoryStandbySyncStandby(mock, "sdw-standby", standbyDataDir) + Expect(cmdFlags.Set(options.HISTORY_SYNC_STANDBY_TIMEOUT, "600")).To(Succeed()) + backupHistoryStandbySyncContextWithTimeout = func(parent context.Context, timeout time.Duration) (context.Context, context.CancelFunc) { + Expect(timeout).To(Equal(600 * time.Second)) + return context.WithDeadline(parent, time.Now().Add(-time.Second)) + } + commandCalls := setBackupHistoryStandbySyncCommands([]backupHistoryStandbySyncCommandResponse{{}, {}}) + start := time.Now() + + _, err := syncBackupHistoryToStandby() + finished := time.Now() + + Expect(err).To(HaveOccurred()) + Expect(errors.Is(err, context.DeadlineExceeded)).To(BeTrue()) + Expect(err.Error()).To(ContainSubstring("rsync standby history snapshot")) + Expect(err.Error()).To(ContainSubstring("timed out after 600 seconds")) + Expect(*commandCalls).To(HaveLen(2)) + Expect((*commandCalls)[0].ctxErr).To(Equal(context.DeadlineExceeded)) + Expect((*commandCalls)[1].ctx).ToNot(BeIdenticalTo((*commandCalls)[0].ctx)) + Expect((*commandCalls)[1].ctxErr).ToNot(HaveOccurred()) + Expect((*commandCalls)[1].hasDeadline).To(BeTrue()) + Expect((*commandCalls)[1].deadline).To(BeTemporally(">=", start.Add(backupHistoryStandbySyncCleanupTimeout))) + Expect((*commandCalls)[1].deadline).To(BeTemporally("<=", finished.Add(backupHistoryStandbySyncCleanupTimeout))) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + }) + It("releases the source lock after transport errors", func() { tmpDir := GinkgoT().TempDir() primaryDataDir := filepath.Join(tmpDir, "primary") @@ -347,6 +402,7 @@ var _ = Describe("backup history standby sync", func() { It("passes rsync paths as arguments and quotes remote shell paths", func() { remoteTempPath := "/data dir/standby's/.gpbackup_history.db.tmp" destPath := "/data dir/standby's/gpbackup_history.db" + Expect(backupHistoryStandbySyncSSHOptions).To(ContainSubstring("BatchMode=yes")) Expect(buildBackupHistoryStandbySyncRsyncArgs("/tmp/snapshot", "sdw-standby", "gpadmin", remoteTempPath)).To(Equal([]string{ "-p", @@ -365,19 +421,29 @@ var _ = Describe("backup history standby sync", func() { }) It("chains remote cleanup errors onto the primary transport error", func() { + cleanupCommandErr := fmt.Errorf("cleanup timeout: %w", context.DeadlineExceeded) commandCalls := setBackupHistoryStandbySyncCommands([]backupHistoryStandbySyncCommandResponse{ - {output: []byte("cleanup failed"), err: errors.New("exit status 255")}, + {output: []byte("cleanup failed"), err: cleanupCommandErr}, }) primaryErr := errors.New("install failed") + start := time.Now() err := cleanupBackupHistoryStandbySyncRemoteTempAfterError(primaryErr, "sdw-standby", "gpadmin", "/standby/.tmp") + finished := time.Now() Expect(err).To(HaveOccurred()) + Expect(errors.Is(err, primaryErr)).To(BeTrue()) + Expect(errors.Is(err, cleanupCommandErr)).To(BeTrue()) + Expect(errors.Is(err, context.DeadlineExceeded)).To(BeTrue()) Expect(err.Error()).To(ContainSubstring("install failed")) Expect(err.Error()).To(ContainSubstring("additionally failed to clean up remote temp file")) Expect(err.Error()).To(ContainSubstring("cleanup failed")) Expect(*commandCalls).To(HaveLen(1)) Expect((*commandCalls)[0].name).To(Equal("ssh")) + Expect((*commandCalls)[0].ctxErr).ToNot(HaveOccurred()) + Expect((*commandCalls)[0].hasDeadline).To(BeTrue()) + Expect((*commandCalls)[0].deadline).To(BeTemporally(">=", start.Add(backupHistoryStandbySyncCleanupTimeout))) + Expect((*commandCalls)[0].deadline).To(BeTemporally("<=", finished.Add(backupHistoryStandbySyncCleanupTimeout))) }) It("logs disabled automatic sync without invoking discovery", func() { @@ -402,13 +468,14 @@ var _ = Describe("backup history standby sync", func() { DeferCleanup(gplog.SetErrorCode, originalErrorCode) gplog.SetErrorCode(0) backupHistoryStandbySync = func() (string, error) { - return "", errors.New("transport failed") + return "", fmt.Errorf("standby history sync transport timed out after 300 seconds: %w", context.DeadlineExceeded) } _, err := syncBackupHistoryToStandbyBestEffort(false) Expect(err).To(HaveOccurred()) - Expect(string(stdout.Contents())).To(ContainSubstring("History db sync to standby coordinator failed; standby history may be stale: transport failed")) + Expect(errors.Is(err, context.DeadlineExceeded)).To(BeTrue()) + Expect(string(stdout.Contents())).To(ContainSubstring("History db sync to standby coordinator failed; standby history may be stale: standby history sync transport timed out after 300 seconds")) Expect(gplog.GetErrorCode()).To(Equal(0)) }) @@ -423,7 +490,7 @@ var _ = Describe("backup history standby sync", func() { defer func() { backupHistoryStandbySync = originalBestEffort }() - backupHistoryStandbySyncCommandExec = func(name string, args ...string) backupHistoryStandbySyncCommand { + backupHistoryStandbySyncCommandExec = func(ctx context.Context, name string, args ...string) backupHistoryStandbySyncCommand { return backupHistoryStandbySyncFakeCommand{} } @@ -478,8 +545,16 @@ func expectBackupHistoryStandbySyncStandby(mock sqlmock.Sqlmock, host, dataDir s func setBackupHistoryStandbySyncCommands(responses []backupHistoryStandbySyncCommandResponse) *[]backupHistoryStandbySyncCommandCall { calls := make([]backupHistoryStandbySyncCommandCall, 0) - backupHistoryStandbySyncCommandExec = func(name string, args ...string) backupHistoryStandbySyncCommand { - calls = append(calls, backupHistoryStandbySyncCommandCall{name: name, args: append([]string{}, args...)}) + backupHistoryStandbySyncCommandExec = func(ctx context.Context, name string, args ...string) backupHistoryStandbySyncCommand { + deadline, hasDeadline := ctx.Deadline() + calls = append(calls, backupHistoryStandbySyncCommandCall{ + ctx: ctx, + ctxErr: ctx.Err(), + deadline: deadline, + hasDeadline: hasDeadline, + name: name, + args: append([]string{}, args...), + }) response := backupHistoryStandbySyncCommandResponse{} if len(calls) <= len(responses) { response = responses[len(calls)-1] diff --git a/backup/validate.go b/backup/validate.go index e3169d6f..2902d1aa 100644 --- a/backup/validate.go +++ b/backup/validate.go @@ -165,6 +165,10 @@ func validateFlagCombinations(flags *pflag.FlagSet) { } func validateFlagValues() { + timeoutSeconds := MustGetFlagInt(options.HISTORY_SYNC_STANDBY_TIMEOUT) + if timeoutSeconds <= 0 || timeoutSeconds > maxHistorySyncStandbyTimeoutSeconds { + gplog.Fatal(errors.Errorf("--%s must be between 1 and %d seconds", options.HISTORY_SYNC_STANDBY_TIMEOUT, maxHistorySyncStandbyTimeoutSeconds), "") + } err := utils.ValidateFullPath(MustGetFlagString(options.BACKUP_DIR)) gplog.FatalOnError(err) err = utils.ValidateFullPath(MustGetFlagString(options.PLUGIN_CONFIG)) diff --git a/backup/validate_test.go b/backup/validate_test.go index 29e200c8..82678257 100644 --- a/backup/validate_test.go +++ b/backup/validate_test.go @@ -215,6 +215,10 @@ var _ = Describe("backup/validate tests", func() { } }, Entry("--backup-dir combo", "--backup-dir /tmp --plugin-config /tmp/config", false), + Entry("standby history sync timeout must be positive", "--history-sync-standby-timeout 0", false), + Entry("standby history sync timeout must not be negative", "--history-sync-standby-timeout -1", false), + Entry("standby history sync timeout accepts one day", "--history-sync-standby-timeout 86400", true), + Entry("standby history sync timeout must not exceed one day", "--history-sync-standby-timeout 86401", false), /* * Below are all the different filter combinations diff --git a/gpbackman/COMMANDS.md b/gpbackman/COMMANDS.md index 4a031c90..6c8fc108 100644 --- a/gpbackman/COMMANDS.md +++ b/gpbackman/COMMANDS.md @@ -47,6 +47,8 @@ The source must resolve to the cluster history database at ` %s:%s", snapshotPath, standbyHost, remoteTempPath) - output, err := execCombinedOutputCommand("rsync", args...).CombinedOutput() + output, err := execCombinedOutputCommand(ctx, "rsync", args...).CombinedOutput() + if ctxErr := ctx.Err(); ctxErr != nil { + err = ctxErr + } if err != nil { return fmt.Errorf("rsync standby history snapshot to %s:%s failed: %w%s", standbyHost, remoteTempPath, err, formatHistoryStandbySyncCommandOutput(output)) } @@ -401,10 +420,10 @@ func buildHistoryStandbySyncRsyncArgs(snapshotPath, standbyHost, userName, remot } } -func installHistoryStandbySyncSnapshotOnStandby(target *historyStandbySyncTarget, userName, remoteTempPath string) error { +func installHistoryStandbySyncSnapshotOnStandby(ctx context.Context, target *historyStandbySyncTarget, userName, remoteTempPath string) error { command := buildHistoryStandbySyncRemoteInstallCommand(remoteTempPath, target.standbyHistoryDBPath) gplog.Debug("Install history db snapshot on standby coordinator: %s:%s", target.standbyHost, target.standbyHistoryDBPath) - output, err := historyStandbySyncRunSSHCommand(command, target.standbyHost, userName) + output, err := historyStandbySyncRunSSHCommand(ctx, command, target.standbyHost, userName) if err != nil { return fmt.Errorf("install standby history snapshot on %s:%s failed: %w%s", target.standbyHost, target.standbyHistoryDBPath, err, formatHistoryStandbySyncCommandOutput(output)) } @@ -428,16 +447,19 @@ func buildHistoryStandbySyncRemoteInstallCommand(remoteTempPath, standbyHistoryD } func cleanupHistoryStandbySyncRemoteTempAfterError(primaryErr error, standbyHost, userName, remoteTempPath string) error { - if cleanupErr := cleanupHistoryStandbySyncRemoteTemp(standbyHost, userName, remoteTempPath); cleanupErr != nil { - return fmt.Errorf("%w; additionally failed to clean up remote temp file: %v", primaryErr, cleanupErr) + cleanupCtx, cancel := context.WithTimeout(context.Background(), historyStandbySyncCleanupTimeout) + defer cancel() + + if cleanupErr := cleanupHistoryStandbySyncRemoteTemp(cleanupCtx, standbyHost, userName, remoteTempPath); cleanupErr != nil { + return fmt.Errorf("%w; additionally failed to clean up remote temp file: %w", primaryErr, cleanupErr) } return primaryErr } -func cleanupHistoryStandbySyncRemoteTemp(standbyHost, userName, remoteTempPath string) error { +func cleanupHistoryStandbySyncRemoteTemp(ctx context.Context, standbyHost, userName, remoteTempPath string) error { command := buildHistoryStandbySyncRemoteCleanupCommand(remoteTempPath) gplog.Debug("Clean up remote standby history sync temp file: %s:%s", standbyHost, remoteTempPath) - output, err := historyStandbySyncRunSSHCommand(command, standbyHost, userName) + output, err := historyStandbySyncRunSSHCommand(ctx, command, standbyHost, userName) if err != nil { return fmt.Errorf("clean up remote standby history sync temp file %s:%s failed: %w%s", standbyHost, remoteTempPath, err, formatHistoryStandbySyncCommandOutput(output)) } @@ -448,16 +470,23 @@ func buildHistoryStandbySyncRemoteCleanupCommand(remoteTempPath string) string { return fmt.Sprintf("rm -f -- %s", shellQuoteHistoryStandbySyncPath(remoteTempPath)) } -func runHistoryStandbySyncSSHCommand(remoteCommand, standbyHost, userName string) ([]byte, error) { - return execCombinedOutputCommand( +func runHistoryStandbySyncSSHCommand(ctx context.Context, remoteCommand, standbyHost, userName string) ([]byte, error) { + output, err := execCombinedOutputCommand( + ctx, "ssh", "-o", + "BatchMode=yes", + "-o", "StrictHostKeyChecking=no", "-o", "ConnectTimeout=30", fmt.Sprintf("%s@%s", userName, standbyHost), remoteCommand, ).CombinedOutput() + if ctxErr := ctx.Err(); ctxErr != nil { + return output, ctxErr + } + return output, err } func historyStandbySyncSQLiteURI(dbPath, mode string) string { diff --git a/gpbackman/cmd/history_standby_sync_test.go b/gpbackman/cmd/history_standby_sync_test.go index b9b05fbd..ad724f8b 100644 --- a/gpbackman/cmd/history_standby_sync_test.go +++ b/gpbackman/cmd/history_standby_sync_test.go @@ -20,6 +20,7 @@ under the License. package cmd import ( + "context" "database/sql" "errors" "fmt" @@ -43,8 +44,12 @@ const ( ) type historyStandbySyncCommandCall struct { - name string - args []string + ctx context.Context + ctxErr error + deadline time.Time + hasDeadline bool + name string + args []string } type historyStandbySyncCommandResponse struct { @@ -71,8 +76,10 @@ var _ = Describe("history standby sync", func() { originalNow func() time.Time originalPID func() int originalCurrentUser func() (string, error) - originalRunSSHCommand func(string, string, string) ([]byte, error) - originalExecCombinedOutputCommand func(string, ...string) combinedOutputCommand + originalRunSSHCommand func(context.Context, string, string, string) ([]byte, error) + originalExecCombinedOutputCommand func(context.Context, string, ...string) combinedOutputCommand + originalContextWithTimeout func(context.Context, time.Duration) (context.Context, context.CancelFunc) + originalTimeoutSeconds int savedRootHistoryDB string savedRootAutoLoadHistoryDB bool savedHistoryStandbySyncEnvironment map[string]string @@ -91,6 +98,8 @@ var _ = Describe("history standby sync", func() { originalCurrentUser = historyStandbySyncCurrentUser originalRunSSHCommand = historyStandbySyncRunSSHCommand originalExecCombinedOutputCommand = execCombinedOutputCommand + originalContextWithTimeout = historyStandbySyncContextWithTimeout + originalTimeoutSeconds = historyStandbySyncTimeoutSeconds savedRootHistoryDB = rootHistoryDB savedRootAutoLoadHistoryDB = rootAutoLoadHistoryDB savedHistoryStandbySyncEnvironment = make(map[string]string) @@ -121,7 +130,9 @@ var _ = Describe("history standby sync", func() { return "gpadmin", nil } historyStandbySyncRunSSHCommand = runHistoryStandbySyncSSHCommand - execCombinedOutputCommand = func(name string, args ...string) combinedOutputCommand { + historyStandbySyncTimeoutSeconds = historySyncStandbyTimeoutDefault + historyStandbySyncContextWithTimeout = context.WithTimeout + execCombinedOutputCommand = func(ctx context.Context, name string, args ...string) combinedOutputCommand { return historyStandbySyncFakeCommand{} } }) @@ -137,6 +148,8 @@ var _ = Describe("history standby sync", func() { historyStandbySyncCurrentUser = originalCurrentUser historyStandbySyncRunSSHCommand = originalRunSSHCommand execCombinedOutputCommand = originalExecCombinedOutputCommand + historyStandbySyncContextWithTimeout = originalContextWithTimeout + historyStandbySyncTimeoutSeconds = originalTimeoutSeconds rootHistoryDB = savedRootHistoryDB rootAutoLoadHistoryDB = savedRootAutoLoadHistoryDB for name, value := range savedHistoryStandbySyncEnvironment { @@ -387,8 +400,11 @@ var _ = Describe("history standby sync", func() { return snapshotDir, nil } commandCalls := setHistoryStandbySyncCommands([]historyStandbySyncCommandResponse{{}, {}}) + historyStandbySyncTimeoutSeconds = 600 + start := time.Now() result := syncHistoryStandby() + finished := time.Now() Expect(result.err).ToNot(HaveOccurred()) Expect(result.skipReason).To(BeEmpty()) @@ -399,7 +415,14 @@ var _ = Describe("history standby sync", func() { Expect((*commandCalls)[0].name).To(Equal("rsync")) Expect((*commandCalls)[0].args).To(Equal(buildHistoryStandbySyncRsyncArgs(snapshotPath, "sdw-standby", "gpadmin", remoteTempPath))) Expect((*commandCalls)[1].name).To(Equal("ssh")) + Expect((*commandCalls)[1].ctx).To(BeIdenticalTo((*commandCalls)[0].ctx)) + deadline, ok := (*commandCalls)[0].ctx.Deadline() + Expect(ok).To(BeTrue()) + Expect(deadline).To(BeTemporally(">=", start.Add(600*time.Second))) + Expect(deadline).To(BeTemporally("<=", finished.Add(600*time.Second))) Expect((*commandCalls)[1].args).To(Equal([]string{ + "-o", + "BatchMode=yes", "-o", "StrictHostKeyChecking=no", "-o", @@ -411,6 +434,40 @@ var _ = Describe("history standby sync", func() { Expect(errors.Is(err, os.ErrNotExist)).To(BeTrue()) }) + It("returns the rsync stage, configured seconds, and DeadlineExceeded without waiting", func() { + tmpDir := GinkgoT().TempDir() + primaryDataDir := filepath.Join(tmpDir, "primary") + standbyDataDir := filepath.Join(tmpDir, "standby") + Expect(os.Mkdir(primaryDataDir, 0o700)).To(Succeed()) + sourceDBPath := filepath.Join(primaryDataDir, historyDBNameConst) + createHistoryStandbySyncSQLiteDB(sourceDBPath) + rootHistoryDB = sourceDBPath + mock := setupHistoryStandbySyncClusterConn(primaryDataDir, standbyDataDir, nil, true) + historyStandbySyncTimeoutSeconds = 600 + historyStandbySyncContextWithTimeout = func(parent context.Context, timeout time.Duration) (context.Context, context.CancelFunc) { + Expect(timeout).To(Equal(600 * time.Second)) + return context.WithDeadline(parent, time.Now().Add(-time.Second)) + } + commandCalls := setHistoryStandbySyncCommands([]historyStandbySyncCommandResponse{{}, {}}) + start := time.Now() + + result := syncHistoryStandby() + finished := time.Now() + + Expect(result.err).To(HaveOccurred()) + Expect(errors.Is(result.err, context.DeadlineExceeded)).To(BeTrue()) + Expect(result.err.Error()).To(ContainSubstring("rsync standby history snapshot")) + Expect(result.err.Error()).To(ContainSubstring("timed out after 600 seconds")) + Expect(*commandCalls).To(HaveLen(2)) + Expect((*commandCalls)[0].ctxErr).To(Equal(context.DeadlineExceeded)) + Expect((*commandCalls)[1].ctx).ToNot(BeIdenticalTo((*commandCalls)[0].ctx)) + Expect((*commandCalls)[1].ctxErr).ToNot(HaveOccurred()) + Expect((*commandCalls)[1].hasDeadline).To(BeTrue()) + Expect((*commandCalls)[1].deadline).To(BeTemporally(">=", start.Add(historyStandbySyncCleanupTimeout))) + Expect((*commandCalls)[1].deadline).To(BeTemporally("<=", finished.Add(historyStandbySyncCleanupTimeout))) + Expect(mock.ExpectationsWereMet()).To(Succeed()) + }) + It("uses the default cluster discovery connection for PGDATABASE resolution", func() { tmpDir := GinkgoT().TempDir() primaryDataDir := filepath.Join(tmpDir, "primary") @@ -490,6 +547,7 @@ var _ = Describe("history standby sync", func() { It("passes rsync paths as arguments and quotes remote shell paths", func() { remoteTempPath := "/data dir/standby's/.gpbackup_history.db.tmp" destPath := "/data dir/standby's/gpbackup_history.db" + Expect(historyStandbySyncSSHOptions).To(ContainSubstring("BatchMode=yes")) Expect(buildHistoryStandbySyncRsyncArgs("/tmp/snapshot", "sdw-standby", "gpadmin", remoteTempPath)).To(Equal([]string{ "-p", @@ -530,24 +588,40 @@ var _ = Describe("history standby sync", func() { Expect(*commandCalls).To(HaveLen(3)) Expect((*commandCalls)[2].name).To(Equal("ssh")) Expect((*commandCalls)[2].args[len((*commandCalls)[2].args)-1]).To(HavePrefix("rm -f -- ")) + Expect((*commandCalls)[2].ctx).ToNot(BeIdenticalTo((*commandCalls)[1].ctx)) + Expect((*commandCalls)[2].ctxErr).ToNot(HaveOccurred()) + Expect((*commandCalls)[2].hasDeadline).To(BeTrue()) Expect(mock.ExpectationsWereMet()).To(Succeed()) }) It("chains remote cleanup errors onto the primary transport error", func() { - historyStandbySyncRunSSHCommand = func(remoteCommand, standbyHost, userName string) ([]byte, error) { + cleanupCommandErr := fmt.Errorf("cleanup timeout: %w", context.DeadlineExceeded) + var cleanupDeadline time.Time + historyStandbySyncRunSSHCommand = func(ctx context.Context, remoteCommand, standbyHost, userName string) ([]byte, error) { + Expect(ctx.Err()).ToNot(HaveOccurred()) + var ok bool + cleanupDeadline, ok = ctx.Deadline() + Expect(ok).To(BeTrue()) Expect(remoteCommand).To(Equal(buildHistoryStandbySyncRemoteCleanupCommand("/standby/.tmp"))) Expect(standbyHost).To(Equal("sdw-standby")) Expect(userName).To(Equal("gpadmin")) - return []byte("cleanup failed"), errors.New("exit status 255") + return []byte("cleanup failed"), cleanupCommandErr } primaryErr := errors.New("install failed") + start := time.Now() err := cleanupHistoryStandbySyncRemoteTempAfterError(primaryErr, "sdw-standby", "gpadmin", "/standby/.tmp") + finished := time.Now() Expect(err).To(HaveOccurred()) + Expect(errors.Is(err, primaryErr)).To(BeTrue()) + Expect(errors.Is(err, cleanupCommandErr)).To(BeTrue()) + Expect(errors.Is(err, context.DeadlineExceeded)).To(BeTrue()) Expect(err.Error()).To(ContainSubstring("install failed")) Expect(err.Error()).To(ContainSubstring("additionally failed to clean up remote temp file")) Expect(err.Error()).To(ContainSubstring("cleanup failed")) + Expect(cleanupDeadline).To(BeTemporally(">=", start.Add(historyStandbySyncCleanupTimeout))) + Expect(cleanupDeadline).To(BeTemporally("<=", finished.Add(historyStandbySyncCleanupTimeout))) }) It("keeps automatic sync best-effort while strict sync treats skips as errors", func() { @@ -637,8 +711,16 @@ func setupHistoryStandbySyncClusterConnWithHook( func setHistoryStandbySyncCommands(responses []historyStandbySyncCommandResponse) *[]historyStandbySyncCommandCall { calls := make([]historyStandbySyncCommandCall, 0) - execCombinedOutputCommand = func(name string, args ...string) combinedOutputCommand { - calls = append(calls, historyStandbySyncCommandCall{name: name, args: append([]string{}, args...)}) + execCombinedOutputCommand = func(ctx context.Context, name string, args ...string) combinedOutputCommand { + deadline, hasDeadline := ctx.Deadline() + calls = append(calls, historyStandbySyncCommandCall{ + ctx: ctx, + ctxErr: ctx.Err(), + deadline: deadline, + hasDeadline: hasDeadline, + name: name, + args: append([]string{}, args...), + }) response := historyStandbySyncCommandResponse{} if len(calls) <= len(responses) { response = responses[len(calls)-1] diff --git a/gpbackman/cmd/history_sync.go b/gpbackman/cmd/history_sync.go index 2efcd399..dbbf14d3 100644 --- a/gpbackman/cmd/history_sync.go +++ b/gpbackman/cmd/history_sync.go @@ -43,6 +43,12 @@ only after the standby file is replaced atomically with a verified snapshot.`, func init() { rootCmd.AddCommand(historySyncCmd) + historySyncCmd.Flags().IntVar( + &historyStandbySyncTimeoutSeconds, + historySyncStandbyTimeoutFlagName, + historySyncStandbyTimeoutDefault, + "shared rsync and remote install timeout in seconds; must be an integer between 1 and 86400", + ) } func doHistorySync() { diff --git a/gpbackman/cmd/history_sync_test.go b/gpbackman/cmd/history_sync_test.go index 2c7f6910..acebdda3 100644 --- a/gpbackman/cmd/history_sync_test.go +++ b/gpbackman/cmd/history_sync_test.go @@ -21,7 +21,9 @@ package cmd import ( "bytes" + "context" "errors" + "fmt" "os" "github.com/apache/cloudberry-go-libs/testhelper" @@ -70,10 +72,53 @@ var _ = Describe("history sync command", func() { Expect(rootCmd.PersistentFlags().Lookup(noHistorySyncStandbyFlagName)).To(BeNil()) }) - It("keeps history-sync strict with inherited global flags and no local flags", func() { + It("registers history-sync-standby-timeout only on sync-capable commands", func() { + syncCommands := map[string]bool{ + "history-sync": true, + "backup-delete": true, + "backup-clean": true, + "history-clean": true, + } + + for _, command := range rootCmd.Commands() { + flag := command.Flags().Lookup(historySyncStandbyTimeoutFlagName) + if syncCommands[command.Name()] { + Expect(flag).ToNot(BeNil(), command.Name()) + Expect(flag.DefValue).To(Equal("300"), command.Name()) + continue + } + Expect(flag).To(BeNil(), command.Name()) + } + Expect(rootCmd.PersistentFlags().Lookup(historySyncStandbyTimeoutFlagName)).To(BeNil()) + }) + + DescribeTable("rejects non-integer timeout values", + func(value string) { + flag := historySyncCmd.Flags().Lookup(historySyncStandbyTimeoutFlagName) + Expect(flag).ToNot(BeNil()) + Expect(flag.Value.Set(value)).ToNot(Succeed()) + }, + Entry("fractional", "1.5"), + Entry("duration", "5m"), + ) + + It("accepts a custom timeout in seconds", func() { + originalTimeout := historyStandbySyncTimeoutSeconds + flag := historySyncCmd.Flags().Lookup(historySyncStandbyTimeoutFlagName) + originalChanged := flag.Changed + DeferCleanup(func() { + historyStandbySyncTimeoutSeconds = originalTimeout + flag.Changed = originalChanged + }) + + Expect(historySyncCmd.Flags().Set(historySyncStandbyTimeoutFlagName, "600")).To(Succeed()) + Expect(historyStandbySyncTimeoutSeconds).To(Equal(600)) + }) + + It("keeps history-sync strict with inherited global flags and its timeout flag", func() { Expect(commandByName("history-sync")).To(Equal(historySyncCmd)) Expect(historySyncCmd.Args(historySyncCmd, []string{"unexpected"})).To(HaveOccurred()) - Expect(flagNames(historySyncCmd.LocalFlags())).To(BeEmpty()) + Expect(flagNames(historySyncCmd.LocalFlags())).To(Equal([]string{historySyncStandbyTimeoutFlagName})) Expect(historySyncCmd.Flags().Lookup(noHistorySyncStandbyFlagName)).To(BeNil()) for _, flagName := range []string{ historyDBFlagName, @@ -136,6 +181,22 @@ var _ = Describe("history sync command", func() { Expect(exitCodes).To(BeEmpty()) }) + DescribeTable("rejects a standby history sync timeout outside the supported range", func(timeoutSeconds int) { + originalTimeout := historyStandbySyncTimeoutSeconds + DeferCleanup(func() { + historyStandbySyncTimeoutSeconds = originalTimeout + }) + historyStandbySyncTimeoutSeconds = timeoutSeconds + + doRootFlagValidation(historySyncCmd.Flags(), false) + + Expect(exitCodes).To(Equal([]int{exitErrorCode})) + }, + Entry("zero", 0), + Entry("negative", -1), + Entry("more than one day", 86401), + ) + It("treats the default working-directory source as a strict error", func() { stdout, stderr, _ := testhelper.SetupTestLogger() historyStandbySync = syncHistoryStandby @@ -175,6 +236,11 @@ var _ = Describe("history sync command", func() { result: historyStandbySyncResult{err: errors.New("validate standby history sync snapshot quick_check failed")}, want: "validate standby history sync snapshot quick_check failed", }, + { + name: "transport timeout", + result: historyStandbySyncResult{err: fmt.Errorf("rsync standby history snapshot timed out after 300 seconds: %w", context.DeadlineExceeded)}, + want: "rsync standby history snapshot timed out after 300 seconds", + }, } for _, tt := range tests { diff --git a/gpbackman/cmd/root.go b/gpbackman/cmd/root.go index 79156cea..b49ddceb 100644 --- a/gpbackman/cmd/root.go +++ b/gpbackman/cmd/root.go @@ -21,6 +21,7 @@ package cmd import ( "fmt" + "strconv" "github.com/apache/cloudberry-backup/gpbackman/gpbckpconfig" "github.com/apache/cloudberry-backup/gpbackman/textmsg" @@ -92,6 +93,16 @@ func getVersion() string { // These flag checks are applied for all commands: func doRootFlagValidation(flags *pflag.FlagSet, checkFileExists bool) { var err error + if flags.Lookup(historySyncStandbyTimeoutFlagName) != nil { + timeoutSeconds, timeoutErr := flags.GetInt(historySyncStandbyTimeoutFlagName) + if timeoutErr == nil && (timeoutSeconds <= 0 || timeoutSeconds > historySyncStandbyTimeoutMax) { + timeoutErr = fmt.Errorf("must be between 1 and %d seconds", historySyncStandbyTimeoutMax) + } + if timeoutErr != nil { + gplog.Error("%s", textmsg.ErrorTextUnableValidateFlag(strconv.Itoa(timeoutSeconds), historySyncStandbyTimeoutFlagName, timeoutErr)) + execOSExit(exitErrorCode) + } + } // If history-db flag is specified and full path. // The existence of the file is checked by condition from each specific command. // Not all commands require a history db file to exist. diff --git a/gpbackman/cmd/wrappers.go b/gpbackman/cmd/wrappers.go index a3c66425..9086c6ad 100644 --- a/gpbackman/cmd/wrappers.go +++ b/gpbackman/cmd/wrappers.go @@ -20,6 +20,7 @@ under the License. package cmd import ( + "context" "database/sql" "fmt" "os" @@ -41,8 +42,8 @@ type combinedOutputCommand interface { CombinedOutput() ([]byte, error) } -var execCombinedOutputCommand = func(name string, args ...string) combinedOutputCommand { - return exec.Command(name, args...) +var execCombinedOutputCommand = func(ctx context.Context, name string, args ...string) combinedOutputCommand { + return exec.CommandContext(ctx, name, args...) } func logHeadersDebug() { diff --git a/options/flag.go b/options/flag.go index f5e2083b..f79a23a5 100644 --- a/options/flag.go +++ b/options/flag.go @@ -14,47 +14,49 @@ import ( ) const ( - BACKUP_DIR = "backup-dir" - COMPRESSION_TYPE = "compression-type" - COMPRESSION_LEVEL = "compression-level" - DATA_ONLY = "data-only" - DBNAME = "dbname" - DEBUG = "debug" - EXCLUDE_RELATION = "exclude-table" - EXCLUDE_RELATION_FILE = "exclude-table-file" - EXCLUDE_SCHEMA = "exclude-schema" - EXCLUDE_SCHEMA_FILE = "exclude-schema-file" - FROM_TIMESTAMP = "from-timestamp" - INCLUDE_RELATION = "include-table" - INCLUDE_RELATION_FILE = "include-table-file" - INCLUDE_SCHEMA = "include-schema" - INCLUDE_SCHEMA_FILE = "include-schema-file" - INCREMENTAL = "incremental" - JOBS = "jobs" - LEAF_PARTITION_DATA = "leaf-partition-data" - METADATA_ONLY = "metadata-only" - NO_COMPRESSION = "no-compression" - NO_HISTORY = "no-history" - NO_HISTORY_SYNC_STANDBY = "no-history-sync-standby" - PLUGIN_CONFIG = "plugin-config" - QUIET = "quiet" - SINGLE_DATA_FILE = "single-data-file" - COPY_QUEUE_SIZE = "copy-queue-size" - VERBOSE = "verbose" - WITH_STATS = "with-stats" - CREATE_DB = "create-db" - ON_ERROR_CONTINUE = "on-error-continue" - REDIRECT_DB = "redirect-db" - RUN_ANALYZE = "run-analyze" - SINGLE_BACKUP_DIR = "single-backup-dir" - TIMESTAMP = "timestamp" - WITH_GLOBALS = "with-globals" - REDIRECT_SCHEMA = "redirect-schema" - TRUNCATE_TABLE = "truncate-table" - WITHOUT_GLOBALS = "without-globals" - RESIZE_CLUSTER = "resize-cluster" - NO_INHERITS = "no-inherits" - REPORT_DIR = "report-dir" + BACKUP_DIR = "backup-dir" + COMPRESSION_TYPE = "compression-type" + COMPRESSION_LEVEL = "compression-level" + DATA_ONLY = "data-only" + DBNAME = "dbname" + DEBUG = "debug" + EXCLUDE_RELATION = "exclude-table" + EXCLUDE_RELATION_FILE = "exclude-table-file" + EXCLUDE_SCHEMA = "exclude-schema" + EXCLUDE_SCHEMA_FILE = "exclude-schema-file" + FROM_TIMESTAMP = "from-timestamp" + HISTORY_SYNC_STANDBY_TIMEOUT = "history-sync-standby-timeout" + INCLUDE_RELATION = "include-table" + INCLUDE_RELATION_FILE = "include-table-file" + INCLUDE_SCHEMA = "include-schema" + INCLUDE_SCHEMA_FILE = "include-schema-file" + INCREMENTAL = "incremental" + JOBS = "jobs" + LEAF_PARTITION_DATA = "leaf-partition-data" + METADATA_ONLY = "metadata-only" + NO_COMPRESSION = "no-compression" + NO_HISTORY = "no-history" + NO_HISTORY_SYNC_STANDBY = "no-history-sync-standby" + PLUGIN_CONFIG = "plugin-config" + QUIET = "quiet" + SINGLE_DATA_FILE = "single-data-file" + COPY_QUEUE_SIZE = "copy-queue-size" + VERBOSE = "verbose" + WITH_STATS = "with-stats" + CREATE_DB = "create-db" + ON_ERROR_CONTINUE = "on-error-continue" + REDIRECT_DB = "redirect-db" + RUN_ANALYZE = "run-analyze" + SINGLE_BACKUP_DIR = "single-backup-dir" + TIMESTAMP = "timestamp" + WITH_GLOBALS = "with-globals" + REDIRECT_SCHEMA = "redirect-schema" + TRUNCATE_TABLE = "truncate-table" + WITHOUT_GLOBALS = "without-globals" + RESIZE_CLUSTER = "resize-cluster" + NO_INHERITS = "no-inherits" + REPORT_DIR = "report-dir" + DEFAULT_HISTORY_SYNC_STANDBY_TIMEOUT = 300 ) func SetBackupFlagDefaults(flagSet *pflag.FlagSet) { @@ -75,6 +77,7 @@ func SetBackupFlagDefaults(flagSet *pflag.FlagSet) { flagSet.StringArray(INCLUDE_RELATION, []string{}, "Back up only the specified table(s). --include-table can be specified multiple times.") flagSet.String(INCLUDE_RELATION_FILE, "", "A file containing a list of fully-qualified tables to be included in the backup") flagSet.Bool(INCREMENTAL, false, "Only back up data for AO tables that have been modified since the last backup") + flagSet.Int(HISTORY_SYNC_STANDBY_TIMEOUT, DEFAULT_HISTORY_SYNC_STANDBY_TIMEOUT, "Shared rsync and remote install timeout in seconds; must be an integer between 1 and 86400") flagSet.Int(JOBS, 1, "The number of parallel connections to use when backing up data") flagSet.Bool(LEAF_PARTITION_DATA, false, "For partition tables, create one data file per leaf partition instead of one data file for the whole table") flagSet.Bool(METADATA_ONLY, false, "Only back up metadata, do not back up data") diff --git a/options/flag_test.go b/options/flag_test.go index 456f52d4..57e51995 100644 --- a/options/flag_test.go +++ b/options/flag_test.go @@ -58,6 +58,39 @@ var _ = Describe("utils/flag tests", func() { }) }) Context("SetBackupFlagDefaults", func() { + It("registers history-sync-standby-timeout with a 300 second default", func() { + flagSet := pflag.NewFlagSet("gpbackup", pflag.ContinueOnError) + options.SetBackupFlagDefaults(flagSet) + + flag := flagSet.Lookup(options.HISTORY_SYNC_STANDBY_TIMEOUT) + Expect(flag).ToNot(BeNil()) + Expect(flag.DefValue).To(Equal("300")) + value, err := flagSet.GetInt(options.HISTORY_SYNC_STANDBY_TIMEOUT) + Expect(err).ToNot(HaveOccurred()) + Expect(value).To(Equal(300)) + }) + + It("accepts a custom standby history sync timeout in seconds", func() { + flagSet := pflag.NewFlagSet("gpbackup", pflag.ContinueOnError) + options.SetBackupFlagDefaults(flagSet) + + Expect(flagSet.Parse([]string{"--" + options.HISTORY_SYNC_STANDBY_TIMEOUT, "600"})).To(Succeed()) + value, err := flagSet.GetInt(options.HISTORY_SYNC_STANDBY_TIMEOUT) + Expect(err).ToNot(HaveOccurred()) + Expect(value).To(Equal(600)) + }) + + DescribeTable("rejects non-integer standby history sync timeout values", + func(value string) { + flagSet := pflag.NewFlagSet("gpbackup", pflag.ContinueOnError) + options.SetBackupFlagDefaults(flagSet) + + Expect(flagSet.Parse([]string{"--" + options.HISTORY_SYNC_STANDBY_TIMEOUT, value})).ToNot(Succeed()) + }, + Entry("fractional", "1.5"), + Entry("duration", "5m"), + ) + It("registers no-history-sync-standby for gpbackup with a false default", func() { flagSet := pflag.NewFlagSet("gpbackup", pflag.ContinueOnError) options.SetBackupFlagDefaults(flagSet) @@ -74,6 +107,7 @@ var _ = Describe("utils/flag tests", func() { options.SetRestoreFlagDefaults(flagSet) Expect(flagSet.Lookup(options.NO_HISTORY_SYNC_STANDBY)).To(BeNil()) + Expect(flagSet.Lookup(options.HISTORY_SYNC_STANDBY_TIMEOUT)).To(BeNil()) }) }) }) From c29c2a4b976ef40120331bddb880b3997cfbac7b Mon Sep 17 00:00:00 2001 From: woblerr Date: Tue, 11 Aug 2026 01:05:25 +0300 Subject: [PATCH 17/19] Protect standby history sync rsync paths. Pass remote paths with rsync's `-s` option so shell metacharacters are not interpreted by the remote shell, and document the resulting rsync 3.0.0 requirement. --- README.md | 13 +++-- backup/history_standby_sync.go | 3 +- backup/history_standby_sync_test.go | 64 +++++++++++++++++++++- gpbackman/COMMANDS.md | 2 + gpbackman/README.md | 4 ++ gpbackman/cmd/history_standby_sync.go | 1 + gpbackman/cmd/history_standby_sync_test.go | 3 +- 7 files changed, 81 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 1557fc23..db40bc23 100644 --- a/README.md +++ b/README.md @@ -121,17 +121,18 @@ The synchronization process: 1. Takes a non-waiting lock next to the canonical source database. 2. Creates a consistent SQLite snapshot with `VACUUM INTO` and accepts it only when `PRAGMA quick_check` returns `ok`. -3. Transfers the snapshot with `rsync -p` to a unique temporary file in the +3. Transfers the snapshot with `rsync -p -s` to a unique temporary file in the standby coordinator data directory. 4. Preserves the existing standby file's owner, group, and mode when it exists, then atomically renames the temporary file to `gpbackup_history.db`. -The host running `gpbackup` must have `ssh` and `rsync`, and the current OS -user must have non-interactive SSH access to the standby host. That user must -be able to create files in the standby coordinator data directory and preserve -the destination file's ownership and permissions. The cluster must expose an -up standby in `gp_segment_configuration`. +`rsync` 3.0.0 or later must be installed on both the host running `gpbackup` +and the standby coordinator. The `gpbackup` host must also have `ssh`, and the +current OS user must have non-interactive SSH access to the standby host. That +user must be able to create files in the standby coordinator data directory +and preserve the destination file's ownership and permissions. The cluster +must expose an up standby in `gp_segment_configuration`. The atomic rename prevents readers from observing a partially copied database, but it is not a failover coordination mechanism. A coordinator role change diff --git a/backup/history_standby_sync.go b/backup/history_standby_sync.go index 2ad27fc2..62f8f214 100644 --- a/backup/history_standby_sync.go +++ b/backup/history_standby_sync.go @@ -39,7 +39,7 @@ import ( ) const ( - backupHistoryDBName = "gpbackup_history.db" + backupHistoryDBName = "gpbackup_history.db" // Leave enough time for the 30-second SSH connection timeout and remote removal // while keeping failure cleanup bounded. backupHistoryStandbySyncCleanupTimeout = 120 * time.Second @@ -358,6 +358,7 @@ func rsyncBackupHistoryStandbySyncSnapshot(ctx context.Context, snapshotPath, st func buildBackupHistoryStandbySyncRsyncArgs(snapshotPath, standbyHost, userName, remoteTempPath string) []string { return []string{ "-p", + "-s", "-e", backupHistoryStandbySyncSSHOptions, "--", diff --git a/backup/history_standby_sync_test.go b/backup/history_standby_sync_test.go index 3ee5e2cd..c3ffbb4b 100644 --- a/backup/history_standby_sync_test.go +++ b/backup/history_standby_sync_test.go @@ -25,8 +25,10 @@ import ( "errors" "fmt" "os" + "os/exec" "path/filepath" "regexp" + "strconv" "time" "github.com/DATA-DOG/go-sqlmock" @@ -399,13 +401,14 @@ var _ = Describe("backup history standby sync", func() { Expect(mock.ExpectationsWereMet()).To(Succeed()) }) - It("passes rsync paths as arguments and quotes remote shell paths", func() { + It("protects rsync paths and quotes remote shell paths", func() { remoteTempPath := "/data dir/standby's/.gpbackup_history.db.tmp" destPath := "/data dir/standby's/gpbackup_history.db" Expect(backupHistoryStandbySyncSSHOptions).To(ContainSubstring("BatchMode=yes")) Expect(buildBackupHistoryStandbySyncRsyncArgs("/tmp/snapshot", "sdw-standby", "gpadmin", remoteTempPath)).To(Equal([]string{ "-p", + "-s", "-e", backupHistoryStandbySyncSSHOptions, "--", @@ -420,6 +423,39 @@ var _ = Describe("backup history standby sync", func() { Expect(buildBackupHistoryStandbySyncRemoteCleanupCommand(remoteTempPath)).To(Equal("rm -f -- " + shellQuoteBackupHistoryStandbySyncPath(remoteTempPath))) }) + It("keeps protected rsync paths out of the remote shell command", func() { + rsyncPath := requireBackupHistoryStandbySyncRsync3() + tmpDir := GinkgoT().TempDir() + remoteArgsPath := filepath.Join(tmpDir, "remote-args") + fakeShellPath := filepath.Join(tmpDir, "fake-ssh") + fakeShell := "#!/bin/sh\nprintf '%s\\n' \"$@\" > \"$RSYNC_REMOTE_ARGS_FILE\"\nexit 1\n" + Expect(os.WriteFile(fakeShellPath, []byte(fakeShell), 0o700)).To(Succeed()) + + snapshotPath := filepath.Join(tmpDir, "snapshot") + Expect(os.WriteFile(snapshotPath, []byte("snapshot"), 0o600)).To(Succeed()) + remoteTempPath := "/data dir/standby's/$HOME/[history]*;RSYNC_REMOTE_PATH_SENTINEL" + args := buildBackupHistoryStandbySyncRsyncArgs(snapshotPath, "sdw-standby", "gpadmin", remoteTempPath) + remoteShellReplaced := false + for i := range args { + if args[i] == "-e" && i+1 < len(args) { + args[i+1] = fakeShellPath + remoteShellReplaced = true + break + } + } + Expect(remoteShellReplaced).To(BeTrue()) + + command := exec.Command(rsyncPath, args...) + command.Env = append(os.Environ(), "RSYNC_REMOTE_ARGS_FILE="+remoteArgsPath) + _, err := command.CombinedOutput() + Expect(err).To(HaveOccurred()) + + remoteArgs, err := os.ReadFile(remoteArgsPath) + Expect(err).ToNot(HaveOccurred()) + Expect(string(remoteArgs)).To(ContainSubstring("--server")) + Expect(string(remoteArgs)).ToNot(ContainSubstring("RSYNC_REMOTE_PATH_SENTINEL")) + }) + It("chains remote cleanup errors onto the primary transport error", func() { cleanupCommandErr := fmt.Errorf("cleanup timeout: %w", context.DeadlineExceeded) commandCalls := setBackupHistoryStandbySyncCommands([]backupHistoryStandbySyncCommandResponse{ @@ -516,6 +552,32 @@ var _ = Describe("backup history standby sync", func() { }) }) +func requireBackupHistoryStandbySyncRsync3() string { + GinkgoHelper() + + rsyncPath, err := exec.LookPath("rsync") + if err != nil { + Skip("rsync is not installed") + return "" + } + output, err := exec.Command(rsyncPath, "--version").CombinedOutput() + if err != nil { + Skip(fmt.Sprintf("cannot determine rsync version: %v", err)) + return "" + } + match := regexp.MustCompile(`(?m)^rsync\s+version\s+([0-9]+)\.`).FindStringSubmatch(string(output)) + if len(match) != 2 { + Skip("cannot parse rsync version") + return "" + } + majorVersion, err := strconv.Atoi(match[1]) + if err != nil || majorVersion < 3 { + Skip("rsync 3.0.0 or later is required") + return "" + } + return rsyncPath +} + func createBackupHistoryStandbySyncSQLiteDB(path string) { db, err := sql.Open("sqlite3", backupHistoryStandbySyncSQLiteURI(path, "rwc")) Expect(err).ToNot(HaveOccurred()) diff --git a/gpbackman/COMMANDS.md b/gpbackman/COMMANDS.md index 6c8fc108..715536b9 100644 --- a/gpbackman/COMMANDS.md +++ b/gpbackman/COMMANDS.md @@ -49,6 +49,8 @@ After a successful `backup-delete`, `backup-clean`, or `history-clean`, gpBackMa The `history-sync`, `backup-delete`, `backup-clean`, and `history-clean` commands accept `--history-sync-standby-timeout SECONDS`. The default is 300 seconds. `SECONDS` must be an integer from 1 to 86400 seconds; `0`, `86401`, fractions, and duration strings such as `5m` are rejected. The 24-hour upper bound guards against accidentally oversized values; a longer timeout is not meaningful for this synchronization. This value is one shared budget for `rsync` and remote install, not a separate timeout for each command. The timeout starts only after snapshot validation and does not include standby discovery, `VACUUM INTO`, or `PRAGMA quick_check`. Remote cleanup after a transport failure uses a separate fixed timeout of 120 seconds. Read-only commands do not accept this option. +`rsync` 3.0.0 or later must be installed on both the host running gpBackMan and the standby coordinator. The current OS user must have non-interactive SSH access to the standby host. + Only `gpbackup_history.db` is synchronized. Report files, backup data, and other backup artifacts are not synchronized. # Delete all existing backups older than the specified time condition (`backup-clean`) diff --git a/gpbackman/README.md b/gpbackman/README.md index ac2bae74..2c9b4af9 100644 --- a/gpbackman/README.md +++ b/gpbackman/README.md @@ -87,6 +87,10 @@ validation. Standby discovery and SQLite snapshot creation and validation transport step fails, remote cleanup of the temporary file uses its own fixed 120-second timeout, independent of `--history-sync-standby-timeout`. +`rsync` 3.0.0 or later must be installed on both the host running gpBackMan +and the standby coordinator. The current OS user must have non-interactive SSH +access to the standby host. + Only `gpbackup_history.db` is synchronized. Report files, backup data, and other backup artifacts are not synchronized. ### Detail info about commands diff --git a/gpbackman/cmd/history_standby_sync.go b/gpbackman/cmd/history_standby_sync.go index ff4c1f3e..0f778f22 100644 --- a/gpbackman/cmd/history_standby_sync.go +++ b/gpbackman/cmd/history_standby_sync.go @@ -412,6 +412,7 @@ func rsyncHistoryStandbySyncSnapshot(ctx context.Context, snapshotPath, standbyH func buildHistoryStandbySyncRsyncArgs(snapshotPath, standbyHost, userName, remoteTempPath string) []string { return []string{ "-p", + "-s", "-e", historyStandbySyncSSHOptions, "--", diff --git a/gpbackman/cmd/history_standby_sync_test.go b/gpbackman/cmd/history_standby_sync_test.go index ad724f8b..ccd397c3 100644 --- a/gpbackman/cmd/history_standby_sync_test.go +++ b/gpbackman/cmd/history_standby_sync_test.go @@ -544,13 +544,14 @@ var _ = Describe("history standby sync", func() { Expect(mock.ExpectationsWereMet()).To(Succeed()) }) - It("passes rsync paths as arguments and quotes remote shell paths", func() { + It("protects rsync paths and quotes remote shell paths", func() { remoteTempPath := "/data dir/standby's/.gpbackup_history.db.tmp" destPath := "/data dir/standby's/gpbackup_history.db" Expect(historyStandbySyncSSHOptions).To(ContainSubstring("BatchMode=yes")) Expect(buildHistoryStandbySyncRsyncArgs("/tmp/snapshot", "sdw-standby", "gpadmin", remoteTempPath)).To(Equal([]string{ "-p", + "-s", "-e", historyStandbySyncSSHOptions, "--", From 75524346378ea32af3d2cb2aac64718fa6356597 Mon Sep 17 00:00:00 2001 From: woblerr Date: Tue, 11 Aug 2026 16:06:02 +0000 Subject: [PATCH 18/19] Guard standby history sync context error checks with command result. When rsync or ssh succeeds and the context deadline fires immediately after, the successful transfer was incorrectly treated as a timeout. Only prefer ctx.Err() when the command itself returned an error. --- gpbackman/cmd/history_standby_sync.go | 4 ++-- gpbackman/cmd/history_standby_sync_test.go | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/gpbackman/cmd/history_standby_sync.go b/gpbackman/cmd/history_standby_sync.go index 0f778f22..06589b5b 100644 --- a/gpbackman/cmd/history_standby_sync.go +++ b/gpbackman/cmd/history_standby_sync.go @@ -400,7 +400,7 @@ func rsyncHistoryStandbySyncSnapshot(ctx context.Context, snapshotPath, standbyH args := buildHistoryStandbySyncRsyncArgs(snapshotPath, standbyHost, userName, remoteTempPath) gplog.Debug("Transfer history db snapshot to standby coordinator: %s -> %s:%s", snapshotPath, standbyHost, remoteTempPath) output, err := execCombinedOutputCommand(ctx, "rsync", args...).CombinedOutput() - if ctxErr := ctx.Err(); ctxErr != nil { + if ctxErr := ctx.Err(); ctxErr != nil && err != nil { err = ctxErr } if err != nil { @@ -484,7 +484,7 @@ func runHistoryStandbySyncSSHCommand(ctx context.Context, remoteCommand, standby fmt.Sprintf("%s@%s", userName, standbyHost), remoteCommand, ).CombinedOutput() - if ctxErr := ctx.Err(); ctxErr != nil { + if ctxErr := ctx.Err(); ctxErr != nil && err != nil { return output, ctxErr } return output, err diff --git a/gpbackman/cmd/history_standby_sync_test.go b/gpbackman/cmd/history_standby_sync_test.go index ccd397c3..822063ed 100644 --- a/gpbackman/cmd/history_standby_sync_test.go +++ b/gpbackman/cmd/history_standby_sync_test.go @@ -448,7 +448,10 @@ var _ = Describe("history standby sync", func() { Expect(timeout).To(Equal(600 * time.Second)) return context.WithDeadline(parent, time.Now().Add(-time.Second)) } - commandCalls := setHistoryStandbySyncCommands([]historyStandbySyncCommandResponse{{}, {}}) + commandCalls := setHistoryStandbySyncCommands([]historyStandbySyncCommandResponse{ + {err: context.DeadlineExceeded}, + {}, + }) start := time.Now() result := syncHistoryStandby() From 0cb94c28009f406016983c7ba03a8bdca50de3c3 Mon Sep 17 00:00:00 2001 From: woblerr Date: Tue, 11 Aug 2026 23:16:56 +0300 Subject: [PATCH 19/19] Extend standby history sync context error guards to gpbackup. --- backup/history_standby_sync.go | 4 ++-- backup/history_standby_sync_test.go | 5 ++++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/backup/history_standby_sync.go b/backup/history_standby_sync.go index 62f8f214..53a7374f 100644 --- a/backup/history_standby_sync.go +++ b/backup/history_standby_sync.go @@ -346,7 +346,7 @@ func rsyncBackupHistoryStandbySyncSnapshot(ctx context.Context, snapshotPath, st args := buildBackupHistoryStandbySyncRsyncArgs(snapshotPath, standbyHost, userName, remoteTempPath) gplog.Debug("Transfer history db snapshot to standby coordinator: %s -> %s:%s", snapshotPath, standbyHost, remoteTempPath) output, err := backupHistoryStandbySyncCommandExec(ctx, "rsync", args...).CombinedOutput() - if ctxErr := ctx.Err(); ctxErr != nil { + if ctxErr := ctx.Err(); ctxErr != nil && err != nil { err = ctxErr } if err != nil { @@ -430,7 +430,7 @@ func runBackupHistoryStandbySyncSSHCommand(ctx context.Context, remoteCommand, s fmt.Sprintf("%s@%s", userName, standbyHost), remoteCommand, ).CombinedOutput() - if ctxErr := ctx.Err(); ctxErr != nil { + if ctxErr := ctx.Err(); ctxErr != nil && err != nil { return output, ctxErr } return output, err diff --git a/backup/history_standby_sync_test.go b/backup/history_standby_sync_test.go index c3ffbb4b..dc22f901 100644 --- a/backup/history_standby_sync_test.go +++ b/backup/history_standby_sync_test.go @@ -327,7 +327,10 @@ var _ = Describe("backup history standby sync", func() { Expect(timeout).To(Equal(600 * time.Second)) return context.WithDeadline(parent, time.Now().Add(-time.Second)) } - commandCalls := setBackupHistoryStandbySyncCommands([]backupHistoryStandbySyncCommandResponse{{}, {}}) + commandCalls := setBackupHistoryStandbySyncCommands([]backupHistoryStandbySyncCommandResponse{ + {err: context.DeadlineExceeded}, + {}, + }) start := time.Now() _, err := syncBackupHistoryToStandby()