public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Dylan Whyte <d.whyte@proxmox.com>
To: Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>,
	Hannes Laimer <h.laimer@proxmox.com>
Subject: Re: [pbs-devel] [PATCH proxmox-backup v10 0/6] closes #3071: maintenance mode for datastore
Date: Tue, 12 Apr 2022 07:40:35 +0200	[thread overview]
Message-ID: <0f1b92fa-224a-2ba8-820f-a2b3d9731fe3@proxmox.com> (raw)
In-Reply-To: <20220412052601.4709-1-h.laimer@proxmox.com>

Hi,

I tested most datastore operations, as well as cases such as enabling 
maintenance mode mid task, and everything seems to work as expected, 
except for one potential issue.

If you enable maintenance mode during a vzdump backup job, the backup 
job will complete for the current guest, however, the task log 
'client.blob.log' won't get uploaded to the otherwise successful 
snapshot due to the server being in maintenance mode (this is noted in 
the PVE task log). Without looking into it, I imagine that this could be 
somewhat awkward to fix. However, it's also probably not much of an 
issue, as the task log still exists on the PVE side. I just wanted to 
make it known.

Tested-by: Dylan Whyte <d.whyte@proxmox.com>


On 4/12/22 07:25, Hannes Laimer wrote:
> Adds maintenance mode and tracking of active reading/writing operations.
> The maintenance mode prevents the start of new operations if the type of
> operation they would perform on the datastore would conflict with the
> maintenance type that is currently set. This check is performed when
> lookup_datastore is called. Tasks only call this function once at the
> beginning, therefore updating the maintenance type cannot interfere with
> already running tasks.
>
> active operations tracking:
> Changed file layout to now also keep track of the pid+starttime and the
> counts of operations that that pid started, like this it is possible to
> not count operations that were started by a dead process, since they
> are also not active anymore. Whenever the file is updated, also entries
> of dead processes are removed. When the file is read, only entries of
> active processes are counted.
>
> The UI shows a spinner with the count of conflictintg tasks (the tasks
> that were started before the maintenance type was updated) next to it.
> As soon as all conflicting tasks are finished a checkmark appears.
>
> v10:
>   - rebase onto master
>   - minor changes suggested by Dylan on v9
>
> v9:
>   - bump proxmox-schema dep to 1.2.1
>   - loosen MAINTENANCE_MESSAGE_REGEX
>   - MaintenanceMode fn check: use and_then()
>   - ui: properly (un)escape double quotes in message
>
> v8:
>   - replace enum with MaintenanceMode struct
>   - impl check function on MaintenanceMode struct
>   - api now encodes type+msg in one string
>   - message has to be encoded to ensure proper parsing
> (everywhere except the UI the message has to be already encoded
> when passed, API+CLI)
>
> v7:
>   - tracking: fix counting on clone
>   - ui: correct gettext usages + remove usage of capitalize
>
> v6:
>   - also use process start time in order to avoid pid clashes(as suggested
>   by Thomas and somehow missed by me in the last version)
>   - now a single call of get_active_operations return reads and writes
>   - improved code structure
>   - don't lock when reading
>
> v5:
>   - use simple struct and serde instead of manual parsing for file
>   - move tracking related stuff into new file (task_tracking.rs)
>
> v4:
>   - clones are not also tracked
>   - use lockfile, instead of locking the file
>   - track pid of the process which started smth
>   - updating maintenance mode is now always possible
>   - add get_active_operations endpoint for datastore
>   - ui: show count of conflicting tasks (or checkmark if no conflicting
>       operations are active)
>
>   v3, based on Dominik Csapak <d.csapak@proxmox.com>'s feedback:
>   - added Operation enum(r/w), as suggested by
>   - added active operation tracking
>   - combine type and message into on field
>
> v2:
>   - check for maintenance now directly in lookup_datastore
>   - parameter for checking is now the last acceptable maintenance type,
>     description in commit msg of 2nd patch
>   - ui cleanup
>
> Hannes Laimer (6):
>    api-types: add maintenance type
>    datastore: add check for maintenance in lookup
>    pbs-datastore: add active operations tracking
>    api: make maintenance_type updatable
>    api: add get_active_operations endpoint
>    ui: add option to change the maintenance type
>
>   pbs-api-types/Cargo.toml             |   2 +-
>   pbs-api-types/src/datastore.rs       |  23 ++++-
>   pbs-api-types/src/lib.rs             |   3 +
>   pbs-api-types/src/maintenance.rs     |  78 +++++++++++++++++
>   pbs-datastore/Cargo.toml             |   1 +
>   pbs-datastore/src/datastore.rs       | 126 +++++++++++++++++++--------
>   pbs-datastore/src/lib.rs             |   4 +
>   pbs-datastore/src/snapshot_reader.rs |   6 +-
>   pbs-datastore/src/task_tracking.rs   | 110 +++++++++++++++++++++++
>   src/api2/admin/datastore.rs          |  81 +++++++++++------
>   src/api2/backup/mod.rs               |   4 +-
>   src/api2/config/datastore.rs         |   5 ++
>   src/api2/reader/mod.rs               |   6 +-
>   src/api2/status.rs                   |   4 +-
>   src/api2/tape/backup.rs              |   6 +-
>   src/api2/tape/restore.rs             |   6 +-
>   src/bin/proxmox-backup-api.rs        |   1 +
>   src/bin/proxmox-backup-proxy.rs      |   4 +-
>   src/server/mod.rs                    |  16 +++-
>   src/server/prune_job.rs              |   4 +-
>   src/server/pull.rs                   |   4 +-
>   src/server/verify_job.rs             |   4 +-
>   www/Makefile                         |   1 +
>   www/Utils.js                         |  23 +++++
>   www/datastore/OptionView.js          |  30 +++++++
>   www/window/MaintenanceOptions.js     |  77 ++++++++++++++++
>   26 files changed, 539 insertions(+), 90 deletions(-)
>   create mode 100644 pbs-api-types/src/maintenance.rs
>   create mode 100644 pbs-datastore/src/task_tracking.rs
>   create mode 100644 www/window/MaintenanceOptions.js
>




  parent reply	other threads:[~2022-04-12  5:41 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-12  5:25 Hannes Laimer
2022-04-12  5:25 ` [pbs-devel] [PATCH proxmox-backup v10 1/6] api-types: add maintenance type Hannes Laimer
2022-04-12  5:25 ` [pbs-devel] [PATCH proxmox-backup v10 2/6] datastore: add check for maintenance in lookup Hannes Laimer
2022-04-12  5:25 ` [pbs-devel] [PATCH proxmox-backup v10 3/6] pbs-datastore: add active operations tracking Hannes Laimer
2022-04-12  5:25 ` [pbs-devel] [PATCH proxmox-backup v10 4/6] api: make maintenance_type updatable Hannes Laimer
2022-04-12  5:26 ` [pbs-devel] [PATCH proxmox-backup v10 5/6] api: add get_active_operations endpoint Hannes Laimer
2022-04-12  5:26 ` [pbs-devel] [PATCH proxmox-backup v10 6/6] ui: add option to change the maintenance type Hannes Laimer
2022-04-12  5:40 ` Dylan Whyte [this message]
2022-04-12 15:00 ` [pbs-devel] applied-series: [PATCH proxmox-backup v10 0/6] closes #3071: maintenance mode for datastore Thomas Lamprecht

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=0f1b92fa-224a-2ba8-820f-a2b3d9731fe3@proxmox.com \
    --to=d.whyte@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