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. _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel