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

miaoliyao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/shardingsphere-on-cloud.git


The following commit(s) were added to refs/heads/main by this push:
     new e4c7276  chore: delete backup file when backup failed.
     new 766de49  Merge pull request #413 from Xu-Wentao/pitr
e4c7276 is described below

commit e4c7276caa12e864f089f209fb62f9795156c890
Author: xuwentao <[email protected]>
AuthorDate: Tue Jun 13 14:14:36 2023 +0800

    chore: delete backup file when backup failed.
---
 pitr/agent/internal/handler/restore.go       | 15 +++++++++++----
 pitr/cli/internal/cmd/backup.go              |  8 ++++++--
 pitr/cli/internal/cmd/backup_test.go         | 23 +++++++++++++++++++++--
 pitr/cli/internal/cmd/restore.go             |  4 ++++
 pitr/cli/internal/cmd/root.go                |  4 ++--
 pitr/cli/internal/pkg/local-storage.go       |  9 +++++++++
 pitr/cli/internal/pkg/mocks/local-storage.go | 16 +++++++++++++++-
 7 files changed, 68 insertions(+), 11 deletions(-)

diff --git a/pitr/agent/internal/handler/restore.go 
b/pitr/agent/internal/handler/restore.go
index c0e2326..88cd933 100644
--- a/pitr/agent/internal/handler/restore.go
+++ b/pitr/agent/internal/handler/restore.go
@@ -53,6 +53,7 @@ func Restore(ctx *fiber.Ctx) (err error) {
                err = fmt.Errorf("stop openGauss failure,err=%w", err)
                return
        }
+
        defer func() {
                if err != nil {
                        err2 := pkg.OG.Start()
@@ -69,16 +70,22 @@ func Restore(ctx *fiber.Ctx) (err error) {
                return
        }
 
+       var status = "restoring"
+       defer func() {
+               if status != "restore success" {
+                       err2 := pkg.OG.MvTempToPgData()
+                       err = fmt.Errorf("resotre 
failre[err=%s],pkg.OG.MvTempToPgData return err=%w", err, err2)
+               }
+       }()
+
        // restore data from backup
        if err = pkg.OG.Restore(in.DnBackupPath, in.Instance, in.DnBackupID); 
err != nil {
                efmt := "pkg.OG.Restore 
failure[path=%s,instance=%s,backupID=%s],err=%w"
                err = fmt.Errorf(efmt, in.DnBackupPath, in.Instance, 
in.DnBackupID, err)
-
-               err2 := pkg.OG.MvTempToPgData()
-               err = fmt.Errorf("resotre failre[err=%s],pkg.OG.MvTempToPgData 
return err=%w", err, err2)
-
+               status = "restore failure"
                return
        }
+       status = "restore success"
 
        // clean temp
        if err = pkg.OG.CleanPgDataTemp(); err != nil {
diff --git a/pitr/cli/internal/cmd/backup.go b/pitr/cli/internal/cmd/backup.go
index 9a2249e..ea2a4fe 100644
--- a/pitr/cli/internal/cmd/backup.go
+++ b/pitr/cli/internal/cmd/backup.go
@@ -124,7 +124,7 @@ func backup() error {
 
                        if lsBackup != nil {
                                logging.Warn("Try to delete backup data ...")
-                               deleteBackupFiles(lsBackup)
+                               deleteBackupFiles(ls, lsBackup)
                        }
                }
        }()
@@ -424,7 +424,7 @@ func doCheck(as pkg.IAgentServer, sn *model.StorageNode, 
backupID string, retrie
        return backupInfo.Status, nil
 }
 
-func deleteBackupFiles(lsBackup *model.LsBackup) {
+func deleteBackupFiles(ls pkg.ILocalStorage, lsBackup *model.LsBackup) {
        var (
                dataNodeMap = make(map[string]*model.DataNode)
                totalNum    = len(lsBackup.SsBackup.StorageNodes)
@@ -479,6 +479,10 @@ func deleteBackupFiles(lsBackup *model.LsBackup) {
 
        t.Render()
 
+       if err := ls.DeleteByName(filename); err != nil {
+               logging.Warn("Delete backup info file failed")
+       }
+
        logging.Info("Delete backup files finished")
 }
 
diff --git a/pitr/cli/internal/cmd/backup_test.go 
b/pitr/cli/internal/cmd/backup_test.go
index e36ea29..2ddab5c 100644
--- a/pitr/cli/internal/cmd/backup_test.go
+++ b/pitr/cli/internal/cmd/backup_test.go
@@ -402,6 +402,10 @@ var _ = Describe("test backup mock", func() {
        })
 
        Context("test delete backup data", func() {
+               var (
+                       ls *mock_pkg.MockILocalStorage
+               )
+
                bak := &model.LsBackup{
                        Info: nil,
                        DnList: []*model.DataNode{
@@ -419,8 +423,22 @@ var _ = Describe("test backup mock", func() {
                                },
                        },
                }
+               BeforeEach(func() {
+                       ctrl = gomock.NewController(GinkgoT())
+                       ls = mock_pkg.NewMockILocalStorage(ctrl)
+
+                       monkey.Patch(pkg.NewLocalStorage, func(rootDir string) 
(pkg.ILocalStorage, error) {
+                               return ls, nil
+                       })
+               })
+               AfterEach(func() {
+                       monkey.UnpatchAll()
+                       ctrl.Finish()
+               })
+
                It("should delete failed", func() {
-                       deleteBackupFiles(bak)
+                       
ls.EXPECT().DeleteByName(gomock.Any()).Return(errors.New("failed"))
+                       deleteBackupFiles(ls, bak)
                })
 
                It("should delete success", func() {
@@ -433,7 +451,8 @@ var _ = Describe("test backup mock", func() {
                        defer monkey.UnpatchAll()
                        defer ctrl.Finish()
                        as.EXPECT().DeleteBackup(gomock.Any()).Return(nil)
-                       deleteBackupFiles(bak)
+                       ls.EXPECT().DeleteByName(gomock.Any()).Return(nil)
+                       deleteBackupFiles(ls, bak)
                })
        })
 })
diff --git a/pitr/cli/internal/cmd/restore.go b/pitr/cli/internal/cmd/restore.go
index e669db7..0813e95 100644
--- a/pitr/cli/internal/cmd/restore.go
+++ b/pitr/cli/internal/cmd/restore.go
@@ -234,6 +234,10 @@ func execRestore(lsBackup *model.LsBackup) error {
 
        t.Render()
 
+       if restoreFinalStatus == "Failed" {
+               return xerr.NewCliErr("restore failed, please check the log for 
more details.")
+       }
+
        return nil
 }
 
diff --git a/pitr/cli/internal/cmd/root.go b/pitr/cli/internal/cmd/root.go
index 7deb386..87a67a4 100644
--- a/pitr/cli/internal/cmd/root.go
+++ b/pitr/cli/internal/cmd/root.go
@@ -107,10 +107,10 @@ func checkAgentServerStatus(lsBackup *model.LsBackup) 
bool {
                        Password: sn.Password,
                }
                if err := as.CheckStatus(in); err != nil {
-                       statusList = append(statusList, 
&model.AgentServerStatus{IP: sn.IP, Port: sn.Port, Status: "Unavailable"})
+                       statusList = append(statusList, 
&model.AgentServerStatus{IP: sn.IP, Port: AgentPort, Status: "Unavailable"})
                        available = false
                } else {
-                       statusList = append(statusList, 
&model.AgentServerStatus{IP: sn.IP, Port: sn.Port, Status: "Available"})
+                       statusList = append(statusList, 
&model.AgentServerStatus{IP: sn.IP, Port: AgentPort, Status: "Available"})
                }
        }
 
diff --git a/pitr/cli/internal/pkg/local-storage.go 
b/pitr/cli/internal/pkg/local-storage.go
index 596fd6c..2bc8227 100644
--- a/pitr/cli/internal/pkg/local-storage.go
+++ b/pitr/cli/internal/pkg/local-storage.go
@@ -43,6 +43,7 @@ type (
                ReadAll() ([]*model.LsBackup, error)
                ReadByID(id string) (*model.LsBackup, error)
                ReadByCSN(csn string) (*model.LsBackup, error)
+               DeleteByName(name string) error
        }
 
        Extension string
@@ -212,3 +213,11 @@ func (ls *localStorage) GenFilename(extn Extension) string 
{
                return fmt.Sprintf("%s_%s", prefix, suffix)
        }
 }
+
+func (ls *localStorage) DeleteByName(name string) error {
+       path := fmt.Sprintf("%s/%s", ls.backupDir, name)
+       if err := os.Remove(path); err != nil {
+               return xerr.NewCliErr(fmt.Sprintf("delete file failed,err=%s", 
err))
+       }
+       return nil
+}
diff --git a/pitr/cli/internal/pkg/mocks/local-storage.go 
b/pitr/cli/internal/pkg/mocks/local-storage.go
index aa18261..9284834 100644
--- a/pitr/cli/internal/pkg/mocks/local-storage.go
+++ b/pitr/cli/internal/pkg/mocks/local-storage.go
@@ -16,7 +16,7 @@
  */
 
 // Code generated by MockGen. DO NOT EDIT.
-// Source: internal/pkg/local-storage.go
+// Source: local-storage.go
 
 // Package mock_pkg is a generated GoMock package.
 package mock_pkg
@@ -52,6 +52,20 @@ func (m *MockILocalStorage) EXPECT() 
*MockILocalStorageMockRecorder {
        return m.recorder
 }
 
+// DeleteByName mocks base method.
+func (m *MockILocalStorage) DeleteByName(name string) error {
+       m.ctrl.T.Helper()
+       ret := m.ctrl.Call(m, "DeleteByName", name)
+       ret0, _ := ret[0].(error)
+       return ret0
+}
+
+// DeleteByName indicates an expected call of DeleteByName.
+func (mr *MockILocalStorageMockRecorder) DeleteByName(name interface{}) 
*gomock.Call {
+       mr.mock.ctrl.T.Helper()
+       return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteByName", 
reflect.TypeOf((*MockILocalStorage)(nil).DeleteByName), name)
+}
+
 // GenFilename mocks base method.
 func (m *MockILocalStorage) GenFilename(extn pkg.Extension) string {
        m.ctrl.T.Helper()

Reply via email to