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 2D5E41FF141 for ; Mon, 16 Mar 2026 15:04:40 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 314601BFFD; Mon, 16 Mar 2026 15:04:52 +0100 (CET) Message-ID: Date: Mon, 16 Mar 2026 15:04:46 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH proxmox{,-backup} v5 00/33] partially fix #6563: add s3 request and traffic counter statistics To: Christian Ebner , pbs-devel@lists.proxmox.com References: <20260311130823.724888-1-c.ebner@proxmox.com> Content-Language: en-US From: Hannes Laimer In-Reply-To: <20260311130823.724888-1-c.ebner@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1773669846382 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.078 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_MSPIKE_H2 0.001 Average reputation (+2) 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: ERXGSBLXYWA6FGGA5THUC4D7HVG6NBKS X-Message-ID-Hash: ERXGSBLXYWA6FGGA5THUC4D7HVG6NBKS X-MailFrom: h.laimer@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: This is a really well put together series, and with the things I've mentioned this looks very good to me! I did do some testing, and everything worked. I am not super sure if having a "hard" limit makes sense, so no new operations after that? On 2026-03-11 14:08, Christian Ebner wrote: > This patch series implements request and traffic counters for the s3 > client, being shared as atomic counters via shared memory and mmapping > across all s3-client instances. > > The shared counters are instantiated on demand for individual > datastores with s3 backend on s3 client instantiation and stored as > part of the datastore implementation by loading the mmapped file on > datastore instantiation, being cached for further access. > > Further, counters are being loaded during rrd metrics collection and > exposed as charts in the datastore summary, including the total > request counts per method, the total upload and download traffic as > well as the averaged upload and download rate. > > Usage statistics, are not included in this series an will be tackled > as followup series. > > Link to the bugtracker issue: > https://bugzilla.proxmox.com/show_bug.cgi?id=6563 > > Changes since version 4: > - Rebase on top of patch series https://lore.proxmox.com/pbs-devel/20260311095906.202410-1-c.ebner@proxmox.com/T/ > as requested by Fabian in order to facilitate review flow. > > Changes since version 3: > - Extend previous implementation by threshold values for counters and > mechanism to send notifications once thresholds are exceeded > > Changes since version 2: > - Introduce msync flushing mechanism to periodically persist counters > state to mmap backing file. > > Changes since version 1 (thanks @Robert for feedback): > - Keep tmpfs check in shmem mapping for pre-existing code, add > dedicated methods to create mmapped files in persistent locations. > - Reorder and align atomic counters to 32-byte (half default cache > line size) to reduce false sharing. > - Rework request counter init logic, avoid unsafe code and undefined > behaviour. > - Rework page size calculation based on new counter alignment. > - Avoid the need to open and mmap counter file for each rrd data > collection, keep the per-datastores filehandle cached instead. > > > proxmox: > > Christian Ebner (13): > proxmox-sys: expose msync to flush mmapped contents to filesystem > shared-memory: add method without tmpfs check for mmap file location > shared-memory: expose msync to flush in-memory contents to filesystem > s3-client: add persistent shared request counters for client > s3-client: add counters for upload/download traffic > s3-client: account for upload traffic on successful request sending > s3-client: account for downloaded bytes in incoming response body > s3-client: request counters: periodically persist counters to file > s3-client: sync flush request counters on client instance drop > pbs-api-types: define api type for s3 request statistics > s3-client: api-types: define request counter thresholds > pbs-api-types: add notification thresholds to datastore config > s3-client: implement request counter threshold and exceeding callback > > pbs-api-types/src/datastore.rs | 39 ++ > proxmox-s3-client/Cargo.toml | 6 + > proxmox-s3-client/debian/control | 3 + > proxmox-s3-client/examples/s3_client.rs | 1 + > proxmox-s3-client/src/api_types.rs | 59 ++ > proxmox-s3-client/src/client.rs | 96 +++- > proxmox-s3-client/src/lib.rs | 7 +- > proxmox-s3-client/src/response_reader.rs | 75 ++- > .../src/shared_request_counters.rs | 525 ++++++++++++++++++ > proxmox-shared-memory/src/lib.rs | 48 +- > proxmox-sys/src/mmap.rs | 12 + > 11 files changed, 852 insertions(+), 19 deletions(-) > create mode 100644 proxmox-s3-client/src/shared_request_counters.rs > > > proxmox-backup: > > Christian Ebner (20): > metrics: split common module imports into individual use statements > datastore: collect request statistics for s3 backed datastores > datastore: expose request counters for s3 backed datastores > api: s3: add endpoint to reset s3 request counters > bin: s3: expose request counter reset method as cli command > datastore: add helper method to get datastore backend type > ui: improve variable name indirectly fixing typo > ui: datastore summary: move store to be part of summary panel > ui: expose s3 request counter statistics in the datastore summary > metrics: collect s3 datastore statistics as rrd metrics > api: admin: expose s3 statistics in datastore rrd data > partially fix #6563: ui: expose s3 rrd charts in datastore summary > datastore: refactor shared request counter loading into helper > api: config: update notification thresholds for config and counters > ui: utils: add helper to render notification threshold property string > ui: add notification thresholds edit window > notifications: template data: fix typos in docstrings > notification: define templates and template data for thresholds > datastore: add thresholds notification callback on datastore lookup > api/ui: notifications: add 'thresholds' as notification type value > > debian/proxmox-backup-server.install | 2 + > pbs-datastore/src/datastore.rs | 158 ++++++-- > pbs-datastore/src/lib.rs | 3 +- > pbs-datastore/src/snapshot_reader.rs | 1 + > src/api2/admin/datastore.rs | 23 +- > src/api2/admin/s3.rs | 86 ++++- > src/api2/config/datastore.rs | 33 +- > src/api2/config/notifications/mod.rs | 1 + > src/api2/config/s3.rs | 9 +- > src/bin/proxmox_backup_manager/s3.rs | 33 ++ > src/server/metric_collection/metric_server.rs | 8 +- > src/server/metric_collection/mod.rs | 108 +++++- > src/server/metric_collection/pull_metrics.rs | 18 +- > src/server/metric_collection/rrd.rs | 34 +- > src/server/notifications/mod.rs | 39 +- > src/server/notifications/template_data.rs | 38 +- > src/tools/mod.rs | 9 +- > templates/Makefile | 2 + > .../default/thresholds-exceeded-body.txt.hbs | 9 + > .../thresholds-exceeded-subject.txt.hbs | 1 + > www/Makefile | 1 + > www/Utils.js | 10 + > www/datastore/OptionView.js | 9 + > www/datastore/Summary.js | 341 ++++++++++++------ > www/window/NotificationThresholds.js | 113 ++++++ > 25 files changed, 907 insertions(+), 182 deletions(-) > create mode 100644 templates/default/thresholds-exceeded-body.txt.hbs > create mode 100644 templates/default/thresholds-exceeded-subject.txt.hbs > create mode 100644 www/window/NotificationThresholds.js > > > Summary over all repositories: > 36 files changed, 1759 insertions(+), 201 deletions(-) >