Re: [pbs-devel] [PATCH proxmox-backup v9 0/6] closes #3071: maintenance mode for datastore

[Dylan Whyte <d.whyte@proxmox.com>]

Hi,

On 2/17/22 18:14, 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.
>
> 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
>
>   debian/control                       |   6 +-
>   pbs-api-types/Cargo.toml             |   2 +-
>   pbs-api-types/src/datastore.rs       |  24 +++++-
>   pbs-api-types/src/lib.rs             |   3 +
>   pbs-api-types/src/maintenance.rs     |  78 +++++++++++++++++
>   pbs-datastore/Cargo.toml             |   1 +
>   pbs-datastore/src/datastore.rs       | 121 +++++++++++++++++++--------
>   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 +++++++++++++++++
>   27 files changed, 540 insertions(+), 91 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

I just wanted to reply to this series, to get the ball rolling again. 
However, I can't say that I found much real issues with it on first 
glance, so I've just added some very minor suggestions.

As a rebase is now required for it to apply, I haven't done any testing. 
I'd be happy to test once it's rebased though.


regards,

Dylan