[pacman-dev] Downloader options - Was: Add configuration options for libcurl's "low speed" timeout

[Olivier Brunel]

On Sat, 3 Sep 2016 09:40:52 -0400
Dave Reisner <d at falconindy.com> wrote:

> On Sat, Sep 03, 2016 at 10:45:42PM +1000, Allan McRae wrote:
> > On 30/08/16 22:48, Christian Hesse wrote:  
> > > Dave Reisner <d at falconindy.com> on Tue, 2016/08/30 08:46:  
> > >> On Tue, Aug 30, 2016 at 02:12:23PM +0200, Christian Hesse
> > >> wrote:  
> > >>> From: Christian Hesse <mail at eworm.de>
> > >>>
> > >>> Add LowSpeedLimit and LowSpeedTime configuration options to
> > >>> correspond to libcurl's CURLOPT_LOW_SPEED_LIMIT and
> > >>> CURLOPT_LOW_SPEED_TIME options. This allows, e.g., transfers
> > >>> behind corporate virus-scanning firewalls to survive the
> > >>> delays. Increasing the timeout may not be desirable in all
> > >>> situations; similarly, disabling the check prevents detection
> > >>> of disappearing networks.    
> > >>
> > >> FWIW, I'm strongly opposed to having a 1:1 mapping between pacman
> > >> options and curl config options. Please look at the bigger
> > >> picture -- it's dead connection detection. We might reimplement
> > >> that in the future in some other way (e.g. via the progress
> > >> callback or by some other transfer library entirely).
> > >>
> > >> To that end, I think it would be reasonable to add a boolean
> > >> toggle for the dead connection detection (default on). The patch
> > >> in its current state makes me rather itchy from an API
> > >> perspective.  
> > > 
> > > That is what my stupid-proxy patch does...
> > > Now it is up to Allan to decide. ;)  
> > 
> > Crap...  Why do I need to make decisions?
> > 
> > OK - lets think on the go here...  The two options we "want" to
> > support based on submitted patches are:
> > 1) Disabling the low speed timeout
> > 2) Setting maximum download speed
> > 
> > In the future, we might also add a parallel download option.
> > 
> > I'm guessing these want both global config values and command line
> > options.  Anyone want to suggest what these could be.  I'll choose
> > the colour of the bikeshed.
> > 
> > Allan  
> 
> 1) DetectDeadConnections
> 2) MaxDownloadKBps
> future) MaxConcurrentDownloads
> 
> For #2, I'll suggest that you can already do this with a program like
> netbrake. Curl's internal rate limiting is not good and only uses a
> flat average, rather than a rolling window of recent samples. It's

FWIW I've actually submitted a patch[1] recently to improve curl's
limiting algo that should improve this. (Of course that'll only apply
to users with a quite recent libcurl.)

In my patch[2] (for pacman this time) I used MaxDlSpeed as name so one
can specify a unit, whereas MaxDownloadKBps implies one (though likely
the most common one I suppose).

Using "Dl" instead of "Download" in the option name seemed like still
pretty common/understandable, all the while making things a bit
shorter, especially nice for the command line option.

With similar intent, maybe a shorter one for #1 might be better... e.g.
NoLowSpeedTimeout, or simply NoSlowTimeout ?
(Also I guess yours should be NoDetectDeadConnections or
similar, as the option is actually meant to *disable* the low speed
timeout.)

> idea of "limiting" bandwidth is inserting an artificial delay before
> the next recv call, which might just be draining a buffer of already
> received data from the kernel. This is particularly true if you're
> behind a router and not the actual gateway device (and I suspect this
> is the common case).
> 
> d

[1] https://github.com/curl/curl/pull/971
[2]
https://lists.archlinux.org/pipermail/pacman-dev/2016-August/021265.html