{"thread":{"id":"4915","subject":"[PATCH] upload-pack: fix timeout in create_pack_file","startedAt":"2006-07-18T17:14:51Z","lastAt":"2006-07-24T07:55:18Z","messageCount":4,"participants":["Matthias Lederhofer","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"23923","messageId":"E1G2t9n-0005PW-72@moooo.ath.cx","threadId":"4915","inReplyTo":null,"subject":"[PATCH] upload-pack: fix timeout in create_pack_file","fromName":"Matthias Lederhofer","fromEmail":"matled@gmx.net","sentAt":"2006-07-18T17:14:51Z","receivedAt":"2006-07-18T17:14:51Z","isPatch":true,"sender":{"key":"matled@gmx.net","avatar":null},"body":"Signed-off-by: Matthias Lederhofer \n---\n fatal: packfile '../linux-2.6/.git/objects/pack/tmp-7iPJo5'\n SHA1 mismatch\n error: git-fetch-pack: unable to read from git-index-pack\n error: git-index-pack died with error code 128\n Any idea what this means?\nThis happens after ~12 minutes. The problem is that the loop in\nupload-pack.c actually sending the pack does not reset the timeout.\nI'd guess --timeout is 600 or a bit more on git.kernel.org :)\n\nThis does not help for low timeouts with slow clients. If a client is\nslow enough so the server is blocked for more time than specified by\ntimeout the connection will be closed too (e.g. 15kb/s with a timeout\nof 30 (git adds 10 extra) is not enough). We should either add a\nwarning to the man page or try to fix this. I don't know if this can\nbe fixed not using non-blocking sockets.\n\nPerhaps support for resume would be quite useful too but I've no idea\nhow hard this is to implement.\n\nA workaround for is to pull a part of the repository first and the\nrest later. For example using this:\n$ git init-db\n$ git fetch URL refs/tags/old-tag:refs/tags/old-tag\n$ git fetch URL refs/tags/newer-tag:refs/tags/newer-tag\n..\n---\n upload-pack.c | 2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex f6f5a7e..07ecdb4 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -182,6 +182,8 @@ static void create_pack_file(void)\n \t\tssize_t sz;\n \t\tint pe, pu, pollsize;\n \n+\t\treset_timeout();\n+\n \t\tpollsize = 0;\n \t\tpe = pu = -1;\n \n-- \n1.4.2.rc1.ge7a0\n"},{"id":"24037","messageId":"7vd5bvh67p.fsf@assigned-by-dhcp.cox.net","threadId":"4915","inReplyTo":"E1G2t9n-0005PW-72@moooo.ath.cx","subject":"Re: [PATCH] upload-pack: fix timeout in create_pack_file","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-24T06:23:06Z","receivedAt":"2006-07-24T06:23:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthias Lederhofer writes:\n\n> Signed-off-by: Matthias Lederhofer \n> ---\n> fatal: packfile '../linux-2.6/.git/objects/pack/tmp-7iPJo5'\n> SHA1 mismatch\n> error: git-fetch-pack: unable to read from git-index-pack\n> error: git-index-pack died with error code 128\n> Any idea what this means?\n> This happens after ~12 minutes. The problem is that the loop in\n> upload-pack.c actually sending the pack does not reset the timeout.\n> I'd guess --timeout is 600 or a bit more on git.kernel.org :)\n>\n> This does not help for low timeouts with slow clients. If a client is\n> slow enough so the server is blocked for more time than specified by\n> timeout the connection will be closed too (e.g. 15kb/s with a timeout\n> of 30 (git adds 10 extra) is not enough).\n\nWhere do we add 10 extra?\n\n> We should either add a\n> warning to the man page or try to fix this. I don't know if this can\n> be fixed not using non-blocking sockets.\n\nI think the intent of \"timeout\" was to protect us from funny\nclients by avoiding talking with the ones that take too much\ntime doing something that should not take too long. When we are\nin create_pack_file(), we are already committed to the heaviest\noperation anyway, so one possibility might be to stop doing\ntimeout at that point. I am not sure if that is acceptable,\nthough -- it opens up the daemon to even easier DoS than it\ncurrently is.\n\nMy gut feeling is that your patch would be fine as is (have you\ntried and confirmed that it helps cases other than slow\nclients?)\n\n> Perhaps support for resume would be quite useful too but I've no idea\n> how hard this is to implement.\n\nThat would be _very_ hard.\n"},{"id":"24051","messageId":"E1G4uZw-0006vZ-VO@moooo.ath.cx","threadId":"4915","inReplyTo":"7vd5bvh67p.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] upload-pack: fix timeout in create_pack_file","fromName":"Matthias Lederhofer","fromEmail":"matled@gmx.net","sentAt":"2006-07-24T07:10:12Z","receivedAt":"2006-07-24T07:10:12Z","isPatch":true,"sender":{"key":"matled@gmx.net","avatar":null},"body":"Junio C Hamano wrote:\n> > This does not help for low timeouts with slow clients. If a client is\n> > slow enough so the server is blocked for more time than specified by\n> > timeout the connection will be closed too (e.g. 15kb/s with a timeout\n> > of 30 (git adds 10 extra) is not enough).\n> \n> Where do we add 10 extra?\nOops, never mind, I misread the source.\n\n> > We should either add a\n> > warning to the man page or try to fix this. I don't know if this can\n> > be fixed not using non-blocking sockets.\n> \n> I think the intent of \"timeout\" was to protect us from funny\n> clients by avoiding talking with the ones that take too much\n> time doing something that should not take too long. When we are\n> in create_pack_file(), we are already committed to the heaviest\n> operation anyway, so one possibility might be to stop doing\n> timeout at that point. I am not sure if that is acceptable,\n> though -- it opens up the daemon to even easier DoS than it\n> currently is.\n> \n> My gut feeling is that your patch would be fine as is (have you\n> tried and confirmed that it helps cases other than slow\n> clients?)\nWhat else? The problem solved by this patch is the missing\nreset_timeout while downloading pack files and this affects all\nclients that are too slow (cannot download the packfile within\n--timeout seconds). I tried a server with --timeout=600 and a client\nthat cannot download the whole pack file in 10 minutes. Without the\npatch the server closes the connection, with this patch it works.\nAdditionally I tried it with a client reading so slow that the server\nwas blocked for more than --timeout seconds, then the server closes\nthe connection too (this is more or less the DoS case).\n\n> > Perhaps support for resume would be quite useful too but I've no idea\n> > how hard this is to implement.\n> \n> That would be _very_ hard.\nI don't know how much demand there is for resuming but there is always rsync\n(at least on kernel.org and other repositories that need it can install it too)\nto get the new objects with support for resuming.\n"},{"id":"24054","messageId":"7vac6ze8t5.fsf@assigned-by-dhcp.cox.net","threadId":"4915","inReplyTo":"E1G4uZw-0006vZ-VO@moooo.ath.cx","subject":"Re: [PATCH] upload-pack: fix timeout in create_pack_file","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-07-24T07:55:18Z","receivedAt":"2006-07-24T07:55:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthias Lederhofer writes:\n\n> Junio C Hamano wrote:\n>\n>> My gut feeling is that your patch would be fine as is (have you\n>> tried and confirmed that it helps cases other than slow\n>> clients?)\n> ... I tried a server with --timeout=600 and a client\n> that cannot download the whole pack file in 10 minutes. Without the\n> patch the server closes the connection, with this patch it works.\n> Additionally I tried it with a client reading so slow that the server\n> was blocked for more than --timeout seconds, then the server closes\n> the connection too (this is more or less the DoS case).\n\nThanks, that's all I wanted to know. Let's have it.\n"}]}