From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 33BB869CE4 for ; Mon, 14 Mar 2022 16:19:50 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 1DDD15E34 for ; Mon, 14 Mar 2022 16:19:50 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 787DE5E29 for ; Mon, 14 Mar 2022 16:19:49 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 4B97146396 for ; Mon, 14 Mar 2022 16:19:49 +0100 (CET) Message-ID: <915cbd4b-a195-5714-501c-41932b2c2558@proxmox.com> Date: Mon, 14 Mar 2022 16:19:47 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.6.2 Content-Language: en-US To: Thomas Lamprecht , Wolfgang Bumiller Cc: Proxmox Backup Server development discussion References: <20220309135031.1995207-1-s.sterz@proxmox.com> <717c8999-d3f8-a01b-a8f5-da0f5960d23f@proxmox.com> <20220314093617.n2mc2jv4k6ntzroo@wobu-vie.proxmox.com> <738d037f-ed3c-db76-287f-5b6d37a3b7f3@proxmox.com> <5ca8cb1b-f677-67d0-a4c5-052de38472b2@proxmox.com> From: Stefan Sterz In-Reply-To: <5ca8cb1b-f677-67d0-a4c5-052de38472b2@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.000 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.001 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pbs-devel] [PATCH proxmox-backup] fix #3336: api: remove backup group if the last snapshot is removed X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Mar 2022 15:19:50 -0000 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?