Re: [PATCH proxmox v5 13/13] s3-client: implement request counter threshold and exceeding callback

[Hannes Laimer <h.laimer@proxmox.com>]

On 2026-03-13 15:56, Christian Ebner wrote:
> On 3/13/26 3:26 PM, Hannes Laimer wrote:
>> On 2026-03-11 14:09, Christian Ebner wrote:
>>> Allows to set the threshold values to trigger exceeding callbacks
>>> for the s3 request counters.
>>> The globally settable callback function is executed whenever one of
>>> the request counters is exceeding it's set threshold value, but
>>> only once when passing over the threshold.
>>>
>>> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>>> ---
>>
>> [..]
>>
>>> +
>>> +    /// Update the callback executed when one of the set thresholds
>>> is exceeded
>>> +    pub fn update_thresholds_exceeded_callback(callback:
>>> ThresholdExceededCallback) {
>>> +        SHARED_COUNTER_THRESHOLD_EXCEEDED_CALLBACK
>>> +            .write()
>>> +            .unwrap()
>>> +            .replace(callback);
>>> +    }
>>
>> this is called on every datastore load, so
>> `SHARED_COUNTER_THRESHOLD_EXCEEDED_CALLBACK` only holds the callback
>> associated with the latest datastore that was looked-up. Every call
>> would replace whatever callback was there before, at least if we don't
>> hit the cache...
> 
> Yes that is true, might be better to only set it once. The callback is
> however shared behind an Arc for all datastores and already should use
> the correct counters and threshold for the given datastore, so the
> notifications are send for that, independent of which was the last store
> to be instantiated since not cached.
> 
> But it makes sense to avoid updating the function pointer each time.
> 

right, but isn't the datastore's name captured when we assign the
callback?

```
+
SharedRequestCounters::update_thresholds_exceeded_callback(Box::new(
+                    move |counter_id, threshold, value| {
+                        thresholds_exceeded_callback.map(|cb| {
+                            if let Err(err) = cb(&config.name,
counter_id, threshold, value) {
+                                log::error!("failed to send
notification: {err:?}");
+                            }
+                        });
+                    },
+                ));
```
am I missing something?

>>> +
>>> +    /// Update the request counter thresholds to given values.
>>> +    pub fn update_thresholds(&self, thresholds:
>>> &RequestCounterThresholds) {
>>> +        self.shared_memory
>>> +            .data()
>>> +            .counters
>>> +            .update_thresholds(thresholds);
>>> +    }
>>>   }
>>>     const FLUSH_THRESHOLD: Duration = Duration::from_secs(5);
>>
>