public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Christian Ebner <c.ebner@proxmox.com>,
	Hannes Laimer <h.laimer@proxmox.com>,
	pbs-devel@lists.proxmox.com
Subject: Re: [PATCH proxmox-backup v7 2/9] datastore: add move-group
Date: Wed, 22 Apr 2026 12:05:01 +0200	[thread overview]
Message-ID: <1776851956.4wbeejt3wr.astroid@yuna.none> (raw)
In-Reply-To: <8fa35401-0228-4f09-a9fc-1c66de829d48@proxmox.com>

On April 22, 2026 11:30 am, Christian Ebner wrote:
> On 4/22/26 11:23 AM, Hannes Laimer wrote:
>> On 2026-04-22 11:06, Fabian Grünbichler wrote:
>>> On April 22, 2026 10:40 am, Christian Ebner wrote:
>>>> On 4/16/26 7:18 PM, Hannes Laimer wrote:
>>>>> Add support for moving a single backup group to a different namespace
>>>>> within the same datastore.
>>>>>
>>>>> For the filesystem backend each snapshot directory is renamed
>>>>> individually. For S3 all objects are copied to the target prefix
>>>>> before deleting the source, per snapshot.
>>>>>
>>>>> Exclusive locks on the group and all its snapshots are acquired
>>>>> before the move to ensure no concurrent operations are active.
>>>>> Snapshots are locked and moved in batches to avoid exhausting file
>>>>> descriptors on groups with many snapshots.
>>>>
>>>> Unless I overlook it, there currently is still one major issue which can
>>>> lead to data loss with this:
>>>>
>>>> Garbage collection uses the Datastore's list_index_files() method to
>>>> collect all index files at the start of phase 1. This is to know which
>>>> chunks need atime updates to mark them as in use. Snapshots which
>>>> disappear in the mean time can be ignored, as the chunks may then no
>>>> longer be in use. Snapshots created in the mean time are safe, as there
>>>> it is the cutoff time protecting newly written chunks which are not
>>>> referenced by any of the index files which are now not in the list.
>>>>
>>>> But if the move happens after GC started and collected the index files,
>>>> but before reaching that index files. the moved index file still might
>>>> reference chunks which are in-use, but now never get an atime update.
>>>>
>>>> Locking unfortunately does not protect against this.
>>>>
>>>> So if there is an ongoing garbage collection phase 1, there is the need
>>>> for some mechanism to re-inject the index files in the list of indices
>>>> and therefore chunks to process.
>>>> This might require to write the moved indices to a file, so they can be
>>>> read and processed at the end of GC phase 1 even if GC is running in a
>>>> different process. And it requires to flock that file and wait for it to
>>>> become available before continuing.
>>>
>>> or moving could obtain the GC lock, and you simply cannot move while a
>>> GC is running or start a GC while a move is in progress? though the
>>> latter might be problematic.. it is already possible to block GC in
>>> practice if you have a writer that never finishes (assuming the proxy is
>>> reloaded every once in a while, which happens once per day at least).
>>>
>>> I guess your approach is similar to the trash feature we've discussed a
>>> while back (just without restoring from trash and all the associated
>>> complexity ;)).. it would only require blocking moves during this "phase
>>> 1.5" instead of the whole GC, which would of course be nice.. but it
>>> also increases the amount of work move needs to do by quite a bit..
>> 
>> is it that much though? it would be just appending a line to a file for
>> every moved index, compared to the actual moving itself, this seems
>> rather minor, no?

it is another readdir + parsing for each moved snapshot, which is
definitely not nothing.. but it also isn't that bad.

we'd also need a mechanism to support multiple moves, so we need a
shared (obtained by moves) lock that allows creating and writing such
files, and an exclusive lock (obtained by GC) that allows processing and
clearing them?

>> and gc would just have to read this file (if it exists) once at the end
>> of phase one
> 
> Another option might be to re-list all the index files at the end of 
> phase1, and process any not already processed one?

that is much more expensive, and in particular, also makes the "there
were no parallel moves" case more expensive as well which we should
avoid.. and also we'd need to forbid further moves while this second
processing is going on, or repeat this possibly very often..




  reply	other threads:[~2026-04-22 10:05 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-16 17:18 [PATCH proxmox-backup v7 0/9] fixes #6195: add support for moving groups and namespaces Hannes Laimer
2026-04-16 17:18 ` [PATCH proxmox-backup v7 1/9] ui: show empty groups Hannes Laimer
2026-04-16 17:18 ` [PATCH proxmox-backup v7 2/9] datastore: add move-group Hannes Laimer
2026-04-20 14:49   ` Fabian Grünbichler
2026-04-21 10:43     ` Hannes Laimer
2026-04-22  8:40   ` Christian Ebner
2026-04-22  9:07     ` Fabian Grünbichler
2026-04-22  9:24       ` Christian Ebner
2026-04-22  9:25       ` Hannes Laimer
2026-04-22  9:30         ` Christian Ebner
2026-04-22 10:05           ` Fabian Grünbichler [this message]
2026-04-22 10:17             ` Hannes Laimer
2026-04-22 10:23               ` Christian Ebner
2026-04-22 10:26                 ` Christian Ebner
2026-04-22 10:32                 ` Hannes Laimer
2026-04-22 11:11                   ` Hannes Laimer
2026-04-16 17:18 ` [PATCH proxmox-backup v7 3/9] datastore: add move-namespace Hannes Laimer
2026-04-20 14:49   ` Fabian Grünbichler
2026-04-16 17:18 ` [PATCH proxmox-backup v7 4/9] docs: add section on moving namespaces and groups Hannes Laimer
2026-04-16 17:18 ` [PATCH proxmox-backup v7 5/9] api: add POST endpoint for move-group Hannes Laimer
2026-04-20 14:49   ` Fabian Grünbichler
2026-04-16 17:18 ` [PATCH proxmox-backup v7 6/9] api: add POST endpoint for move-namespace Hannes Laimer
2026-04-16 17:18 ` [PATCH proxmox-backup v7 7/9] ui: add move group action Hannes Laimer
2026-04-16 17:18 ` [PATCH proxmox-backup v7 8/9] ui: add move namespace action Hannes Laimer
2026-04-16 17:18 ` [PATCH proxmox-backup v7 9/9] cli: add move-namespace and move-group commands Hannes Laimer
2026-04-20 15:02 ` [PATCH proxmox-backup v7 0/9] fixes #6195: add support for moving groups and namespaces Fabian Grünbichler
2026-04-22 13:41 ` superseded: " Hannes Laimer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1776851956.4wbeejt3wr.astroid@yuna.none \
    --to=f.gruenbichler@proxmox.com \
    --cc=c.ebner@proxmox.com \
    --cc=h.laimer@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal