[pbs-devel] applied-series: [PATCH v5 proxmox-backup 0/5] fix #5331: GC: avoid multiple atime updates

[Thomas Lamprecht]

Am 26.03.25 um 11:03 schrieb Christian Ebner:> This patches implement the logic to greatly improve the performance
> of phase 1 garbage collection by avoiding multiple atime updates on
> the same chunk.
> 
> Currently, phase 1 GC iterates over all folders in the datastore
> looking and collecting all image index files without taking any
> logical assumptions (e.g. namespaces, groups, snapshots, ...). This
> is to avoid accidentally missing image index files located in
> unexpected paths and therefore not marking their chunks as in use,
> leading to potential data losses.
> 
> This patches improve phase 1 by:
> - Iterating index images using the datatstore's iterators for detecting
>   regular index files. Paths outside of the iterator logic are still taken
>   into account and processed as well by generating a list of all the found
>   images first, removing index files encountered while iterating, finally
>   leaving a list of indexes with unexpected paths. These unexpected paths
>   are now also logged, for the user to potentially take action.
> - Keeping track of recently touched chunks by storing their digests in a
>   LRU cache, skipping over expensive atime updates for chunks already
>   present in the cache.


> Christian Ebner (5):
>   tools: lru cache: tell if node was already present or newly inserted
>   garbage collection: format error including anyhow error context
>   datastore: add helper method to open index reader from path
>   garbage collection: generate index file list via datastore iterators
>   fix #5331: garbage collection: avoid multiple chunk atime updates
> 
>  pbs-datastore/src/datastore.rs  | 179 ++++++++++++++++++++++----------
>  pbs-tools/src/lru_cache.rs      |   4 +-
>  src/api2/admin/datastore.rs     |   6 +-
>  src/bin/proxmox-backup-proxy.rs |   2 +-
>  4 files changed, 131 insertions(+), 60 deletions(-)
> 

I decided to move this forward as is, getting the other metrics measured
like proposed in my reply to patch 5/5 might be still nice, if that really
results in us choosing a different default or what not we can record those
metrics in a new commit, else they are at least on the list and all is
fine anyway. So:

applied, thanks!