krz/gitbay

A CLI-first git forge. cli forge git self-hosted https://gitbay.org

Commit 5f1152bca4

5f1152bca4baa83655bb922e097c182d063789fc

parent: d201d6acb1

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-28 22:27 UTC 

backup: gc and mr prune refuse during a full backup; skip objects removed mid-walk

Ref #259

Layout: unified · split

.gitbay/wiki/Admin.org +6 −3
@@ -456,9 +456,12 @@ missing repository or a missing object.
456 456
457A full backup holds =<root>/backup.lock= from its database snapshot to 457A full backup holds =<root>/backup.lock= from its database snapshot to
458its last repository. While it runs, =repo delete=, =repo rename=, 458its last repository. While it runs, =repo delete=, =repo rename=,
459=repo transfer=, =admin repo delete= and =org rename= refuse with "a 459=repo transfer=, =admin repo delete=, =org rename=, =admin mr prune=
460backup is running"; retry when it finishes. Database-only backups take 460and =gitbayd admin gc= refuse with "a backup is running"; retry when it
461no lock. 461finishes. Database-only backups take no lock. A pack that git's own
462automatic gc removes during the walk is skipped; each repository's refs
463are archived before its objects, so the refs still find their objects,
464and =--verify= reports it if one does not.
462 465
463Verifying an archive of unknown origin is safe as an unprivileged 466Verifying an archive of unknown origin is safe as an unprivileged
464user: the connectivity check runs git against the archived 467user: the connectivity check runs git against the archived
cmd/gitbayd/backup.go +47 −12
@@ -162,14 +162,17 @@ func runBackup(cfg config.Config, out string, dbOnly bool) error {
162 repoCount := 0 162 repoCount := 0
163 root := cfg.Server.Root 163 root := cfg.Server.Root
164 if !dbOnly { 164 if !dbOnly {
165 err = filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { 165 err = filepath.WalkDir(root, func(path string, d fs.DirEntry, walkErr error) error {
166 if err != nil {
167 return err
168 }
169 rel, err := filepath.Rel(root, path) 166 rel, err := filepath.Rel(root, path)
170 if err != nil { 167 if err != nil {
171 return err 168 return err
172 } 169 }
170 if walkErr != nil {
171 if vanished(walkErr, rel) {
172 return nil
173 }
174 return walkErr
175 }
173 if rel == "." { 176 if rel == "." {
174 return nil 177 return nil
175 } 178 }
@@ -195,6 +198,9 @@ func runBackup(cfg config.Config, out string, dbOnly bool) error {
195 // ref is packed), so extraction recreates it: git's own 198 // ref is packed), so extraction recreates it: git's own
196 // repository discovery needs refs/ to exist. 199 // repository discovery needs refs/ to exist.
197 if err := addDir(tw, path, filepath.ToSlash(rel)); err != nil { 200 if err := addDir(tw, path, filepath.ToSlash(rel)); err != nil {
201 if vanished(err, rel) {
202 return filepath.SkipDir
203 }
198 return err 204 return err
199 } 205 }
200 if strings.HasSuffix(rel, ".git") { 206 if strings.HasSuffix(rel, ".git") {
@@ -206,7 +212,11 @@ func runBackup(cfg config.Config, out string, dbOnly bool) error {
206 } 212 }
207 return nil 213 return nil
208 } 214 }
209 return addFile(tw, path, filepath.ToSlash(rel)) 215 beforeAdd(path)
216 if err := addFile(tw, path, filepath.ToSlash(rel)); !vanished(err, rel) {
217 return err
218 }
219 return nil
210 }) 220 })
211 if err != nil { 221 if err != nil {
212 return err 222 return err
@@ -295,6 +305,29 @@ func addRefs(tw *tar.Writer, path, name string) error {
295 }) 305 })
296} 306}
297 307
308// beforeAdd runs before each file the walk archives outside refs. Tests
309// use it to remove a file between listing and reading.
310var beforeAdd = func(path string) {}
311
312// vanished reports a file or directory under a repository's objects/
313// that went between the walk listing it and reading it: a pack or loose
314// object a concurrent gc or receive.autogc removed. The walk skips it.
315// Refs archived earlier reach only objects that are still reachable, and
316// a repack writes those into a new pack before removing the old one; if
317// one is lost regardless, verify's fsck reports it.
318func vanished(err error, rel string) bool {
319 if !errors.Is(err, fs.ErrNotExist) {
320 return false
321 }
322 parts := strings.Split(filepath.ToSlash(rel), "/")
323 for i := 0; i+2 < len(parts); i++ {
324 if strings.HasSuffix(parts[i], ".git") && parts[i+1] == "objects" {
325 return true
326 }
327 }
328 return false
329}
330
298// syncDir makes a rename in dir durable. 331// syncDir makes a rename in dir durable.
299func syncDir(dir string) error { 332func syncDir(dir string) error {
300 d, err := os.Open(dir) 333 d, err := os.Open(dir)
@@ -313,8 +346,15 @@ func snapshotDB(st *store.Store, dest string) error {
313 return err 346 return err
314} 347}
315 348
349// addFile opens before writing the header, so a file removed after the
350// walk listed it fails before the archive has a member for it.
316func addFile(tw *tar.Writer, path, name string) error { 351func addFile(tw *tar.Writer, path, name string) error {
317 info, err := os.Stat(path) 352 src, err := os.Open(path)
353 if err != nil {
354 return err
355 }
356 defer src.Close()
357 info, err := src.Stat()
318 if err != nil { 358 if err != nil {
319 return err 359 return err
320 } 360 }
@@ -326,12 +366,7 @@ func addFile(tw *tar.Writer, path, name string) error {
326 if err := tw.WriteHeader(hdr); err != nil { 366 if err := tw.WriteHeader(hdr); err != nil {
327 return err 367 return err
328 } 368 }
329 src, err := os.Open(path) 369 _, err = io.CopyN(tw, src, hdr.Size)
330 if err != nil {
331 return err
332 }
333 defer src.Close()
334 _, err = io.Copy(tw, src)
335 return err 370 return err
336} 371}
337 372
cmd/gitbayd/backup_test.go +78
@@ -5,6 +5,7 @@ import (
5 "compress/gzip" 5 "compress/gzip"
6 "errors" 6 "errors"
7 "io" 7 "io"
8 "io/fs"
8 "os" 9 "os"
9 "os/exec" 10 "os/exec"
10 "path/filepath" 11 "path/filepath"
@@ -548,3 +549,80 @@ func TestBackupArchivesRefsBeforeObjects(t *testing.T) {
548 } 549 }
549 gitIn(t, repo, "cat-file", "-e", second) 550 gitIn(t, repo, "cat-file", "-e", second)
550} 551}
552
553// A pack removed between the walk listing it and reading it is skipped,
554// and verify's fsck then reports what it held; a vanished file outside
555// objects/ still fails the backup.
556func TestBackupSkipsVanishedObjects(t *testing.T) {
557 cfg := testConfig(t)
558 st, err := openStore(cfg)
559 if err != nil {
560 t.Fatal(err)
561 }
562 uid, err := st.CreateUser("krz", false)
563 if err != nil {
564 t.Fatal(err)
565 }
566 if _, err := st.CreateRepo("user", uid, "thing", "public"); err != nil {
567 t.Fatal(err)
568 }
569 st.Close()
570
571 work := t.TempDir()
572 gitIn(t, work, "init", "-q", "-b", "main")
573 if err := os.WriteFile(filepath.Join(work, "a.txt"), []byte("a\n"), 0o644); err != nil {
574 t.Fatal(err)
575 }
576 gitIn(t, work, "add", "a.txt")
577 gitIn(t, work, "commit", "-q", "-m", "one")
578 dir := filepath.Join(cfg.Server.Root, "repos", "krz", "thing.git")
579 gitIn(t, work, "clone", "-q", "--bare", work, dir)
580 gitIn(t, dir, "repack", "-q", "-a", "-d")
581
582 removed := ""
583 beforeAdd = func(path string) {
584 if removed == "" && strings.HasSuffix(path, ".pack") {
585 removed = path
586 os.Remove(path)
587 }
588 }
589 t.Cleanup(func() { beforeAdd = func(string) {} })
590 archive := filepath.Join(t.TempDir(), "b.tar.gz")
591 if err := runBackup(cfg, archive, false); err != nil {
592 t.Fatalf("backup with a vanished pack: %v", err)
593 }
594 if removed == "" {
595 t.Fatal("no pack was archived")
596 }
597 for _, n := range members(t, archive) {
598 if strings.HasSuffix(n, ".pack") {
599 t.Errorf("archive carries %s", n)
600 }
601 }
602 if err := verifyBackup(archive, ""); err == nil || !strings.Contains(err.Error(), "connectivity") {
603 t.Errorf("verify of an archive missing its pack: %v", err)
604 }
605
606 beforeAdd = func(path string) {
607 if strings.HasSuffix(path, filepath.Join("thing.git", "config")) {
608 os.Remove(path)
609 }
610 }
611 err = runBackup(cfg, filepath.Join(t.TempDir(), "c.tar.gz"), false)
612 if !errors.Is(err, fs.ErrNotExist) {
613 t.Fatalf("backup with a vanished config: %v, want not-exist", err)
614 }
615}
616
617// gc refuses while a full backup holds the lock.
618func TestGCRefusedDuringBackup(t *testing.T) {
619 cfg := testConfig(t)
620 release, err := backuplock.Hold(cfg.Server.Root)
621 if err != nil {
622 t.Fatal(err)
623 }
624 defer release()
625 if err := runGC(cfg, "", false, false); !errors.Is(err, backuplock.ErrBusy) {
626 t.Fatalf("gc during a backup: %v", err)
627 }
628}
cmd/gitbayd/maint.go +51 −38
@@ -8,6 +8,7 @@ import (
8 8
9 "github.com/spf13/cobra" 9 "github.com/spf13/cobra"
10 10
11 "gitbay.org/gitbay/internal/backuplock"
11 "gitbay.org/gitbay/internal/config" 12 "gitbay.org/gitbay/internal/config"
12 "gitbay.org/gitbay/internal/control" 13 "gitbay.org/gitbay/internal/control"
13 "gitbay.org/gitbay/internal/gitutil" 14 "gitbay.org/gitbay/internal/gitutil"
@@ -27,44 +28,7 @@ func gcCmd() *cobra.Command {
27 if err != nil { 28 if err != nil {
28 return err 29 return err
29 } 30 }
30 st, err := openStore(cfg) 31 return runGC(cfg, repoPath, aggressive, withLFS)
31 if err != nil {
32 return err
33 }
34 defer st.Close()
35
36 var repos []store.Repo
37 if repoPath != "" {
38 r, err := st.RepoByPath(repoPath)
39 if err != nil {
40 return fmt.Errorf("no repository %q", repoPath)
41 }
42 repos = []store.Repo{r}
43 } else if repos, err = st.ListAllRepos(); err != nil {
44 return err
45 }
46
47 var before, after int64
48 for _, r := range repos {
49 dir := control.RepoDir(cfg.Server.Root, r.OwnerName, r.Name)
50 b := gitutil.DirSize(dir)
51 gcArgs := []string{"-C", dir, "gc", "--quiet"}
52 if aggressive {
53 gcArgs = append(gcArgs, "--aggressive")
54 }
55 if out, err := exec.Command(toolpath.Look("git"), gcArgs...).CombinedOutput(); err != nil {
56 fmt.Fprintf(os.Stderr, "%s: gc failed: %v\n%s", r.Path(), err, out)
57 continue
58 }
59 a := gitutil.DirSize(dir)
60 before, after = before+b, after+a
61 fmt.Printf("%s\t%s -> %s\n", r.Path(), human(b), human(a))
62 }
63 fmt.Printf("total\t%s -> %s (freed %s)\n", human(before), human(after), human(before-after))
64 if !withLFS {
65 return nil
66 }
67 return gcLFS(cfg, st)
68 }, 32 },
69 } 33 }
70 cmd.Flags().StringVar(&repoPath, "repo", "", "one repository (owner/name) instead of all") 34 cmd.Flags().StringVar(&repoPath, "repo", "", "one repository (owner/name) instead of all")
@@ -73,6 +37,55 @@ func gcCmd() *cobra.Command {
73 return cmd 37 return cmd
74} 38}
75 39
40// runGC refuses while a full backup runs: a prune removing a pack the
41// backup has listed but not yet read would leave it out of the archive
42// (#259).
43func runGC(cfg config.Config, repoPath string, aggressive, withLFS bool) error {
44 release, err := backuplock.TryShared(cfg.Server.Root)
45 if err != nil {
46 return err
47 }
48 defer release()
49 st, err := openStore(cfg)
50 if err != nil {
51 return err
52 }
53 defer st.Close()
54
55 var repos []store.Repo
56 if repoPath != "" {
57 r, err := st.RepoByPath(repoPath)
58 if err != nil {
59 return fmt.Errorf("no repository %q", repoPath)
60 }
61 repos = []store.Repo{r}
62 } else if repos, err = st.ListAllRepos(); err != nil {
63 return err
64 }
65
66 var before, after int64
67 for _, r := range repos {
68 dir := control.RepoDir(cfg.Server.Root, r.OwnerName, r.Name)
69 b := gitutil.DirSize(dir)
70 gcArgs := []string{"-C", dir, "gc", "--quiet"}
71 if aggressive {
72 gcArgs = append(gcArgs, "--aggressive")
73 }
74 if out, err := exec.Command(toolpath.Look("git"), gcArgs...).CombinedOutput(); err != nil {
75 fmt.Fprintf(os.Stderr, "%s: gc failed: %v\n%s", r.Path(), err, out)
76 continue
77 }
78 a := gitutil.DirSize(dir)
79 before, after = before+b, after+a
80 fmt.Printf("%s\t%s -> %s\n", r.Path(), human(b), human(a))
81 }
82 fmt.Printf("total\t%s -> %s (freed %s)\n", human(before), human(after), human(before-after))
83 if !withLFS {
84 return nil
85 }
86 return gcLFS(cfg, st)
87}
88
76func human(b int64) string { 89func human(b int64) string {
77 switch { 90 switch {
78 case b >= 1<<30: 91 case b >= 1<<30:
internal/backuplock/backuplock.go +7 −7
@@ -1,9 +1,9 @@
1// Package backuplock keeps repository deletes, renames and transfers 1// Package backuplock keeps repository deletes, renames, transfers and
2// out of a full backup's way (#259). The backup runs in its own process 2// prunes out of a full backup's way (#259). The backup runs in its own
3// (gitbayd admin backup) and a delete in the daemon's, so the lock is 3// process (gitbayd admin backup) and a delete in the daemon's, so the
4// flock(2) on a file under server.root: the backup holds it exclusively 4// lock is flock(2) on a file under server.root: the backup holds it
5// from its database snapshot until the last repository is archived, and 5// exclusively from its database snapshot until the last repository is
6// each delete or move holds it shared while it runs. 6// archived, and each delete, move or prune holds it shared while it runs.
7package backuplock 7package backuplock
8 8
9import ( 9import (
@@ -17,7 +17,7 @@ import (
17const Name = "backup.lock" 17const Name = "backup.lock"
18 18
19// ErrBusy is TryShared's answer while a backup holds the lock. 19// ErrBusy is TryShared's answer while a backup holds the lock.
20var ErrBusy = errors.New("a backup is running; repositories cannot be deleted, renamed or moved until it finishes, usually within minutes") 20var ErrBusy = errors.New("a backup is running; repositories cannot be deleted, renamed, moved or pruned until it finishes, usually within minutes")
21 21
22// open opens the lock file read-only, which is all flock needs, so the 22// open opens the lock file read-only, which is all flock needs, so the
23// daemon's user can lock a file a root-run backup created. O_NOFOLLOW 23// daemon's user can lock a file a root-run backup created. O_NOFOLLOW
internal/control/admin.go +8
@@ -655,6 +655,14 @@ func runAdminMRPrune(c *Ctx, args []string) int {
655 mrs = append(mrs, mr) 655 mrs = append(mrs, mr)
656 } 656 }
657 657
658 // The prune would remove objects a running full backup has listed
659 // and not yet read.
660 release, code := holdOffBackup(c)
661 if code >= 0 {
662 return code
663 }
664 defer release()
665
658 // The record is written as each ref goes, not after the gc: a failure 666 // The record is written as each ref goes, not after the gc: a failure
659 // past this point leaves refs deleted, and the audit log and the MR 667 // past this point leaves refs deleted, and the audit log and the MR
660 // thread must say so. Re-running the same command finishes the job. 668 // thread must say so. Re-running the same command finishes the job.
internal/control/backuplock_test.go +17
@@ -38,3 +38,20 @@ func TestRepoDeleteAndRenameRefusedDuringBackup(t *testing.T) {
38 t.Fatalf("delete after the backup: exit %d, %s", code, errOut) 38 t.Fatalf("delete after the backup: exit %d, %s", code, errOut)
39 } 39 }
40} 40}
41
42func TestAdminMRPruneRefusedDuringBackup(t *testing.T) {
43 st, repo, root, headSHA := prunedRepo(t)
44 dir := RepoDir(root, repo.OwnerName, repo.Name)
45 release, err := backuplock.Hold(root)
46 if err != nil {
47 t.Fatal(err)
48 }
49 defer release()
50 c, errOut := pruneCtx(st, root, rootUser(t, st))
51 if code := Dispatch(c, []string{"admin", "mr", "prune", repo.Path(), "1", "--yes"}); code != protocol.ExitFailure || !strings.Contains(errOut.String(), "a backup is running") {
52 t.Fatalf("prune during a backup: exit %d, %s", code, errOut)
53 }
54 if !refExists(dir, mrHeadRef(1)) || !objectExists(dir, headSHA) {
55 t.Fatal("prune during a backup changed the repository")
56 }
57}