[pbs-devel] [PATCH proxmox] pbs api types: add garbage collection cache capacity tuning option

[pbs-devel] [PATCH proxmox] pbs api types: add garbage collection cache capacity tuning option

[Christian Ebner]

Allows to adjust the capacity for the LRU cache used to keep track of
recently touched chunks during phase 1 of garbage collection.

Values are provided as multiples of 1024 cache entries, the default
value of 1024  was chosen as tradeoff between runtime improvements
and memory usage [0]. The maximum of 8192 was chosen based on the
linear regression from [1], resulting in about 8 * 80 MiB = 640 MiB
of memory requirement, while allowing to keep chunks which can
reference about 32 TiB of data in case of 4 MiB fixed size chunks.

[0] https://git.proxmox.com/?p=proxmox-backup.git;a=commit;h=03143eee0a59cf319be0052e139f7e20e124d572
[1] https://lore.proxmox.com/pbs-devel/fa3800dd-e812-4c9a-9d3d-2d8673e05355@proxmox.com/

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-api-types/src/datastore.rs | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs
index ddd8d3c6..e6cc1975 100644
--- a/pbs-api-types/src/datastore.rs
+++ b/pbs-api-types/src/datastore.rs
@@ -223,12 +223,23 @@ pub enum DatastoreFSyncLevel {
     Filesystem,
 }
 
+pub const GC_CACHE_CAPACITY_SCHEMA: Schema =
+    IntegerSchema::new("Garbage collection LRU cache capacity (in multiples of 1024 chunk digests")
+        .minimum(0)
+        .maximum(8192)
+        .default(1024)
+        .schema();
+
 #[api(
     properties: {
         "chunk-order": {
             type: ChunkOrder,
             optional: true,
         },
+        "gc-cache-capacity": {
+            schema: GC_CACHE_CAPACITY_SCHEMA,
+            optional: true,
+        },
     },
 )]
 #[derive(Serialize, Deserialize, Default)]
@@ -240,6 +251,8 @@ pub struct DatastoreTuning {
     pub chunk_order: Option<ChunkOrder>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub sync_level: Option<DatastoreFSyncLevel>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub gc_cache_capacity: Option<usize>,
 }
 
 pub const DATASTORE_TUNING_STRING_SCHEMA: Schema = StringSchema::new("Datastore tuning options")
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup 2/4] garbage collection: set phase1 LRU cache capacity by tuning option

[Christian Ebner]

Allow to control the capacity of the cache used to track recently
touched chunks via the configured value in the datastore tuning
options. Log the configured value to the task log, if an explicit
value is set, allowing the user to confirm the setting and debug.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/datastore.rs | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index 008449e51..aa9a033b4 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -1113,6 +1113,7 @@ impl DataStore {
         &self,
         status: &mut GarbageCollectionStatus,
         worker: &dyn WorkerTaskContext,
+        cache_capacity: usize,
     ) -> Result<(), Error> {
         // Iterate twice over the datastore to fetch index files, even if this comes with an
         // additional runtime cost:
@@ -1128,7 +1129,7 @@ impl DataStore {
         let mut unprocessed_index_list = self.list_index_files()?;
         let index_count = unprocessed_index_list.len();
 
-        let mut chunk_lru_cache = LruCache::new(1024 * 1024);
+        let mut chunk_lru_cache = LruCache::new(cache_capacity);
         let mut processed_index_files = 0;
         let mut last_percentage: usize = 0;
 
@@ -1237,9 +1238,20 @@ impl DataStore {
                 ..Default::default()
             };
 
+            let tuning: DatastoreTuning = serde_json::from_value(
+                DatastoreTuning::API_SCHEMA
+                    .parse_property_string(gc_store_config.tuning.as_deref().unwrap_or(""))?,
+            )?;
+            let gc_cache_capacity = if let Some(capacity) = tuning.gc_cache_capacity {
+                info!("Using cache capacity of {capacity} (multiples of 1024 chunk digests).");
+                capacity * 1024
+            } else {
+                1024 * 1024
+            };
+
             info!("Start GC phase1 (mark used chunks)");
 
-            self.mark_used_chunks(&mut gc_status, worker)
+            self.mark_used_chunks(&mut gc_status, worker, gc_cache_capacity)
                 .context("marking used chunks failed")?;
 
             info!("Start GC phase2 (sweep unused chunks)");
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup 3/4] ui: expose GC cache capacity in datastore tuning parameters.

[Christian Ebner]

Displays and allows to edit the configured LRU cache capacity via the
datastore tuning parameters.

A step of 512 is used in the number field for convenience when using
the buttons, more fine grained values can be set by typing.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 www/Utils.js                |  4 ++++
 www/datastore/OptionView.js | 14 ++++++++++++++
 2 files changed, 18 insertions(+)

diff --git a/www/Utils.js b/www/Utils.js
index 2746ef0b5..1572f6e56 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -846,6 +846,10 @@ Ext.define('PBS.Utils', {
 	sync = PBS.Utils.tuningOptions['sync-level'][sync ?? '__default__'];
 	options.push(`${gettext('Sync Level')}: ${sync}`);
 
+	let gc_cache_capacity = tuning['gc-cache-capacity'];
+	delete tuning['gc-cache-capacity'];
+	options.push(`${gettext('GC cache capacity')}: ${gc_cache_capacity ?? Proxmox.Utils.defaultText}`);
+
 	for (const [k, v] of Object.entries(tuning)) {
 	    options.push(`${k}: ${v}`);
 	}
diff --git a/www/datastore/OptionView.js b/www/datastore/OptionView.js
index e1f38af6f..84ef9fc51 100644
--- a/www/datastore/OptionView.js
+++ b/www/datastore/OptionView.js
@@ -271,6 +271,20 @@ Ext.define('PBS.Datastore.Options', {
 			    deleteEmpty: true,
 			    value: '__default__',
 			},
+			{
+			    xtype: 'proxmoxintegerfield',
+			    name: 'gc-cache-capacity',
+			    fieldLabel: gettext('GC cache capacity'),
+			    emptyText: Proxmox.Utils.defaultText,
+			    minValue: 0,
+			    maxValue: 8192,
+			    deleteEmpty: true,
+			    step: 512,
+			    autoEl: {
+				tag: 'div',
+				'data-qtip': gettext('GC LRU cache capacity (in multiples of 1024 chunk digests)'),
+			    },
+			},
 		    ],
 		},
 	    },
-- 
2.39.5



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

[pbs-devel] [PATCH proxmox-backup 4/4] docs: add description for gc-cache-capacity tuning parameter

[Christian Ebner]

Adds a bullet point to the listed datastore tuning parameters,
describing its functionality, implications and typical values.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 docs/storage.rst | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/docs/storage.rst b/docs/storage.rst
index 490302955..cab65ef79 100644
--- a/docs/storage.rst
+++ b/docs/storage.rst
@@ -435,9 +435,17 @@ There are some tuning related options for the datastore that are more advanced:
 
   This can be set with:
 
-.. code-block:: console
+  .. code-block:: console
+
+    # proxmox-backup-manager datastore update <storename> --tuning 'sync-level=filesystem'
 
-  # proxmox-backup-manager datastore update <storename> --tuning 'sync-level=filesystem'
+* ``gc-cache-capacity``: Datastore GC least recently used cache capacity:
+  Allows to control the cache capacity used to keep track of chunks for which
+  the access time has already been updated during phase 1 of garbage collection.
+  This avoids multiple updates and increases GC runtime performance. The
+  capacity is set as the given value multiplied by 1024. Higher values can
+  reduce GC runtime at the cost of increase memory usage, setting the value to 0
+  disables caching.
 
 If you want to set multiple tuning options simultaneously, you can separate them
 with a comma, like this:
-- 
2.39.5



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

Re: [pbs-devel] [PATCH proxmox-backup 4/4] docs: add description for gc-cache-capacity tuning parameter

[Lukas Wagner]

On  2025-04-03 14:27, Christian Ebner wrote:
> Adds a bullet point to the listed datastore tuning parameters,
> describing its functionality, implications and typical values.
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
>  docs/storage.rst | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/storage.rst b/docs/storage.rst
> index 490302955..cab65ef79 100644
> --- a/docs/storage.rst
> +++ b/docs/storage.rst
> @@ -435,9 +435,17 @@ There are some tuning related options for the datastore that are more advanced:
>  
>    This can be set with:
>  
> -.. code-block:: console
> +  .. code-block:: console
> +
> +    # proxmox-backup-manager datastore update <storename> --tuning 'sync-level=filesystem'
>  
> -  # proxmox-backup-manager datastore update <storename> --tuning 'sync-level=filesystem'
> +* ``gc-cache-capacity``: Datastore GC least recently used cache capacity:
> +  Allows to control the cache capacity used to keep track of chunks for which
> +  the access time has already been updated during phase 1 of garbage collection.
> +  This avoids multiple updates and increases GC runtime performance. The
> +  capacity is set as the given value multiplied by 1024. Higher values can
> +  reduce GC runtime at the cost of increase memory usage, setting the value to 0
> +  disables caching.

I think we could completely omit the "the capacity is set as the given value multiplied by 1024" sentence here
and consider the fact that the LRU cache size is value * 1024 an implementation detail.
For the user, the exact number of cached digests in the backend is probably not really that important, right?
In reality, they just want some knob that they can adjust in a range from 0 (no caching) to some maximum.

Same of course applies also for the GUI patch and the log message.

What do you think?

-- 
- Lukas



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

Re: [pbs-devel] [PATCH proxmox-backup 4/4] docs: add description for gc-cache-capacity tuning parameter

[Christian Ebner]

On 4/4/25 13:58, Lukas Wagner wrote:
> 
> 
> On  2025-04-03 14:27, Christian Ebner wrote:
>> Adds a bullet point to the listed datastore tuning parameters,
>> describing its functionality, implications and typical values.
>>
>> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
>> ---
>>   docs/storage.rst | 12 ++++++++++--
>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/docs/storage.rst b/docs/storage.rst
>> index 490302955..cab65ef79 100644
>> --- a/docs/storage.rst
>> +++ b/docs/storage.rst
>> @@ -435,9 +435,17 @@ There are some tuning related options for the datastore that are more advanced:
>>   
>>     This can be set with:
>>   
>> -.. code-block:: console
>> +  .. code-block:: console
>> +
>> +    # proxmox-backup-manager datastore update <storename> --tuning 'sync-level=filesystem'
>>   
>> -  # proxmox-backup-manager datastore update <storename> --tuning 'sync-level=filesystem'
>> +* ``gc-cache-capacity``: Datastore GC least recently used cache capacity:
>> +  Allows to control the cache capacity used to keep track of chunks for which
>> +  the access time has already been updated during phase 1 of garbage collection.
>> +  This avoids multiple updates and increases GC runtime performance. The
>> +  capacity is set as the given value multiplied by 1024. Higher values can
>> +  reduce GC runtime at the cost of increase memory usage, setting the value to 0
>> +  disables caching.
> 
> I think we could completely omit the "the capacity is set as the given value multiplied by 1024" sentence here
> and consider the fact that the LRU cache size is value * 1024 an implementation detail.
> For the user, the exact number of cached digests in the backend is probably not really that important, right?

Agreed, I opted for including this as the option states "capacity", so 
one might expect the exact capacity. But you are completely right that 
this is mostly unimportant for the end user.

> In reality, they just want some knob that they can adjust in a range from 0 (no caching) to some maximum.
> 
> Same of course applies also for the GUI patch and the log message.
> 
> What do you think?

Will send a new version which drops the mentions of multiples of 1024, 
thanks for review and testing!


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

Re: [pbs-devel] [PATCH proxmox-backup 4/4] docs: add description for gc-cache-capacity tuning parameter

[Thomas Lamprecht]

Am 04.04.25 um 13:58 schrieb Lukas Wagner:
> I think we could completely omit the "the capacity is set as the given value multiplied by 1024" sentence here
> and consider the fact that the LRU cache size is value * 1024 an implementation detail.

But once that option is available it's part of the API and not just
an implementation detail anymore?

> For the user, the exact number of cached digests in the backend is probably not really that important, right?
> In reality, they just want some knob that they can adjust in a range from 0 (no caching) to some maximum.

For some, probably even a lot, users it might be indeed enough to
like double or half the number depending on if they want to improve
performance or reduce memory footprint.

But I know users that have a hard time working with a numerical
setting without knowing what it's exactly doing on a lower level, at
least for me such settings are often rather irritating, as I cannot
really have a good thought process for how I'd choose the number
depending on what I want to achieve.

So while I agree with your underlying point, I'd still like users
being able to relatively easily find out how much change translates
in what impact.

> Same of course applies also for the GUI patch and the log message.
> 
> What do you think?

Two alternatives:
- Changes this to the shift width, i.e. the x from 2^x, similar to the
  ZFS setting. Makes it nice small number to configure and for most
  use cases the exponential nature should be still granular enough.
  That said, it's not very user-friendly, at least to those without
  some level of CS background or the like.

- just drop the * 1024 factor and allow users to enter the full number,
  it then can be simply described as numbers of chunks which is trivial
  to understand and relate too.

Personally I'd favor the second option, mainly because it's so simple,
and having big numbers here is not that of a huge problem.

ps. secret option three: adapt the human byte selector in the frontend
to expose selecting kilo-chunks and mega-chunks ;-)


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

Re: [pbs-devel] [PATCH proxmox-backup 4/4] docs: add description for gc-cache-capacity tuning parameter

[Lukas Wagner]

On  2025-04-04 14:20, Thomas Lamprecht wrote:
> Am 04.04.25 um 13:58 schrieb Lukas Wagner:
>> I think we could completely omit the "the capacity is set as the given value multiplied by 1024" sentence here
>> and consider the fact that the LRU cache size is value * 1024 an implementation detail.
> 
> But once that option is available it's part of the API and not just
> an implementation detail anymore?
> 
>> For the user, the exact number of cached digests in the backend is probably not really that important, right?
>> In reality, they just want some knob that they can adjust in a range from 0 (no caching) to some maximum.
> 
> For some, probably even a lot, users it might be indeed enough to
> like double or half the number depending on if they want to improve
> performance or reduce memory footprint.
> 
> But I know users that have a hard time working with a numerical
> setting without knowing what it's exactly doing on a lower level, at
> least for me such settings are often rather irritating, as I cannot
> really have a good thought process for how I'd choose the number
> depending on what I want to achieve.
> 
> So while I agree with your underlying point, I'd still like users
> being able to relatively easily find out how much change translates
> in what impact.
> 
>> Same of course applies also for the GUI patch and the log message.
>>
>> What do you think?
> 
> Two alternatives:
> - Changes this to the shift width, i.e. the x from 2^x, similar to the
>   ZFS setting. Makes it nice small number to configure and for most
>   use cases the exponential nature should be still granular enough.
>   That said, it's not very user-friendly, at least to those without
>   some level of CS background or the like.
> 
> - just drop the * 1024 factor and allow users to enter the full number,
>   it then can be simply described as numbers of chunks which is trivial
>   to understand and relate too.
> 
> Personally I'd favor the second option, mainly because it's so simple,
> and having big numbers here is not that of a huge problem.

Sounds like a good idea, I like it.
My main gripe with the "times 1024" option was that it makes it a bit
more confusing to the user (e.g. me, when reading 'GC LRU cache capacity (in multiples of 1024 chunk digests)'
in the UI I first thought that the value itself must be a multiple of 1024).

Changing the setting to the full number, we avoid this potential for confusion while
still giving power-users a good sense of what is going on under the hood.

> 
> ps. secret option three: adapt the human byte selector in the frontend
> to expose selecting kilo-chunks and mega-chunks ;-)
> 


-- 
- Lukas



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

Re: [pbs-devel] [PATCH proxmox-backup 4/4] docs: add description for gc-cache-capacity tuning parameter

[Christian Ebner]

On 4/4/25 14:28, Lukas Wagner wrote:
> 
> 
> On  2025-04-04 14:20, Thomas Lamprecht wrote:
>> Two alternatives:
>> - Changes this to the shift width, i.e. the x from 2^x, similar to the
>>    ZFS setting. Makes it nice small number to configure and for most
>>    use cases the exponential nature should be still granular enough.
>>    That said, it's not very user-friendly, at least to those without
>>    some level of CS background or the like.
>>
>> - just drop the * 1024 factor and allow users to enter the full number,
>>    it then can be simply described as numbers of chunks which is trivial
>>    to understand and relate too.
>>
>> Personally I'd favor the second option, mainly because it's so simple,
>> and having big numbers here is not that of a huge problem.
> 
> Sounds like a good idea, I like it.
> My main gripe with the "times 1024" option was that it makes it a bit
> more confusing to the user (e.g. me, when reading 'GC LRU cache capacity (in multiples of 1024 chunk digests)'
> in the UI I first thought that the value itself must be a multiple of 1024).
> 
> Changing the setting to the full number, we avoid this potential for confusion while
> still giving power-users a good sense of what is going on under the hood.

Okay, so let's go with the full values there, that should be the least 
confusing one.


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

Re: [pbs-devel] [PATCH proxmox] pbs api types: add garbage collection cache capacity tuning option

[Lukas Wagner]

I left one comment on Patch 4/4 regarding the '(multiples of 1024)' phrasing,
apart from that consider this:

Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>
Tested-by: Lukas Wagner <l.wagner@proxmox.com>


-- 
- Lukas



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

[pbs-devel] superseded: [PATCH proxmox] pbs api types: add garbage collection cache capacity tuning option

[Christian Ebner]

superseded-by version 2: 
https://lore.proxmox.com/pbs-devel/20250404130713.376630-1-c.ebner@proxmox.com/T/


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