Re: [RFC PATCH proxmox-backup] fix #7670: datastore: s3: allow for per-chunk file lock cleanup

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On June 5, 2026 5:17 pm, Christian Ebner wrote:
> On 6/5/26 5:06 PM, Christian Ebner wrote:
>> On 6/5/26 3:39 PM, Fabian Grünbichler wrote:
>>> Quoting Christian Ebner (2026-06-04 16:09:19)

[..]

>>> another approach would be to switch to a different kind of locking 
>>> mechanism
>>> entirely (though with the cross-process and multi-threaded 
>>> requirements we
>>> have, this might not be easy either ;)) or to reduce the lock 
>>> granularity.
>> 
>> Yeah, reducing the lock granularity and moving to something with 
>> deterministic count could be a way out, and given the usage estimates 
>> from above, using the 4 hex-digit prefix used also for chunk store 
>> directory hierarchy might be a viable candidate for defining such lock- 
>> files.

that would give us 64M at most if we keep it on tmpfs, but might be a
bit too coarse (the lock is held for the duration of an upload after
all, which can take a bit..).

>>> given that a mistake in the handling of retries below can cause 
>>> dataloss, doing
>>> it for every lock/unlock pair sounds a bit dangerous. there's also the
>>> additional overhead if the lock is actually contended to account for - 
>>> we need
>>> at least two loop iterations if it is, potentially a lot more?
>> 
>> This is however not so frequently happening though? But yes, there is 
>> overhead by the then unavoidable retry/retries.
>> 
>>> or we go back to square one, and revisit the whole interaction here to 
>>> see if
>>> we can get rid of the per-chunk locks again in favor of something that 
>>> scales
>>> with the number of pending uploads.. the last time we tried this 
>>> turned out to
>>> be quite tricky though.
>> The major downside for all solutions is unfortunately that moving 
>> forward can only happen with the current locking mechanism still in 
>> place, so a transitional step for cleanup unavoidable. But at least it 
>> is fine to remove the per-chunk lock files once they are being held in 
>> the process instance implementing the new locking logic.

yeah, it requires the usual "set flag-file, keep old behaviour until
reboot/while it exists, switch over after" migration.

> Oh, and another thing to consider as well: The backup snapshot 
> lock-files are also not cleaned up AFAIKT? While way less problematic, 
> they will also accumulate and add unneeded memory overhead on systems 
> with high frequency backups which are not rebooted frequently.
> 
> There removing the lock-file on prune and retry for vanished lock-files 
> after flock calls might be acceptable? Also performance wise.

snapshot counts are in a different ball park though.. but still the
next-most-problematic one ;) so we might need to reconsider other
tmpfs-located locks as well.