@matheusbsbs / matheus-bagliano / commits / 4ff0f01cb7

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 <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Michael Haggerty committed Aug 21, 2017 at 13:51 UTC 4ff0f01cb7dd92fad49b4d0799590bb33a88168a

4 files changed +39 -5

Documentation/config.txt

@@ -776,6 +776,12 @@ core.commentChar::

776 If set to "auto", `git-commit` would select a character that is not

777 the beginning character of any line in existing commit messages.

778

779 +core.filesRefLockTimeout::

780 + The length of time, in milliseconds, to retry when trying to

781 + lock an individual reference. Value 0 means not to retry at

782 + all; -1 means to try indefinitely. Default is 100 (i.e.,

783 + retry for 100ms).

784 +

785 core.packedRefsTimeout::

786 The length of time, in milliseconds, to retry when trying to

787 lock the `packed-refs` file. Value 0 means not to retry at

refs.c

+21 -3

@@ -561,6 +561,21 @@ enum ref_type ref_type(const char *refname)

561 return REF_TYPE_NORMAL;

562 }

563

564 +long get_files_ref_lock_timeout_ms(void)

565 +{

566 + static int configured = 0;

567 +

568 + /* The default timeout is 100 ms: */

569 + static int timeout_ms = 100;

570 +

571 + if (!configured) {

572 + git_config_get_int("core.filesreflocktimeout", &timeout_ms);

573 + configured = 1;

574 + }

575 +

576 + return timeout_ms;

577 +}

578 +

579 static int write_pseudoref(const char *pseudoref, const unsigned char *sha1,

580 const unsigned char *old_sha1, struct strbuf *err)

581 {

@@ -573,7 +588,9 @@ static int write_pseudoref(const char *pseudoref, const unsigned char *sha1,

588 strbuf_addf(&buf, "%s\n", sha1_to_hex(sha1));

589

590 filename = git_path("%s", pseudoref);

576 - fd = hold_lock_file_for_update(&lock, filename, LOCK_DIE_ON_ERROR);

591 + fd = hold_lock_file_for_update_timeout(&lock, filename,

592 + LOCK_DIE_ON_ERROR,

593 + get_files_ref_lock_timeout_ms());

594 if (fd < 0) {

595 strbuf_addf(err, "could not open '%s' for writing: %s",

596 filename, strerror(errno));

@@ -616,8 +633,9 @@ static int delete_pseudoref(const char *pseudoref, const unsigned char *old_sha1

633 int fd;

634 unsigned char actual_old_sha1[20];

635

619 - fd = hold_lock_file_for_update(&lock, filename,

620 - LOCK_DIE_ON_ERROR);

636 + fd = hold_lock_file_for_update_timeout(

637 + &lock, filename, LOCK_DIE_ON_ERROR,

638 + get_files_ref_lock_timeout_ms());

639 if (fd < 0)

640 die_errno(_("Could not open '%s' for writing"), filename);

641 if (read_ref(pseudoref, actual_old_sha1))

refs/files-backend.c

+6 -2

@@ -855,7 +855,9 @@ retry:

855 if (!lock->lk)

856 lock->lk = xcalloc(1, sizeof(struct lock_file));

857

858 - if (hold_lock_file_for_update(lock->lk, ref_file.buf, LOCK_NO_DEREF) < 0) {

858 + if (hold_lock_file_for_update_timeout(

859 + lock->lk, ref_file.buf, LOCK_NO_DEREF,

860 + get_files_ref_lock_timeout_ms()) < 0) {

861 if (errno == ENOENT && --attempts_remaining > 0) {

862 /*

863 * Maybe somebody just deleted one of the

@@ -1181,7 +1183,9 @@ static int create_reflock(const char *path, void *cb)

1183 {

1184 struct lock_file *lk = cb;

1185

1184 - return hold_lock_file_for_update(lk, path, LOCK_NO_DEREF) < 0 ? -1 : 0;

1186 + return hold_lock_file_for_update_timeout(

1187 + lk, path, LOCK_NO_DEREF,

1188 + get_files_ref_lock_timeout_ms()) < 0 ? -1 : 0;

1189 }

1190

1191 /*

refs/refs-internal.h

@@ -61,6 +61,12 @@

61 */

62 #define REF_DELETED_LOOSE 0x200

64 +/*

65 + * Return the length of time to retry acquiring a loose reference lock

66 + * before giving up, in milliseconds:

67 + */

68 +long get_files_ref_lock_timeout_ms(void);

69 +

70 /*

71 * Return true iff refname is minimally safe. "Safe" here means that

72 * deleting a loose reference by this name will not do any damage, for