Re: [pbs-devel] [PATCH proxmox-backup v3 0/6] datastore: remove config reload on hot path

[Samuel Rufinatscha <s.rufinatscha@proxmox.com>]

On 11/20/25 3:50 PM, Fabian Grünbichler wrote:
> On November 20, 2025 2:03 pm, Samuel Rufinatscha wrote:
>> Hi,
>>
>> [..]
> 
> nit: this is getting a bit long ;)
> 
>>
>> ## Patch summary
>>
>> [PATCH 1/6] partial fix #6049: config: enable config version cache for datastore
>> [PATCH 2/6] partial fix #6049: datastore: impl ConfigVersionCache fast path for lookups
>> [PATCH 3/6] partial fix #6049: datastore: use config fast-path in Drop
>> [PATCH 4/6] partial fix #6049: datastore: add TTL fallback to catch manual config edits
>> [PATCH 5/6] to add a reload flag to the config cache helper.
>> [PATCH 6/6] to only bump generation when the config digest changes.
>>
>> ## Changes from v2:
>>
>> Added:
>> - [PATCH 5/6]: Add a reload flag to the config cache helper.
>> - [PATCH 6/6]: Only bump generation when the config digest changes.
> 
> please fold those into the existing version where they make sense, and
> include a per-patch changelog to know *what* changed ;)
> 
> e.g., the digest part can already go into the first patch (if the
> generation bumping is also moved thre from patch #4), or into patch #4.
> 
> the structural changes I suggested are missing, and I think the
> readability got worse as a result since v2, we now have six instances of
> checking whether there is some cache we are operating on or not..
> 
> I'll give more detailed feedback on the two new patches..
> 

Thanks for the review Fabian! I actually somehow missed your comment on 
PATCH 2/4 v2, sorry for that! Will make sure its included in the new 
version for sure.

>>
>> ## Maintainer notes
>>
>> No dependency bumps, no API changes and no breaking changes.
>>
>> Thanks,
>> Samuel
>>
>> [1] Bugzilla #6049: https://bugzilla.proxmox.com/show_bug.cgi?id=6049
>> [2] cargo-flamegraph: https://github.com/flamegraph-rs/flamegraph
>> [3] Bugzilla #7017: https://bugzilla.proxmox.com/show_bug.cgi?id=7017
>>
>> Samuel Rufinatscha (6):
>>    partial fix #6049: config: enable config version cache for datastore
>>    partial fix #6049: datastore: impl ConfigVersionCache fast path for
>>      lookups
>>    partial fix #6049: datastore: use config fast-path in Drop
>>    partial fix #6049: datastore: add TTL fallback to catch manual config
>>      edits
>>    partial fix #6049: datastore: add reload flag to config cache helper
>>    datastore: only bump generation when config digest changes
>>
>>   pbs-config/src/config_version_cache.rs |  10 +-
>>   pbs-datastore/Cargo.toml               |   1 +
>>   pbs-datastore/src/datastore.rs         | 232 ++++++++++++++++++++-----
>>   3 files changed, 197 insertions(+), 46 deletions(-)
>>
>> -- 
>> 2.47.3
>>
>>
>>
>> _______________________________________________
>> pbs-devel mailing list
>> pbs-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>>
>>
>>
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 



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