From 86b9cc385bf3c6d9706bafce0edce57cf3078873 Mon Sep 17 00:00:00 2001 From: Anton Kurochkin <45575813+woblerr@users.noreply.github.com> Date: Wed, 9 Sep 2026 06:12:05 +0300 Subject: [PATCH 1/3] Update golangci-lint and resolve lint findings. (#114) * Resolve production lint findings. Mark existing best-effort cleanup and transaction errors as intentionally ignored without changing their execution order or error-handling policy. Remove unused parameters and return values, ineffective assignments, redundant conversions, and an unreachable duplicate branch. Correct misspelled source comments. * Resolve test lint findings. Remove assignments whose values are overwritten before use. Assert errors from test file writes, and correct misspelled descriptions and variable names. * Enable additional golangci-lint checks. Enable ineffassign, misspell, nakedret, SA staticcheck checks except SA1019, and unconvert with read-only module resolution. Run golangci-lint 2.12.2 through the official GitHub Action. Keep make lint limited to invoking the installed binary and document the workflow. * Install the pinned golangci-lint version with Go. Restore make lint bootstrapping without executing a downloaded installation script. --- .github/workflows/build_and_unit_test.yml | 6 +++ .golangci.yml | 12 ++++++ Makefile | 13 +++---- README.md | 7 ++-- backup/backup.go | 11 +++--- backup/data.go | 2 +- backup/dependencies.go | 2 +- backup/dependencies_test.go | 4 +- backup/incremental.go | 4 +- backup/predata_externals.go | 2 - backup/predata_externals_test.go | 2 +- backup/predata_functions.go | 2 +- backup/predata_relations.go | 4 +- backup/predata_shared.go | 2 +- backup/queries_acl.go | 2 +- backup/queries_shared.go | 2 +- backup/queries_table_defs.go | 4 +- backup/wrappers.go | 6 +-- end_to_end/end_to_end_suite_test.go | 8 ++-- filepath/filepath.go | 2 +- gpbackman/cmd/wrappers.go | 10 +++-- gpbackman/gpbckpconfig/utils_db.go | 4 +- helper/backup_helper.go | 10 ++--- helper/backup_helper_pipes.go | 4 +- helper/helper.go | 6 +-- helper/restore_helper.go | 24 +++++------- history/history.go | 37 +++++++++++-------- integration/predata_acl_queries_test.go | 6 +-- integration/predata_functions_queries_test.go | 2 +- options/options.go | 1 - options/options_test.go | 2 + plugins/s3plugin/backup.go | 12 ++++-- plugins/s3plugin/restore.go | 4 +- plugins/s3plugin/s3plugin.go | 8 ++-- restore/parallel.go | 4 +- restore/restore.go | 12 ++---- restore/validate.go | 4 +- restore/wrappers.go | 4 +- utils/agent_remote_test.go | 2 +- utils/plugin_test.go | 3 +- 40 files changed, 140 insertions(+), 116 deletions(-) diff --git a/.github/workflows/build_and_unit_test.yml b/.github/workflows/build_and_unit_test.yml index c4e64a24..cf03988e 100644 --- a/.github/workflows/build_and_unit_test.yml +++ b/.github/workflows/build_and_unit_test.yml @@ -33,6 +33,12 @@ jobs: cd ${GOPATH}/src/github.com/apache/cloudberry-backup make depend + - name: Run linters + uses: golangci/golangci-lint-action@v9 + with: + version: v2.12.2 + working-directory: go/src/github.com/apache/cloudberry-backup + - name: Build run: | cd ${GOPATH}/src/github.com/apache/cloudberry-backup diff --git a/.golangci.yml b/.golangci.yml index a00057b8..ac3492b7 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -22,7 +22,12 @@ linters: enable: - errcheck - govet + - ineffassign + - misspell + - nakedret - revive + - staticcheck + - unconvert - unparam - unused settings: @@ -31,6 +36,12 @@ linters: - shadow revive: confidence: 0.1 + misspell: + locale: US + staticcheck: + checks: + - "SA*" + - "-SA1019" exclusions: generated: lax rules: @@ -67,3 +78,4 @@ linters: run: timeout: 5m + modules-download-mode: readonly diff --git a/Makefile b/Makefile index 69675e9d..19e53427 100644 --- a/Makefile +++ b/Makefile @@ -27,6 +27,7 @@ EXPORTER_VERSION_STR=-X github.com/prometheus/common/version.Version=$(GIT_VERSI # note that /testutils is not a production directory, but has unit tests to validate testing tools SUBDIRS_HAS_UNIT=backup/ filepath/ history/ helper/ options/ report/ restore/ toc/ utils/ testutils/ plugins/s3plugin/ gpbackman/cmd/ gpbackman/gpbckpconfig/ gpbackman/textmsg/ exporter/ SUBDIRS_ALL=$(SUBDIRS_HAS_UNIT) integration/ end_to_end/ +GOLANGCI_LINT_VERSION=v2.12.2 GOLANG_LINTER=$(GOPATH)/bin/golangci-lint GINKGO=$(GOPATH)/bin/ginkgo GOIMPORTS=$(GOPATH)/bin/goimports @@ -56,18 +57,16 @@ $(GOIMPORTS) : $(GOSQLITE) : go install github.com/mattn/go-sqlite3 +$(GOLANG_LINTER) : + GOBIN=$(GOPATH)/bin go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION) + format : $(GOIMPORTS) @goimports -w $(shell find . -type f -name '*.go' -not -path "./vendor/*") -LINTER_VERSION=2.10.1 -$(GOLANG_LINTER) : - mkdir -p $(GOPATH)/bin - curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/main/install.sh | sh -s -- -b $(GOPATH)/bin v${LINTER_VERSION} - -.PHONY : coverage integration end_to_end +.PHONY : coverage integration end_to_end lint lint : $(GOLANG_LINTER) - golangci-lint run + $(GOLANG_LINTER) run unit : $(GINKGO) TEST_DB_TYPE=CBDB TEST_DB_VERSION=2.999.0 ginkgo $(GINKGO_FLAGS) $(SUBDIRS_HAS_UNIT) 2>&1 diff --git a/README.md b/README.md index db40bc23..f16a12e4 100644 --- a/README.md +++ b/README.md @@ -174,7 +174,7 @@ popd **NOTE**: The integration and end_to_end tests require a running Cloudberry instance. -* To run all tests except end-to-end (linters, unit, and integration), use `make test`. +* To build and run unit and integration tests, use `make test`. * To run only unit tests, use `make unit`. * To run only integration tests (requires a running Cloudberry instance), use `make integration`. * To run end to end tests (requires a running Cloudberry instance), use `make end_to_end`. @@ -183,8 +183,9 @@ We provide the following targets to help developers ensure their code fits Go standard formatting guidelines: * To run a linting tool that checks for basic coding errors, use: `make lint`. -This target runs [gometalinter](https://github.com/alecthomas/gometalinter). -Note: The lint target will fail if code is not formatted properly. +This target runs [golangci-lint](https://golangci-lint.run/) installed in +`$(GOPATH)/bin`. CI installs its pinned version through the official GitHub +Action. * To automatically format your code and add/remove imports, use `make format`. This target runs diff --git a/backup/backup.go b/backup/backup.go index c0032746..07c2d253 100644 --- a/backup/backup.go +++ b/backup/backup.go @@ -172,7 +172,7 @@ func DoBackup() { gplog.FatalOnError(err) } else { err = history.StoreBackupHistory(historyDB, &backupReport.BackupConfig) - historyDB.Close() + _ = historyDB.Close() gplog.FatalOnError(err) } } @@ -245,7 +245,6 @@ func backupPredata(metadataFile *utils.FileWithByteCount, tables []Table, tableO } gplog.Info("Writing pre-data metadata") - var protocols []ExternalProtocol var functions []Function var funcInfoMap map[uint32]FunctionInfo objects := make([]Sortable, 0) @@ -259,7 +258,7 @@ func backupPredata(metadataFile *utils.FileWithByteCount, tables []Table, tableO addToMetadataMap(relationMetadata, metadataMap) if !tableOnly { - protocols = retrieveProtocols(&objects, metadataMap) + retrieveProtocols(&objects, metadataMap) backupSchemas(metadataFile, createAlteredPartitionSchemaSet(tables)) backupExtensions(metadataFile) backupCollations(metadataFile) @@ -283,7 +282,7 @@ func backupPredata(metadataFile *utils.FileWithByteCount, tables []Table, tableO sequences := retrieveAndBackupSequences(metadataFile, relationMetadata) domainConstraints, nonDomainConstraints, conMetadata := retrieveConstraints(&objects, metadataMap) - viewsDependingOnConstraints := backupDependentObjects(metadataFile, tables, protocols, metadataMap, domainConstraints, objects, sequences, funcInfoMap, tableOnly) + viewsDependingOnConstraints := backupDependentObjects(metadataFile, tables, metadataMap, domainConstraints, objects, sequences, funcInfoMap) backupConversions(metadataFile) @@ -523,7 +522,7 @@ func DoCleanup(backupFailed bool) { gplog.Error("Unable to update history database. Error: %v", err) } else { _, err := historyDB.Exec(fmt.Sprintf("UPDATE backups SET status='%s', end_time='%s' WHERE timestamp='%s'", statusString, backupReport.BackupConfig.EndTime, globalFPInfo.Timestamp)) - historyDB.Close() + _ = historyDB.Close() if err != nil { gplog.Error("Unable to update history database. Error: %v", err) } else { @@ -683,7 +682,7 @@ func getTableLocks(table Table) []TableLocks { return locksResults } -func logTableLocks(table Table, whichConn int) { +func logTableLocks(table Table) { locks := getTableLocks(table) jsonData, _ := json.Marshal(&locks) gplog.Warn("Locks held on table %s: %s", table.FQN(), jsonData) diff --git a/backup/data.go b/backup/data.go index e4571408..4ffb5172 100644 --- a/backup/data.go +++ b/backup/data.go @@ -234,7 +234,7 @@ func BackupDataForAllTables(tables []Table) []map[uint32]int64 { fmt.Printf("\n") } gplog.Warn("Worker %d could not acquire AccessShareLock for table %s.", whichConn, table.FQN()) - logTableLocks(table, whichConn) + logTableLocks(table) // rollback transaction and defer table err = connectionPool.Rollback(whichConn) if err != nil { diff --git a/backup/dependencies.go b/backup/dependencies.go index 64445e52..b80d390d 100644 --- a/backup/dependencies.go +++ b/backup/dependencies.go @@ -239,7 +239,7 @@ func assignCohorts(slice []Sortable, dependencies DependencyMap, isDependentOn m // to the toc. for id := range tierMap { cohort := objectToCohortMap[id] - tierMap[id][1] = uint32(cohort) + tierMap[id][1] = cohort } return } diff --git a/backup/dependencies_test.go b/backup/dependencies_test.go index fd645182..dd220513 100644 --- a/backup/dependencies_test.go +++ b/backup/dependencies_test.go @@ -86,7 +86,7 @@ var _ = Describe("backup/dependencies tests", func() { testhelper.ExpectRegexp(logfile, "\tpublic.relation2 {ClassID:1259 Oid:2}") }() defer testhelper.ShouldPanicWithMessage("Dependency resolution failed; see log file gbytes.Buffer for details. This is a bug, please report.") - sortable, _ = backup.TopologicalSort(sortable, depMap) + _, _ = backup.TopologicalSort(sortable, depMap) }) It("aborts if dependencies are not met", func() { depMap[backup.UniqueID{ClassID: backup.PG_CLASS_OID, Oid: 1}] = map[backup.UniqueID]bool{{ClassID: backup.PG_CLASS_OID, Oid: 2}: true} @@ -94,7 +94,7 @@ var _ = Describe("backup/dependencies tests", func() { sortable := []backup.Sortable{relation1, relation2} defer testhelper.ShouldPanicWithMessage("Dependency resolution failed; see log file gbytes.Buffer for details. This is a bug, please report.") - sortable, _ = backup.TopologicalSort(sortable, depMap) + _, _ = backup.TopologicalSort(sortable, depMap) }) }) Describe("PrintDependentObjectStatements", func() { diff --git a/backup/incremental.go b/backup/incremental.go index dcf70619..dd0124c0 100644 --- a/backup/incremental.go +++ b/backup/incremental.go @@ -89,7 +89,9 @@ func GetLatestMatchingBackupConfig(historyDBPath string, currentBackupConfig *hi gplog.Error("%s", err.Error()) return nil } - defer timestampRows.Close() + defer func() { + _ = timestampRows.Close() + }() timestamps := make([]string, 0) for timestampRows.Next() { diff --git a/backup/predata_externals.go b/backup/predata_externals.go index 399146fc..49c18c2f 100644 --- a/backup/predata_externals.go +++ b/backup/predata_externals.go @@ -292,8 +292,6 @@ func PrintExternalTableStatements(metadataFile *utils.FileWithByteCount, tableNa if extTableDef.Type == READABLE || (extTableDef.Type == WRITABLE_WEB && extTableDef.Protocol == S3) { if extTableDef.ExecLocation == "COORDINATOR_ONLY" { metadataFile.MustPrintf(" ON COORDINATOR") - } else if extTableDef.ExecLocation == "COORDINATOR_ONLY" { - metadataFile.MustPrintf(" ON COORDINATOR") } } if extTableDef.Type == READABLE_WEB || extTableDef.Type == WRITABLE_WEB { diff --git a/backup/predata_externals_test.go b/backup/predata_externals_test.go index a9515d97..8df24a77 100644 --- a/backup/predata_externals_test.go +++ b/backup/predata_externals_test.go @@ -320,7 +320,7 @@ SEGMENT REJECT LIMIT 2 ROWS`) Expect(resultStatement).To(Equal(`FORMAT 'TEXT'`)) }) - It("generates a FORMAT statment with some options provided", func() { + It("generates a FORMAT statement with some options provided", func() { extTableDef.FormatType = "t" extTableDef.FormatOpts = `delimiter '\t' null '\N' escape '\'` diff --git a/backup/predata_functions.go b/backup/predata_functions.go index 9f6324ef..ef31921d 100644 --- a/backup/predata_functions.go +++ b/backup/predata_functions.go @@ -291,7 +291,7 @@ func PrintCreateExtensionStatements(metadataFile *utils.FileWithByteCount, objTo for _, extensionDef := range extensionDefs { start := metadataFile.ByteCount if (connectionPool.Version.IsGPDB() && connectionPool.Version.AtLeast("7")) || connectionPool.Version.IsCBDB() { - // changes to gp_toolkit in gpdb7 require explicilty creating the schema before the extension + // changes to gp_toolkit in gpdb7 require explicitly creating the schema before the extension metadataFile.MustPrintf( "\n\nCREATE SCHEMA IF NOT EXISTS %[1]s;\nSET search_path=%[1]s,pg_catalog;\nCREATE EXTENSION IF NOT EXISTS %[2]s WITH SCHEMA %[1]s;\nSET search_path=pg_catalog;\n", extensionDef.Schema, extensionDef.Name) diff --git a/backup/predata_relations.go b/backup/predata_relations.go index ead394ab..4de2b542 100644 --- a/backup/predata_relations.go +++ b/backup/predata_relations.go @@ -463,8 +463,8 @@ func PrintCreatePostdataViewStatements(metadataFile *utils.FileWithByteCount, ob for _, view := range views { start := metadataFile.ByteCount metadataFile.MustPrintf("\n\nCREATE OR REPLACE VIEW %s%s AS %s\n", view.FQN(), view.Options, view.Definition.String) - section, entry := view.GetMetadataEntry() - section = "postdata" + _, entry := view.GetMetadataEntry() + section := "postdata" tier := globalTierMap[view.GetUniqueID()] objToc.AddMetadataEntry(section, entry, start, metadataFile.ByteCount, tier) } diff --git a/backup/predata_shared.go b/backup/predata_shared.go index 760d36ea..372e0da1 100644 --- a/backup/predata_shared.go +++ b/backup/predata_shared.go @@ -38,7 +38,7 @@ func PrintConstraintStatements(metadataFile *utils.FileWithByteCount, objToc *to for _, constraint := range constraints { start := metadataFile.ByteCount - // ConIsLocal should always return true from GetConstraints because we filter out constraints that are inherited using the INHERITS clause, or inherited from a parent partition table. This field only accurately reflects constraints in GPDB6+ because check constraints on parent tables must propogate to children. For GPDB versions 5 or lower, this field will default to false. + // ConIsLocal should always return true from GetConstraints because we filter out constraints that are inherited using the INHERITS clause, or inherited from a parent partition table. This field only accurately reflects constraints in GPDB6+ because check constraints on parent tables must propagate to children. For GPDB versions 5 or lower, this field will default to false. objStr := "TABLE ONLY" if constraint.IsPartitionParent || (constraint.ConType == "c" && constraint.ConIsLocal) { objStr = toc.OBJ_TABLE diff --git a/backup/queries_acl.go b/backup/queries_acl.go index 11d02b6b..688c88c1 100644 --- a/backup/queries_acl.go +++ b/backup/queries_acl.go @@ -324,7 +324,7 @@ func GetDefaultPrivileges(connectionPool *dbconn.DBConn) []DefaultPrivileges { // Cannot use unnest() in CASE statements anymore in GPDB 7+ so convert // it to a LEFT JOIN LATERAL. We do not use LEFT JOIN LATERAL for GPDB 6 // because the CASE unnest() logic is more performant. - aclCols := "''" + var aclCols string aclLateralJoin := "" if (connectionPool.Version.IsGPDB() && connectionPool.Version.AtLeast("7")) || connectionPool.Version.IsCBDB() { aclLateralJoin = diff --git a/backup/queries_shared.go b/backup/queries_shared.go index c9998d5f..030f1c3b 100644 --- a/backup/queries_shared.go +++ b/backup/queries_shared.go @@ -114,7 +114,7 @@ func GetConstraints(connectionPool *dbconn.DBConn, includeTables ...Relation) [] // filter out constraints that are inherited using the INHERITS clause, or // inherited from a parent partition table. This field only accurately // reflects constraints in GPDB6+ because check constraints on parent - // tables must propogate to children. For GPDB versions 5 or lower, this + // tables must propagate to children. For GPDB versions 5 or lower, this // field will default to false. conIsLocal := "" if (connectionPool.Version.IsGPDB() && connectionPool.Version.AtLeast("6")) || connectionPool.Version.IsCBDB() { diff --git a/backup/queries_table_defs.go b/backup/queries_table_defs.go index 49282fd2..3b5d1351 100644 --- a/backup/queries_table_defs.go +++ b/backup/queries_table_defs.go @@ -375,7 +375,7 @@ func GetColumnDefinitions(connectionPool *dbconn.DBConn) map[uint32][]ColumnDefi AND a.attisdropped = 'f' ORDER BY a.attrelid, a.attnum`, relationAndSchemaFilterClause()) - query := `` + var query string if connectionPool.Version.IsGPDB() && connectionPool.Version.Before("6") { query = before6Query } else if connectionPool.Version.IsGPDB() && connectionPool.Version.Is("6") { @@ -736,7 +736,7 @@ func GetTableInheritance(connectionPool *dbconn.DBConn, tables []Relation) map[u return resultMap } -// Used to contruct root tables for GPDB 7+, because the root partition must be +// Used to construct root tables for GPDB 7+, because the root partition must be // constructed by itself first. func GetPartitionKeyDefs(connectionPool *dbconn.DBConn) map[uint32]string { if connectionPool.Version.IsGPDB() && connectionPool.Version.Before("7") { diff --git a/backup/wrappers.go b/backup/wrappers.go index 284348cd..33704b0e 100644 --- a/backup/wrappers.go +++ b/backup/wrappers.go @@ -85,7 +85,7 @@ func initializeConnectionPool(timestamp string) { } func SetSessionGUCs(connNum int) { - // These GUCs ensure the dumps portability accross systems + // These GUCs ensure the dumps portability across systems connectionPool.MustExec("SET search_path TO pg_catalog", connNum) connectionPool.MustExec("SET statement_timeout = 0", connNum) connectionPool.MustExec("SET DATESTYLE = ISO", connNum) @@ -616,8 +616,8 @@ func addToMetadataMap(newMetadata MetadataMap, metadataMap MetadataMap) { // This function is fairly unwieldy, but there's not really a good way to break it down func backupDependentObjects(metadataFile *utils.FileWithByteCount, tables []Table, - protocols []ExternalProtocol, filteredMetadata MetadataMap, domainConstraints []Constraint, - sortables []Sortable, sequences []Sequence, funcInfoMap map[uint32]FunctionInfo, tableOnly bool) []View { + filteredMetadata MetadataMap, domainConstraints []Constraint, + sortables []Sortable, sequences []Sequence, funcInfoMap map[uint32]FunctionInfo) []View { var sortedSlice []Sortable gplog.Verbose("Writing CREATE statements for dependent objects to metadata file") diff --git a/end_to_end/end_to_end_suite_test.go b/end_to_end/end_to_end_suite_test.go index 4986c3c4..585c2057 100644 --- a/end_to_end/end_to_end_suite_test.go +++ b/end_to_end/end_to_end_suite_test.go @@ -927,13 +927,13 @@ var _ = Describe("backup and restore end to end tests", func() { helperLogs, _ := path.Glob(path.Join(homeDir, "gpAdminLogs/gprestore_*")) cmdStr := fmt.Sprintf("tail -n 40 %s | grep \"Creating skip file\" || true", helperLogs[len(helperLogs)-1]) - attemts := 1000 + attempts := 1000 err = errors.New("Timeout to discover skip file") - for attemts > 0 { + for attempts > 0 { output := mustRunCommand(exec.Command("bash", "-c", cmdStr)) if strings.TrimSpace(string(output)) == "" { time.Sleep(5 * time.Millisecond) - attemts-- + attempts-- } else { err = nil break @@ -2511,7 +2511,7 @@ LANGUAGE plpgsql NO SQL;`) Expect(err).To(HaveOccurred()) Expect(string(output)).To(MatchRegexp("Segment count for backup with timestamp [0-9]+ is unknown, cannot restore using --resize-cluster flag")) }) - It("Will not restore to a different-size cluster without the approprate flag", func() { + It("Will not restore to a different-size cluster without the appropriate flag", func() { command := exec.Command("tar", "-xzf", "resources/5-segment-db.tar.gz", "-C", backupDir) mustRunCommand(command) diff --git a/filepath/filepath.go b/filepath/filepath.go index 818844ef..4b8c4019 100644 --- a/filepath/filepath.go +++ b/filepath/filepath.go @@ -273,7 +273,7 @@ func GetTimestampFromBackupDirectory(backupDir string) (string, error) { return "", fmt.Errorf("Multiple timestamp directories found under %s, please specify a timestamp using the --timestamp flag", backupDir) } - timestamp := path.Base(string(timestampDirs[0])) + timestamp := path.Base(timestampDirs[0]) return timestamp, nil } diff --git a/gpbackman/cmd/wrappers.go b/gpbackman/cmd/wrappers.go index 9086c6ad..a35c4c94 100644 --- a/gpbackman/cmd/wrappers.go +++ b/gpbackman/cmd/wrappers.go @@ -97,7 +97,7 @@ func setLogLevelFile(level string) error { // COORDINATOR_DATA_DIRECTORY environment variable exported by the standard // Cloudberry environment scripts. As a final fallback, return the bare // filename so it is resolved against the current working directory, -// preserving the original behaviour for the default invocation. +// preserving the original behavior for the default invocation. func getHistoryDBPath(historyDBPath string, autoLoad bool) string { if historyDBPath != "" { return historyDBPath @@ -252,7 +252,9 @@ func getBackupMasterDirClusterInfo(dbName string) string { gplog.Error("%s", textmsg.ErrorTextUnableConnectLocalCluster(err)) return "" } - defer db.Close() + defer func() { + _ = db.Close() + }() sqlQuery := "SELECT datadir FROM gp_segment_configuration WHERE content = -1 AND role = 'p';" queryResult, err := gpbckpconfig.ExecuteQueryLocalClusterConn[string](db, sqlQuery) if err != nil { @@ -270,7 +272,9 @@ func getSegmentConfigurationClusterInfo(dbName string) ([]gpbckpconfig.SegmentCo gplog.Error("%s", textmsg.ErrorTextUnableConnectLocalCluster(err)) return queryResult, err } - defer db.Close() + defer func() { + _ = db.Close() + }() sqlQuery := "SELECT content as contentid, hostname, datadir FROM gp_segment_configuration WHERE role = 'p' and content != -1 ORDER BY content;" queryResult, err = gpbckpconfig.ExecuteQueryLocalClusterConn[[]gpbckpconfig.SegmentConfig](db, sqlQuery) if err != nil { diff --git a/gpbackman/gpbckpconfig/utils_db.go b/gpbackman/gpbckpconfig/utils_db.go index cd022feb..6850307f 100644 --- a/gpbackman/gpbckpconfig/utils_db.go +++ b/gpbackman/gpbckpconfig/utils_db.go @@ -216,7 +216,9 @@ func execQueryFunc(query string, historyDB *sql.DB, args ...any) ([]string, erro if err != nil { return nil, err } - defer sqlRow.Close() + defer func() { + _ = sqlRow.Close() + }() var resultList []string for sqlRow.Next() { var b string diff --git a/helper/backup_helper.go b/helper/backup_helper.go index 1c77ee48..88a22bed 100644 --- a/helper/backup_helper.go +++ b/helper/backup_helper.go @@ -86,7 +86,7 @@ func doBackupAgent() error { _ = readHandle.Close() logInfo(fmt.Sprintf("Oid %d: Deleting pipe: %s\n", oid, currentPipe)) - deletePipe(currentPipe) + _ = deletePipe(currentPipe) } _ = pipeWriter.Close() @@ -141,19 +141,19 @@ func getBackupPipeWriter() (pipe BackupPipeWriterCloser, writeCmd *exec.Cmd, err if *compressionLevel == 0 { pipe = NewCommonBackupPipeWriterCloser(writeHandle) - return + return pipe, writeCmd, nil } if *compressionType == "gzip" { pipe, err = NewGZipBackupPipeWriterCloser(writeHandle, *compressionLevel) - return + return pipe, writeCmd, err } if *compressionType == "zstd" { pipe, err = NewZSTDBackupPipeWriterCloser(writeHandle, *compressionLevel) - return + return pipe, writeCmd, err } - writeHandle.Close() + _ = writeHandle.Close() // error logging handled by calling functions return nil, nil, fmt.Errorf("unknown compression type '%s' (compression level %d)", *compressionType, *compressionLevel) } diff --git a/helper/backup_helper_pipes.go b/helper/backup_helper_pipes.go index 29edebfb..04718f26 100644 --- a/helper/backup_helper_pipes.go +++ b/helper/backup_helper_pipes.go @@ -56,7 +56,7 @@ func NewGZipBackupPipeWriterCloser(writeHandle io.WriteCloser, compressLevel int gzPipe.cPipe = NewCommonBackupPipeWriterCloser(writeHandle) gzPipe.gzipWriter, err = gzip.NewWriterLevel(gzPipe.cPipe.bufIoWriter, compressLevel) if err != nil { - gzPipe.cPipe.Close() + _ = gzPipe.cPipe.Close() } return } @@ -80,7 +80,7 @@ func NewZSTDBackupPipeWriterCloser(writeHandle io.WriteCloser, compressLevel int zstdPipe.cPipe = NewCommonBackupPipeWriterCloser(writeHandle) zstdPipe.zstdEncoder, err = zstd.NewWriter(zstdPipe.cPipe.bufIoWriter, zstd.WithEncoderLevel(zstd.EncoderLevelFromZstd(compressLevel))) if err != nil { - zstdPipe.cPipe.Close() + _ = zstdPipe.cPipe.Close() } return } diff --git a/helper/helper.go b/helper/helper.go index 216783ce..7e59581e 100644 --- a/helper/helper.go +++ b/helper/helper.go @@ -251,7 +251,7 @@ func getOidListFromFile(oidFileName string) ([]int, error) { func flushAndCloseRestoreWriter(pipeName string, oid int) error { if writer != nil { - writer.Write([]byte{}) // simulate writer connected in case of error + _, _ = writer.Write([]byte{}) // simulate writer connected in case of error err := writer.Flush() if err != nil { logError("Oid %d: Failed to flush pipe %s", oid, pipeName) @@ -318,9 +318,9 @@ func DoCleanup() { logVerbose("Cleanup complete") } -func logInfo(s string, v ...interface{}) { +func logInfo(s string) { s = fmt.Sprintf("Segment %d: %s", *content, s) - gplog.Info(s, v...) + gplog.Info("%s", s) } func logWarn(s string, v ...interface{}) { diff --git a/helper/restore_helper.go b/helper/restore_helper.go index e0c67857..edb16b1c 100644 --- a/helper/restore_helper.go +++ b/helper/restore_helper.go @@ -26,8 +26,8 @@ type ReaderType string const ( SEEKABLE ReaderType = "seekable" // reader which supports seek - NONSEEKABLE = "discard" // reader which is not seekable - SUBSET = "subset" // reader which operates on pre filtered data + NONSEEKABLE ReaderType = "discard" // reader which is not seekable + SUBSET ReaderType = "subset" // reader which operates on pre filtered data ) var ( @@ -214,7 +214,7 @@ func doRestoreAgent() error { // Close file before it gets overwritten. Free up these // resources when the reader is not needed anymore. if reader, ok := readers[contentToRestore]; ok { - reader.fileHandle.Close() + _ = reader.fileHandle.Close() } // We pre-create readers above for the sake of not re-opening SDF readers. For MDF we can't // re-use them but still having them in a map simplifies overall code flow. We repeatedly assign @@ -242,7 +242,6 @@ func doRestoreAgent() error { // invocation from gprestore (e.g. create a --db-version flag option). if *onErrorContinue && utils.FileExists(fmt.Sprintf("%s_skip_%d", *pipeFile, tableOid)) { logWarn(fmt.Sprintf("Oid %d, Batch %d: Skip file discovered, skipping this relation.", tableOid, batchNum)) - err = nil goto LoopEnd } else { // keep trying to open the pipe @@ -260,7 +259,7 @@ func doRestoreAgent() error { // the writer for the pipe. To avoid having to write complex buffer // logic for when os.write() returns EAGAIN due to full buffer, set // the file descriptor to block on IO. - unix.SetNonblock(int(writeHandle.Fd()), false) + _ = unix.SetNonblock(int(writeHandle.Fd()), false) logVerbose(fmt.Sprintf("Oid %d, Batch %d: Reader connected to pipe %s", tableOid, batchNum, path.Base(currentPipe))) break } @@ -300,11 +299,6 @@ func doRestoreAgent() error { if *singleDataFile { lastByte[contentToRestore] += uint64(bytesRead) } - if errBuf.Len() > 0 { - err = errors.Wrap(err, strings.Trim(errBuf.String(), "\x00")) - } else { - err = errors.Wrap(err, "Error copying data") - } goto LoopEnd } @@ -368,7 +362,7 @@ func getRestoreDataReader(fileToRead string, objToc *toc.SegmentTOC, oidList []i var gzipReader *gzip.Reader var zstdReader *zstd.Decoder var isSubset bool - var err error = nil + var err error restoreReader := new(RestoreReader) if *pluginConfigFile != "" { @@ -458,15 +452,15 @@ func startRestorePluginCommand(fileToRead string, objToc *toc.SegmentTOC, oidLis if objToc != nil && pluginConfig.CanRestoreSubset() && *isFiltered && !strings.HasSuffix(fileToRead, ".gz") && !strings.HasSuffix(fileToRead, ".zst") { offsetsFile, _ := os.CreateTemp("/tmp", "gprestore_offsets_") defer func() { - offsetsFile.Close() + _ = offsetsFile.Close() }() w := bufio.NewWriter(offsetsFile) - w.WriteString(fmt.Sprintf("%v", len(oidList))) + _, _ = w.WriteString(fmt.Sprintf("%v", len(oidList))) for _, oid := range oidList { - w.WriteString(fmt.Sprintf(" %v %v", objToc.DataEntries[uint(oid)].StartByte, objToc.DataEntries[uint(oid)].EndByte)) + _, _ = w.WriteString(fmt.Sprintf(" %v %v", objToc.DataEntries[uint(oid)].StartByte, objToc.DataEntries[uint(oid)].EndByte)) } - w.Flush() + _ = w.Flush() cmdStr = fmt.Sprintf("%s restore_data_subset %s %s %s", pluginConfig.ExecutablePath, pluginConfig.ConfigPath, fileToRead, offsetsFile.Name()) isSubset = true } else { diff --git a/history/history.go b/history/history.go index 1ee0c7ec..f2b6861b 100644 --- a/history/history.go +++ b/history/history.go @@ -89,7 +89,7 @@ func InitializeHistoryDatabase(historyDBPath string) (*sql.DB, error) { return nil, err } else if err == nil { // We don't want an fd handle to it, so close it - fd.Close() + _ = fd.Close() } db, err := sql.Open("sqlite3", historyDBPath) @@ -134,8 +134,8 @@ func InitializeHistoryDatabase(historyDBPath string) (*sql.DB, error) { );` _, err = tx.Exec(createBackupsTable) if err != nil { - tx.Rollback() - db.Close() + _ = tx.Rollback() + _ = db.Close() return nil, err } @@ -150,8 +150,8 @@ func InitializeHistoryDatabase(historyDBPath string) (*sql.DB, error) { for _, auxTable := range auxTables { _, err = tx.Exec(fmt.Sprintf(createAuxTableQuery, auxTable)) if err != nil { - tx.Rollback() - db.Close() + _ = tx.Rollback() + _ = db.Close() return nil, err } } @@ -167,8 +167,8 @@ func InitializeHistoryDatabase(historyDBPath string) (*sql.DB, error) { );` _, err = tx.Exec(createRestorePlansTable) if err != nil { - tx.Rollback() - db.Close() + _ = tx.Rollback() + _ = db.Close() return nil, err } @@ -181,14 +181,14 @@ func InitializeHistoryDatabase(historyDBPath string) (*sql.DB, error) { );` _, err = tx.Exec(createRestorePlanTablesTable) if err != nil { - tx.Rollback() - db.Close() + _ = tx.Rollback() + _ = db.Close() return nil, err } err = tx.Commit() if err != nil { - db.Close() + _ = db.Close() return nil, err } return db, nil @@ -281,12 +281,12 @@ func StoreBackupHistory(db *sql.DB, currentBackupConfig *BackupConfig) error { return err CleanupError: - tx.Rollback() + _ = tx.Rollback() return err } func GetMainBackupInfo(timestamp string, historyDB *sql.DB) (BackupConfig, error) { - // Retreive main backups information. SQLite doesn't have booleans so convert from ints + // Retrieve main backups information. SQLite doesn't have booleans so convert from ints // TODO -- consider passing in a tx instead so that aux tables are coherent with main backups // table. Need to confirm this is possible with sqlite. Unclear if we ever pull in and use aux // table info, so it may not be needed. @@ -349,7 +349,9 @@ func getAuxTable(db *sql.DB, timestamp, tableName string) ([]string, error) { if err != nil { return nil, err } - defer auxTableRows.Close() + defer func() { + _ = auxTableRows.Close() + }() auxTableSlice := make([]string, 0) for auxTableRows.Next() { @@ -397,7 +399,9 @@ func GetBackupConfig(timestamp string, historyDB *sql.DB) (*BackupConfig, error) if err != nil { return nil, err } - defer restorePlanRows.Close() + defer func() { + _ = restorePlanRows.Close() + }() backupConfig.RestorePlan = make([]RestorePlanEntry, 0) for restorePlanRows.Next() { @@ -418,8 +422,9 @@ func GetBackupConfig(timestamp string, historyDB *sql.DB) (*BackupConfig, error) if err != nil { return nil, err } - defer restorePlanTableRows.Close() - + defer func(rows *sql.Rows) { + _ = rows.Close() + }(restorePlanTableRows) for restorePlanTableRows.Next() { var tableFQN string err = restorePlanTableRows.Scan(&tableFQN) diff --git a/integration/predata_acl_queries_test.go b/integration/predata_acl_queries_test.go index 0eb0bbc4..53714b3b 100644 --- a/integration/predata_acl_queries_test.go +++ b/integration/predata_acl_queries_test.go @@ -432,7 +432,6 @@ LANGUAGE SQL`) structmatcher.ExpectStructsToMatch(&dictionaryMetadata, &resultMetadata) }) It("returns a slice of default metadata for a text search configuration", func() { - resultMetadataMap := backup.GetMetadataForObjectType(connectionPool, backup.TYPE_TS_CONFIGURATION) configurationMetadata := testutils.DefaultMetadata(toc.OBJ_TEXT_SEARCH_CONFIGURATION, false, true, true, false) testhelper.AssertQueryRuns(connectionPool, `CREATE TEXT SEARCH CONFIGURATION public.testconfiguration (PARSER = pg_catalog."default");`) @@ -440,7 +439,7 @@ LANGUAGE SQL`) testhelper.AssertQueryRuns(connectionPool, "COMMENT ON TEXT SEARCH CONFIGURATION public.testconfiguration IS 'This is a text search configuration comment.'") uniqueID := testutils.UniqueIDFromObjectName(connectionPool, "public", "testconfiguration", backup.TYPE_TS_CONFIGURATION) - resultMetadataMap = backup.GetMetadataForObjectType(connectionPool, backup.TYPE_TS_CONFIGURATION) + resultMetadataMap := backup.GetMetadataForObjectType(connectionPool, backup.TYPE_TS_CONFIGURATION) Expect(resultMetadataMap).To(HaveLen(1)) resultMetadata := resultMetadataMap[uniqueID] @@ -778,7 +777,6 @@ LANGUAGE SQL`) structmatcher.ExpectStructsToMatch(&dictionaryMetadata, &resultMetadata) }) It("returns a slice of default metadata for a text search configuration in a specific schema", func() { - resultMetadataMap := backup.GetMetadataForObjectType(connectionPool, backup.TYPE_TS_CONFIGURATION) configurationMetadata := testutils.DefaultMetadata(toc.OBJ_TEXT_SEARCH_CONFIGURATION, false, true, true, false) testhelper.AssertQueryRuns(connectionPool, `CREATE TEXT SEARCH CONFIGURATION public.testconfiguration (PARSER = pg_catalog."default");`) @@ -790,7 +788,7 @@ LANGUAGE SQL`) testhelper.AssertQueryRuns(connectionPool, "COMMENT ON TEXT SEARCH CONFIGURATION testschema.testconfiguration IS 'This is a text search configuration comment.'") _ = backupCmdFlags.Set(options.INCLUDE_SCHEMA, "testschema") - resultMetadataMap = backup.GetMetadataForObjectType(connectionPool, backup.TYPE_TS_CONFIGURATION) + resultMetadataMap := backup.GetMetadataForObjectType(connectionPool, backup.TYPE_TS_CONFIGURATION) Expect(resultMetadataMap).To(HaveLen(1)) uniqueID := testutils.UniqueIDFromObjectName(connectionPool, "testschema", "testconfiguration", backup.TYPE_TS_CONFIGURATION) diff --git a/integration/predata_functions_queries_test.go b/integration/predata_functions_queries_test.go index 026af163..26e4fb83 100644 --- a/integration/predata_functions_queries_test.go +++ b/integration/predata_functions_queries_test.go @@ -905,7 +905,7 @@ LANGUAGE SQL`) BeforeEach(func() { testutils.SkipIfBefore7(connectionPool) }) - It("returns a slice of transfroms", func() { + It("returns a slice of transforms", func() { testhelper.AssertQueryRuns(connectionPool, "CREATE TRANSFORM FOR pg_catalog.int4 LANGUAGE c (FROM SQL WITH FUNCTION numeric_support(internal), TO SQL WITH FUNCTION int4recv(internal));") defer testhelper.AssertQueryRuns(connectionPool, "DROP TRANSFORM FOR int4 LANGUAGE c") diff --git a/options/options.go b/options/options.go index 8dff3ffe..716c249f 100644 --- a/options/options.go +++ b/options/options.go @@ -392,7 +392,6 @@ func (o Options) GetUserTableRelationsWithIncludeFiltering(connectionPool *dbcon if len(childOids) > 0 { childPartitionFilter = fmt.Sprintf(`OR c.oid IN (%s)`, strings.Join(childOids, ", ")) } - includeOids = childOids } } diff --git a/options/options_test.go b/options/options_test.go index fef85568..539f82b1 100644 --- a/options/options_test.go +++ b/options/options_test.go @@ -134,6 +134,7 @@ var _ = Describe("options", func() { _, err = file.WriteString("\n") Expect(err).To(Not(HaveOccurred())) _, err = file.WriteString("\n") + Expect(err).To(Not(HaveOccurred())) err = file.Close() Expect(err).To(Not(HaveOccurred())) @@ -167,6 +168,7 @@ var _ = Describe("options", func() { _, err = file.WriteString("\n") Expect(err).To(Not(HaveOccurred())) _, err = file.WriteString("\n") + Expect(err).To(Not(HaveOccurred())) err = file.Close() Expect(err).To(Not(HaveOccurred())) diff --git a/plugins/s3plugin/backup.go b/plugins/s3plugin/backup.go index 0a7d0c50..c0494849 100644 --- a/plugins/s3plugin/backup.go +++ b/plugins/s3plugin/backup.go @@ -32,7 +32,9 @@ func SetupPluginForBackup(c *cli.Context) error { testFilePath := fmt.Sprintf("%s/%s", localBackupDir, testFileName) fileKey := GetS3Path(config.Options.Folder, testFilePath) file, err := os.Create("/tmp/" + testFileName) // dummy empty reader for probe - defer file.Close() + defer func() { + _ = file.Close() + }() if err != nil { return err } @@ -48,7 +50,9 @@ func BackupFile(c *cli.Context) error { fileName := c.Args().Get(1) fileKey := GetS3Path(config.Options.Folder, fileName) file, err := os.Open(fileName) - defer file.Close() + defer func() { + _ = file.Close() + }() if err != nil { return err } @@ -77,7 +81,7 @@ func BackupDirectory(c *cli.Context) error { // Populate a list of files to be backed up fileList := make([]string, 0) _ = filepath.Walk(dirName, func(path string, f os.FileInfo, err error) error { - isDir, _ := isDirectoryGetSize(path) + isDir := isDirectory(path) if !isDir { fileList = append(fileList, path) } @@ -125,7 +129,7 @@ func BackupDirectoryParallel(c *cli.Context) error { // Populate a list of files to be backed up fileList := make([]string, 0) _ = filepath.Walk(dirName, func(path string, f os.FileInfo, err error) error { - isDir, _ := isDirectoryGetSize(path) + isDir := isDirectory(path) if !isDir { fileList = append(fileList, path) } diff --git a/plugins/s3plugin/restore.go b/plugins/s3plugin/restore.go index deb3cc98..54733d47 100644 --- a/plugins/s3plugin/restore.go +++ b/plugins/s3plugin/restore.go @@ -36,7 +36,9 @@ func RestoreFile(c *cli.Context) error { bucket := config.Options.Bucket fileKey := GetS3Path(config.Options.Folder, fileName) file, err := os.Create(fileName) - defer file.Close() + defer func() { + _ = file.Close() + }() if err != nil { return err } diff --git a/plugins/s3plugin/s3plugin.go b/plugins/s3plugin/s3plugin.go index f1b43d56..92e2897e 100644 --- a/plugins/s3plugin/s3plugin.go +++ b/plugins/s3plugin/s3plugin.go @@ -282,19 +282,19 @@ func ShouldEnableEncryption(encryption string) bool { return !isOff } -func isDirectoryGetSize(path string) (bool, int64) { +func isDirectory(path string) bool { fd, err := os.Stat(path) if err != nil { gplog.FatalOnError(err) } switch mode := fd.Mode(); { case mode.IsDir(): - return true, 0 + return true case mode.IsRegular(): - return false, fd.Size() + return false } gplog.FatalOnError(errors.New(fmt.Sprintf("INVALID file %s", path))) - return false, 0 + return false } func getFileSize(S3 s3iface.S3API, bucket string, fileKey string) (int64, error) { diff --git a/restore/parallel.go b/restore/parallel.go index 1697ff3d..b4a67f9a 100644 --- a/restore/parallel.go +++ b/restore/parallel.go @@ -28,7 +28,7 @@ func executeStatementsForConn(statements chan toc.StatementWithType, fatalErr *e gplog.Error("Error detected on connection %d. Terminating transactions.", whichConn) txMutex.Lock() if connectionPool.Tx[whichConn] != nil { - connectionPool.Rollback(whichConn) + _ = connectionPool.Rollback(whichConn) } txMutex.Unlock() } @@ -65,7 +65,7 @@ func executeStatementsForConn(statements chan toc.StatementWithType, fatalErr *e if executeInParallel { txMutex.Lock() if connectionPool.Tx[whichConn] != nil { - connectionPool.Commit(whichConn) + _ = connectionPool.Commit(whichConn) } txMutex.Unlock() } diff --git a/restore/restore.go b/restore/restore.go index 91c6c5ab..84b7a20b 100644 --- a/restore/restore.go +++ b/restore/restore.go @@ -250,21 +250,15 @@ func verifyIncrementalState() { var schemasToCreate []string var tableFQNsToCreate []string - var schemasExcludedByUserInput []string - var tablesExcludedByUserInput []string for _, table := range tableFQNsToRestore { schemaName := strings.Split(table, ".")[0] if utils.SchemaIsExcludedByUser(opts.IncludedSchemas, opts.ExcludedSchemas, schemaName) { - if !utils.Exists(schemasExcludedByUserInput, schemaName) { - schemasExcludedByUserInput = append(schemasExcludedByUserInput, schemaName) - } - tablesExcludedByUserInput = append(tablesExcludedByUserInput, table) continue } if _, exists := existingTablesMap[table]; !exists { if utils.RelationIsExcludedByUser(opts.IncludedRelations, opts.ExcludedRelations, table) { - tablesExcludedByUserInput = append(tablesExcludedByUserInput, table) + continue } else { _, schemaExists := existingSchemasMap[schemaName] preFilteredToCreate := utils.Exists(schemasToCreate, schemaName) @@ -352,7 +346,7 @@ func restorePredata(metadataFilename string) { } numErrors = ExecuteRestoreMetadataStatements("predata", statements, "Pre-data objects", progressBar, utils.PB_VERBOSE, false) if !MustGetFlagBool(options.ON_ERROR_CONTINUE) { - connectionPool.Commit(0) + _ = connectionPool.Commit(0) } } @@ -691,7 +685,7 @@ func writeErrorTables(isMetadata bool) { } _, _ = errorWriter.WriteString(table) } - err = errorWriter.Flush() + _ = errorWriter.Flush() err = errorFile.Close() if err != nil { gplog.Warn("Could not close error tables file: %v", err) diff --git a/restore/validate.go b/restore/validate.go index f9fb55fb..35c1e798 100644 --- a/restore/validate.go +++ b/restore/validate.go @@ -37,7 +37,7 @@ func ValidateExcludeSchemasInBackupSet(schemaList []string) { } } -/* This only checks the globalTOC, but will still succesfully validate tables +/* This only checks the globalTOC, but will still successfully validate tables * in incremental backups since incremental backups will always take backups of * the metadata (--incremental and --data-only backup flags are not compatible) */ @@ -304,7 +304,7 @@ func ValidateSafeToResizeCluster() { timestamp := MustGetFlagString(options.TIMESTAMP) gplog.Fatal(errors.Errorf("Segment count for backup with timestamp %s is unknown, cannot restore using --resize-cluster flag.", timestamp), "") } else if origSize == destSize { - cmdFlags.Set(options.RESIZE_CLUSTER, "false") + _ = cmdFlags.Set(options.RESIZE_CLUSTER, "false") gplog.Warn("Backup segment count matches restore segment count; the --resize-cluster flag is not needed. Proceeding with a normal restore.") } else { gplog.Info("Resize restore specified, will restore a backup set from a %d-segment cluster to a %d-segment cluster", origSize, destSize) diff --git a/restore/wrappers.go b/restore/wrappers.go index ceafeb25..229b517c 100644 --- a/restore/wrappers.go +++ b/restore/wrappers.go @@ -256,7 +256,9 @@ func FindHistoricalPluginVersion(timestamp string) string { if err != nil { return historicalPluginVersion } - defer historyDB.Close() + defer func() { + _ = historyDB.Close() + }() foundBackupConfig, err := history.GetBackupConfig(timestamp, historyDB) if err != nil && err.Error() != "timestamp doesn't match any existing backups" { diff --git a/utils/agent_remote_test.go b/utils/agent_remote_test.go index a09cc2d5..8ff3d8e2 100644 --- a/utils/agent_remote_test.go +++ b/utils/agent_remote_test.go @@ -149,7 +149,7 @@ var _ = Describe("agent remote", func() { }) }) Describe("CheckAgentErrorsOnSegments", func() { - It("constructs the correct ssh call to check for the existance of an error file on each segment", func() { + It("constructs the correct ssh call to check for the existence of an error file on each segment", func() { err := utils.CheckAgentErrorsOnSegments(testCluster, fpInfo) Expect(err).ToNot(HaveOccurred()) diff --git a/utils/plugin_test.go b/utils/plugin_test.go index 99989b1d..d2d66cf6 100644 --- a/utils/plugin_test.go +++ b/utils/plugin_test.go @@ -157,6 +157,7 @@ options: field3: 567 ` err := os.WriteFile(testConfigPath, []byte(testConfigContents), 0777) + Expect(err).ToNot(HaveOccurred()) subject.Options["password_encryption"] = "on" mdd := testCluster.GetDirForContent(-1) _ = os.MkdirAll(mdd, 0777) @@ -372,7 +373,7 @@ options: Expect(err).To(HaveOccurred()) Expect(err.Error()).To(Equal("ERROR: Failed to get plugin name. Failed with error: error executing plugin")) }) - It("did not recieve expected information from plugin", func() { + It("did not receive expected information from plugin", func() { executor.LocalOutput = "bad output" pluginName, err := subject.GetPluginName(testCluster) From 2fc77fa85924ceb32a38dfb21ad2ece6710aff53 Mon Sep 17 00:00:00 2001 From: Dianjin Wang Date: Mon, 14 Sep 2026 17:24:32 +0800 Subject: [PATCH 2/3] Fix s3_plugin_e2e: dl.min.io no longer serves MinIO binaries MinIO archived the open-source Server and Client (mc) and stopped serving their binaries: every artifact under dl.min.io now returns HTTP 410 Gone. The s3_plugin_e2e test case downloads both, so it fails on the first curl. CI has not run since 2026-09-09, before the shutdown, so this has not gone red yet -- it breaks on the next run of that test case. Pin the final open-source releases from their GitHub release assets, which are still published, and verify each binary against the sha256sum MinIO publishes next to it. The previous code chmod +x'd an unverified download. Server RELEASE.2025-09-07T16-13-09Z, client RELEASE.2025-08-13T08-35-41Z. Both keep the minio/mc file names and the default minioadmin credentials, so the server startup, health probe and bucket setup are unchanged. The release/checksum values are plain shell variables rather than exported env: anything with a MINIO_ prefix would be read as configuration by the MinIO server started a few lines below. --- .github/workflows/cloudberry-backup-ci.yml | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/.github/workflows/cloudberry-backup-ci.yml b/.github/workflows/cloudberry-backup-ci.yml index 4f025b81..1a36d4fd 100644 --- a/.github/workflows/cloudberry-backup-ci.yml +++ b/.github/workflows/cloudberry-backup-ci.yml @@ -349,7 +349,22 @@ jobs: make end_to_end 2>&1 | tee "${TEST_LOG_ROOT}/cloudberry-backup-end_to_end.log" ;; s3_plugin_e2e) - curl -fsSL https://dl.min.io/server/minio/release/linux-amd64/minio -o /tmp/minio + # dl.min.io now returns HTTP 410: MinIO archived the + # open-source Server and Client and no longer serves them from + # that host. The final open-source releases are still + # published as GitHub release assets, so pin them there and + # verify the checksums MinIO publishes next to each binary. + # + # These are plain shell variables on purpose: anything + # exported with a MINIO_ prefix would be picked up as + # configuration by the server started below. + minio_release="RELEASE.2025-09-07T16-13-09Z" + minio_sha256="7c5bd8512c6e966455b1d198209358b2d191c77a83ab377c4073281065fb855f" + mc_release="RELEASE.2025-08-13T08-35-41Z" + mc_sha256="01f866e9c5f9b87c2b09116fa5d7c06695b106242d829a8bb32990c00312e891" + + curl -fsSL "https://github.com/minio/minio/releases/download/${minio_release}/minio.linux-amd64.${minio_release}" -o /tmp/minio + echo "${minio_sha256} /tmp/minio" | sha256sum -c - chmod +x /tmp/minio mkdir -p /tmp/minio-data /tmp/minio server --address ":9000" /tmp/minio-data > "${TEST_LOG_ROOT}/minio.log" 2>&1 & @@ -360,7 +375,8 @@ jobs: sleep 1 done - curl -fsSL https://dl.min.io/client/mc/release/linux-amd64/mc -o /tmp/mc + curl -fsSL "https://github.com/minio/mc/releases/download/${mc_release}/mc.linux-amd64.${mc_release}" -o /tmp/mc + echo "${mc_sha256} /tmp/mc" | sha256sum -c - chmod +x /tmp/mc /tmp/mc alias set local http://127.0.0.1:9000 minioadmin minioadmin /tmp/mc mb --ignore-existing local/cloudberry-backup-s3-test From 8cc854d6c999f92dc68549e7c326043769725ce2 Mon Sep 17 00:00:00 2001 From: Dianjin Wang Date: Tue, 15 Sep 2026 12:11:20 +0800 Subject: [PATCH 3/3] Remove license text from NOTICE Justin Mclean noted during the 2.2.0-rc1 IPMC vote that this NOTICE carries the full Apache-2.0 boilerplate, and that NOTICE is reserved for notifications that are legally required. The text came from upstream: github.com/greenplum-db/gpbackup-archive ships a NOTICE whose body is the VMware attribution followed by the Apache-2.0 grant boilerplate, and the whole file was copied in when the code was donated. Copying all of it is more than section 4(d) of the Apache License asks for. It requires "the attribution notices contained within such NOTICE file", not the file itself, and infra.apache.org/licensing-howto.html is explicit that a bundled dependency's NOTICE "must be analyzed and the relevant portions bubbled up", and that one should "keep NOTICE as brief and simple as possible" and "not add anything to NOTICE which is not legally required". So keep the part that is an attribution notice: This product includes software originally developed by VMware. Greenplum Database Backup Copyright 2017-Present VMware, Inc. or its affiliates. All Rights Reserved. and drop the license grant, which is not one. The Apache-2.0 terms continue to reach users through LICENSE and the source file headers, which is where they belong. Assisted-by: Claude Code --- NOTICE | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/NOTICE b/NOTICE index cf7f007c..a5e00dc2 100644 --- a/NOTICE +++ b/NOTICE @@ -9,17 +9,4 @@ The Apache Software Foundation (http://www.apache.org/). This product includes software originally developed by VMware. Greenplum Database Backup - Copyright 2017-Present VMware, Inc. or its affiliates. All Rights Reserved. - -Licensed 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.