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 42F381FF16F for ; Tue, 22 Jul 2025 10:13:24 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id D6C21326EF; Tue, 22 Jul 2025 10:14:38 +0200 (CEST) Mime-Version: 1.0 Date: Tue, 22 Jul 2025 10:14:33 +0200 Message-Id: Cc: "pbs-devel" From: "Hannes Laimer" To: "Proxmox Backup Server development discussion" X-Mailer: aerc 0.20.1-112-gd31995f1e20b References: <20250721164507.1045869-1-c.ebner@proxmox.com> In-Reply-To: <20250721164507.1045869-1-c.ebner@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1753172064515 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.024 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 0.001 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.001 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.001 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 URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [mod.rs, push.rs, upload-speed.rs, verify.rs, s3.rs, maintenance.rs, pull.rs, directory.rs, main.rs, proxmox-backup-manager.rs, sync.rs, benchmark.rs, datastore.rs, environment.rs, lib.rs, zfs.rs, proxmox.com] Subject: Re: [pbs-devel] [PATCH proxmox{, -backup} v10 00/49] fix #2943: S3 storage backend for datastores X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" Both the max S3 object key length check and the leftover `.unwrap()` look good now! The rather small changes from v9 -> v10 also look fine, consider this Reviewed-by: Hannes Laimer On Mon Jul 21, 2025 at 6:44 PM CEST, Christian Ebner wrote: > Disclaimer: These patches are still in an experimental state and not > intended for production use. > > This patch series aims to add S3 compatible object stores as storage > backend for PBS datastores. A PBS local cache store using the regular > datastore layout is used for faster operation, bypassing requests to > the S3 api when possible. Further, the local cache store allows to > keep frequently used chunks and is used to avoid expensive metadata > updates on the object store, e.g. by using local marker file during > garbage collection. > > Backups are created by upload chunks to the corresponding S3 bucket, > while keeping the index files in the local cache store, on backup > finish, the snapshot metadata are persisted to the S3 storage backend. > > Snapshot restores read chunks preferably from the local cache store, > downloading and insterting them if not present from the S3 object > store. Listing and snapsoht metadata operation currently rely soly on > the local cache store. > > Currently chunks use a 1:1 mapping to S3 objects. An advanced packing > mechanism for chunks to significantly reduce the number of api > requests and therefore be more cost effective will be implemented as > followup patches. > > Most notably changes since version 8 of the patches (thanks @Lukas for > more testing and debugging and Hannes for review): > - Moved s3 refresh button to the datastore content, and made it less > visible by placing it into a 'More' dropdown. > - Added basic unit tests for s3 object key helpers > - Extend missing public function and type documentation > - Use pbs_buildcfg::configdir macro to build config paths > - Mostly refactoring based on feedback, please refer to the per-patch > changelog for details. > - Fixes typos in docs (thanks @Maximiliano) > > Most notably changes since version 8 of the patches (thanks @Lukas for > code review): > - Moved s3 refresh button to the datastore content, and made it less > visible by placing it into a 'More' dropdown. > - Added basic unit tests for s3 object key helpers > - Extend missing public function and type documentation > - Use pbs_buildcfg::configdir macro to build config paths > - Mostly refactoring based on feedback, please refer to the per-patch > changelog for details. > - Fixes typos in docs (thanks @Maximiliano) > > Most notably changes since version 7 of the patches (thanks @Thomas > and @Lukas for feedback, testing and debugging): > - Improve self-signed certificate fingerprint check, verify valid > expected fingerprint is passed to client on instantiation. > - Rename previously missed host to endpoint is s3 client selector > - Use more specific `S3 Client ID` over ambiguous `Unique Identifier` > - Implement missing cli commands for s3 client manipulation > - Add in-use marker to s3 object stores to avoid accitental reuse of > object stores which are already used as datastore by another > instance, adding also flags to overwrite. > - Automatically perform an s3-refresh when recreating a datastore, > pre-populating the contents without further user interaction. > - Add documentation > - Fix formatting issue in proxmox-s3-client > > Most notably changes since version 6 of the patches (thanks @Thomas > for feedback): > - Reworked uri encoding logic, instead of doing this in the > S3ObjectKey, perform this in the build_uri helper used by all > client api requests. > - Add cache-size optional parameter to datastore backend config, > allows to define the local datastore LRU cache capacity. > - Increase s3 client timeout, as otherwise delete objects operations > on Cloudflare R2 would run into a timeout error. > - Also upload client log, previously not uploaded to s3 backend. > - Add missing documentation to some pub types in the response reader > - Use s3 object key generation helper for index file upload, which > fixes the missing key prefix. > - Add basic regression tests for uri encoder and decoder helper > functions. > - Include some baseline performance tests for garbage collection as > well as chunk up-/download when caching. > > Most notably changes since version 5 of the patches (thanks @Thomas > for feedback): > - Move s3 client into its own, dedicated crate in the proxmox repo > - Factor out any directly PBS related code from the client > - Guard implementation behind feature cfg, so api types can be used > independently > - Add basic example and extend on crate documentation > > Most notably changes since version 4 of the patches: > - Fix race between S3 backend upload and local cache store insert, > avoiding possibly chunk loss for concurrent backups. > - Use the local datastore cache also for local chunk reader instances > - Fallback to fetching chunks from S3 backend if they should be cached > but the local chunk file is missing or empty, instead of failing > - Rename chunks detected as corrupt also on the S3 object store > - Retry chunk uploads via put objects in case of errors. > - Add possibility to add rate limits for the s3 client put requests, as > otherwise object stores can be overloaded. > - Allow for Cloudflare R2 compatible `auto` region, as otherwise AWS > sign v4 request authentication will fail > - Use `Async` instead of `Sync` variant for the api handler of the > s3-refresh command, as otherwise this fails. > - Take into account that some type folders might not be present when > performing an s3-refresh. > - Use `Local` instead of `Regular` to refer to normal datastores in the > creation window. > > Most notably changes since version 3 of the patches: > - Rebased onto current master, fixed incompatibilities with upgraded > dependencies > - Added method to uri decode s3 object keys, as they are required in > order to download contents to a local store > - Added api endpoint to allow resyncing of the datastore contents to > the local cache store, introducing a new maintenance mode s3-refresh > to guarantee consistency. > > Most notably changes since RFC version 2 of the patches (thanks > @Lukas for feedback): > - Extend S3 client implementation to also support path style bucket > addressing. > - Keep bucket name as config option for the datastore, allowing more > flexible reuse of a configured S3 client. > - Use the datastore name as additional object key prefix to allow for > multiple datastores on the same bucket. > - Allow bucket and region templating in S3 endpoint, making this more > flexible with respect to possible DNS records. > - Rework datastore create window to be less overloaded. > - Drop dead code in the S3 client implementation, since tagging and > object copying is currently not required. > - Fix missing locking when deleting chunks from s3 store during > garbage collection, avoiding possible chunk loss for concurrent > backups. > - Remove chunks from LRU cache when deleting chunks during garbage > collection, avoiding possible chunk loss for concurrent backups. > - Add dedicated types for object prefix and relative s3 key paths to > avoid misuse. > - Use more fitting icon for S3 client. > > Link to the bugtracker issue: > https://bugzilla.proxmox.com/show_bug.cgi?id=2943 > > Steps to setup a local S3 object store using RADOS gateway or MinIO > can be found at (internal only, external users might use the steps > outlined in the cover letter and comments of RFC version 2): > https://wiki.intra.proxmox.com/PBS_Setup_S3_Object_Store > > proxmox: > > Christian Ebner (3): > pbs-api-types: extend datastore config by backend config enum > pbs-api-types: maintenance: add new maintenance mode S3 refresh > s3 client: Add missing S3 object key max length check > > Cargo.toml | 1 + > pbs-api-types/Cargo.toml | 1 + > pbs-api-types/debian/control | 2 + > pbs-api-types/src/datastore.rs | 114 +++++++++++++++++++++++- > pbs-api-types/src/maintenance.rs | 4 + > proxmox-s3-client/examples/s3_client.rs | 4 +- > proxmox-s3-client/src/object_key.rs | 26 ++++-- > 7 files changed, 142 insertions(+), 10 deletions(-) > > > proxmox-backup: > > Christian Ebner (46): > datastore: add helpers for path/digest to s3 object key conversion > config: introduce s3 object store client configuration > api: config: implement endpoints to manipulate and list s3 configs > api: datastore: check s3 backend bucket access on datastore create > api/cli: add endpoint and command to check s3 client connection > datastore: allow to get the backend for a datastore > api: backup: store datastore backend in runtime environment > api: backup: conditionally upload chunks to s3 object store backend > api: backup: conditionally upload blobs to s3 object store backend > api: backup: conditionally upload indices to s3 object store backend > api: backup: conditionally upload manifest to s3 object store backend > api: datastore: conditionally upload client log to s3 backend > sync: pull: conditionally upload content to s3 backend > api: reader: fetch chunks based on datastore backend > datastore: local chunk reader: read chunks based on backend > verify worker: add datastore backed to verify worker > verify: implement chunk verification for stores with s3 backend > datastore: create namespace marker in s3 backend > datastore: create/delete protected marker file on s3 storage backend > datastore: prune groups/snapshots from s3 object store backend > datastore: get and set owner for s3 store backend > datastore: implement garbage collection for s3 backend > ui: add datastore type selector and reorganize component layout > ui: add s3 client edit window for configuration create/edit > ui: add s3 client view for configuration > ui: expose the s3 client view in the navigation tree > ui: add s3 client selector and bucket field for s3 backend setup > tools: lru cache: add removed callback for evicted cache nodes > tools: async lru cache: implement insert, remove and contains methods > datastore: add local datastore cache for network attached storages > api: backup: use local datastore cache on s3 backend chunk upload > api: reader: use local datastore cache on s3 backend chunk fetching > datastore: local chunk reader: get cached chunk from local cache store > backup writer: refactor parameters into backup writer options struct > api: backup: add no-cache flag to bypass local datastore cache > api/datastore: implement refresh endpoint for stores with s3 backend > cli: add dedicated subcommand for datastore s3 refresh > ui: render s3 refresh as valid maintenance type and task description > ui: expose s3 refresh button for datastores backed by object store > datastore: conditionally upload atime marker chunk to s3 backend > bin: implement client subcommands for s3 configuration manipulation > bin: expose reuse-datastore flag for proxmox-backup-manager > datastore: mark store as in-use by setting marker on s3 backend > datastore: run s3-refresh when reusing a datastore with s3 backend > api/ui: add flag to allow overwriting in-use marker for s3 backend > docs: Add section describing how to setup s3 backed datastore > > Cargo.toml | 2 + > docs/storage.rst | 73 ++ > examples/upload-speed.rs | 17 +- > pbs-client/src/backup_writer.rs | 47 +- > pbs-config/Cargo.toml | 1 + > pbs-config/src/lib.rs | 1 + > pbs-config/src/s3.rs | 89 +++ > pbs-datastore/Cargo.toml | 5 + > pbs-datastore/src/backup_info.rs | 63 +- > pbs-datastore/src/cached_chunk_reader.rs | 6 +- > pbs-datastore/src/chunk_store.rs | 29 +- > pbs-datastore/src/datastore.rs | 696 ++++++++++++++++-- > pbs-datastore/src/dynamic_index.rs | 1 + > pbs-datastore/src/lib.rs | 5 + > pbs-datastore/src/local_chunk_reader.rs | 66 +- > .../src/local_datastore_lru_cache.rs | 180 +++++ > pbs-datastore/src/s3.rs | 114 +++ > pbs-tools/src/async_lru_cache.rs | 46 +- > pbs-tools/src/lru_cache.rs | 42 +- > proxmox-backup-client/src/benchmark.rs | 17 +- > proxmox-backup-client/src/main.rs | 26 +- > src/api2/admin/datastore.rs | 97 ++- > src/api2/admin/mod.rs | 2 + > src/api2/admin/s3.rs | 83 +++ > src/api2/backup/environment.rs | 82 ++- > src/api2/backup/mod.rs | 131 ++-- > src/api2/backup/upload_chunk.rs | 114 ++- > src/api2/config/datastore.rs | 149 +++- > src/api2/config/mod.rs | 2 + > src/api2/config/s3.rs | 307 ++++++++ > src/api2/node/disks/directory.rs | 2 +- > src/api2/node/disks/zfs.rs | 2 +- > src/api2/reader/environment.rs | 12 +- > src/api2/reader/mod.rs | 62 +- > src/backup/verify.rs | 138 +++- > src/bin/proxmox-backup-manager.rs | 1 + > src/bin/proxmox_backup_manager/datastore.rs | 42 ++ > src/bin/proxmox_backup_manager/mod.rs | 2 + > src/bin/proxmox_backup_manager/s3.rs | 102 +++ > src/server/pull.rs | 76 +- > src/server/push.rs | 27 +- > src/server/sync.rs | 22 +- > src/server/verify_job.rs | 2 +- > www/Makefile | 3 + > www/NavigationTree.js | 6 + > www/Utils.js | 4 + > www/config/S3ClientView.js | 141 ++++ > www/datastore/Content.js | 48 ++ > www/form/S3ClientSelector.js | 33 + > www/window/DataStoreEdit.js | 132 +++- > www/window/MaintenanceOptions.js | 6 +- > www/window/S3ClientEdit.js | 148 ++++ > 52 files changed, 3151 insertions(+), 353 deletions(-) > create mode 100644 pbs-config/src/s3.rs > create mode 100644 pbs-datastore/src/local_datastore_lru_cache.rs > create mode 100644 pbs-datastore/src/s3.rs > create mode 100644 src/api2/admin/s3.rs > create mode 100644 src/api2/config/s3.rs > create mode 100644 src/bin/proxmox_backup_manager/s3.rs > create mode 100644 www/config/S3ClientView.js > create mode 100644 www/form/S3ClientSelector.js > create mode 100644 www/window/S3ClientEdit.js > > > Summary over all repositories: > 59 files changed, 3293 insertions(+), 363 deletions(-) _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel