From 5bb0dc9390edf9f056cd67c12c258b10d0648871 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 13 Sep 2012 14:02:46 -0700 Subject: [PATCH 1/3] ODB: re-load packfiles on failed lookup The old method was avoiding re-loading of packfiles by watching the mtime of the pack directory. This causes the ODB to become stale if the directory and packfile are written within the same clock millisecond, as when cloning a fairly small repo. This method tries to find the object in the cached packs, and forces a refresh when that fails. This will cause extra stat'ing on a miss, but speeds up the success case and avoids this race condition. --- src/odb_pack.c | 36 ++++++++++++++++++++++++++++-------- tests-clar/clone/clone.c | 2 +- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/odb_pack.c b/src/odb_pack.c index b4f958b6f..5d3f0e6e5 100644 --- a/src/odb_pack.c +++ b/src/odb_pack.c @@ -233,10 +233,11 @@ static int packfile_load__cb(void *_data, git_buf *path) return git_vector_insert(&backend->packs, pack); } -static int packfile_refresh_all(struct pack_backend *backend) +static int packfile_refresh_all_maybe(struct pack_backend *backend, bool force) { int error; struct stat st; + git_buf path = GIT_BUF_INIT; if (backend->pack_folder == NULL) return 0; @@ -244,8 +245,7 @@ static int packfile_refresh_all(struct pack_backend *backend) if (p_stat(backend->pack_folder, &st) < 0 || !S_ISDIR(st.st_mode)) return git_odb__error_notfound("failed to refresh packfiles", NULL); - if (st.st_mtime != backend->pack_folder_mtime) { - git_buf path = GIT_BUF_INIT; + if (force || st.st_mtime != backend->pack_folder_mtime) { git_buf_sets(&path, backend->pack_folder); /* reload all packs */ @@ -263,9 +263,14 @@ static int packfile_refresh_all(struct pack_backend *backend) return 0; } -static int pack_entry_find(struct git_pack_entry *e, struct pack_backend *backend, const git_oid *oid) +static int packfile_refresh_all(struct pack_backend *backend) { + return packfile_refresh_all_maybe(backend, false); +} + +static int pack_entry_find_inner(struct git_pack_entry *e, + struct pack_backend *backend, + const git_oid *oid) { - int error; unsigned int i; struct git_pack_file *last_found = backend->last_found; @@ -273,9 +278,6 @@ static int pack_entry_find(struct git_pack_entry *e, struct pack_backend *backen git_pack_entry_find(e, last_found, oid, GIT_OID_HEXSZ) == 0) return 0; - if ((error = packfile_refresh_all(backend)) < 0) - return error; - for (i = 0; i < backend->packs.length; ++i) { struct git_pack_file *p; @@ -289,6 +291,24 @@ static int pack_entry_find(struct git_pack_entry *e, struct pack_backend *backen } } + return -1; +} + +static int pack_entry_find(struct git_pack_entry *e, struct pack_backend *backend, const git_oid *oid) +{ + int error; + + if (backend->last_found && + git_pack_entry_find(e, backend->last_found, oid, GIT_OID_HEXSZ) == 0) + return 0; + + if (!pack_entry_find_inner(e, backend, oid)) + return 0; + if ((error = packfile_refresh_all_maybe(backend, true)) < 0) + return error; + if (!pack_entry_find_inner(e, backend, oid)) + return 0; + return git_odb__error_notfound("failed to find pack entry", oid); } diff --git a/tests-clar/clone/clone.c b/tests-clar/clone/clone.c index 4cca15ffe..237e607dd 100644 --- a/tests-clar/clone/clone.c +++ b/tests-clar/clone/clone.c @@ -5,7 +5,7 @@ #define DO_LOCAL_TEST 0 #define DO_LIVE_NETWORK_TESTS 0 -#define LIVE_REPO_URL "http://github.com/libgit2/node-gitteh" +#define LIVE_REPO_URL "git://github.com/nulltoken/TestGitRepository" static git_repository *g_repo; From 78216495b0aaca66dc76ae33516a679385495c6a Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Thu, 13 Sep 2012 20:08:05 -0700 Subject: [PATCH 2/3] Remove mtime checks from ODB packfile backend Now forcing refresh on a foreach, and on missed full-oid or short-oid lookups. --- src/odb_pack.c | 55 +++++++++++++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/src/odb_pack.c b/src/odb_pack.c index 5d3f0e6e5..50949bc6b 100644 --- a/src/odb_pack.c +++ b/src/odb_pack.c @@ -24,7 +24,6 @@ struct pack_backend { git_vector packs; struct git_pack_file *last_found; char *pack_folder; - time_t pack_folder_mtime; }; /** @@ -233,7 +232,7 @@ static int packfile_load__cb(void *_data, git_buf *path) return git_vector_insert(&backend->packs, pack); } -static int packfile_refresh_all_maybe(struct pack_backend *backend, bool force) +static int packfile_refresh_all(struct pack_backend *backend) { int error; struct stat st; @@ -245,28 +244,21 @@ static int packfile_refresh_all_maybe(struct pack_backend *backend, bool force) if (p_stat(backend->pack_folder, &st) < 0 || !S_ISDIR(st.st_mode)) return git_odb__error_notfound("failed to refresh packfiles", NULL); - if (force || st.st_mtime != backend->pack_folder_mtime) { - git_buf_sets(&path, backend->pack_folder); + git_buf_sets(&path, backend->pack_folder); - /* reload all packs */ - error = git_path_direach(&path, packfile_load__cb, (void *)backend); + /* reload all packs */ + error = git_path_direach(&path, packfile_load__cb, (void *)backend); - git_buf_free(&path); + git_buf_free(&path); - if (error < 0) - return error; + if (error < 0) + return error; - git_vector_sort(&backend->packs); - backend->pack_folder_mtime = st.st_mtime; - } + git_vector_sort(&backend->packs); return 0; } -static int packfile_refresh_all(struct pack_backend *backend) { - return packfile_refresh_all_maybe(backend, false); -} - static int pack_entry_find_inner(struct git_pack_entry *e, struct pack_backend *backend, const git_oid *oid) @@ -304,7 +296,7 @@ static int pack_entry_find(struct git_pack_entry *e, struct pack_backend *backen if (!pack_entry_find_inner(e, backend, oid)) return 0; - if ((error = packfile_refresh_all_maybe(backend, true)) < 0) + if ((error = packfile_refresh_all(backend)) < 0) return error; if (!pack_entry_find_inner(e, backend, oid)) return 0; @@ -312,11 +304,10 @@ static int pack_entry_find(struct git_pack_entry *e, struct pack_backend *backen return git_odb__error_notfound("failed to find pack entry", oid); } -static int pack_entry_find_prefix( - struct git_pack_entry *e, - struct pack_backend *backend, - const git_oid *short_oid, - size_t len) +static unsigned pack_entry_find_prefix_inner(struct git_pack_entry *e, + struct pack_backend *backend, + const git_oid *short_oid, + size_t len) { int error; unsigned int i; @@ -351,6 +342,25 @@ static int pack_entry_find_prefix( } } + return found; +} + +static int pack_entry_find_prefix( + struct git_pack_entry *e, + struct pack_backend *backend, + const git_oid *short_oid, + size_t len) +{ + unsigned found = 0; + int error; + + if ((found = pack_entry_find_prefix_inner(e, backend, short_oid, len)) > 0) + goto cleanup; + if ((error = packfile_refresh_all(backend)) < 0) + return error; + found = pack_entry_find_prefix_inner(e, backend, short_oid, len); + +cleanup: if (!found) return git_odb__error_notfound("no matching pack entry for prefix", short_oid); else if (found > 1) @@ -535,7 +545,6 @@ int git_odb_backend_pack(git_odb_backend **backend_out, const char *objects_dir) if (git_path_isdir(git_buf_cstr(&path)) == true) { backend->pack_folder = git_buf_detach(&path); - backend->pack_folder_mtime = 0; } backend->parent.read = &pack_backend__read; From 63409451860749cfe1efbb509fbe8c5a0a1648a0 Mon Sep 17 00:00:00 2001 From: Ben Straub Date: Wed, 19 Sep 2012 04:55:16 -0700 Subject: [PATCH 3/3] ODB pack: snapshot last_found to avoid race Also removed unnecessary refresh call and fixed some indentation. --- src/odb_pack.c | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/odb_pack.c b/src/odb_pack.c index 50949bc6b..964e82afb 100644 --- a/src/odb_pack.c +++ b/src/odb_pack.c @@ -259,12 +259,13 @@ static int packfile_refresh_all(struct pack_backend *backend) return 0; } -static int pack_entry_find_inner(struct git_pack_entry *e, - struct pack_backend *backend, - const git_oid *oid) +static int pack_entry_find_inner( + struct git_pack_entry *e, + struct pack_backend *backend, + const git_oid *oid, + struct git_pack_file *last_found) { unsigned int i; - struct git_pack_file *last_found = backend->last_found; if (last_found && git_pack_entry_find(e, last_found, oid, GIT_OID_HEXSZ) == 0) @@ -289,33 +290,32 @@ static int pack_entry_find_inner(struct git_pack_entry *e, static int pack_entry_find(struct git_pack_entry *e, struct pack_backend *backend, const git_oid *oid) { int error; + struct git_pack_file *last_found = backend->last_found; if (backend->last_found && git_pack_entry_find(e, backend->last_found, oid, GIT_OID_HEXSZ) == 0) return 0; - if (!pack_entry_find_inner(e, backend, oid)) + if (!pack_entry_find_inner(e, backend, oid, last_found)) return 0; if ((error = packfile_refresh_all(backend)) < 0) return error; - if (!pack_entry_find_inner(e, backend, oid)) + if (!pack_entry_find_inner(e, backend, oid, last_found)) return 0; return git_odb__error_notfound("failed to find pack entry", oid); } -static unsigned pack_entry_find_prefix_inner(struct git_pack_entry *e, - struct pack_backend *backend, - const git_oid *short_oid, - size_t len) +static unsigned pack_entry_find_prefix_inner( + struct git_pack_entry *e, + struct pack_backend *backend, + const git_oid *short_oid, + size_t len, + struct git_pack_file *last_found) { int error; unsigned int i; unsigned found = 0; - struct git_pack_file *last_found = backend->last_found; - - if ((error = packfile_refresh_all(backend)) < 0) - return error; if (last_found) { error = git_pack_entry_find(e, last_found, short_oid, len); @@ -353,12 +353,13 @@ static int pack_entry_find_prefix( { unsigned found = 0; int error; + struct git_pack_file *last_found = backend->last_found; - if ((found = pack_entry_find_prefix_inner(e, backend, short_oid, len)) > 0) + if ((found = pack_entry_find_prefix_inner(e, backend, short_oid, len, last_found)) > 0) goto cleanup; if ((error = packfile_refresh_all(backend)) < 0) return error; - found = pack_entry_find_prefix_inner(e, backend, short_oid, len); + found = pack_entry_find_prefix_inner(e, backend, short_oid, len, last_found); cleanup: if (!found)