Re: svn commit: r1464769 - in /subversion/trunk/subversion/libsvn_client: revert.c switch.c update.c

Philip Martin wrote:

>> URL: http://svn.apache.org/r1464769
>> Log:
>> Sleep for timestamps even if using commit times as timestamps.
>>
>> Before r1463721, 'revert' did not sleep if using commit times, while
>> 'checkout' and 'update' and 'switch' would sleep regardless.  r1463721 made
>> all of those operations omit the sleep if using commit times, but Philip and
>> Ben discussed and pointed out that the commit time could still be within the
>> timestamp resolution and so that could be unsafe.  Therefore we will sleep
>> anyway, in all of those operations, to be safe.
>>
>> (The commit time could also be *after* the local time because of clock skew,
>> which adds the possibility that the sleep takes us *into* an unsafe time
>> period, but we're not trying to address that problem here.)
>
> I don't really understand this log and it mentions my name.  I still
> think if the user has use-commit-times=yes then we should not sleep.  In
> that mode we are setting the timestamps to completely arbitrary times,
> times that can be changed by changing svn:date.  How does sleeping make
> anything more reliable?

The client-side operations where we were already sleeping for timestamps (before I started this sequence of changes) are: checkout, update, switch, revert and commit.  The first four of those have a "use-commit-times" mode, so that's why those four are the subject of this patch.  Bert points out that operations such as "status" can also update the WC recorded info, so we might consider adding a sleep in such places too, but that's out of scope for this use-commit-times patch.

I refute your claim that the commit time is a completely arbitrary time.  Of course those times *can* be set to any arbitrary value, but I aim to support the following two scenarios when use-commit-times is in effect:

  * Far past: The recorded commit time, compared with the client-side
    clock time, is in the past by more than the timestamp resolution,
    even though there is some clock skew.

    - We do not need to sleep.
    - My earlier commit eliminated the sleep, which was OK.
    - This commit restores the sleep, which is safe but redundant.

  * Near past: The recorded commit time, compared with the client-side
    clock time, is within the timestamp granularity and in the past.

    - We do need to sleep.
    - My earlier commit eliminated the sleep, which broke this case.
    - This commit fixes this case.

That second scenario is likely after a commit from the local client, and when doing rapid work across multiple clients, assuming the client(s) and server clocks are sufficiently well synchronized which seems a reasonable assumption if you expect that sort of thing to work.

I *do not* aim to support the remaining possible scenario, which can be summarized as:

  * The recorded commit time is in the future with respect to the client.

    - Sleeping could be neutral or could avoid or create a problem.
    - This commit just shifts the windows of opportunity for avoiding
      or creating a problem.

That scenario is not easily supported in the current code (it requires support in libsvn_wc, the subject of an issue I filed the other day), and the recent changes change it but don't make it significantly better or worse, and in my opinion it is not a reasonable use case to try to support at this point.

Does this make sense now, and how can I change the log message wording to clarify and so it doesn't misrepresent you?

- Julian
Received on 2013-04-05 17:11:56 CEST