[pbs-devel] [RFC v2 proxmox-backup 10/21] datastore: mark namespace as trash instead of deleting it

[Christian Ebner]

On 5/12/25 11:46, Fabian Grünbichler wrote:
> On May 12, 2025 9:47 am, Christian Ebner wrote:
>> On 5/9/25 14:27, Fabian Grünbichler wrote:
>>> On May 8, 2025 3:05 pm, Christian Ebner wrote:
>>>> As for backup snapshots and groups, mark the namespace as trash
>>>> instead of removing it and the contents right away, if the trash
>>>> should not be bypassed.
>>>>
>>>> Actual removal of the hirarchical folder structure has to be taken
>>>> care of by the garbage collection.
>>>>
>>>> In order to avoid races during removal, first mark the namespaces as
>>>> trash pending, mark the snapshots and groups as trash and only after
>>>> rename the pending marker file to the trash marker file. By this,
>>>> concurrent backups can remove the trash pending marker to avoid the
>>>> namespace being trashed.
>>>>
>>>> On re-creation of a trashed namespace remove the marker file on itself
>>>> and any parent component from deepest to shallowest. As trashing a full
>>>> namespace can also set the trash pending state for recursive namespace
>>>> cleanup, remove encounters of that marker file as well to avoid the
>>>> namespace or its parent being trashed.
>>>
>>> this is fairly involved since we don't have locks on namespaces..
>>>
>>> should we have them for creation/removal/trashing/untrashing?
>>
>> That is what I did try to avoid at all cost with the two-marker
>> approach, as locking the namespace might be rather invasive. But if that
>> does not work out as intended, I see no other way as to add exclusive
>> locking for namespaces as well, yes.
>>
>>>
>>> I assume those are fairly rare occurrences, I haven't yet analyzed the
>>> interactions here to see whether the two-marker approach is actually
>>> race-free..
>>>
>>> OTOH, do we really need to (be able to) trash namespaces?
>>
>> Yes, I think we do need that as well since the datastore's hierarchy
>> should remain in place, and the namespace iterator requires a way to
>> distinguish between a namespace which has been trashed/deleted and a
>> namespace which has not, but might contain trashed items. Otherwise a
>> user requesting to forget a namespace, still sees the (empty as only
>> trashed contents) namespace tree after the operation. Which would be
>> rather unexpected?
> 
> we could also require emptying the trash as part of forgetting a
> namespace? we already have the `delete_groups` option and only remove a
> non-empty namespace is set, we could re-use that or add a new
> `empty_trash` option next to it, if we want double-opt-in ;)
> 
> else, we'd also need to support trashing whole datastores if
> we follow this line of thinking..
> 
> like I said, I don't think forgetting namespaces is something that is
> done very often, as namespaces are normally fairly static.. and if I
> want to remove a namespace, I probably also want to remove all its
> contents (trash or regular ;)).

Ah okay, requiring to clear trashed contents as well when deleting a 
namespace seems the better approach indeed. Will adapt the code 
accordingly.