Commit 2cc61da3 authored by Kirill Smelkov's avatar Kirill Smelkov

pull: Don't leave backup repository locked on error

On pull git-backup locks backup repository to make sure another
concurrent `git-backup pull` process is not running. However until now,
if a pull was failing, the lock was left unreleased, which made followup
pull attempts to fail while acquiring the lock until the lock was
manually removed with `git update-ref -d ...`. Probably originally I
made it like this in 6f237f22 (git-backup: Initial draft) to make sure
that if there is a problem it does not go unnoticed and forces me to
investigate. But in general we do _not_ need to keep the lock on error
return after `git-backup pull` completes even abnormally.

This "lock left unreleased" is causing operational issues on
lab.nexedi.com from time to time: if a pull try fails for some, even
temporary, reason, all next pull tries will fail until a human intervene
and remove the lock ref.

Fix it.

See also: kirr/git-backup!4
parent 9791c04e
// Copyright (C) 2015-2016 Nexedi SA and Contributors. // Copyright (C) 2015-2020 Nexedi SA and Contributors.
// Kirill Smelkov <kirr@nexedi.com> // Kirill Smelkov <kirr@nexedi.com>
// //
// This program is free software: you can Use, Study, Modify and Redistribute // This program is free software: you can Use, Study, Modify and Redistribute
...@@ -371,10 +371,11 @@ func cmd_pull_(gb *git.Repository, pullspecv []PullSpec) { ...@@ -371,10 +371,11 @@ func cmd_pull_(gb *git.Repository, pullspecv []PullSpec) {
// unique work refs namespace. // unique work refs namespace.
backup_time := time.Now().Format("20060102-1504") // %Y%m%d-%H%M backup_time := time.Now().Format("20060102-1504") // %Y%m%d-%H%M
backup_refs_work := fmt.Sprintf("refs/backup/%s/", backup_time) // refs/backup/20150820-2109/ backup_refs_work := fmt.Sprintf("refs/backup/%s/", backup_time) // refs/backup/20150820-2109/
backup_lock := "refs/backup.locked"
// make sure another `git-backup pull` is not running // prevent another `git-backup pull` from running simultaneously
backup_lock := "refs/backup.locked"
xgit("update-ref", backup_lock, mktree_empty(), Sha1{}) xgit("update-ref", backup_lock, mktree_empty(), Sha1{})
defer xgit("update-ref", "-d", backup_lock)
// make sure there is root commit // make sure there is root commit
var HEAD Sha1 var HEAD Sha1
...@@ -654,9 +655,6 @@ func cmd_pull_(gb *git.Repository, pullspecv []PullSpec) { ...@@ -654,9 +655,6 @@ func cmd_pull_(gb *git.Repository, pullspecv []PullSpec) {
infof("%s", diffstat) infof("%s", diffstat)
} }
} }
// we are done - unlock
xgit("update-ref", "-d", backup_lock)
} }
// fetch makes sure all objects from a repository are present in backup place. // fetch makes sure all objects from a repository are present in backup place.
......
// Copyright (C) 2015-2016 Nexedi SA and Contributors. // Copyright (C) 2015-2020 Nexedi SA and Contributors.
// Kirill Smelkov <kirr@nexedi.com> // Kirill Smelkov <kirr@nexedi.com>
// //
// This program is free software: you can Use, Study, Modify and Redistribute // This program is free software: you can Use, Study, Modify and Redistribute
...@@ -68,6 +68,11 @@ func xgittype(s string) git.ObjectType { ...@@ -68,6 +68,11 @@ func xgittype(s string) git.ObjectType {
return type_ return type_
} }
// xnoref asserts that git reference ref does not exists.
func xnoref(ref string) {
xgit("update-ref", "--stdin", RunWith{stdin: fmt.Sprintf("verify refs/%s %s\n", ref, Sha1{})})
}
// verify end-to-end pull-restore // verify end-to-end pull-restore
func TestPullRestore(t *testing.T) { func TestPullRestore(t *testing.T) {
...@@ -292,8 +297,8 @@ func TestPullRestore(t *testing.T) { ...@@ -292,8 +297,8 @@ func TestPullRestore(t *testing.T) {
defer exc.Catch(func(e *exc.Error) { defer exc.Catch(func(e *exc.Error) {
// it ok - pull should raise // it ok - pull should raise
// git-backup leaves backup repo locked on error // git-backup should not leave backup repo locked on error
xgit("update-ref", "-d", "refs/backup.locked") xnoref("backup.locked")
}) })
cmd_pull(gb, []string{my2 + ":b2"}) cmd_pull(gb, []string{my2 + ":b2"})
...@@ -318,8 +323,8 @@ func TestPullRestore(t *testing.T) { ...@@ -318,8 +323,8 @@ func TestPullRestore(t *testing.T) {
t.Fatalf("pull incomplete-send-pack.git/%s: complained, but error is wrong:\n%s\nerror: %s", kind, bad, estr) t.Fatalf("pull incomplete-send-pack.git/%s: complained, but error is wrong:\n%s\nerror: %s", kind, bad, estr)
} }
// git-backup leaves backup repo locked on error // git-backup should not leave backup repo locked on error
xgit("update-ref", "-d", "refs/backup.locked") xnoref("backup.locked")
}) })
// for incomplete-send-pack.git to indeed send incomplete pack, its git // for incomplete-send-pack.git to indeed send incomplete pack, its git
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment