Patchwork [2,of,2] setdiscovery: limit the size of the initial sample (issue4411)

login
register
mail settings
Submitter Pierre-Yves David
Date Oct. 27, 2014, 5:23 p.m.
Message ID <6467e9577b3ec59e84eb.1414430601@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/6475/
State Accepted
Headers show

Comments

Pierre-Yves David - Oct. 27, 2014, 5:23 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1414428753 -3600
#      Mon Oct 27 17:52:33 2014 +0100
# Branch stable
# Node ID 6467e9577b3ec59e84ebc263e7e6cb04d3305907
# Parent  9abdfe27dd006ff12d721fc127d9cea519f1a8b9
setdiscovery: limit the size of the initial sample (issue4411)

The set discovery start by sending a "known" command with all local heads. When
the number of local heads is massive (eg: using hidden changesets) such request
becomes too large. This lead to 414 error over http, aborting the whole
process.

We limit the size of the sample used by the first query to fix this.

The test are impacted because they do test massive number of heads. But they do
not test it over real world http setup.
Matt Mackall - Oct. 27, 2014, 6:18 p.m.
On Mon, 2014-10-27 at 18:23 +0100, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1414428753 -3600
> #      Mon Oct 27 17:52:33 2014 +0100
> # Branch stable
> # Node ID 6467e9577b3ec59e84ebc263e7e6cb04d3305907
> # Parent  9abdfe27dd006ff12d721fc127d9cea519f1a8b9
> setdiscovery: limit the size of the initial sample (issue4411)

These are queued for stable, thanks.

Patch

diff --git a/mercurial/setdiscovery.py b/mercurial/setdiscovery.py
--- a/mercurial/setdiscovery.py
+++ b/mercurial/setdiscovery.py
@@ -131,11 +131,11 @@  def findcommonheads(ui, local, remote,
 
     # early exit if we know all the specified remote heads already
     ui.debug("query 1; heads\n")
     roundtrips += 1
     ownheads = dag.heads()
-    sample = ownheads
+    sample = _limitsample(ownheads, initialsamplesize)
     if remote.local():
         # stopgap until we have a proper localpeer that supports batch()
         srvheadhashes = remote.heads()
         yesno = remote.known(dag.externalizeall(sample))
     elif remote.capable('batch'):
diff --git a/tests/test-setdiscovery.t b/tests/test-setdiscovery.t
--- a/tests/test-setdiscovery.t
+++ b/tests/test-setdiscovery.t
@@ -309,20 +309,20 @@  One with >200 heads, which used to use u
   comparing with b
   query 1; heads
   searching for changes
   taking quick initial sample
   searching: 2 queries
-  query 2; still undecided: 1080, sample size is: 260
+  query 2; still undecided: 1240, sample size is: 260
   sampling from both directions
   searching: 3 queries
-  query 3; still undecided: 820, sample size is: 260
+  query 3; still undecided: 980, sample size is: 260
   sampling from both directions
   searching: 4 queries
-  query 4; still undecided: 560, sample size is: 260
+  query 4; still undecided: 720, sample size is: 260
   sampling from both directions
   searching: 5 queries
-  query 5; still undecided: 300, sample size is: 200
+  query 5; still undecided: 460, sample size is: 200
   5 total queries
   common heads: 3ee37d65064a
 
 Test actual protocol when pulling one new head in addition to common heads