Re: [PATCH 2/2] lock_packed_refs(): allow retries when acquiring the packed-refs lock

From: Stefan Beller <sbeller@google.com>
Date: May 11, 2015, 17:49 UTC
Message-ID: <CAGZ79kYc91z5S9j_hpA3b9-TvMBxr9jkdxhqEgHhOCT3mGzH5w@mail.gmail.com>
In-Reply-To: <20150511164941.GA30541@peff.net>

On Mon, May 11, 2015 at 9:49 AM, Jeff King <peff@peff.net> wrote:

Show 12 quoted lines

> On Mon, May 11, 2015 at 12:26:23PM +0200, Michael Haggerty wrote:
>
>> > So something like 100ms max backoff makes sense to me, in that it keeps
>> > us in the same order of magnitude as the expected time that the lock is
>> > held. [...]
>>
>> I don't understand your argument. If another process blocks us for on
>> the order of 100 ms, the backoff time (reading from my table) is less
>> than half of that.
>
> I think it is just that I was agreeing with you, but communicated it
> badly. I think your series is fine as-is.

By now I also think your series is fine as is. I am currently implementing something similar for Gerrit and I realize testing time based things is a royal pain in the butt. The tests you propose just take wall clock time and all should is good, though slowing down the test suite by another second or three in the worst case.

So for tests in Gerrit I use a dedicated java class which is specialized to pretend different times, such that you can write:

    doThings();
    pretendTimePassing(1 second);
    checkResultsFromThreads();

but running the tests takes less than 1 second as it's no real wall clock time passing.

On my machine there is /bin/usleep - sleep some number of microseconds which may be better for running the test suite. I don't know if it's worth the hassle of checking if that program is there and selectively running a faster version of said test though.

That said, I'm personally fine with the waste of 1 second as I view running the test suite as a long waiting game anyway. Maybe Junio as the maintainer (I assume he runs more tests than all of us?) dislikes waiting for the second all the time?

>
> -Peff