Skip to content

Commit 5e296ff

Browse files
SlachSlach
committed
refactoring execution UpdateBackupMetrics, to avoid context canceled error, fix #814 
1 parent e89354c commit 5e296ff

File tree

3 files changed

+52
-42
lines changed

3 files changed

+52
-42
lines changed

‎ChangeLog.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
# v2.4.21
2+
BUG FIXES
3+
- refactoring execution UpdateBackupMetrics, to avoid context canceled error, fix [814](https://github.com/Altinity/clickhouse-backup/issues/814)
4+
15
# v2.4.20
26
IMPROVEMENTS
37
- refactoring of `create` command to allow parallel execution of `FREEZE` and `UNFREEZE` and table level parallelization `object_disk.CopyObject`

‎pkg/server/server.go

Lines changed: 38 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@ func Run(cliCtx *cli.Context, cliApp *cli.App, configPath string, clickhouseBack
116116
}
117117

118118
go func() {
119-
if err := api.UpdateBackupMetrics(context.Background(), false); err != nil {
120-
log.Errorf("UpdateBackupMetrics return error: %v", err)
119+
if metricsErr := api.UpdateBackupMetrics(context.Background(), false); metricsErr != nil {
120+
log.Errorf("UpdateBackupMetrics return error: %v", metricsErr)
121121
}
122122
}()
123123

@@ -390,8 +390,8 @@ func (api *APIServer) actionsDeleteHandler(row status.ActionRow, args []string,
390390
}
391391
api.log.Info("DELETED")
392392
go func() {
393-
if err := api.UpdateBackupMetrics(context.Background(), args[1] == "local"); err != nil {
394-
api.log.Errorf("UpdateBackupMetrics return error: %v", err)
393+
if metricsErr := api.UpdateBackupMetrics(context.Background(), args[1] == "local"); metricsErr != nil {
394+
api.log.Errorf("UpdateBackupMetrics return error: %v", metricsErr)
395395
}
396396
}()
397397
actionsResults = append(actionsResults, actionsResultsRow{
@@ -417,7 +417,7 @@ func (api *APIServer) actionsAsyncCommandsHandler(command string, args []string,
417417
return
418418
}
419419
go func() {
420-
if err := api.UpdateBackupMetrics(context.Background(), command == "create" || command == "restore"); err != nil {
420+
if err := api.UpdateBackupMetrics(context.Background(), command == "create" || strings.HasPrefix(command, "restore") || command == "download"); err != nil {
421421
api.log.Errorf("UpdateBackupMetrics return error: %v", err)
422422
}
423423
}()
@@ -452,7 +452,7 @@ func (api *APIServer) actionsCleanRemoteBrokenHandler(w http.ResponseWriter, row
452452
api.log.Warn(ErrAPILocked.Error())
453453
return actionsResults, ErrAPILocked
454454
}
455-
commandId, ctx := status.Current.Start(command)
455+
commandId, _ := status.Current.Start(command)
456456
cfg, err := api.ReloadConfig(w, "clean_remote_broken")
457457
if err != nil {
458458
status.Current.Stop(commandId, err)
@@ -466,10 +466,11 @@ func (api *APIServer) actionsCleanRemoteBrokenHandler(w http.ResponseWriter, row
466466
return actionsResults, err
467467
}
468468
api.log.Info("CLEANED")
469-
metricsErr := api.UpdateBackupMetrics(ctx, false)
470-
if metricsErr != nil {
471-
api.log.Errorf("UpdateBackupMetrics return error: %v", metricsErr)
472-
}
469+
go func() {
470+
if metricsErr := api.UpdateBackupMetrics(context.Background(), false); metricsErr != nil {
471+
api.log.Errorf("UpdateBackupMetrics return error: %v", metricsErr)
472+
}
473+
}()
473474
status.Current.Stop(commandId, nil)
474475
actionsResults = append(actionsResults, actionsResultsRow{
475476
Status: "success",
@@ -857,7 +858,7 @@ func (api *APIServer) httpCreateHandler(w http.ResponseWriter, r *http.Request)
857858
return
858859
}
859860

860-
commandId, ctx := status.Current.Start(fullCommand)
861+
commandId, _ := status.Current.Start(fullCommand)
861862
go func() {
862863
err, _ := api.metrics.ExecuteWithMetrics("create", 0, func() error {
863864
b := backup.NewBackuper(cfg)
@@ -869,12 +870,12 @@ func (api *APIServer) httpCreateHandler(w http.ResponseWriter, r *http.Request)
869870
api.errorCallback(context.Background(), err, callback)
870871
return
871872
}
872-
if err := api.UpdateBackupMetrics(ctx, true); err != nil {
873-
api.log.Errorf("UpdateBackupMetrics return error: %v", err)
874-
status.Current.Stop(commandId, err)
875-
api.errorCallback(context.Background(), err, callback)
876-
return
877-
}
873+
go func() {
874+
if metricsErr := api.UpdateBackupMetrics(context.Background(), true); metricsErr != nil {
875+
api.log.Errorf("UpdateBackupMetrics return error: %v", metricsErr)
876+
}
877+
}()
878+
878879
status.Current.Stop(commandId, nil)
879880
api.successCallback(context.Background(), callback)
880881
}()
@@ -1011,7 +1012,7 @@ func (api *APIServer) httpCleanRemoteBrokenHandler(w http.ResponseWriter, _ *htt
10111012
if err != nil {
10121013
return
10131014
}
1014-
commandId, ctx := status.Current.Start("clean_remote_broken")
1015+
commandId, _ := status.Current.Start("clean_remote_broken")
10151016
defer status.Current.Stop(commandId, err)
10161017

10171018
b := backup.NewBackuper(cfg)
@@ -1021,13 +1022,11 @@ func (api *APIServer) httpCleanRemoteBrokenHandler(w http.ResponseWriter, _ *htt
10211022
api.writeError(w, http.StatusInternalServerError, "clean_remote_broken", err)
10221023
return
10231024
}
1024-
1025-
err = api.UpdateBackupMetrics(ctx, false)
1026-
if err != nil {
1027-
api.log.Errorf("Clean remote broken error: %v", err)
1028-
api.writeError(w, http.StatusInternalServerError, "clean_remote_broken", err)
1029-
return
1030-
}
1025+
go func() {
1026+
if metricsErr := api.UpdateBackupMetrics(context.Background(), false); metricsErr != nil {
1027+
api.log.Errorf("UpdateBackupMetrics return error: %v", metricsErr)
1028+
}
1029+
}()
10311030

10321031
api.sendJSONEachRow(w, http.StatusOK, struct {
10331032
Status string `json:"status"`
@@ -1094,8 +1093,8 @@ func (api *APIServer) httpUploadHandler(w http.ResponseWriter, r *http.Request)
10941093
return
10951094
}
10961095

1097-
commandId, ctx := status.Current.Start(fullCommand)
10981096
go func() {
1097+
commandId, ctx := status.Current.Start(fullCommand)
10991098
err, _ := api.metrics.ExecuteWithMetrics("upload", 0, func() error {
11001099
b := backup.NewBackuper(cfg)
11011100
return b.Upload(name, diffFrom, diffFromRemote, tablePattern, partitionsToBackup, schemaOnly, resume, commandId)
@@ -1106,12 +1105,11 @@ func (api *APIServer) httpUploadHandler(w http.ResponseWriter, r *http.Request)
11061105
api.errorCallback(context.Background(), err, callback)
11071106
return
11081107
}
1109-
if err := api.UpdateBackupMetrics(ctx, false); err != nil {
1110-
api.log.Errorf("UpdateBackupMetrics return error: %v", err)
1111-
status.Current.Stop(commandId, err)
1112-
api.errorCallback(context.Background(), err, callback)
1113-
return
1114-
}
1108+
go func() {
1109+
if metricsErr := api.UpdateBackupMetrics(context.Background(), false); metricsErr != nil {
1110+
api.log.Errorf("UpdateBackupMetrics return error: %v", metricsErr)
1111+
}
1112+
}()
11151113
status.Current.Stop(commandId, nil)
11161114
api.successCallback(context.Background(), callback)
11171115
}()
@@ -1289,8 +1287,8 @@ func (api *APIServer) httpDownloadHandler(w http.ResponseWriter, r *http.Request
12891287
return
12901288
}
12911289

1292-
commandId, ctx := status.Current.Start(fullCommand)
12931290
go func() {
1291+
commandId, _ := status.Current.Start(fullCommand)
12941292
err, _ := api.metrics.ExecuteWithMetrics("download", 0, func() error {
12951293
b := backup.NewBackuper(cfg)
12961294
return b.Download(name, tablePattern, partitionsToBackup, schemaOnly, resume, commandId)
@@ -1301,12 +1299,11 @@ func (api *APIServer) httpDownloadHandler(w http.ResponseWriter, r *http.Request
13011299
api.errorCallback(context.Background(), err, callback)
13021300
return
13031301
}
1304-
if err := api.UpdateBackupMetrics(ctx, true); err != nil {
1305-
api.log.Errorf("UpdateBackupMetrics return error: %v", err)
1306-
status.Current.Stop(commandId, err)
1307-
api.errorCallback(context.Background(), err, callback)
1308-
return
1309-
}
1302+
go func() {
1303+
if metricsErr := api.UpdateBackupMetrics(context.Background(), true); metricsErr != nil {
1304+
api.log.Errorf("UpdateBackupMetrics return error: %v", metricsErr)
1305+
}
1306+
}()
13101307
status.Current.Stop(commandId, nil)
13111308
api.successCallback(context.Background(), callback)
13121309
}()
@@ -1351,8 +1348,8 @@ func (api *APIServer) httpDeleteHandler(w http.ResponseWriter, r *http.Request)
13511348
return
13521349
}
13531350
go func() {
1354-
if err := api.UpdateBackupMetrics(context.Background(), vars["where"] == "local"); err != nil {
1355-
api.log.Errorf("UpdateBackupMetrics return error: %v", err)
1351+
if metricsErr := api.UpdateBackupMetrics(context.Background(), vars["where"] == "local"); metricsErr != nil {
1352+
api.log.Errorf("UpdateBackupMetrics return error: %v", metricsErr)
13561353
}
13571354
}()
13581355
api.sendJSONEachRow(w, http.StatusOK, struct {

‎test/integration/integration_test.go

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -848,6 +848,10 @@ func testAPIDeleteLocalDownloadRestore(r *require.Assertions) {
848848
log.Debug(out)
849849
r.NoError(err)
850850
r.NotContains(out, "another operation is currently running")
851+
r.NotContains(out, "error")
852+
853+
out, err = dockerExecOut("clickhouse-backup", "curl", "-sfL", "http://localhost:7171/backup/actions?filter=download")
854+
r.NoError(err)
851855
r.NotContains(out, "\"status\":\"error\"")
852856

853857
out, err = dockerExecOut("clickhouse-backup", "curl", "http://localhost:7171/metrics")
@@ -895,8 +899,13 @@ func testAPIBackupUpload(r *require.Assertions) {
895899
)
896900
log.Debug(out)
897901
r.NoError(err)
898-
r.NotContains(out, "\"status\":\"error\"")
902+
r.NotContains(out, "error")
899903
r.NotContains(out, "another operation is currently running")
904+
905+
out, err = dockerExecOut("clickhouse-backup", "curl", "-sfL", "http://localhost:7171/backup/actions?filter=upload")
906+
r.NoError(err)
907+
r.NotContains(out, "error")
908+
900909
out, err = dockerExecOut("clickhouse-backup", "curl", "http://localhost:7171/metrics")
901910
r.NoError(err)
902911
r.Contains(out, "clickhouse_backup_last_upload_status 1")

0 commit comments

Comments
 (0)