From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 7C3221FF139 for ; Tue, 24 Feb 2026 13:17:22 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 2CA0F85DF; Tue, 24 Feb 2026 13:18:18 +0100 (CET) Message-ID: <872656d9-21cc-4bf8-91c5-063a8fab94e3@proxmox.com> Date: Tue, 24 Feb 2026 13:17:43 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [pbs-devel] [PATCH proxmox 3/3] fix #6858: s3-client: retry request on 500 and 503 response status To: =?UTF-8?Q?Fabian_Gr=C3=BCnbichler?= , Proxmox Backup Server development discussion References: <20260123145835.625914-1-c.ebner@proxmox.com> <20260123145835.625914-4-c.ebner@proxmox.com> <1771934648.hk9upfhes9.astroid@yuna.none> Content-Language: en-US, de-DE From: Christian Ebner In-Reply-To: <1771934648.hk9upfhes9.astroid@yuna.none> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1771935448636 X-SPAM-LEVEL: Spam detection results: 0 AWL -1.015 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 1.179 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.717 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.236 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: LCIHW4UQ6XV4SYGTLBG3B7O6NM2YNMC3 X-Message-ID-Hash: LCIHW4UQ6XV4SYGTLBG3B7O6NM2YNMC3 X-MailFrom: c.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 2/24/26 1:05 PM, Fabian Grünbichler wrote: > On January 23, 2026 3:58 pm, Christian Ebner wrote: >> Follow the best practices for AWS S3 error handling [0] and perform >> retries on requests with http status code 500 or 503 in the response. >> >> This is done for all requests unconditionally, maximum number of >> retires and optional request timeout being honored. >> >> [0] https://docs.aws.amazon.com/AmazonS3/latest/API/ErrorBestPractices.html >> >> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=6858 >> Signed-off-by: Christian Ebner >> --- >> proxmox-s3-client/src/client.rs | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/proxmox-s3-client/src/client.rs b/proxmox-s3-client/src/client.rs >> index f3e5eb45..f8cae7da 100644 >> --- a/proxmox-s3-client/src/client.rs >> +++ b/proxmox-s3-client/src/client.rs >> @@ -389,7 +389,14 @@ impl S3Client { >> }; >> >> match response { >> - Ok(response) => return Ok(response), >> + Ok(response) => match response.status() { >> + StatusCode::INTERNAL_SERVER_ERROR | StatusCode::SERVICE_UNAVAILABLE => { >> + if retry >= MAX_S3_HTTP_REQUEST_RETRY - 1 { >> + bail!("request failed exceeding retries"); > > it would be less of a change in behaviour and provide more context to > callers if we returned Ok(response) here, like before? True, will resend, maybe also including the 504 status code as retryable status code, although not explicitly documented as such. This did however cause some churn for users using Hetzner's S3 offerings as reported in https://forum.proxmox.com/threads/180956/, so worth to retry as well.