Dennis Kaarsemaker <dennis@xxxxxxxxxxxxxxx> writes: > On di, 2014-10-21 at 10:56 -0700, Junio C Hamano wrote: >> Dennis Kaarsemaker <dennis@xxxxxxxxxxxxxxx> writes: >> >> > By not clearing the request buffer in stateless-rpc mode, fetch-pack >> > would keep sending already known-common commits, leading to ever bigger >> > http requests, eventually getting too large for git-http-backend to >> > handle properly without filling up the pipe buffer in inflate_request. >> > --- >> > I'm still not quite sure whether this is the right thing to do, but make >> > test still passes :) The new testcase demonstrates the problem, when >> > running t5551 with EXPENSIVE, this test will hang without the patch to >> > fetch-pack.c and succeed otherwise. >> >> IIUC, because "stateless" is just that, i.e. the server-end does not >> keep track of what is already known, not telling what is known to be >> common in each request would fundamentally break the protocol. Am I >> mistaken? > > That sounds plausible, but why then does the fetch complete with this > line removed, and why does 'make test' still pass? The fetch-pack program tries to help the upload-pack program(s) running on the other end find what nodes in the graph both repositories have in common by sending what the repository on its end has. Some commits may not be known by the other side (e.g. your new commits that haven't been pushed there that are made on a branch forked from the common history), and some others may be known (i.e. you drilled down the history from the tips of your refs and reached a commit that you fetched from the common history previously). The latter are ACKed by the upload-pack process and are remembered to be re-sent to the _next_ incarnation of the upload-pack process when stateless RPC is in use. With your patch, you stop telling the upload-pack process what these two programs already found to be common in their exchange. After the first exchange, fetch-pack and upload-pack may have noticed that both ends have version 2.0, but because you do not convey that fact to the other side, the new incarnation of upload-pack may end up deciding that the version 1.9 is the newest common commit between the two, and sending commits between 1.9 and 2.0. If you imagine an extreme case, it would be easy to see why "the fetch completes" and "make test passes" are not sufficient to say anything about this change. Even if you break the protocol in in a way different from your patch, by not sending any "have", such a butchered "fetch-pack" will become "fetch everything from scratch", aka "clone". The end result will still have correct history and "fetch completes" would be true. But I'd prefer deferring a more detailed analysis/explanation to Shawn, as stateless RPC is his creation. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html