From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68])
	by lore.proxmox.com (Postfix) with ESMTPS id 807E31FF18A
	for <inbox@lore.proxmox.com>; Mon, 26 May 2025 16:18:25 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
	by firstgate.proxmox.com (Proxmox) with ESMTP id 2AE2A3436A;
	Mon, 26 May 2025 16:18:38 +0200 (CEST)
Message-ID: <e1a6415a-2555-41de-a231-1eef126afea7@proxmox.com>
Date: Mon, 26 May 2025 16:18:34 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
To: pbs-devel@lists.proxmox.com
References: <20240903123401.91513-1-h.laimer@proxmox.com>
Content-Language: en-US
From: Hannes Laimer <h.laimer@proxmox.com>
In-Reply-To: <20240903123401.91513-1-h.laimer@proxmox.com>
X-SPAM-LEVEL: Spam detection results:  0
 AWL 0.025 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
 SPF_HELO_NONE           0.001 SPF: HELO does not publish an SPF Record
 SPF_PASS               -0.001 SPF: sender matches SPF record
Subject: Re: [pbs-devel] superseded: [PATCH proxmox-backup RFC 00/10]
 introduce typestate for datastore/chunkstore
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
 <pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>, 
 <mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Backup Server development discussion
 <pbs-devel@lists.proxmox.com>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>

superseded by 
https://lore.proxmox.com/pbs-devel/20240903123401.91513-1-h.laimer@proxmox.com/

On 9/3/24 14:33, Hannes Laimer wrote:
> This patch series introduces two traits, CanRead and CanWrite, to define whether
> a datastore reference is readable, writable, or neither. Functions that read
> or write are now implemented in `impl<T: CanRead>` or `impl<T: CanWrite>` blocks, ensuring
> that they are only available to references that are supposed to read/write.
> 
> Motivation:
> Currently, we track the number of read/write references of a datastore but we don't
> track Lookup operations as they don't read or write, they still need a chunkstore, so
> eventhough they don't neccessarily directly do IO, they hold an open file handle.
> This is a problem for things like unmounting, currently lookup operations are only really
> short, so you'd need really unlucky timing to actually run into problems, but still,
> if a datastore is in "offline" maintenance mode, we shouldn't open filehandles on it.
> 
> By encoding state in the type:
> 1. We can assign non-readable/writable references for lookup operations.
> 2. The compiler ensures correct usage of references. Since it is easy to miss
>      what might happen a few function calls down the line, having the compiler
>      yell at you for easily missed things like this, is a really good thing
>      I think.
> 
> Changes:
> * Added CanRead and CanWrite traits.
> * Separated functions into impl<T: CanRead> or impl<T: CanWrite>.
> * Introduced three new datastore lookup functions that return concrete types implementing
>     CanRead, CanWrite, or neither.
> * Renamed lookup_datastore() to open_datastore() and made it private.
> 
> The main downside is needing separate datastore caches for read and write references due to
> concrete type requirements in the cache HashMap.
> 
> Almost all changes are either adding generics or moving functions into the appropriate
> trait implementations. The logic itself is only touched twice, once in datastore_lookup()
> and once check_privs_and_load_store() in /api/admin/datastore, this function now only checks
> the privs, the datastore opening happens in the endpoint function directly.
> 
>   
> 
> Hannes Laimer (10):
>    chunkstore: add CanRead and CanWrite trait
>    chunkstore: separate functions into impl block
>    datastore: add generics and new lookup functions
>    datastore: separate functions into impl block
>    backup_info: add generics and separate functions into impl blocks
>    pbs-datastore: add generics and separate functions into impl blocks
>    api: replace datastore_lookup with new, state-typed datastore
>      returning functions
>    server/bin: replace datastore_lookup with new, state-typed datastore
>      returning functions
>    api: add generics and separate functions into impl blocks
>    backup/server/tape: add generics and separate functions into impl
>      blocks
> 
>   pbs-datastore/src/backup_info.rs            |  179 +-
>   pbs-datastore/src/chunk_store.rs            |  228 ++-
>   pbs-datastore/src/datastore.rs              | 1688 ++++++++++---------
>   pbs-datastore/src/dynamic_index.rs          |   22 +-
>   pbs-datastore/src/fixed_index.rs            |   50 +-
>   pbs-datastore/src/hierarchy.rs              |   92 +-
>   pbs-datastore/src/local_chunk_reader.rs     |   13 +-
>   pbs-datastore/src/prune.rs                  |   19 +-
>   pbs-datastore/src/snapshot_reader.rs        |   30 +-
>   src/api2/admin/datastore.rs                 |  170 +-
>   src/api2/admin/namespace.rs                 |    8 +-
>   src/api2/backup/environment.rs              |  176 +-
>   src/api2/backup/mod.rs                      |   25 +-
>   src/api2/backup/upload_chunk.rs             |   19 +-
>   src/api2/reader/environment.rs              |   31 +-
>   src/api2/reader/mod.rs                      |    9 +-
>   src/api2/status.rs                          |    8 +-
>   src/api2/tape/backup.rs                     |   21 +-
>   src/api2/tape/drive.rs                      |    2 +-
>   src/api2/tape/restore.rs                    |   75 +-
>   src/backup/hierarchy.rs                     |   26 +-
>   src/backup/verify.rs                        |   53 +-
>   src/bin/proxmox-backup-proxy.rs             |    4 +-
>   src/server/gc_job.rs                        |    8 +-
>   src/server/prune_job.rs                     |    9 +-
>   src/server/pull.rs                          |   29 +-
>   src/server/verify_job.rs                    |    4 +-
>   src/tape/file_formats/snapshot_archive.rs   |    5 +-
>   src/tape/pool_writer/mod.rs                 |   11 +-
>   src/tape/pool_writer/new_chunks_iterator.rs |    7 +-
>   30 files changed, 1585 insertions(+), 1436 deletions(-)
> 



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel