Skip to content

Commit

Permalink
fixed Unknown setting base_backup for `use_embedded_backup_restore:…
Browse files Browse the repository at this point in the history
… true` and `create --diff-from-remote`, affected 2.5.0+ versions, fix #735
  • Loading branch information
Slach committed Apr 24, 2024
1 parent 5b53513 commit 15b6e7a
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 21 deletions.
4 changes: 4 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# v2.5.3
BUG FIXES
- fixed `Unknown setting base_backup` for `use_embedded_backup_restore: true` and `create --diff-from-remote`, affected 2.5.0+ versions, fix [735](https://github.com/Altinity/clickhouse-backup/issues/735)

# v2.5.2
BUG FIXES
- fixed issue after [865](https://github.com/Altinity/clickhouse-backup/pull/865) we can't use `create_remote --diff-from-remote` for `remote_storage: custom`, affected versions 2.5.0, 2.5.1, fix [900](https://github.com/Altinity/clickhouse-backup/issue/900)
Expand Down
6 changes: 5 additions & 1 deletion pkg/backup/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -508,7 +508,11 @@ func (b *Backuper) generateEmbeddedBackupSQL(ctx context.Context, backupName str
}
// incremental native backup https://github.com/Altinity/clickhouse-backup/issues/735
if baseBackup != "" {
backupSettings = append(backupSettings, fmt.Sprintf("base_backup='%s'", baseBackup))
baseBackup, err = b.getEmbeddedBackupLocation(ctx, baseBackup)
if err != nil {
return "", "", err
}
backupSettings = append(backupSettings, "base_backup="+baseBackup)
}
if len(backupSettings) > 0 {
backupSQL += " SETTINGS " + strings.Join(backupSettings, ", ")
Expand Down
39 changes: 19 additions & 20 deletions test/integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1479,7 +1479,7 @@ func TestKeepBackupRemoteAndDiffFromRemote(t *testing.T) {
}
out, err := dockerExecOut("clickhouse-backup", "bash", "-ce", "clickhouse-backup -c /etc/clickhouse-backup/config-s3.yml list local")
r.NoError(err)
// shall not delete any backup, cause all deleted backup have links as required in other backups
// shall not delete any backup, cause all deleted backups have links as required in other backups
for _, backupName := range backupNames {
r.Contains(out, backupName)
r.NoError(dockerExec("clickhouse-backup", "clickhouse-backup", "-c", "/etc/clickhouse-backup/config-s3.yml", "delete", "local", backupName))
Expand Down Expand Up @@ -2002,11 +2002,11 @@ func TestIntegrationEmbedded(t *testing.T) {
//t.Parallel()
r := require.New(t)
if compareVersion(version, "24.3") >= 0 {
//CUSTOM backup create folder in each disk, need to clear
//CUSTOM backup creates folder in each disk, need to clear
r.NoError(dockerExec("clickhouse", "rm", "-rfv", "/var/lib/clickhouse/disks/backups_local/backup/"))
runMainIntegrationScenario(t, "EMBEDDED_LOCAL", "config-s3-embedded-local.yml")
}
//CUSTOM backup create folder in each disk, need to clear
//CUSTOM backup creates folder in each disk, need to clear
r.NoError(dockerExec("clickhouse", "rm", "-rfv", "/var/lib/clickhouse/disks/backups_s3/backup/"))
runMainIntegrationScenario(t, "EMBEDDED_S3", "config-s3-embedded.yml")

Expand Down Expand Up @@ -2303,7 +2303,7 @@ func runMainIntegrationScenario(t *testing.T, remoteStorageType, backupConfig st

// test end
log.Info("Clean after finish")
// during download increment, partially downloaded full will clean
// during download increment, partially downloaded full will clean
fullCleanup(t, r, ch, []string{incrementBackupName}, []string{"local"}, nil, true, false, backupConfig)
fullCleanup(t, r, ch, []string{testBackupName, incrementBackupName}, []string{"remote"}, databaseList, true, true, backupConfig)
replaceStorageDiskNameForReBalance(r, ch, remoteStorageType, true)
Expand Down Expand Up @@ -2418,7 +2418,7 @@ func testBackupSpecifiedPartitions(t *testing.T, r *require.Assertions, ch *Test
r.NoError(dockerExec("clickhouse-backup", "clickhouse-backup", "-c", "/etc/clickhouse-backup/"+backupConfig, "delete", "local", fullBackupName))
r.NoError(dockerExec("clickhouse-backup", "clickhouse-backup", "-c", "/etc/clickhouse-backup/"+backupConfig, "download", "--partitions=(0,'2022-01-02'),(0,'2022-01-03')", fullBackupName))
fullBackupDir := "/var/lib/clickhouse/backup/" + fullBackupName + "/shadow/" + dbName + "/t?/default/"
// embedded storage with embedded disks contain object disk files and will download additional data parts
// embedded storage with embedded disks contains object disk files and will download additional data parts
if strings.HasPrefix(remoteStorageType, "EMBEDDED") {
fullBackupDir = "/var/lib/clickhouse/disks/backups" + strings.ToLower(strings.TrimPrefix(remoteStorageType, "EMBEDDED")) + "/" + fullBackupName + "/data/" + dbName + "/t?"
}
Expand Down Expand Up @@ -2464,7 +2464,7 @@ func testBackupSpecifiedPartitions(t *testing.T, r *require.Assertions, ch *Test

expectedLines = "17"
fullBackupDir = "/var/lib/clickhouse/backup/" + fullBackupName + "/shadow/" + dbName + "/t?/default/"
// embedded storage with embedded disks contain hardLink files and will download additional data parts
// embedded storage with embedded disks contains hardLink files and will download additional data parts
if strings.HasPrefix(remoteStorageType, "EMBEDDED") {
fullBackupDir = "/var/lib/clickhouse/disks/backups" + strings.ToLower(strings.TrimPrefix(remoteStorageType, "EMBEDDED")) + "/" + fullBackupName + "/data/" + dbName + "/t?"
}
Expand Down Expand Up @@ -2495,7 +2495,7 @@ func testBackupSpecifiedPartitions(t *testing.T, r *require.Assertions, ch *Test
if strings.HasPrefix(remoteStorageType, "EMBEDDED") && !strings.HasSuffix(remoteStorageType, "_URL") {
partitionBackupDir = "/var/lib/clickhouse/disks/backups" + strings.ToLower(strings.TrimPrefix(remoteStorageType, "EMBEDDED")) + "/" + partitionBackupName + "/data/" + dbName + "/t1"
}
//embedded backup without disk have only local metadata
//embedded backup without a disk has only local metadata
if strings.HasPrefix(remoteStorageType, "EMBEDDED") && strings.HasSuffix(remoteStorageType, "_URL") {
partitionBackupDir = "/var/lib/clickhouse/backup/" + partitionBackupName + "/metadata/" + dbName + "/t?.json"
expectedLines = "1"
Expand All @@ -2512,7 +2512,7 @@ func testBackupSpecifiedPartitions(t *testing.T, r *require.Assertions, ch *Test
if strings.HasPrefix(remoteStorageType, "EMBEDDED") && !strings.HasSuffix(remoteStorageType, "_URL") {
partitionBackupDir = "/var/lib/clickhouse/disks/backups" + strings.ToLower(strings.TrimPrefix(remoteStorageType, "EMBEDDED")) + "/" + partitionBackupName + "/data/" + dbName + "/t1"
}
//embedded backup without disk have only local metadata
//embedded backup without a disk has only local metadata
if strings.HasPrefix(remoteStorageType, "EMBEDDED") && strings.HasSuffix(remoteStorageType, "_URL") {
partitionBackupDir = "/var/lib/clickhouse/backup/" + partitionBackupName + "/metadata/" + dbName + "/t?.json"
expectedLines = "1"
Expand Down Expand Up @@ -2573,7 +2573,6 @@ func checkResumeAlreadyProcessed(backupCmd, testBackupName, resumeKind string, r
}

func fullCleanup(t *testing.T, r *require.Assertions, ch *TestClickHouse, backupNames, backupTypes, databaseList []string, checkDeleteErr, checkDropErr bool, backupConfig string) {
//WTF? why table switched to readonly during drop?
for _, backupName := range backupNames {
for _, backupType := range backupTypes {
err := dockerExec("clickhouse-backup", "bash", "-xce", "clickhouse-backup -c /etc/clickhouse-backup/"+backupConfig+" delete "+backupType+" "+backupName)
Expand Down Expand Up @@ -2977,17 +2976,17 @@ func (ch *TestClickHouse) queryWithNoError(r *require.Assertions, query string,

var dockerExecTimeout = 180 * time.Second

func dockerExecBackground(container string, cmd ...string) error {
out, err := dockerExecBackgroundOut(container, cmd...)
log.Info(out)
return err
}

func dockerExecBackgroundOut(container string, cmd ...string) (string, error) {
dcmd := []string{"exec", "-d", container}
dcmd = append(dcmd, cmd...)
return utils.ExecCmdOut(context.Background(), dockerExecTimeout, "docker", dcmd...)
}
//func dockerExecBackground(container string, cmd ...string) error {
// out, err := dockerExecBackgroundOut(container, cmd...)
// log.Info(out)
// return err
//}

//func dockerExecBackgroundOut(container string, cmd ...string) (string, error) {
// dcmd := []string{"exec", "-d", container}
// dcmd = append(dcmd, cmd...)
// return utils.ExecCmdOut(context.Background(), dockerExecTimeout, "docker", dcmd...)
//}

func dockerExec(container string, cmd ...string) error {
out, err := dockerExecOut(container, cmd...)
Expand Down

0 comments on commit 15b6e7a

Please sign in to comment.