From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id A3E2E8D68A; Tue, 8 Nov 2022 16:54:24 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 79ED1A385; Tue, 8 Nov 2022 16:53:54 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS; Tue, 8 Nov 2022 16:53:53 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 519B142B1F; Tue, 8 Nov 2022 16:53:53 +0100 (CET) Message-ID: Date: Tue, 8 Nov 2022 16:53:52 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:107.0) Gecko/20100101 Thunderbird/107.0 Content-Language: en-GB To: Proxmox VE development discussion , Dominik Csapak , pbs-devel@lists.proxmox.com References: <20220527082203.1653182-1-d.csapak@proxmox.com> <20220527082203.1653182-2-d.csapak@proxmox.com> <15393855-d2d4-8a6f-6916-dd895e7b6cef@proxmox.com> <8dd1d4cb-509f-e000-9c19-4165c9a74b6f@proxmox.com> From: Thomas Lamprecht In-Reply-To: <8dd1d4cb-509f-e000-9c19-4165c9a74b6f@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.033 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com] Subject: Re: [pve-devel] [PATCH common v3 1/1] PBSClient: file_restore_list: add timeout parameter X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 08 Nov 2022 15:54:24 -0000 Am 08/11/2022 um 12:20 schrieb Dominik Csapak: > On 11/7/22 15:17, Thomas Lamprecht wrote: >> subject is not wrong but worded rather confusingly, as of now it rather >> implies that this adds a new parameter allowing callers to control the >> timeout, but actually it sets the timeout hard-coded to 25s. >> >> Am 27/05/2022 um 10:22 schrieb Dominik Csapak: >>> we always want the restore_list to use a timeout here. Set it to 25 seconds >> >> Such statements could be a bit more useful with some actual reasoning >> (e.g., short sentence about ill effects of lacking timeout) > > ok i thought the sentence below would be enough reasoning > not really as it doesn't explains much for **why** headroom would be required, if the clients gets the response cut short anyway at 30s what benefits do we gain here, just another error message or otherwise improved behavior? Why not just alarm($foo) on the call site. main point is, it really doesn't hurts to have that relevant information here too, not just in the pbs side of the commit.. >> >>> so there is a little headroom between this and pveproxys 30s one. >> >> what if we'd add a call site outside the sync API response context >> (e.g., task worker or CLI rpcenv)? could be an artificial limitation >> in that case. > > i followed your suggestion from the v1 version by hardcoding the options > and you applied the pbs ones from v2 partially without > complaining about this ;) You mean https://lists.proxmox.com/pipermail/pve-devel/2022-February/051664.html ? That was a comment about a horrid general "pass anything" interface Wolfgang agreed too, not specific to the timeout param and it's implications, that's why I asked here about if you thought about that (which you did not replied at all...) > > in any case, since i have to touch this again when doing the > user dependent memory increase for the file restore, > i'd rather use the other approach wolfang mentioned > by having a %param hash with the 'timeout' (and > dynamic memory) option. > > i'd send these two things together in one (pve) series. > is that ok for you? > not sure, my question about what happens if I call this in CLI and if, whatever does, should happen is not really answered.