* [PATCH 2/2] http-push: remove MOVE step after PUT when sending objects to server
@ 2009-01-17 20:24 Ray Chuan
2009-01-18 0:48 ` Junio C Hamano
0 siblings, 1 reply; 10+ messages in thread
From: Ray Chuan @ 2009-01-17 20:24 UTC (permalink / raw
To: git
Currently, git PUTs to
/repo.git/objects/1a/1a2b...9z_opaquelocktoken:1234-....
then MOVEs to
/repo.git/objects/1a/1a2b...9z
This is needless. In fact, the only time MOVE requests are sent is for
this sole purpose (ie. of renaming an object).
A concern raised was repository corruption in the event of failure
during PUT. "put && move" won't afford any more protection than using
simply "put", since info/refs is not updated if a PUT fails, so there
is no cause for concern.
Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---
http-push.c | 45 +--------------------------------------------
1 files changed, 1 insertions(+), 44 deletions(-)
diff --git a/http-push.c b/http-push.c
index 4517cf2..d21fd3b 100644
--- a/http-push.c
+++ b/http-push.c
@@ -31,7 +31,6 @@ enum XML_Status {
/* DAV methods */
#define DAV_LOCK "LOCK"
#define DAV_MKCOL "MKCOL"
-#define DAV_MOVE "MOVE"
#define DAV_PROPFIND "PROPFIND"
#define DAV_PUT "PUT"
#define DAV_UNLOCK "UNLOCK"
@@ -105,7 +104,6 @@ enum transfer_state {
NEED_PUSH,
RUN_MKCOL,
RUN_PUT,
- RUN_MOVE,
ABORTED,
COMPLETE,
};
@@ -531,11 +529,6 @@ static void start_put(struct transfer_request *request)
posn += 2;
*(posn++) = '/';
strcpy(posn, hex + 2);
- request->dest = xmalloc(strlen(request->url) + 14);
- sprintf(request->dest, "Destination: %s", request->url);
- posn += 38;
- *(posn++) = '_';
- strcpy(posn, request->lock->token);
if_header = xmalloc(strlen(request->lock->token) + 25);
sprintf(if_header, "If: <%s>", request->lock->token);
@@ -565,32 +558,6 @@ static void start_put(struct transfer_request *request)
}
}
-static void start_move(struct transfer_request *request)
-{
- struct active_request_slot *slot;
- struct curl_slist *dav_headers = NULL;
-
- slot = get_active_slot();
- slot->callback_func = process_response;
- slot->callback_data = request;
- curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1); /* undo PUT setup */
- curl_easy_setopt(slot->curl, CURLOPT_CUSTOMREQUEST, DAV_MOVE);
- dav_headers = curl_slist_append(dav_headers, request->dest);
- dav_headers = curl_slist_append(dav_headers, "Overwrite: T");
- curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, dav_headers);
- curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_null);
- curl_easy_setopt(slot->curl, CURLOPT_URL, request->url);
-
- if (start_active_slot(slot)) {
- request->slot = slot;
- request->state = RUN_MOVE;
- } else {
- request->state = ABORTED;
- free(request->url);
- request->url = NULL;
- }
-}
-
static int refresh_lock(struct remote_lock *lock)
{
struct active_request_slot *slot;
@@ -713,23 +680,13 @@ static void finish_request(struct
transfer_request *request)
}
} else if (request->state == RUN_PUT) {
if (request->curl_result == CURLE_OK) {
- start_move(request);
- } else {
- fprintf(stderr, "PUT %s failed, aborting (%d/%ld)\n",
- sha1_to_hex(request->obj->sha1),
- request->curl_result, request->http_code);
- request->state = ABORTED;
- aborted = 1;
- }
- } else if (request->state == RUN_MOVE) {
- if (request->curl_result == CURLE_OK) {
if (push_verbosely)
fprintf(stderr, " sent %s\n",
sha1_to_hex(request->obj->sha1));
request->obj->flags |= REMOTE;
release_request(request);
} else {
- fprintf(stderr, "MOVE %s failed, aborting (%d/%ld)\n",
+ fprintf(stderr, "PUT %s failed, aborting (%d/%ld)\n",
sha1_to_hex(request->obj->sha1),
request->curl_result, request->http_code);
request->state = ABORTED;
--
1.5.6.3
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] http-push: remove MOVE step after PUT when sending objects to server
2009-01-17 20:24 [PATCH 2/2] http-push: remove MOVE step after PUT when sending objects to server Ray Chuan
@ 2009-01-18 0:48 ` Junio C Hamano
2009-01-18 3:19 ` Ray Chuan
0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2009-01-18 0:48 UTC (permalink / raw
To: Ray Chuan; +Cc: git
"Ray Chuan" <rctay89@gmail.com> writes:
> Currently, git PUTs to
>
> /repo.git/objects/1a/1a2b...9z_opaquelocktoken:1234-....
>
> then MOVEs to
>
> /repo.git/objects/1a/1a2b...9z
>
> This is needless. In fact, the only time MOVE requests are sent is for
> this sole purpose (ie. of renaming an object).
>
> A concern raised was repository corruption in the event of failure
> during PUT. "put && move" won't afford any more protection than using
> simply "put", since info/refs is not updated if a PUT fails, so there
> is no cause for concern.
That's a completely bogus reasoning. Normal operation inside the
repository that was pushed into won't look at info/refs at all.
The true reason you want to avoid put-then-move is...?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] http-push: remove MOVE step after PUT when sending objects to server
2009-01-18 0:48 ` Junio C Hamano
@ 2009-01-18 3:19 ` Ray Chuan
2009-01-18 13:32 ` Johannes Schindelin
0 siblings, 1 reply; 10+ messages in thread
From: Ray Chuan @ 2009-01-18 3:19 UTC (permalink / raw
To: Junio C Hamano; +Cc: git
Hi,
On Sun, Jan 18, 2009 at 12:48 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "Ray Chuan" <rctay89@gmail.com> writes:
>
>> Currently, git PUTs to
>>
>> /repo.git/objects/1a/1a2b...9z_opaquelocktoken:1234-....
>>
>> then MOVEs to
>>
>> /repo.git/objects/1a/1a2b...9z
>>
>> This is needless. In fact, the only time MOVE requests are sent is for
>> this sole purpose (ie. of renaming an object).
this would be my "true" reason. i do believe PUTting to a "temporary"
file (ie with '__opaquelocktoken:1234-....' appended) to be a needless
step.
>> A concern raised was repository corruption in the event of failure
>> during PUT. "put && move" won't afford any more protection than using
>> simply "put", since info/refs is not updated if a PUT fails, so there
>> is no cause for concern.
>
> That's a completely bogus reasoning. Normal operation inside the
> repository that was pushed into won't look at info/refs at all.
>
> The true reason you want to avoid put-then-move is...?
i mentioned this "repository corruption" issue as it was raised
previously by Johannes (towards the bottom):
http://article.gmane.org/gmane.comp.version-control.git/106031
--
Cheers,
Ray Chuan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] http-push: remove MOVE step after PUT when sending objects to server
2009-01-18 3:19 ` Ray Chuan
@ 2009-01-18 13:32 ` Johannes Schindelin
2009-01-18 20:05 ` Junio C Hamano
0 siblings, 1 reply; 10+ messages in thread
From: Johannes Schindelin @ 2009-01-18 13:32 UTC (permalink / raw
To: Ray Chuan; +Cc: Junio C Hamano, git
Hi,
On Sun, 18 Jan 2009, Ray Chuan wrote:
> On Sun, Jan 18, 2009 at 12:48 AM, Junio C Hamano <gitster@pobox.com> wrote:
>
> > "Ray Chuan" <rctay89@gmail.com> writes:
> >
> >> A concern raised was repository corruption in the event of failure
> >> during PUT. "put && move" won't afford any more protection than using
> >> simply "put", since info/refs is not updated if a PUT fails, so there
> >> is no cause for concern.
> >
> > That's a completely bogus reasoning. Normal operation inside the
> > repository that was pushed into won't look at info/refs at all.
>
> i mentioned this "repository corruption" issue as it was raised
> previously by Johannes (towards the bottom):
>
> http://article.gmane.org/gmane.comp.version-control.git/106031
,.. and Junio just raised a new concern. The repository as it is on the
server could very well be served by other means, i.e. git:// and rsync://,
and there could be cron jobs and interactive Git sessions trying to work
with it.
The point is: the repository inside the document root of the web server is
still a valid repository.
And the assumption is that whenever you have a file that looks like a
valid object/pack inside a valid repository, that it does not need
replacing.
So even when optimizing the uncommon (HTTP push is 2nd class citizen), we
have to keep the common workflow intact (1st class citizens _are_ push by
file system, ssh or git://).
Which unfortunately means that put && move must stay.
The same reasoning explains why http-push has to ignore the info/refs file
when looking for common refs, BTW.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] http-push: remove MOVE step after PUT when sending objects to server
2009-01-18 13:32 ` Johannes Schindelin
@ 2009-01-18 20:05 ` Junio C Hamano
2009-01-18 21:14 ` Johannes Schindelin
0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2009-01-18 20:05 UTC (permalink / raw
To: Johannes Schindelin; +Cc: Ray Chuan, git
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> The point is: the repository inside the document root of the web server is
> still a valid repository.
>
> And the assumption is that whenever you have a file that looks like a
> valid object/pack inside a valid repository, that it does not need
> replacing.
>
> So even when optimizing the uncommon (HTTP push is 2nd class citizen), we
> have to keep the common workflow intact (1st class citizens _are_ push by
> file system, ssh or git://).
The first class citizens are "local use", not "copying out of the area
that looks like a repository only to http:// transport users".
> Which unfortunately means that put && move must stay.
I still do not understand why it is unfortunate.
As far as I understand, Ray's issue was that his filesystem did not like
the temporary file name that is used for the initial "put" phase because
it contained a character it did not like (colon, perhaps). Why isn't the
patch about changing _that_ single issue?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] http-push: remove MOVE step after PUT when sending objects to server
2009-01-18 20:05 ` Junio C Hamano
@ 2009-01-18 21:14 ` Johannes Schindelin
2009-01-18 21:43 ` Junio C Hamano
0 siblings, 1 reply; 10+ messages in thread
From: Johannes Schindelin @ 2009-01-18 21:14 UTC (permalink / raw
To: Junio C Hamano; +Cc: Ray Chuan, git
Hi,
On Sun, 18 Jan 2009, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Which unfortunately means that put && move must stay.
>
> I still do not understand why it is unfortunate.
Because it is slow.
Ciao,
Dscho
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] http-push: remove MOVE step after PUT when sending objects to server
2009-01-18 21:14 ` Johannes Schindelin
@ 2009-01-18 21:43 ` Junio C Hamano
2009-01-18 21:58 ` Johannes Schindelin
2009-01-19 0:12 ` Sverre Rabbelier
0 siblings, 2 replies; 10+ messages in thread
From: Junio C Hamano @ 2009-01-18 21:43 UTC (permalink / raw
To: Johannes Schindelin; +Cc: Ray Chuan, git
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Sun, 18 Jan 2009, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>> > Which unfortunately means that put && move must stay.
>>
>> I still do not understand why it is unfortunate.
>
> Because it is slow.
If "slow" is a problem, why are you using http to begin with ;-)?
I'd take slow but reliable any day over fast and mostly works but
unreliable.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] http-push: remove MOVE step after PUT when sending objects to server
2009-01-18 21:43 ` Junio C Hamano
@ 2009-01-18 21:58 ` Johannes Schindelin
2009-01-19 0:12 ` Sverre Rabbelier
1 sibling, 0 replies; 10+ messages in thread
From: Johannes Schindelin @ 2009-01-18 21:58 UTC (permalink / raw
To: Junio C Hamano; +Cc: Ray Chuan, git
Hi,
On Sun, 18 Jan 2009, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Sun, 18 Jan 2009, Junio C Hamano wrote:
> >
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >>
> >> > Which unfortunately means that put && move must stay.
> >>
> >> I still do not understand why it is unfortunate.
> >
> > Because it is slow.
>
> If "slow" is a problem, why are you using http to begin with ;-)?
Heh, indeed.
> I'd take slow but reliable any day over fast and mostly works but
> unreliable.
Right. Issue settled,
Dscho
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] http-push: remove MOVE step after PUT when sending objects to server
2009-01-18 21:43 ` Junio C Hamano
2009-01-18 21:58 ` Johannes Schindelin
@ 2009-01-19 0:12 ` Sverre Rabbelier
2009-01-19 3:07 ` Junio C Hamano
1 sibling, 1 reply; 10+ messages in thread
From: Sverre Rabbelier @ 2009-01-19 0:12 UTC (permalink / raw
To: Junio C Hamano; +Cc: Johannes Schindelin, Ray Chuan, git
On Sun, Jan 18, 2009 at 22:43, Junio C Hamano <gitster@pobox.com> wrote:
> If "slow" is a problem, why are you using http to begin with ;-)?
Because http might be the only available protocol?
> I'd take slow but reliable any day over fast and mostly works but
> unreliable.
Yes, but if we want, say, git support at code.google.com (which I do,
I totally detest having to use svn because of this), it would be nice
to not dismiss http as "being slow anyway, so who cares about not
making it faster"?
--
Cheers,
Sverre Rabbelier
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] http-push: remove MOVE step after PUT when sending objects to server
2009-01-19 0:12 ` Sverre Rabbelier
@ 2009-01-19 3:07 ` Junio C Hamano
0 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2009-01-19 3:07 UTC (permalink / raw
To: Sverre Rabbelier; +Cc: Junio C Hamano, Johannes Schindelin, Ray Chuan, git
"Sverre Rabbelier" <srabbelier@gmail.com> writes:
> I totally detest having to use svn because of this), it would be nice
> to not dismiss http as "being slow anyway, so who cares about not
> making it faster"?
Nobody is arguing for "keeping it slow at all costs".
But "replace put && move with a single put, and we do not care if that
corrupts the resulting repository if it gets interrupted" is a different
story.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2009-01-19 3:08 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-17 20:24 [PATCH 2/2] http-push: remove MOVE step after PUT when sending objects to server Ray Chuan
2009-01-18 0:48 ` Junio C Hamano
2009-01-18 3:19 ` Ray Chuan
2009-01-18 13:32 ` Johannes Schindelin
2009-01-18 20:05 ` Junio C Hamano
2009-01-18 21:14 ` Johannes Schindelin
2009-01-18 21:43 ` Junio C Hamano
2009-01-18 21:58 ` Johannes Schindelin
2009-01-19 0:12 ` Sverre Rabbelier
2009-01-19 3:07 ` Junio C Hamano
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.