threads / discuss / 12553

Re: warning: no common commits - slow pull

Subject: Re: warning: no common commits - slow pull

## tl;dr

9 messages between Mar 7, 2008 and Mar 10, 2008.

replies: 8people: 2as markdown or json

David Brownell· Mar 7, 2008, 01:35 UTC · lore
Any progress on fixing this?

I'll report that with git version 1.5.4.3.447.gc95b3.dirty (just a couple days old) I've observed this when updating a clone by pulling from

 (a) a parent on the same disk partition
 (b) a parent on a non-mirrored network server

So that would seem to trash the assumptions that this is related to version mismatch between mirrors, and that the fix can (or should!) wait till 1.6.0 ...

I was glad to see the "^C" workaround, that seems to work.

When I "git pull" it first fetches a bunch of files, then concludes (wrongly) "no common commits", then starts a second fetch of a *HUGE* number of files ... 400 MB is too much to pay when updating from rc3-last-week to rc4. But if I interrupt that second one with ^C, it seems that the first one fetched enough to make the next "git pull" go pretty quickly.

This does seem appear to show up more often lately because of RC4 having been tagged ... but I don't know for sure.

I've got a couple kernel workspaces still on last Friday's version, which -- if this holds true to form! -- will show this bug when I "git pull". So if there are experiments that would help nail down what's going on here, please spell them out to me ("this command, then this ... send this output...").

- Dave
Daniel Barkalow· Mar 8, 2008, 01:22 UTC · re: David Brownell · lore
On Thu, 6 Mar 2008, David Brownell wrote:
Show 33 quoted lines
> Any progress on fixing this?
> 
> I'll report that with git version 1.5.4.3.447.gc95b3.dirty
> (just a couple days old) I've observed this when updating
> a clone by pulling from
> 
>  (a) a parent on the same disk partition
>  (b) a parent on a non-mirrored network server
> 
> So that would seem to trash the assumptions that this is
> related to version mismatch between mirrors, and that the
> fix can (or should!) wait till 1.6.0 ...
> 
> I was glad to see the "^C" workaround, that seems to work.
> 
> When I "git pull" it first fetches a bunch of files, then
> concludes (wrongly) "no common commits", then starts a
> second fetch of a *HUGE* number of files ... 400 MB is too
> much to pay when updating from rc3-last-week to rc4.  But
> if I interrupt that second one with ^C, it seems that the
> first one fetched enough to make the next "git pull" go
> pretty quickly.
> 
> 
> This does seem appear to show up more often lately because
> of RC4 having been tagged ... but I don't know for sure.
> 
> I've got a couple kernel workspaces still on last Friday's
> version, which -- if this holds true to form! -- will show
> this bug when I "git pull".  So if there are experiments
> that would help nail down what's going on here, please
> spell them out to me ("this command, then this ... send
> this output...").

Actually, if you can make a tarball of the .git directory of one of those workspaces, and see if the bug is reproducable with that .git directory every time (particularly when pulling a local repository), it would be really helpful to have a reliable test case.

There's a debugging thing that would help, but it's not in your version. It's in next as

49aaddd102aff Teach upload-pack to log the received need lines to an fd
With that commit, you should be able to do:
GIT_DEBUG_SEND_PACK=3 git pull 3>UPLOAD_LOG

and get a file UPLOAD_LOG that will show what it's doing, although there's a reasonable chance that it'll only demonstrate that it's doing nothing helpful, which we already pretty much know.

	-Daniel
*This .sig left intentionally blank*
David Brownell· Mar 8, 2008, 22:48 UTC · re: Daniel Barkalow · lore
On Friday 07 March 2008, Daniel Barkalow wrote:
Show 14 quoted lines
> On Thu, 6 Mar 2008, David Brownell wrote:
> 
> > When I "git pull" it first fetches a bunch of files, then
> > concludes (wrongly) "no common commits", then starts a
> > second fetch of a *HUGE* number of files ... 400 MB is too
> > much to pay when updating from rc3-last-week to rc4.  But
> > if I interrupt that second one with ^C, it seems that the
> > first one fetched enough to make the next "git pull" go
> > pretty quickly.
> 
> Actually, if you can make a tarball of the .git directory of one of those 
> workspaces, and see if the bug is reproducable with that .git directory 
> every time (particularly when pulling a local repository), it would be 
> really helpful to have a reliable test case.

Seems to be. Let me know where I can stash a ~300 MB tar.bz2 file for you ...

> There's a debugging thing that would help, but it's not in your version.
So I updated.  :)
Show 10 quoted lines
> It's in next as
> 
> 49aaddd102aff Teach upload-pack to log the received need lines to an fd
> 
> With that commit, you should be able to do:
> 
> GIT_DEBUG_SEND_PACK=3 git pull 3>UPLOAD_LOG
> 
> and get a file UPLOAD_LOG that will show what it's doing, although there's 
> a reasonable chance that it'll only demonstrate that it's doing nothing 
That file has always been empty.
> helpful, which we already pretty much know.

Oddly, a few times when I tried that the bug didn't reproduce. One factor may be workspaces cloned a long time ago with early versions of GIT (or cogito).

- Dave
Daniel Barkalow· Mar 8, 2008, 22:58 UTC · re: David Brownell · lore
On Sat, 8 Mar 2008, David Brownell wrote:
Show 18 quoted lines
> On Friday 07 March 2008, Daniel Barkalow wrote:
> > On Thu, 6 Mar 2008, David Brownell wrote:
> > 
> > > When I "git pull" it first fetches a bunch of files, then
> > > concludes (wrongly) "no common commits", then starts a
> > > second fetch of a *HUGE* number of files ... 400 MB is too
> > > much to pay when updating from rc3-last-week to rc4.  But
> > > if I interrupt that second one with ^C, it seems that the
> > > first one fetched enough to make the next "git pull" go
> > > pretty quickly.
> > 
> > Actually, if you can make a tarball of the .git directory of one of those 
> > workspaces, and see if the bug is reproducable with that .git directory 
> > every time (particularly when pulling a local repository), it would be 
> > really helpful to have a reliable test case.
> 
> Seems to be.  Let me know where I can stash a ~300 MB tar.bz2 file
> for you ...

Wherever you've got web space, I guess. It shouldn't be a problem for me on the download side.

Show 17 quoted lines
> > There's a debugging thing that would help, but it's not in your version.
> 
> So I updated.  :)
> 
> 
> > It's in next as
> > 
> > 49aaddd102aff Teach upload-pack to log the received need lines to an fd
> > 
> > With that commit, you should be able to do:
> > 
> > GIT_DEBUG_SEND_PACK=3 git pull 3>UPLOAD_LOG
> > 
> > and get a file UPLOAD_LOG that will show what it's doing, although there's 
> > a reasonable chance that it'll only demonstrate that it's doing nothing 
> 
> That file has always been empty.

Hmm; if the testing code is working, you should get at least one pair of "#B/#E" lines. You probably need to either test with current "next" or cherry-pick that commit, since I don't think that commit's in a particular hurry to get to "master"

Show 5 quoted lines
> > helpful, which we already pretty much know.
> 
> Oddly, a few times when I tried that the bug didn't reproduce.
> One factor may be workspaces cloned a long time ago with early
> versions of GIT (or cogito).

Is it a few times on different workspaces, or with copies of the same workspace? I think that there's a dependance on how your current state and the new state happen to line up.

	-Daniel
*This .sig left intentionally blank*
David Brownell· Mar 8, 2008, 23:25 UTC · re: Daniel Barkalow · lore
On Saturday 08 March 2008, Daniel Barkalow wrote:
Show 7 quoted lines
> On Sat, 8 Mar 2008, David Brownell wrote:
> > Oddly, a few times when I tried that the bug didn't reproduce.
> > One factor may be workspaces cloned a long time ago with early
> > versions of GIT (or cogito).
> 
> Is it a few times on different workspaces, or with copies of the same 
> workspace?

I had several workspaces that weren't updated ... which by previous experience should *all* have done an extra update. I did the pull with the GIT_DEBUG on a few of them, and the bogus one didn't happen.

This seemed odd to me so the next time I tried without the GIT_DEBUG thing ... and got the bogus pull with "no common commits". This was unfortunately *after* I deleted tarballs of the previous workspaces, so that exact experiment can't be reproduced. Those were, however, "newer" workspaces -- probably created last summer, rather than a few years ago.

> I think that there's a dependance on how your current state and  
> the new state happen to line up.

The workspace in the tarball I can send you doesn't have any issues with reproducibility ... now.

- Dave
Daniel Barkalow· Mar 8, 2008, 23:27 UTC · re: David Brownell · lore
On Sat, 8 Mar 2008, David Brownell wrote:
Show 6 quoted lines
> On Saturday 08 March 2008, Daniel Barkalow wrote:
> > I think that there's a dependance on how your current state and  
> > the new state happen to line up.
> 
> The workspace in the tarball I can send you doesn't have any issues
> with reproducibility ... now.  

Let me see if I can write up a patch that you should be able to test without moving the big file around. I may run out of time for tonight, though.

	-Daniel
*This .sig left intentionally blank*
Daniel Barkalow· Mar 9, 2008, 18:47 UTC · re: Daniel Barkalow · lore
On Sat, 8 Mar 2008, Daniel Barkalow wrote:
Show 12 quoted lines
> On Sat, 8 Mar 2008, David Brownell wrote:
> 
> > On Saturday 08 March 2008, Daniel Barkalow wrote:
> > > I think that there's a dependance on how your current state and  
> > > the new state happen to line up.
> > 
> > The workspace in the tarball I can send you doesn't have any issues
> > with reproducibility ... now.  
> 
> Let me see if I can write up a patch that you should be able to test 
> without moving the big file around. I may run out of time for tonight, 
> though.

Try this. I'm not at all sure that it's doing what I want, but it passes all the current tests, and it should only affect your test case if it's actually right.

diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c
index 29b38e4..37cae35 100644
--- a/builtin-fetch-pack.c
+++ b/builtin-fetch-pack.c
@@ -26,6 +26,8 @@ static const char fetch_pack_usage[] =
 #define SEEN		(1U << 3)
 #define POPPED		(1U << 4)
 
+static int marked;
+
 /*
  * After sending this many "have"s if we do not get any new ACK , we
  * give up traversing our history.
@@ -61,6 +63,16 @@ static int rev_list_insert_ref(const char *path, const unsigned char *sha1, int
 	return 0;
 }
 
+static int clear_marks(const char *path, const unsigned char *sha1, int flag, void *cb_data)
+{
+	struct object *o = deref_tag(parse_object(sha1), path, 0);
+
+	if (o && o->type == OBJ_COMMIT)
+		clear_commit_marks((struct commit *)o, 
+				   COMMON | COMMON_REF | SEEN | POPPED);
+	return 0;
+}
+
 /*
    This function marks a rev and its ancestors as common.
    In some cases, it is desirable to mark only the ancestors (for example
@@ -153,6 +165,10 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 	unsigned in_vain = 0;
 	int got_continue = 0;
 
+	if (marked)
+		for_each_ref(clear_marks, NULL);
+	marked = 1;
+
 	for_each_ref(rev_list_insert_ref, NULL);
 
 	fetching = 0;
David Brownell· Mar 10, 2008, 17:18 UTC · re: Daniel Barkalow · lore
On Sunday 09 March 2008, Daniel Barkalow wrote:
> Try this. I'm not at all sure that it's doing what I want, but it passes 
> all the current tests, and it should only affect your test case if it's 
> actually right.
Seems to resolve that problem for me ... well timed, in terms of RC5!
But some stuff still looks a bit fishy.  See this RC4 --> RC5 pull:
  remote: Counting objects: 1329, done.
  remote: Compressing objects: 100% (276/276), done.
  remote: Total 908 (delta 749), reused 760 (delta 631)
  Receiving objects: 100% (908/908), 146.35 KiB | 40 KiB/s, done.
  Resolving deltas: 100% (749/749), completed with 287 local objects.
  remote: Counting objects: 1330, done.
  remote: Compressing objects: 100% (277/277), done.
  remote: Total 909 (delta 749), reused 760 (delta 631)
  Receiving objects: 100% (909/909), 146.63 KiB | 38 KiB/s, done.
  Resolving deltas: 100% (749/749), completed with 287 local objects.
  From git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6
   * [new tag]         v2.6.25-rc5 -> v2.6.25-rc5
  Updating 29e8c3c..cdeeeae
  Fast forward
  Auto packing your repository for optimum performance. You may also
  run "git gc" manually. See "git help gc" for more information.
  ...

That's pretty typical: two count/compress/receive/resolve passes, with almost an identical shape and size. Every time I see that, I suspect that most of the second one should not be needed...

However, the grace of not re-fetching the entire GIT database (at several hundred MBytes) the second time is much appreciated!

- Dave
Show 40 quoted lines
> --- a/builtin-fetch-pack.c
> +++ b/builtin-fetch-pack.c
> @@ -26,6 +26,8 @@ static const char fetch_pack_usage[] =
>  #define SEEN		(1U << 3)
>  #define POPPED		(1U << 4)
>  
> +static int marked;
> +
>  /*
>   * After sending this many "have"s if we do not get any new ACK , we
>   * give up traversing our history.
> @@ -61,6 +63,16 @@ static int rev_list_insert_ref(const char *path, const unsigned char *sha1, int
>  	return 0;
>  }
>  
> +static int clear_marks(const char *path, const unsigned char *sha1, int flag, void *cb_data)
> +{
> +	struct object *o = deref_tag(parse_object(sha1), path, 0);
> +
> +	if (o && o->type == OBJ_COMMIT)
> +		clear_commit_marks((struct commit *)o, 
> +				   COMMON | COMMON_REF | SEEN | POPPED);
> +	return 0;
> +}
> +
>  /*
>     This function marks a rev and its ancestors as common.
>     In some cases, it is desirable to mark only the ancestors (for example
> @@ -153,6 +165,10 @@ static int find_common(int fd[2], unsigned char *result_sha1,
>  	unsigned in_vain = 0;
>  	int got_continue = 0;
>  
> +	if (marked)
> +		for_each_ref(clear_marks, NULL);
> +	marked = 1;
> +
>  	for_each_ref(rev_list_insert_ref, NULL);
>  
>  	fetching = 0;
> 
Daniel Barkalow· Mar 10, 2008, 17:40 UTC · re: David Brownell · lore
On Mon, 10 Mar 2008, David Brownell wrote:
Show 30 quoted lines
> On Sunday 09 March 2008, Daniel Barkalow wrote:
> > Try this. I'm not at all sure that it's doing what I want, but it passes 
> > all the current tests, and it should only affect your test case if it's 
> > actually right.
> 
> Seems to resolve that problem for me ... well timed, in terms of RC5!
> 
> But some stuff still looks a bit fishy.  See this RC4 --> RC5 pull:
> 
>   remote: Counting objects: 1329, done.
>   remote: Compressing objects: 100% (276/276), done.
>   remote: Total 908 (delta 749), reused 760 (delta 631)
>   Receiving objects: 100% (908/908), 146.35 KiB | 40 KiB/s, done.
>   Resolving deltas: 100% (749/749), completed with 287 local objects.
>   remote: Counting objects: 1330, done.
>   remote: Compressing objects: 100% (277/277), done.
>   remote: Total 909 (delta 749), reused 760 (delta 631)
>   Receiving objects: 100% (909/909), 146.63 KiB | 38 KiB/s, done.
>   Resolving deltas: 100% (749/749), completed with 287 local objects.
>   From git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6
>    * [new tag]         v2.6.25-rc5 -> v2.6.25-rc5
>   Updating 29e8c3c..cdeeeae
>   Fast forward
>   Auto packing your repository for optimum performance. You may also
>   run "git gc" manually. See "git help gc" for more information.
>   ...
> 
> That's pretty typical:  two count/compress/receive/resolve passes,
> with almost an identical shape and size.  Every time I see that, I
> suspect that most of the second one should not be needed...

Ah, I bet it's not rereading the list of current refs when it updates them, and therefore only says it has the stuff that it had before. That would actually explain somewhat why the other bug was actually mattering.

Should be easy to fix, but I'm not sure when I'll get a chance to work on it.

	-Daniel
*This .sig left intentionally blank*

← back to recent threads