Re: [pbs-devel] [RFC proxmox-backup 3/4] datastore: move snapshots to trash folder on destroy

From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: "Proxmox Backup Server development discussion"
	<pbs-devel@lists.proxmox.com>,
	"Fabian Grünbichler" <f.gruenbichler@proxmox.com>
Subject: Re: [pbs-devel] [RFC proxmox-backup 3/4] datastore: move snapshots to trash folder on destroy
Date: Fri, 18 Apr 2025 13:06:25 +0200	[thread overview]
Message-ID: <41d4c5c5-ce97-40b1-95df-6b5cbfbce432@proxmox.com> (raw)
In-Reply-To: <1744880920.s4f2qkbhp2.astroid@yuna.none>

Am 17.04.25 um 11:29 schrieb Fabian Grünbichler:
> On April 16, 2025 4:18 pm, Christian Ebner wrote:
>> Instead of directly deleting the snapshot directory and it's contents
>> on a prune, move the snapshot directory into the `.trash` subfolder
>> of the datastore.
>>
>> This allows to mark chunks which were used by these index files if
>> the snapshot was pruned during an ongoing garbage collection.
>> Garbage collection will clean up these files before starting with the
>> marking phase 1 and read all index files after completing that phase,
>> touching these chunks as well.
> 
> some other variants to maybe consider:
> 
> marking the snapshot itself as trash (in the manifest, or by adding a
> trash marker file inside the dir) - this would mean that there is no
> iterator race issue when undoing a prune, no double-pruning collisions,
> .. - but it also means we need to adapt all call sites that should skip
> trashed snapshots (most existing ones), which is more churn.

Shouldn't we use the central iterators implementations to query indexes?
I.e., any call site that doesn't should probably be switched over to
those, just like Chris did for GC recently.
Then it could be defined if trashed indexes should be skipped – the
default – or included when instantiating that iterator, e.g. through a
parameter or probably a dedicated "with_trash" fn – but that's details.

> 
> having a trash dir per group instead of a global one for the whole
> datastore (less likely to incur extra costs in case somebody has a weird
> setup where namespaces/.. are symlinked or bindmounted or similar
> shenanigans). would need to postpone group removal to GC in case all
> snapshots are pruned.

Both of those variants would make restore simpler too, depends IMO a
bit if we already read the manifest everywhere anyway where the info
is needed, in that case I'd slightly favor that as place to store the
info if a backup is trash or not, as that would avoid the need for
another directory or file to store (inode exhaustion) and manage.

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