Re: [PATCH 1/7] receive-pack.c: add protocol support to negotiate atomic-push

From: Stefan Beller <stefanbeller@gmail.com>
Date: Dec 23, 2014, 02:09 UTC
Message-ID: <5498CEEE.9010105@gmail.com>
In-Reply-To: <CAPig+cRStBZMNj_K-81n2aZ3SzvH8VSeXwwk0MM5Q01FANRdPg@mail.gmail.com>

On 22.12.2014 14:52, Eric Sunshine wrote:

Show 32 quoted lines

> On Fri, Dec 19, 2014 at 2:38 PM, Stefan Beller <sbeller@google.com> wrote:
>> From: Ronnie Sahlberg <sahlberg@google.com>
>>
>> This adds support to the protocol between send-pack and receive-pack to
>> * allow receive-pack to inform the client that it has atomic push capability
>> * allow send-pack to request atomic push back.
>>
>> There is currently no setting in send-pack to actually request that atomic
>> pushes are to be used yet. This only adds protocol capability not ability
>> for the user to activate it.
>>
>> Signed-off-by: Ronnie Sahlberg <sahlberg@google.com>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>> diff --git a/Documentation/technical/protocol-capabilities.txt b/Documentation/technical/protocol-capabilities.txt
>> index 6d5424c..4f8a7bf 100644
>> --- a/Documentation/technical/protocol-capabilities.txt
>> +++ b/Documentation/technical/protocol-capabilities.txt
>> @@ -244,6 +245,14 @@ respond with the 'quiet' capability to suppress server-side progress
>>  reporting if the local progress reporting is also being suppressed
>>  (e.g., via `push -q`, or if stderr does not go to a tty).
>>
>> +atomic
>> +------
>> +
>> +If the server sends the 'atomic' capability it is capable of accepting
>> +atomic pushes. If the pushing client requests this capability, the server
>> +will update the refs in one atomic transaction. Either all refs are
> 
> Not itself worth a re-send, but if you re-send for some other reason...
> 
> "one atomic" still smacks of redundancy; "an atomic" sounds better.

I did hear you saying 'one single atomic' being too redundant. And I agree that 'one' and 'single' makes the redundancy.

However I have the impression 'an atomic' is too weak. Not everybody is a careful reader picking up the subtle notions. Not everybody is english native. Or concentrated.

Look at it the other way: How could it be done?

* All of the refs could be updated one at a time, not atomically, so
	foreach ref:
		open refs/heads/bla:
			write new sha1

* All of the refs could be updated at once, not atomically:
	open refs pack file:
		write new content
* All of the refs could be updated, one at a time, atomically:
	foreach ref:
		get lock
		write content to lock
		rename into place
* All of the refs at once, atomically.
	open packed refs file lock:
	write new content
	rename into place

That said, atomicity and how many transactions there are, are orthogonal to each other. That's why I'd keep pointing out 'one atomic' transaction.

Thanks for all the comments. I may be doing cleanup patches for you on top of what Junio queued.

Show 10 quoted lines

> 
>> +updated or none.
>> +
>>  allow-tip-sha1-in-want
>>  ----------------------
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>