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 74DCF1FF137 for ; Tue, 31 Mar 2026 12:16:53 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id BE3A117F94; Tue, 31 Mar 2026 12:17:20 +0200 (CEST) Message-ID: Date: Tue, 31 Mar 2026 12:16:43 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH proxmox{,-backup} v6 00/34] partially fix #6563: add s3 counter for statistics and notifications To: Christian Ebner , pbs-devel@lists.proxmox.com References: <20260319094100.240765-1-c.ebner@proxmox.com> Content-Language: en-US From: Hannes Laimer In-Reply-To: <20260319094100.240765-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: 1774952148093 X-SPAM-LEVEL: Spam detection results: 0 AWL -1.418 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 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 1 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 1 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: T5DYMPSN4M6X2RLIMRF5AYFYUI5M32IS X-Message-ID-Hash: T5DYMPSN4M6X2RLIMRF5AYFYUI5M32IS 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: Aside from the few small things I've mentioned and that can easily be addressed in a follow-up, this looks very good to me! The things form v5 are fixed, and I also did some more extensive testing and everything worked as expected. Consider this Reviewed-by: Hannes Laimer Tested-by: Hannes Laimer (side-note: proxmox-backup patches don't seem to apply with the proxy series underneath, proxmox is fine) On 2026-03-19 10:41, 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 creation, being cached for further access. Counter threshold > values can be defined in the datastore options, allowing to send > notifications via the notification system if counters exceeded the set > threshold values. Further, a per-datastore schedule can be configured > to reset the counters with given interval. > > 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 5 (thanks a lot @Hannes for review and testing): > - Implemented the scheduled reset logic on top of the previous series > - Also upload/download traffic counters are now reset, not just request counters > - Fixed the possible swallowing of errors in `Content`s `Body` implementation > - request_flush() is no longer called by spawning a blocking tokio task > - Fixed the notification callback to not capture the datastore name, but provide > a `label` as callback parameter instead > - Fix the api definition for the reset_counters endpoint > - Fixed incorrect graph labels and typos discovered during review > - Reordered patches in a more logical way rather than development flow > > 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 (12): > tree-wide: fix formatting issue via cargo fmt > 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 > s3-client: api-types: define request counter thresholds > s3-client: implement request counter threshold and exceeding callback > pbs-api-types: define api type for s3 request statistics > pbs-api-types: add notification thresholds to datastore config > pbs-api-types: add reset schedule for notification threshold counters > > pbs-api-types/src/datastore.rs | 56 ++ > pbs-api-types/src/remote.rs | 9 +- > 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 | 101 +++- > proxmox-s3-client/src/lib.rs | 7 +- > proxmox-s3-client/src/response_reader.rs | 67 ++- > .../src/shared_request_counters.rs | 537 ++++++++++++++++++ > 10 files changed, 824 insertions(+), 22 deletions(-) > create mode 100644 proxmox-s3-client/src/shared_request_counters.rs > > > proxmox-backup: > > Christian Ebner (22): > metrics: split common module imports into individual use statements > ui: improve variable name indirectly fixing typo > notifications: template data: fix typos in docstrings > 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 > 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 datastore lookup parameters into dedicated type > 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 > 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 > api: config: allow counter reset schedule editing > ui: expose counter reset schedule edit window > bin: proxy: periodically schedule counter reset task > > debian/proxmox-backup-server.install | 2 + > pbs-datastore/src/datastore.rs | 194 ++++++++-- > pbs-datastore/src/lib.rs | 3 +- > pbs-datastore/src/snapshot_reader.rs | 7 +- > src/api2/admin/datastore.rs | 48 ++- > src/api2/admin/namespace.rs | 9 +- > src/api2/admin/s3.rs | 86 ++++- > src/api2/backup/mod.rs | 3 +- > src/api2/config/datastore.rs | 42 ++- > src/api2/config/notifications/mod.rs | 1 + > src/api2/config/s3.rs | 9 +- > src/api2/reader/mod.rs | 3 +- > src/api2/status/mod.rs | 6 +- > src/api2/tape/backup.rs | 6 +- > src/api2/tape/restore.rs | 6 +- > src/bin/proxmox-backup-proxy.rs | 77 +++- > 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/server/prune_job.rs | 3 +- > src/server/pull.rs | 7 +- > src/server/push.rs | 3 +- > src/server/verify_job.rs | 3 +- > src/tools/mod.rs | 17 +- > templates/Makefile | 2 + > .../default/thresholds-exceeded-body.txt.hbs | 9 + > .../thresholds-exceeded-subject.txt.hbs | 1 + > www/Makefile | 2 + > www/Utils.js | 10 + > www/datastore/OptionView.js | 16 + > www/datastore/Summary.js | 345 ++++++++++++------ > www/window/CounterResetScheduleEdit.js | 27 ++ > www/window/NotificationThresholds.js | 113 ++++++ > 37 files changed, 1106 insertions(+), 232 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/CounterResetScheduleEdit.js > create mode 100644 www/window/NotificationThresholds.js > > > Summary over all repositories: > 47 files changed, 1930 insertions(+), 254 deletions(-) >