git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] give priority to progress messages

From
Nicolas Pitre <nico@fluxnic.net>
Date
Nov 11, 2009, 22:24 UTC
Message-ID
<alpine.LFD.2.00.0911111707390.16711@xanadu.home>
In-Reply-To
<20091111214216.GO12890@machine.or.cz>

In theory it is possible for sideband channel #2 to be delayed if pack data is quick to come up for sideband channel #1. And because data for channel #2 is read only 128 bytes at a time while pack data is read 8192 bytes at a time, it is possible for many pack blocks to be sent to the client before the progress message fifo is emptied, making the situation even worse. This would result in totally garbled progress display on the client's console as local progress gets mixed with partial remote progress lines.

Let's prevent such situations by giving transmission priority to progress messages over pack data at all times.

Signed-off-by: Nicolas Pitre <nico@fluxnic.net>
---
On Wed, 11 Nov 2009, Petr Baudis wrote:
Show 24 quoted lines
> On Wed, Nov 11, 2009 at 04:19:44PM -0500, Nicolas Pitre wrote:
> > On Wed, 11 Nov 2009, Petr Baudis wrote:
> > 
> > > On Wed, Nov 11, 2009 at 02:50:22PM -0500, Nicolas Pitre wrote:
> > > > According to strace, data from sideband channel #2 (prefixed with 
> > > > "remote: ") pertaining to object compression is printed way after pack 
> > > > data has already started to arrive locally.  This is really weird.
> > > > 
> > > > And this occurs only when fetching from repo.or.cz and not from 
> > > > git.kernel.org for example.  So there is something to investigate on the 
> > > > server side.  Pasky: anything you changed in your git installation 
> > > > lately?
> > > 
> > > Yes, but nothing should have changed in git-daemon, that's the only part
> > > of the infrastructure that uses system-wide git (which it perhaps
> > > shouldn't). I cannot reproduce this problem, though. I have changed
> > > git-daemon to use my local git version (about one week old master), does
> > > this still happen for you?
> > 
> > No, it doesn't happen anymore.
> > 
> > What was the git-daemon version before?
> 
> 1.5.6.5, the default version in debian lenny

I don't see why the issue couldn't happen with latest git as well. This patch however would plug any possibilities for this to happen again though.

diff --git a/builtin-upload-archive.c b/builtin-upload-archive.c
index c4cd1e1..29446e8 100644
--- a/builtin-upload-archive.c
+++ b/builtin-upload-archive.c
@@ -132,7 +132,6 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 
 	while (1) {
 		struct pollfd pfd[2];
-		ssize_t processed[2] = { 0, 0 };
 		int status;
 
 		pfd[0].fd = fd1[0];
@@ -147,15 +146,14 @@ int cmd_upload_archive(int argc, const char **argv, const char *prefix)
 			}
 			continue;
 		}
-		if (pfd[0].revents & POLLIN)
-			/* Data stream ready */
-			processed[0] = process_input(pfd[0].fd, 1);
 		if (pfd[1].revents & POLLIN)
 			/* Status stream ready */
-			processed[1] = process_input(pfd[1].fd, 2);
-		/* Always finish to read data when available */
-		if (processed[0] || processed[1])
-			continue;
+			if (process_input(pfd[1].fd, 2))
+				continue;
+		if (pfd[0].revents & POLLIN)
+			/* Data stream ready */
+			if (process_input(pfd[0].fd, 1))
+				continue;
 
 		if (waitpid(writer, &status, 0) < 0)
 			error_clnt("%s", lostchild);
diff --git a/upload-pack.c b/upload-pack.c
index 70badcf..6bfb500 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -310,6 +310,23 @@ static void create_pack_file(void)
 			}
 			continue;
 		}
+		if (0 <= pe && (pfd[pe].revents & (POLLIN|POLLHUP))) {
+			/* Status ready; we ship that in the side-band
+			 * or dump to the standard error.
+			 */
+			sz = xread(pack_objects.err, progress,
+				  sizeof(progress));
+			if (0 < sz)
+				send_client_data(2, progress, sz);
+			else if (sz == 0) {
+				close(pack_objects.err);
+				pack_objects.err = -1;
+			}
+			else
+				goto fail;
+			/* give priority to status messages */
+			continue;
+		}
 		if (0 <= pu && (pfd[pu].revents & (POLLIN|POLLHUP))) {
 			/* Data ready; we keep the last byte to ourselves
 			 * in case we detect broken rev-list, so that we
@@ -347,21 +364,6 @@ static void create_pack_file(void)
 			if (sz < 0)
 				goto fail;
 		}
-		if (0 <= pe && (pfd[pe].revents & (POLLIN|POLLHUP))) {
-			/* Status ready; we ship that in the side-band
-			 * or dump to the standard error.
-			 */
-			sz = xread(pack_objects.err, progress,
-				  sizeof(progress));
-			if (0 < sz)
-				send_client_data(2, progress, sz);
-			else if (sz == 0) {
-				close(pack_objects.err);
-				pack_objects.err = -1;
-			}
-			else
-				goto fail;
-		}
 	}
 
 	if (finish_command(&pack_objects)) {
Previous: Nicolas PitreNext: Jakub Narebski
Message 35 of 36 in “excerpts from tomorrow's "What's cooking" draft”
  1. Junio C HamanoNov 11, 2009
  2. Ben WaltonNov 11, 2009
  3. Johan HerlandNov 11, 2009
  4. Sverre RabbelierNov 11, 2009
  5. Daniel BarkalowNov 11, 2009
  6. Sverre RabbelierNov 15, 2009
  7. Daniel BarkalowNov 15, 2009
  8. ks/precompute-completionJonathan Nieder, Nov 11, 2009
  9. Stephen BoydNov 13, 2009
  10. Jonathan NiederNov 13, 2009
  11. Stephen BoydNov 13, 2009
  12. Speed up bash completion loadingJonathan Nieder, Nov 13, 2009
  13. Jonathan NiederNov 13, 2009
  14. Jonathan NiederNov 13, 2009
  15. Stephen BoydNov 13, 2009
  16. Jonathan NiederNov 14, 2009
  17. Jonathan NiederNov 14, 2009
  18. SZEDER GáborNov 14, 2009
  19. Jonathan NiederNov 14, 2009
  20. Stephen BoydNov 14, 2009
  21. Jonathan NiederNov 15, 2009
  22. Junio C HamanoNov 15, 2009
  23. Speed up bash completion loadingJonathan Nieder, Nov 15, 2009
  24. Shawn O. PearceNov 16, 2009
  25. Stephen BoydNov 16, 2009
  26. Speed up bash completion loadingJonathan Nieder, Nov 18, 2009
  27. Shawn O. PearceNov 18, 2009
  28. Nicolas SebrechtNov 11, 2009
  29. Nicolas PitreNov 11, 2009
  30. Petr BaudisNov 11, 2009
  31. Nicolas PitreNov 11, 2009
  32. Nicolas SebrechtNov 11, 2009
  33. Petr BaudisNov 11, 2009
  34. Nicolas PitreNov 11, 2009
  35. give priority to progress messagesNicolas Pitre, Nov 11, 2009
  36. Jakub NarebskiNov 11, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.