lock_packed_refs(): allow retries when acquiring the packed-refs lock

Currently, there is only one attempt to acquire any lockfile, and if the lock is held by another process, the locking attempt fails immediately. This is not such a limitation for loose reference files. First, they don't take long to rewrite. Second, most reference updates have a known "old" value, so if another process is updating a reference at the same moment that we are trying to lock it, then probably the expected "old" value will not longer be valid, and the update will fail anyway. But these arguments do not hold for packed-refs: * The packed-refs file can be large and take significant time to rewrite. * Many references are stored in a single packed-refs file, so it could be that the other process was changing a different reference than the one that we are interested in. Therefore, it is much more likely for there to be spurious lock conflicts in connection to the packed-refs file, resulting in unnecessary command failures. So, if the first attempt to lock the packed-refs file fails, continue retrying for a configurable length of time before giving up. The default timeout is 1 second. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Michael Haggerty committed May 11, 2015 at 12:35 UTC f4ab4f3ab117cf375ae7bb8908c7b5ad687342b1
3 files changed +34 -1
Documentation/config.txt
+6
@@ -624,6 +624,12 @@ core.commentChar::
624 If set to "auto", `git-commit` would select a character that is not
625 the beginning character of any line in existing commit messages.
626
627 +core.packedRefsTimeout::
628 + The length of time, in milliseconds, to retry when trying to
629 + lock the `packed-refs` file. Value 0 means not to retry at
630 + all; -1 means to try indefinitely. Default is 1000 (i.e.,
631 + retry for 1 second).
632 +
633 sequence.editor::
634 Text editor used by `git rebase -i` for editing the rebase instruction file.
635 The value is meant to be interpreted by the shell when it is used.
refs.c
+11 -1
@@ -2431,9 +2431,19 @@ static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)
2431 /* This should return a meaningful errno on failure */
2432 int lock_packed_refs(int flags)
2433 {
2434 + static int timeout_configured = 0;
2435 + static int timeout_value = 1000;
2436 +
2437 struct packed_ref_cache *packed_ref_cache;
2438
2436 - if (hold_lock_file_for_update(&packlock, git_path("packed-refs"), flags) < 0)
2439 + if (!timeout_configured) {
2440 + git_config_get_int("core.packedrefstimeout", &timeout_value);
2441 + timeout_configured = 1;
2442 + }
2443 +
2444 + if (hold_lock_file_for_update_timeout(
2445 + &packlock, git_path("packed-refs"),
2446 + flags, timeout_value) < 0)
2447 return -1;
2448 /*
2449 * Get the current packed-refs while holding the lock. If the
t/t3210-pack-refs.sh
+17
@@ -187,4 +187,21 @@ test_expect_success 'notice d/f conflict with existing ref' '
187 test_must_fail git branch foo/bar/baz/lots/of/extra/components
188 '
189
190 +test_expect_success 'timeout if packed-refs.lock exists' '
191 + LOCK=.git/packed-refs.lock &&
192 + >"$LOCK" &&
193 + test_when_finished "rm -f $LOCK" &&
194 + test_must_fail git pack-refs --all --prune
195 +'
196 +
197 +test_expect_success 'retry acquiring packed-refs.lock' '
198 + LOCK=.git/packed-refs.lock &&
199 + >"$LOCK" &&
200 + test_when_finished "wait; rm -f $LOCK" &&
201 + {
202 + ( sleep 1 ; rm -f $LOCK ) &
203 + } &&
204 + git -c core.packedrefstimeout=3000 pack-refs --all --prune
205 +'
206 +
207 test_done