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

[Martin von Zweigbergk]

On Tue, Apr 23, 2019 at 11:35 PM Pierre-Yves David <
pierre-yves.david at ens-lyon.org> wrote:

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

Joerg already replied to this thread saying that the answer to question 3
is "yes", so I don't think we need to do anything else.


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