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

[Stefan Sterz <s.sterz@proxmox.com>]

On 14.03.22 15:53, Thomas Lamprecht wrote:
> 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.

-- snip --

>>
>> 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.

agreed, would you mind if i open a bug for overhauling the locking
mechanism and started work on that? i looked a bit into your proposed
solution regarding tmpfs afaict there is no per directory inode limit,
only an overall limit corresponding to halve the physical memory
pages. we could use a completely flat structure based on either
encoded or hashed canonical paths. im assuming thats pretty close to
what you had in mind?