[PATCH 1 of 2 V2] peer: introduce a limitedarguments attributes
pierre-yves.david at ens-lyon.org
Wed Apr 24 02:35:41 EDT 2019
On 4/17/19 8:33 PM, Martin von Zweigbergk wrote:
> On Wed, Apr 17, 2019 at 10:41 AM Pierre-Yves David
> <pierre-yves.david at ens-lyon.org <mailto:pierre-yves.david at ens-lyon.org>>
> On 4/17/19 7:17 PM, Martin von Zweigbergk wrote:
> > On Wed, Apr 17, 2019 at 10:07 AM Pierre-Yves David
> > <pierre-yves.david at ens-lyon.org
> <mailto:pierre-yves.david at ens-lyon.org>
> <mailto:pierre-yves.david at ens-lyon.org
> <mailto:pierre-yves.david at ens-lyon.org>>>
> > wrote:
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david at octobus.net
> <mailto:pierre-yves.david at octobus.net>
> > <mailto:pierre-yves.david at octobus.net
> <mailto:pierre-yves.david at octobus.net>>>
> > # Date 1555516590 -7200
> > # Wed Apr 17 17:56:30 2019 +0200
> > # Node ID 6882a0101d389e27d697bf2e9717de176f273309
> > # Parent 607a0de9bae31df526da75b68ab2853787d8c31e
> > # EXP-Topic discovery-speedup
> > # Available At https://bitbucket.org/octobus/mercurial-devel/
> > # hg pull
> > https://bitbucket.org/octobus/mercurial-devel/ -r 6882a0101d38
> > peer: introduce a limitedarguments attributes
> > When set to True, it signal that the peer cannot receive too
> > arguments
> > and that algorithm must adapt. This should only be True for http
> > peer that does
> > not support argument passed as "post".
> > This will be useful to unlock better discovery performance in
> the next
> > changesets.
> > I am using a dedicated argument because this is not really a
> > "capabilities" things. An alternative approach would be to adds a
> > "large-arguments" to all peer, but the http peers. That
> seemed a bit
> > too hacky
> > to me.
> > diff --git a/mercurial/httppeer.py b/mercurial/httppeer.py
> > --- a/mercurial/httppeer.py
> > +++ b/mercurial/httppeer.py
> > @@ -382,6 +382,7 @@ class httppeer(wireprotov1peer.wirepeer)
> > self._path = path
> > self._url = url
> > self._caps = caps
> > + self.limitedarguments = caps is not None and
> > not in caps
> Should we limit them also if `caps is None`?
> > Is `'httppostargs' not in caps` enough to know that httppostargs
> will be
> > used for the "heads" request?
> the `httppostargs` capabilities is http specific and cannot be used
> outside of the httppeer code (for example, ssh server has no argument
> side issue but still won't says they are httppostargs capable).
> (note: we are talking about the `known` command heres, `heads` takes no
> > As I said in an earlier email, I'm not
> > sure it will be respected for GET requests (and "heads" is a GET
> > request, right?). Did you check that this will not result in a
> GET request?
> I am a bit confused about what you mean here. `known` will be included
> into a batch command (or standalone for quite older server). that
> command will be get of post according to `httppostargs`, won't it?
> I don't think I understood what your final question is. Can you try
> Question 1: Do "heads" and "known" request normally use GET or POST
> (without this patch, without batching)?
> I believe the answer is "GET".
I tried to figure out that part, but could not figure it out. If you
know where to look, I would be happy to get a pointer
> Question 2: Do "batch" requests use POST?
> I believe the answer is "yes".
According to my code reading and test: yes.
> Question 3: Does httppostargs switch GET requests to POST requests?
> I believe the answer is "no".
I have no idea. If not, we should probably do it.
> Together, that seems to mean that we now rely on the requests getting
> batched for the httppostargs thing to take effect. Correct? If correct,
> it seems we should also check if the server supports batching before we
> decide to remove the limit.
I think so. I am fine with that. sending a follow up
More information about the Mercurial-devel