On Sat, 3 Sep 2016 09:40:52 -0400 Dave Reisner <[email protected]> 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 <[email protected]> on Tue, 2016/08/30 08:46: > > >> On Tue, Aug 30, 2016 at 02:12:23PM +0200, Christian Hesse > > >> wrote: > > >>> From: Christian Hesse <[email protected]> > > >>> > > >>> 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
