[PATCH 1 of 2 V2] peer: introduce a limitedarguments attributes

[Martin von Zweigbergk]

On Wed, Apr 17, 2019 at 10:41 AM Pierre-Yves David <
pierre-yves.david at ens-lyon.org> wrote:

>
>
> 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>>
>
> > wrote:
> >
> >     # HG changeset patch
> >     # User Pierre-Yves David <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 larges
> >     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 usual
> >     "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 'httppostargs'
> >     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
> arguments)
>
> > 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 again?
>

Question 1: Do "heads" and "known" request normally use GET or POST
(without this patch, without batching)?

I believe the answer is "GET".

Question 2: Do "batch" requests use POST?

I believe the answer is "yes".

Question 3: Does httppostargs switch GET requests to POST requests?

I believe the answer is "no".

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. We should also add a comment to the with-block where
we call "heads" and "known" so someone knows that we rely on batching there
for it to work.


>
>
>
> --
> Pierre-Yves David
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20190417/de7c8d30/attachment.html>