[pve-devel] [PATCH] increase zfs default timeout to 30sec

[Lauri Tirkkonen]

On Thu, Apr 12 2018 12:59:13 +0200, Fabian Grünbichler wrote:
> On Tue, Apr 10, 2018 at 05:40:50PM +0300, Lauri Tirkkonen wrote:
> > Hi,
> > 
> > On Tue, Mar 13 2018 10:25:47 +0100, Thomas Lamprecht wrote:
> > > What Fabian meant with:
> > > > [...] our API has a timeout per request, [...]
> > > 
> > > is that our API already has 30 seconds as timeout for response,
> > > so using 30 seconds here can be problematic.
> > > 
> > > As a quick easy improvement we could probably increase it from
> > > 5 to say 20-25 seconds, still below the API timeout, but nonetheless
> > > multiple times higher than now.
> > 
> > Now that the CLA stuff is out of the way, here's a diff to do that then.
> 
> if you bump the timeout to 20, you need to check every non-worker call
> path that ends up in zfs_request to verify that you don't call
> zfs_request twice (because then you'd have a total timeout of 40 which
> is over the 30s API request limit).
>
> unless you have done that (and somehow provide a convincing argument
> that you did), this cannot be applied. if you do that, you will quickly
> realize that converting API endpoints that can trigger expensive storage
> operations to workers makes more sense, which is why I originally
> proposed that approach.

I have not done that. I submitted this diff because Thomas implied it
could be done: a quick and easy fix, that fixed a real problem for us.

Knowing very close to zero about PVE code in general I'm much more
hesitant to do things like converting things to workers - I imagine that
will end up being a change so large that I'd likely give up before
finishing. You don't have to take my diff, of course, but I have no
plans to write your convert-to-worker diff either.

Thanks.