hotfix 2.2.8, fix #676 (comment)

Altinity · Jun 26, 2023 · f5b2196 · f5b2196
1 parent e512faa
commit f5b2196
Show file tree

Hide file tree

Showing 9 changed files with 26 additions and 17 deletions.
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -1,3 +1,7 @@
+# v2.2.8
+BUG FIXES
+- hotfix wrong empty files when disk_mapping contains not exists during create, affected 2.2.7 version, look details [676](https://github.com/Altinity/clickhouse-backup/issues/676#issue-1771732489)
+
 # v2.2.7
 IMPROVEMENTS
 - Auto-tuning concurrency and buffer size related parameters depending on remote storage type, fix [658](https://github.com/AlexAkulov/clickhouse-backup/issues/658)

diff --git a/pkg/backup/backuper.go b/pkg/backup/backuper.go
@@ -39,7 +39,7 @@ func NewBackuper(cfg *config.Config) *Backuper {
 func (b *Backuper) init(ctx context.Context, disks []clickhouse.Disk, backupName string) error {
 	var err error
 	if disks == nil {
-		disks, err = b.ch.GetDisks(ctx)
+		disks, err = b.ch.GetDisks(ctx, true)
 		if err != nil {
 			return err
 		}

diff --git a/pkg/backup/create.go b/pkg/backup/create.go
@@ -127,7 +127,7 @@ func (b *Backuper) CreateBackup(backupName, tablePattern string, partitions []st
 		return fmt.Errorf("GetUserDefinedFunctions return error: %v", err)
 	}
 
-	disks, err := b.ch.GetDisks(ctx)
+	disks, err := b.ch.GetDisks(ctx, false)
 	if err != nil {
 		return err
 	}

diff --git a/pkg/backup/delete.go b/pkg/backup/delete.go
@@ -25,7 +25,7 @@ func (b *Backuper) Clean(ctx context.Context) error {
 	}
 	defer b.ch.Close()
 
-	disks, err := b.ch.GetDisks(ctx)
+	disks, err := b.ch.GetDisks(ctx, true)
 	if err != nil {
 		return err
 	}
@@ -105,7 +105,7 @@ func (b *Backuper) RemoveBackupLocal(ctx context.Context, backupName string, dis
 	}
 	defer b.ch.Close()
 	if disks == nil {
-		disks, err = b.ch.GetDisks(ctx)
+		disks, err = b.ch.GetDisks(ctx, true)
 		if err != nil {
 			return err
 		}

diff --git a/pkg/backup/list.go b/pkg/backup/list.go
@@ -165,7 +165,7 @@ func (b *Backuper) GetLocalBackups(ctx context.Context, disks []clickhouse.Disk)
 	}
 	log := b.log.WithField("logger", "GetLocalBackups")
 	if disks == nil {
-		disks, err = b.ch.GetDisks(ctx)
+		disks, err = b.ch.GetDisks(ctx, true)
 		if err != nil {
 			return nil, nil, err
 		}
@@ -397,7 +397,7 @@ func (b *Backuper) PrintTables(printAll bool, tablePattern string) error {
 	if err != nil {
 		return err
 	}
-	disks, err := b.ch.GetDisks(ctx)
+	disks, err := b.ch.GetDisks(ctx, false)
 	if err != nil {
 		return err
 	}

diff --git a/pkg/backup/restore.go b/pkg/backup/restore.go
@@ -55,7 +55,7 @@ func (b *Backuper) Restore(backupName, tablePattern string, databaseMapping, par
 		_ = b.PrintLocalBackups(ctx, "all")
 		return fmt.Errorf("select backup for restore")
 	}
-	disks, err := b.ch.GetDisks(ctx)
+	disks, err := b.ch.GetDisks(ctx, true)
 	if err != nil {
 		return err
 	}

diff --git a/pkg/clickhouse/clickhouse.go b/pkg/clickhouse/clickhouse.go
@@ -132,7 +132,7 @@ func (ch *ClickHouse) Connect() error {
 }
 
 // GetDisks - return data from system.disks table
-func (ch *ClickHouse) GetDisks(ctx context.Context) ([]Disk, error) {
+func (ch *ClickHouse) GetDisks(ctx context.Context, enrich bool) ([]Disk, error) {
 	version, err := ch.GetVersion(ctx)
 	if err != nil {
 		return nil, err
@@ -164,12 +164,14 @@ func (ch *ClickHouse) GetDisks(ctx context.Context) ([]Disk, error) {
 			delete(dm, disks[i].Name)
 		}
 	}
-	for k, v := range dm {
-		disks = append(disks, Disk{
-			Name: k,
-			Path: v,
-			Type: "local",
-		})
+	if enrich {
+		for k, v := range dm {
+			disks = append(disks, Disk{
+				Name: k,
+				Path: v,
+				Type: "local",
+			})
+		}
 	}
 	return disks, nil
 }
@@ -1018,7 +1020,7 @@ func (ch *ClickHouse) GetAccessManagementPath(ctx context.Context, disks []Disk)
 	var rows []string
 	if err := ch.SelectContext(ctx, &rows, "SELECT JSONExtractString(params,'path') AS access_path FROM system.user_directories WHERE type='local directory'"); err != nil || len(rows) == 0 {
 		if disks == nil {
-			disks, err = ch.GetDisks(ctx)
+			disks, err = ch.GetDisks(ctx, false)
 			if err != nil {
 				return "", err
 			}

diff --git a/pkg/server/server.go b/pkg/server/server.go
@@ -1542,7 +1542,7 @@ func (api *APIServer) ResumeOperationsAfterRestart() error {
 			api.log.Errorf("ResumeOperationsAfterRestart can't close clickhouse connection: %v", err)
 		}
 	}()
-	disks, err := ch.GetDisks(context.Background())
+	disks, err := ch.GetDisks(context.Background(), true)
 	if err != nil {
 		return err
 	}

diff --git a/test/integration/config-s3.yml b/test/integration/config-s3.yml
@@ -10,14 +10,17 @@ general:
     - "_temporary_and_external_tables.*"
   restore_schema_on_cluster: "{cluster}"
 clickhouse:
+  # wrong disk name mapping for https://github.com/Altinity/clickhouse-backup/issues/676
+  disk_mapping:
+    default-gp3: /var/lib/clickhouse
   host: 127.0.0.1
   port: 9440
   username: backup
   password: meow=& 123?*%# МЯУ
   secure: true
   skip_verify: true
   sync_replicated_tables: true
-  timeout: 1s
+  timeout: 2s
   restart_command: bash -c 'echo "FAKE RESTART"'
   backup_mutations: true
 s3: