Commit 7535343c authored by Kirill Smelkov's avatar Kirill Smelkov

pull: Don't let a lot of empty directories stay under refs/backup/... work prefix after end of pull

Continuing 62374038 (pull: Turns unused refs are removed not 100% and a
lot of empty directories are accumulated) we just make sure to remove
them in the end of pull.

But NOTE: there could be O(n^2) behaviour still hidden, so it makes
sense to eventually revisit it and cleanup empty dirs earlier.

For now we just care not to degrade future pull performance. The
appropriate time for revisiting could be when reworking pull to do
fetches in parallel.

Updates: https://lab.nexedi.com/lab.nexedi.com/lab.nexedi.com/issues/4
parent ff2f0b67
...@@ -510,15 +510,6 @@ func cmd_pull_(gb *git.Repository, pullspecv []PullSpec) { ...@@ -510,15 +510,6 @@ func cmd_pull_(gb *git.Repository, pullspecv []PullSpec) {
xgit("update-ref", "-m", "git-backup pull", "HEAD", commit_sha1, HEAD) xgit("update-ref", "-m", "git-backup pull", "HEAD", commit_sha1, HEAD)
// remove no-longer needed backup refs & verify they don't stay // remove no-longer needed backup refs & verify they don't stay
// FIXME `delete` deletes only files, but leaves empty dirs around.
// more important: this affect performance of future `git-backup pull` run a *LOT*
//
// reason is: `git pull` first check local refs, and for doing so it
// recourse into all directories, even empty ones.
//
// https://lab.nexedi.com/lab.nexedi.com/lab.nexedi.com/issues/4
//
// -> TODO also remove empty directories.
backup_refs_delete := "" backup_refs_delete := ""
for _, __ := range backup_refs_list { for _, __ := range backup_refs_list {
backup_refs_delete += fmt.Sprintf("delete %s %s\n", __.ref, __.sha1) backup_refs_delete += fmt.Sprintf("delete %s %s\n", __.ref, __.sha1)
...@@ -530,6 +521,25 @@ func cmd_pull_(gb *git.Repository, pullspecv []PullSpec) { ...@@ -530,6 +521,25 @@ func cmd_pull_(gb *git.Repository, pullspecv []PullSpec) {
raisef("Backup refs under %s not deleted properly", backup_refs_work) raisef("Backup refs under %s not deleted properly", backup_refs_work)
} }
// NOTE `delete` deletes only files, but leaves empty dirs around.
// more important: this affect performance of future `git-backup pull` run a *LOT*
//
// reason is: `git pull` first check local refs, and for doing so it
// recourse into all directories, even empty ones.
//
// https://lab.nexedi.com/lab.nexedi.com/lab.nexedi.com/issues/4
//
// So remove all dirs under backup_refs_work prefix in the end.
//
// TODO Revisit this when reworking fetch to be parallel. Reason is: in
// the process of pulling repositories, the more references we
// accumulate, the longer pull starts to be, so it becomes O(n^2).
// We can avoid quadratic behaviour via removing refs from just
// pulled repo right after the pull.
gitdir := xgit("rev-parse", "--git-dir")
err := os.RemoveAll(gitdir+"/"+backup_refs_work)
raiseif(err) // NOTE err is nil if path does not exist
// if we have working copy - update it // if we have working copy - update it
bare := xgit("rev-parse", "--is-bare-repository") bare := xgit("rev-parse", "--is-bare-repository")
if bare != "true" { if bare != "true" {
......
...@@ -138,6 +138,19 @@ func TestPullRestore(t *testing.T) { ...@@ -138,6 +138,19 @@ func TestPullRestore(t *testing.T) {
} }
} }
// verify no garbage is left under refs/backup/
dentryv, err := ioutil.ReadDir("refs/backup/")
if err != nil && !os.IsNotExist(err) {
t.Fatal(err)
}
if len(dentryv) != 0 {
namev := []string{}
for _, fi := range dentryv {
namev = append(namev, fi.Name())
}
t.Fatalf("refs/backup/ not empty after pull: %v", namev)
}
// prune all non-reachable objects (e.g. tags just pulled - they were encoded as commits) // prune all non-reachable objects (e.g. tags just pulled - they were encoded as commits)
xgit("prune") xgit("prune")
......
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