Re: [PATCH proxmox{,-backup} v6 00/34] partially fix #6563: add s3 counter for statistics and notifications

Hannes Laimer
2026-03-31T10:16:53Z

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 <h.laimer@proxmox.com>
Tested-by: Hannes Laimer <h.laimer@proxmox.com>

(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(-)
>