From 4ff0f01cb7dd92fad49b4d0799590bb33a88168a Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Mon, 21 Aug 2017 13:51:34 +0200 Subject: refs: retry acquiring reference locks for 100ms The philosophy of reference locking has been, "if another process is changing a reference, then whatever I'm trying to do to it will probably fail anyway because my old-SHA-1 value is probably no longer current". But this argument falls down if the other process has locked the reference to do something that doesn't actually change the value of the reference, such as `pack-refs` or `reflog expire`. There actually *is* a decent chance that a planned reference update will still be able to go through after the other process has released the lock. So when trying to lock an individual reference (e.g., when creating "refs/heads/master.lock"), if it is already locked, then retry the lock acquisition for approximately 100 ms before giving up. This should eliminate some unnecessary lock conflicts without wasting a lot of time. Add a configuration setting, `core.filesRefLockTimeout`, to allow this setting to be tweaked. Note: the function `get_files_ref_lock_timeout_ms()` cannot be private to the files backend because it is also used by `write_pseudoref()` and `delete_pseudoref()`, which are defined in `refs.c` so that they can be used by other reference backends. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- refs/files-backend.c | 8 ++++++-- refs/refs-internal.h | 6 ++++++ 2 files changed, 12 insertions(+), 2 deletions(-) (limited to 'refs') diff --git a/refs/files-backend.c b/refs/files-backend.c index 0404f2c233..d611e0f7d7 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -855,7 +855,9 @@ retry: if (!lock->lk) lock->lk = xcalloc(1, sizeof(struct lock_file)); - if (hold_lock_file_for_update(lock->lk, ref_file.buf, LOCK_NO_DEREF) < 0) { + if (hold_lock_file_for_update_timeout( + lock->lk, ref_file.buf, LOCK_NO_DEREF, + get_files_ref_lock_timeout_ms()) < 0) { if (errno == ENOENT && --attempts_remaining > 0) { /* * Maybe somebody just deleted one of the @@ -1181,7 +1183,9 @@ static int create_reflock(const char *path, void *cb) { struct lock_file *lk = cb; - return hold_lock_file_for_update(lk, path, LOCK_NO_DEREF) < 0 ? -1 : 0; + return hold_lock_file_for_update_timeout( + lk, path, LOCK_NO_DEREF, + get_files_ref_lock_timeout_ms()) < 0 ? -1 : 0; } /* diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 192f9f85c9..9977fea98b 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -61,6 +61,12 @@ */ #define REF_DELETED_LOOSE 0x200 +/* + * Return the length of time to retry acquiring a loose reference lock + * before giving up, in milliseconds: + */ +long get_files_ref_lock_timeout_ms(void); + /* * Return true iff refname is minimally safe. "Safe" here means that * deleting a loose reference by this name will not do any damage, for -- cgit v1.2.3