Merge branch 'Altinity:master' into sharded-backup

Altinity · Jul 25, 2023 · 23ffab2 · 23ffab2
2 parents fb93d4a + b0df795
commit 23ffab2
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 12 deletions.
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -1,3 +1,10 @@
+# v2.4.0 (not released yet)
+IMPROVEMENTS
+- first implementation for properly backup S3/GCS/Azure disks, support server-side copy to back up bucket during `clickhouse-backup` create and during `clickhouse-backup restore`, requires add `object_disk_path` to `s3`,`gcs`,`azblob` section, fix [447](https://github.com/Altinity/clickhouse-backup/issues/447)
+
+BUG FIXES
+- fix possible create backup failures during UNFREEZE not exists tables, affected 2.2.7+ version, fix [704](https://github.com/Altinity/clickhouse-backup/issues/704)
+
 # v2.3.2
 BUG FIXES
 - fix error when `backups_to_keep_local: -1`, fix [698](https://github.com/Altinity/clickhouse-backup/issues/698)

diff --git a/pkg/backup/create.go b/pkg/backup/create.go
@@ -545,7 +545,12 @@ func (b *Backuper) AddTableToBackup(ctx context.Context, backupName, shadowBacku
 	// Unfreeze to unlock data on S3 disks, https://github.com/Altinity/clickhouse-backup/issues/423
 	if version > 21004000 {
 		if err := b.ch.QueryContext(ctx, fmt.Sprintf("ALTER TABLE `%s`.`%s` UNFREEZE WITH NAME '%s'", table.Database, table.Name, shadowBackupUUID)); err != nil {
-			return disksToPartsMap, realSize, err
+			if (strings.Contains(err.Error(), "code: 60") || strings.Contains(err.Error(), "code: 81")) && b.cfg.ClickHouse.IgnoreNotExistsErrorDuringFreeze {
+				b.ch.Log.Warnf("can't unfreeze table: %v", err)
+			} else {
+				return disksToPartsMap, realSize, err
+			}
+
 		}
 	}
 	if b.dst != nil {

diff --git a/test/integration/integration_test.go b/test/integration/integration_test.go
@@ -1383,7 +1383,7 @@ func TestTablePatterns(t *testing.T) {
 }
 
 func TestSkipNotExistsTable(t *testing.T) {
-	t.Skip("TestSkipNotExistsTable is flaky now, need more precise algorithm for pause calculation")
+	//t.Skip("TestSkipNotExistsTable is flaky now, need more precise algorithm for pause calculation")
 	ch := &TestClickHouse{}
 	r := require.New(t)
 	ch.connectWithWait(r, 0*time.Second, 1*time.Second)
@@ -1395,10 +1395,10 @@ func TestSkipNotExistsTable(t *testing.T) {
 	chVersion, err := ch.chbackend.GetVersion(context.Background())
 	r.NoError(err)
 
-	errorCaught := false
+	freezeErrorHandled := false
 	pauseChannel := make(chan int64)
 	resumeChannel := make(chan int64)
-
+	ch.chbackend.Config.LogSQLQueries = true
 	wg := sync.WaitGroup{}
 	wg.Add(1)
 	go func() {
@@ -1407,28 +1407,48 @@ func TestSkipNotExistsTable(t *testing.T) {
 			wg.Done()
 		}()
 		pause := int64(0)
-		pausePercent := int64(93)
+		// pausePercent := int64(90)
 		for i := 0; i < 100; i++ {
 			testBackupName := fmt.Sprintf("not_exists_%d", i)
 			err = ch.chbackend.Query(ifNotExistsCreateSQL)
 			r.NoError(err)
 			err = ch.chbackend.Query(ifNotExistsInsertSQL)
 			r.NoError(err)
+			log.Infof("pauseChannel <- %d", pause)
 			pauseChannel <- pause
 			startTime := time.Now()
 			out, err := dockerExecOut("clickhouse", "bash", "-ce", "LOG_LEVEL=debug clickhouse-backup create --table default.if_not_exists "+testBackupName)
-			pause = time.Since(startTime).Nanoseconds() * pausePercent / 100
 			log.Info(out)
+			if (err != nil && (strings.Contains(out, "can't freeze") || strings.Contains(out, "no tables for backup"))) ||
+				(err == nil && !strings.Contains(out, "can't freeze")) {
+				parseTime := func(line string) time.Time {
+					parsedTime, err := time.Parse("2006/01/02 15:04:05.999999", line[:26])
+					if err != nil {
+						r.Failf("%s, Error parsing time: %v", line, err)
+					}
+					return parsedTime
+				}
+				lines := strings.Split(out, "\n")
+				firstTime := parseTime(lines[0])
+				var freezeTime time.Time
+				for _, line := range lines {
+					if strings.Contains(line, "create_table_query") {
+						freezeTime = parseTime(line)
+						break
+					}
+				}
+				pause = (firstTime.Sub(startTime) + freezeTime.Sub(firstTime) + (10 * time.Microsecond)).Nanoseconds()
+			}
 			if err != nil {
 				if !strings.Contains(out, "no tables for backup") {
 					assert.NoError(t, err)
-				} else {
+				} /* else {
 					pausePercent += 1
-				}
+				} */
 			}
 
-			if strings.Contains(out, "warn") && strings.Contains(out, "can't freeze") && strings.Contains(out, "code: 60") && err == nil {
-				errorCaught = true
+			if strings.Contains(out, "code: 60") && err == nil {
+				freezeErrorHandled = true
 				<-resumeChannel
 				r.NoError(dockerExec("clickhouse", "clickhouse-backup", "delete", "local", testBackupName))
 				break
@@ -1447,17 +1467,19 @@ func TestSkipNotExistsTable(t *testing.T) {
 			wg.Done()
 		}()
 		for pause := range pauseChannel {
+			log.Infof("%d <- pauseChannel", pause)
 			if pause > 0 {
+				pauseStart := time.Now()
 				time.Sleep(time.Duration(pause) * time.Nanosecond)
-				log.Infof("pause=%s", time.Duration(pause).String())
+				log.Infof("pause=%s pauseStart=%s", time.Duration(pause).String(), pauseStart.String())
 				err = ch.chbackend.DropTable(clickhouse.Table{Database: "default", Name: "if_not_exists"}, ifNotExistsCreateSQL, "", false, chVersion)
 				r.NoError(err)
 			}
 			resumeChannel <- 1
 		}
 	}()
 	wg.Wait()
-	r.True(errorCaught)
+	r.True(freezeErrorHandled)
 }
 
 func TestProjections(t *testing.T) {