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

[Pierre-Yves David]

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>> 
> 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>
>     <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
>     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".

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



-- 
Pierre-Yves David