From 41cd39faa098e9f238254b5c749767c986d0efe0 Mon Sep 17 00:00:00 2001 From: Georgy Shelkovy Date: Thu, 1 Aug 2024 18:50:38 +0500 Subject: [PATCH 1/2] Fix restore helper hides actual err The commit 4371a94 moved the LoopEnd label so that the err variable began to be overwritten and it will no longer be used below. To fix it, this patch renames the err variable returned by flushAndCloseRestoreWriter to errPipe. --- end_to_end/end_to_end_suite_test.go | 13 +++++++++++++ helper/restore_helper.go | 8 ++++---- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/end_to_end/end_to_end_suite_test.go b/end_to_end/end_to_end_suite_test.go index 8556c8754..feebe7d2b 100644 --- a/end_to_end/end_to_end_suite_test.go +++ b/end_to_end/end_to_end_suite_test.go @@ -2519,6 +2519,19 @@ LANGUAGE plpgsql NO SQL;`) Expect(string(output)).To(MatchRegexp(`Error loading data into table public.d: COPY d, line 1: "\d,d": ERROR: null value in column "c" violates not-null constraint`)) Expect(string(output)).To(MatchRegexp(`Error loading data into table public.e: COPY e, line 1: "\d,e": ERROR: null value in column "c" violates not-null constraint`)) Expect(string(output)).To(ContainSubstring(`Encountered 3 error(s) during table data restore`)) + homeDir := os.Getenv("HOME") + helperFiles, _ := path.Glob(path.Join(homeDir, "gpAdminLogs/gpbackup_helper_*")) + helperCommand := exec.Command("grep", "Error copying data", helperFiles[len(helperFiles)-1]) + helperOutput, _ := helperCommand.CombinedOutput() + Expect(string(helperOutput)).To(ContainSubstring(`Segment 2: Oid 16397, Batch 0: Error encountered: Error copying data: write`)) + Expect(string(helperOutput)).To(ContainSubstring(`Segment 0: Oid 16397, Batch 0: Error encountered: Error copying data: write`)) + Expect(string(helperOutput)).To(ContainSubstring(`Segment 1: Oid 16397, Batch 0: Error encountered: Error copying data: write`)) + Expect(string(helperOutput)).To(ContainSubstring(`Segment 2: Oid 16403, Batch 0: Error encountered: Error copying data: write`)) + Expect(string(helperOutput)).To(ContainSubstring(`Segment 0: Oid 16403, Batch 0: Error encountered: Error copying data: write`)) + Expect(string(helperOutput)).To(ContainSubstring(`Segment 1: Oid 16403, Batch 0: Error encountered: Error copying data: write`)) + Expect(string(helperOutput)).To(ContainSubstring(`Segment 2: Oid 16409, Batch 0: Error encountered: Error copying data: write`)) + Expect(string(helperOutput)).To(ContainSubstring(`Segment 0: Oid 16409, Batch 0: Error encountered: Error copying data: write`)) + Expect(string(helperOutput)).To(ContainSubstring(`Segment 1: Oid 16409, Batch 0: Error encountered: Error copying data: write`)) assertArtifactsCleaned("20240710143553") testhelper.AssertQueryRuns(restoreConn, "DROP TABLE a; DROP TABLE b; DROP TABLE c; DROP TABLE d; DROP TABLE e; DROP TABLE f; DROP TABLE g; DROP TABLE h;") }) diff --git a/helper/restore_helper.go b/helper/restore_helper.go index c188b6494..d49029bb7 100644 --- a/helper/restore_helper.go +++ b/helper/restore_helper.go @@ -348,9 +348,9 @@ func doRestoreAgent() error { LoopEnd: logInfo(fmt.Sprintf("Oid %d, Batch %d: Closing pipe %s", tableOid, batchNum, currentPipe)) - err = flushAndCloseRestoreWriter(currentPipe, tableOid) - if err != nil { - logVerbose(fmt.Sprintf("Oid %d, Batch %d: Failed to flush and close pipe: %s", tableOid, batchNum, err)) + errPipe := flushAndCloseRestoreWriter(currentPipe, tableOid) + if errPipe != nil { + logVerbose(fmt.Sprintf("Oid %d, Batch %d: Failed to flush and close pipe: %s", tableOid, batchNum, errPipe)) } logVerbose(fmt.Sprintf("Oid %d, Batch %d: End batch restore", tableOid, batchNum)) @@ -367,7 +367,7 @@ func doRestoreAgent() error { } logVerbose(fmt.Sprintf("Oid %d, Batch %d: Attempt to delete pipe %s", tableOid, batchNum, currentPipe)) - errPipe := deletePipe(currentPipe) + errPipe = deletePipe(currentPipe) if errPipe != nil { logError("Oid %d, Batch %d: Failed to remove pipe %s: %v", tableOid, batchNum, currentPipe, errPipe) } From 17a6b1e8ab6b6dce01590d778ccbc8c5e74197ed Mon Sep 17 00:00:00 2001 From: Georgy Shelkovy Date: Fri, 2 Aug 2024 10:06:56 +0500 Subject: [PATCH 2/2] fix test --- integration/helper_test.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/integration/helper_test.go b/integration/helper_test.go index 75914f3de..e518ca93b 100644 --- a/integration/helper_test.go +++ b/integration/helper_test.go @@ -372,11 +372,9 @@ options: err = helperCmd.Start() Expect(err).ToNot(HaveOccurred()) - for _, i := range []int{1, 3} { - contents, _ := ioutil.ReadFile(fmt.Sprintf("%s_%d_0", pipeFile, i)) - // Empty output - Expect(contents).To(Equal([]byte{})) - } + contents, _ := ioutil.ReadFile(fmt.Sprintf("%s_%d_0", pipeFile, 1)) + // Empty output + Expect(contents).To(Equal([]byte{})) err = helperCmd.Wait() Expect(err).To(HaveOccurred())