Re: [pbs-devel] [PATCH proxmox-backup v2 6/6] api: datastore: wait for active operations to clear before s3 refresh

[Christian Ebner <c.ebner@proxmox.com>]

On 11/13/25 9:15 AM, Fabian Grünbichler wrote:
> On November 12, 2025 5:36 pm, Christian Ebner wrote:
>> Currently, the s3 refresh does not take into consideration already
>> ongoing active operations, only blocking new ones.
>>
>> This will however lead to inconsistencies if there are ongoing read
>> or write operations. Therefore, actively wait for ongoing operatioins
>> to complete before running the actual refresh and keep the datastore
>> config locked so the maintenance mode cannot be altered.
>>
>> If an abort was requested while waiting, clear the maintenance mode
>> as well.
>>
>> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>> ---
>> changes since version 1:
>> - use refactored helpers from identical unmount logic
>>
>>   src/api2/admin/datastore.rs | 28 +++++++++++++++++++++++-----
>>   1 file changed, 23 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs
>> index 91189d7ae..93e085be3 100644
>> --- a/src/api2/admin/datastore.rs
>> +++ b/src/api2/admin/datastore.rs
>> @@ -2716,11 +2716,29 @@ pub fn s3_refresh(store: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Valu
>>           Some(store.clone()),
>>           auth_id.to_string(),
>>           to_stdout,
>> -        move |_worker| {
>> -            proxmox_async::runtime::block_on(datastore.s3_refresh())?;
>> -
>> -            let (_lock, config) = expect_maintenance_type(&store, MaintenanceType::S3Refresh)?;
>> -            unset_maintenance(_lock, config).context("failed to clear maintenance mode")
>> +        move |worker| {
>> +            let mut old_status = String::new();
>> +            let aborted = wait_on_active_operations(
>> +                &store,
>> +                Some(&worker),
>> +                MaintenanceType::S3Refresh,
>> +                |reads, writes| {
>> +                    let status = format!(
>> +                        "waiting for active operations to finsish: read {reads}, write {writes}",
>> +                    );
>> +                    if status != old_status {
>> +                        info!("{status}");
>> +                        old_status = status;
>> +                    }
>> +                },
>> +            )?;
>> +            clear_or_run_maintenance_locked(
>> +                &store,
>> +                Some(&worker),
>> +                MaintenanceType::S3Refresh,
>> +                aborted,
>> +                || proxmox_async::runtime::block_on(datastore.s3_refresh()),
>> +            )
> 
> there is one more call to s3_refresh when (re)creating a datastore, that
> seems to me is running completely unprotected..

Oh, yes indeed, that needs to be adapted as well now.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel