Re: [pbs-devel] [PATCH proxmox-backup] fix #3336: api: remove backup group if the last snapshot is removed

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 14.03.22 15:18, Stefan Sterz wrote:
> On 14.03.22 12:36, Thomas Lamprecht wrote:
>> On the other hand, we also handle creation in a similar implicit matter,
>> so maybe I'm overthinking it and just removing it would actually be more
>> consistent/expected for users.
>>
>> So, if you don't see a problem/issue with that approach and agree with
>> the last paragraph above feel free to go for deleting the owner file only.
> 
> for the most part i agree with you. i would also like to point out
> that when a group is deleted (as in, not the last snapshot, but the
> entire group at once) the owner is also implicitly removed (because
> the entire group directory is removed). so in a way, we already delete
> ownership information implicitly and the proposed solution would just
> be consistent with that behavior.

I really do not think that's comparable or would count as implicit deletion
;-)

A user triggering a whole-group removal explicitly expects that all the
associated stuff gets removed too, including owner + group directory,
there's nothing to own after that.

Iow., the difference would be like:
`rm -rf group-dir/` vs. `rm -rf group-dir/snapshot-dir`

> 
> however, i did some more digging and testing and it turns out that we
> currently assume the owner file to be present when a group directory
> exists. this affects not only sync jobs, but also verification and
> more. thus, i would need to do quite a bit of refactoring to get this
> to work and even more testing. so while this issue seemed simple
> enough, as far as i can tell our current options are:
> > 1. re-factor locking and remove the directory
> 2. re-factor how an empty group directory and the owner file is
> treated

meh, not really liking this one as it could conflict with some assumptions.

> 3. add "empty" groups to the gui

Thinking more of it with past users-behavior in mind, I'd be surprised if we
then would get the bug report for not auto-removing this in one step ^^


> 
> in light of this, taking the gui route is possibly the easiest option.
> sorry, for not being aware of this earlier.

I mean, the locking problem Wolfgang pointed out already exists currently,
meaning that we either could:

1. stay ignorant (for now) and just delete the directory
2. fixing that up-front already as it has its own merits

I don't see 1. as _that_ problematic as the deletion of the last snapshot
always has to be a manual action, (auto)pruning will never cause that.
This would allow the assumption that the user/admin already took care
of periodic backup jobs before cleaning up stuff. But yeah, definitively
has a slight sour taste. Putting this on hold and see how we can best
improve the locking w.r.t. to full backup-dir removals would IMO be the
cleanest solution.