From: Junio C Hamano <hidden> Date: 2016-06-15 22:42:07
Daniel Barkalow [off-list ref] writes:
The code in ssh-fetch already does parallel fetching, actually (only over
one connection, but requests are sent before responses are read), so
multiple requests are in progress at the same time.
This reminds me of one patch:
From: Dan Aloni [off-list ref]
Subject: [PATCH] Fix git+ssh's indefinite halts during long fetches
Date: Sat, 1 Oct 2005 21:39:42 +0300
Message-ID: <20051001183942.GA2099@localdomain>
I'd appreciate it if you had a chance to take a look at it and
comment on it.
The change is isolated to ssh-fetch [*1*], so even if it were to
break something it would only break ssh-fetch and in that sense
it is a safer change.
But it still is a lot of code, and I felt there might be a
simpler way to do this. That is why I am deliberately holding
it off.
[Footnote]
*1* The patch touches sha1_file.c and cache.h but that is to
update write_sha1_from_fd(), which is used only by ssh-fetch
AFAICT. We may want to move it from sha1_file.c to sha1-fetch.c
and make it static, removing it from cache.h.
This reminds me of one patch:
From: Dan Aloni [off-list ref]
Subject: [PATCH] Fix git+ssh's indefinite halts during long fetches
Date: Sat, 1 Oct 2005 21:39:42 +0300
Message-ID: <20051001183942.GA2099@localdomain>
I'd appreciate it if you had a chance to take a look at it and
comment on it.
I personally hate it.
It adds horrible patches to fairly core stuff, all because the prefetching
is not limited.
As far as I can tell, it should be much easier to just limit the
prefetching to some reasonable limit (say, a few objects deep), which
guarantees that the prefetching doesn't fill up the write queues on the
fetching side.
It's not like prefetching improves performance once you get to the point
where you can stream. I suspect having more than two or three objects "in
flight" really only helps with
- lots of small objects
- high latency
- high bandwidth
and the thing is, high latency together with high bandwidth is really
quite uncommon - usually high latency goes along with _low_ bandwidth (the
one exception is things like satellite links, which can have latencies in
the seconds, even with good throughput).
It should be pretty easy to benchmark, but my _suspicion_ is that limiting
the read-ahead to even just five is likely to get you 99% of the way, and
that the performance impact of going higher is very limited.
(It might need some extra code to make the synchronous receiving side
re-start the prefetching if the prefetching has stopped after a few
entries - but at that point the extra code is where it is supposed to be,
rather than having core code work around problems in the fetching. I also
suspect that the prefetch limiting can happily be done in the generic
"pull" code, rather than separately for each protocol, so it would need to
be done in just one place).
Linus
From: Daniel Barkalow <hidden> Date: 2016-06-15 22:42:07
On Mon, 3 Oct 2005, Junio C Hamano wrote:
Daniel Barkalow [off-list ref] writes:
quoted
The code in ssh-fetch already does parallel fetching, actually (only over
one connection, but requests are sent before responses are read), so
multiple requests are in progress at the same time.
This reminds me of one patch:
From: Dan Aloni [off-list ref]
Subject: [PATCH] Fix git+ssh's indefinite halts during long fetches
Date: Sat, 1 Oct 2005 21:39:42 +0300
Message-ID: <20051001183942.GA2099@localdomain>
I'd appreciate it if you had a chance to take a look at it and
comment on it.
I think it's overly hacky; we should be able, in prefetch, to check
whether we've stuffed in a lot of hashes already, and actually read an
object out before requesting another; there's nothing in the fetch
contract that says that an object can't become available at some random
time between the start of the fetch and when it gets requested with
fetch(). (In fact, I had a pack-exchange version of the ssh stuff which
would notice that you have certain things and you're looking for a commit,
and get a bunch of stuff you probably want as a pack before you actually
ask for it, but Linus beat me on that one with send-pack/upload-pack.)
I think that a limit of 100 objects in transit is about right, because the
requests for 100 objects fits well within 4K and I expect that we
commonly have small enough objects that we need to queue up a relatively
large number of requests to maintain streaming.
I've got a patch, which I'll send in the next email.
-Daniel
*This .sig left intentionally blank*
From: Dan Aloni <hidden> Date: 2016-06-15 22:42:07
On Mon, Oct 03, 2005 at 04:16:27PM -0700, Linus Torvalds wrote:
On Mon, 3 Oct 2005, Junio C Hamano wrote:
quoted
This reminds me of one patch:
From: Dan Aloni [off-list ref]
Subject: [PATCH] Fix git+ssh's indefinite halts during long fetches
Date: Sat, 1 Oct 2005 21:39:42 +0300
Message-ID: <20051001183942.GA2099@localdomain>
I'd appreciate it if you had a chance to take a look at it and
comment on it.
I personally hate it.
It adds horrible patches to fairly core stuff, all because the prefetching
is not limited.
Well it can be reworked to be more clean...
As far as I can tell, it should be much easier to just limit the
prefetching to some reasonable limit (say, a few objects deep), which
guarantees that the prefetching doesn't fill up the write queues on the
fetching side.
I'm not sure how this will be completely reliable, even if you limit the
prefetching to one object.
Suppose that this one object's size is larger than the receiving queues of
the receiving end (like 1 MB?) and the bandwidth is high, wouldn't that
break?
--
Dan Aloni
da-x@monatomic.org, da-x@colinux.org, da-x@gmx.net
From: Daniel Barkalow <hidden> Date: 2016-06-15 22:42:07
On Tue, 4 Oct 2005, Dan Aloni wrote:
On Mon, Oct 03, 2005 at 04:16:27PM -0700, Linus Torvalds wrote:
quoted
On Mon, 3 Oct 2005, Junio C Hamano wrote:
quoted
This reminds me of one patch:
From: Dan Aloni [off-list ref]
Subject: [PATCH] Fix git+ssh's indefinite halts during long fetches
Date: Sat, 1 Oct 2005 21:39:42 +0300
Message-ID: <20051001183942.GA2099@localdomain>
I'd appreciate it if you had a chance to take a look at it and
comment on it.
I personally hate it.
It adds horrible patches to fairly core stuff, all because the prefetching
is not limited.
Well it can be reworked to be more clean...
quoted
As far as I can tell, it should be much easier to just limit the
prefetching to some reasonable limit (say, a few objects deep), which
guarantees that the prefetching doesn't fill up the write queues on the
fetching side.
I'm not sure how this will be completely reliable, even if you limit the
prefetching to one object.
Suppose that this one object's size is larger than the receiving queues of
the receiving end (like 1 MB?) and the bandwidth is high, wouldn't that
break?
It shouldn't cause any problem, unless there isn't a 4K buffer between the
git-ssh-fetch and ssh; the fetch side would have to fill this buffer
before getting stuck, even if ssh can't send out any more data until the
object has been read, and 100 requests (each 21 bytes) wouldn't be enough.
I remember that there's a lot that depends on being able to put 4K into an
empty pipe without blocking, and I'd guess that UNIX sockets have a
similar capacity (although I'm not going to look it up tonight).
-Daniel
*This .sig left intentionally blank*
I'd guess that UNIX sockets have a
similar capacity (although I'm not going to look it up tonight).
You can set TCP options to change the buffer sizes.
I would however assume that *nobody* sets both the send and receive
buffers such that their cumulative size is <4k, so 99 object IDs
at 41 bytes definitely should be OK.
--
Matthias Urlichs | {M:U} IT Design @ m-u-it.de | smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
- -
I was going to include an ethnic slur in here, but I couldn't figure out how
to get you into this file.
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2016-06-15 22:42:08
Matthias Urlichs wrote:
Hi, Daniel Barkalow wrote:
quoted
I'd guess that UNIX sockets have a
similar capacity (although I'm not going to look it up tonight).
You can set TCP options to change the buffer sizes.
I would however assume that *nobody* sets both the send and receive
buffers such that their cumulative size is <4k, so 99 object IDs
at 41 bytes definitely should be OK.
For TCP, I think we should simply get our own (or set) packet buffer
size and conform to it. Problem solved...
-hpa
I would however assume that *nobody* sets both the send and receive
buffers such that their cumulative size is <4k, so 99 object IDs
at 41 bytes definitely should be OK.
For TCP, I think we should simply get our own (or set) packet buffer
size and conform to it. Problem solved...
Actually, it isn't -- if you have a ssh connection, you don't have
access to the raw TCP socket.
Just limit the number of objects in flight to 20 or so.
Problem also solved. ;-)
--
Matthias Urlichs | {M:U} IT Design @ m-u-it.de | smurf@smurf.noris.de
Disclaimer: The quote was selected randomly. Really. | http://smurf.noris.de
- -
He hated to set precedents; those who did so were sometimes promoted, more
frequently they joined their ancestors.
-- Robert A. Heinlein
From: Daniel Barkalow <hidden> Date: 2016-06-15 22:42:08
On Tue, 4 Oct 2005, Matthias Urlichs wrote:
Hi, Daniel Barkalow wrote:
quoted
I'd guess that UNIX sockets have a
similar capacity (although I'm not going to look it up tonight).
You can set TCP options to change the buffer sizes.
I would however assume that *nobody* sets both the send and receive
buffers such that their cumulative size is <4k, so 99 object IDs
at 41 bytes definitely should be OK.
I actually mean UNIX (a.k.a. PF_LOCAL) sockets; git-ssh-fetch is connected
to ssh via sockets from "socketpair()". Looks like you can set the buffer
size with a socket option here, too, but I doubt ssh will try setting
socket options on standard in and out, and git-ssh-fetch leaves them at
their defaults. I'm not clear where the defaults for these come from in
general.
-Daniel
*This .sig left intentionally blank*
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2016-06-15 22:42:08
Daniel Barkalow wrote:
I actually mean UNIX (a.k.a. PF_LOCAL) sockets; git-ssh-fetch is connected
to ssh via sockets from "socketpair()". Looks like you can set the buffer
size with a socket option here, too, but I doubt ssh will try setting
socket options on standard in and out, and git-ssh-fetch leaves them at
their defaults. I'm not clear where the defaults for these come from in
general.
Well, git-ssh-fetch could set both SO_SNDBUF and SO_RCVBUF if it cared.
For portability, it would be a good thing to explicitly set the buffer
size rather than just blindly assume it can hold a specific amount of data.
-hpa
From: Junio C Hamano <hidden> Date: 2016-06-15 22:42:08
"H. Peter Anvin" [off-list ref] writes:
If you have an ssh connection, you're writing over a pipe to the ssh
process, and your local buffer is that pipe, which is PIPE_BUF size.
I vaguely recall there was an interesting regression in recent
kernel history when the implementation of the pipe buffer was
changed, with which, writing the same amount of data with
different number of writes made things behave differently and
making the worst case buffer size less than traditional 4K.
I wonder if we are going to be bitten by that one...
I vaguely recall there was an interesting regression in recent
kernel history when the implementation of the pipe buffer was
changed, with which, writing the same amount of data with
different number of writes made things behave differently and
making the worst case buffer size less than traditional 4K.
Just for performance reasons, I ended up doing merging anyway, so in fact
regardless of how you write the current pipe buffer size is up to 16
pages.
I think you can safely assume that pretty much any file descriptor you use
has at least 1kB of buffer. Even 4kB is likely a "safe assumption", and in
reality, most of them end up having even more.
With something like ssh, you probably end up having even deeper ones,
since you end up having the local pty or socket to ssh, and then ssh has
the TCP buffer to the network..
Linus
From: "H. Peter Anvin" <hpa@zytor.com> Date: 2016-06-15 22:42:08
Junio C Hamano wrote:
"H. Peter Anvin" [off-list ref] writes:
quoted
If you have an ssh connection, you're writing over a pipe to the ssh
process, and your local buffer is that pipe, which is PIPE_BUF size.
I vaguely recall there was an interesting regression in recent
kernel history when the implementation of the pipe buffer was
changed, with which, writing the same amount of data with
different number of writes made things behave differently and
making the worst case buffer size less than traditional 4K.
I wonder if we are going to be bitten by that one...
The definition of PIPE_BUF is that a write to a pipe of no more than
PIPE_BUF bytes will either succeed immediately or block; it will not be
broken up into multiple writes (with potential interlace problems.) It
says *nothing* about what happens with multiple writes.
-hpa