[PATCH proxmox-backup] chunk store: s3: mute warning on overwrite empty chunk in local cache

[PATCH proxmox-backup] chunk store: s3: mute warning on overwrite empty chunk in local cache

[Christian Ebner]

For regular datastores overwriting chunks which have 0 size is
unexpected and cannot happen under regular operation. For datastores
backed by an S3 object storage, this is however expected as regular
operation if the cache reaches its capacity, since chunks get evicted
from the cache by leaving an empty file as chunk marker behind.

While the warning is benign, it causes false alerts in the task logs.
Therefore, silence the warning if the chunk insert overwriting the
file happens via the local datastore cache.

Fixes: https://forum.proxmox.com/threads/181330/
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-datastore/src/chunk_store.rs               | 7 +++++--
 pbs-datastore/src/local_datastore_lru_cache.rs | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs
index bd1dc353b..773167a6d 100644
--- a/pbs-datastore/src/chunk_store.rs
+++ b/pbs-datastore/src/chunk_store.rs
@@ -667,7 +667,7 @@ impl ChunkStore {
         let _lock = self.mutex.lock();
 
         // Safety: lock acquired above
-        unsafe { self.insert_chunk_nolock(chunk, digest) }
+        unsafe { self.insert_chunk_nolock(chunk, digest, true) }
     }
 
     /// Safety: requires holding the chunk store mutex!
@@ -675,6 +675,7 @@ impl ChunkStore {
         &self,
         chunk: &DataBlob,
         digest: &[u8; 32],
+        warn_on_overwrite_empty: bool,
     ) -> Result<(bool, u64), Error> {
         // unwrap: only `None` in unit tests
         assert!(self.locker.is_some());
@@ -695,7 +696,9 @@ impl ChunkStore {
                 self.touch_chunk_no_lock(digest)?;
                 return Ok((true, old_size));
             } else if old_size == 0 {
-                log::warn!("found empty chunk '{digest_str}' in store {name}, overwriting");
+                if warn_on_overwrite_empty {
+                    log::warn!("found empty chunk '{digest_str}' in store {name}, overwriting");
+                }
             } else if chunk.is_encrypted() {
                 // incoming chunk is encrypted, possible attack or hash collision!
                 let mut existing_file = std::fs::File::open(&chunk_path)?;
diff --git a/pbs-datastore/src/local_datastore_lru_cache.rs b/pbs-datastore/src/local_datastore_lru_cache.rs
index f39b26ebb..ac27d4637 100644
--- a/pbs-datastore/src/local_datastore_lru_cache.rs
+++ b/pbs-datastore/src/local_datastore_lru_cache.rs
@@ -38,7 +38,7 @@ impl LocalDatastoreLruCache {
 
         // Safety: lock acquire above
         unsafe {
-            self.store.insert_chunk_nolock(chunk, digest)?;
+            self.store.insert_chunk_nolock(chunk, digest, false)?;
         }
         self.cache.insert(*digest, (), |digest| {
             // Safety: lock acquired above, this is executed inline!
-- 
2.47.3

applied: [PATCH proxmox-backup] chunk store: s3: mute warning on overwrite empty chunk in local cache

[Fabian Grünbichler]

On Wed, 04 Mar 2026 11:36:00 +0100, Christian Ebner wrote:
> For regular datastores overwriting chunks which have 0 size is
> unexpected and cannot happen under regular operation. For datastores
> backed by an S3 object storage, this is however expected as regular
> operation if the cache reaches its capacity, since chunks get evicted
> from the cache by leaving an empty file as chunk marker behind.
> 
> While the warning is benign, it causes false alerts in the task logs.
> Therefore, silence the warning if the chunk insert overwriting the
> file happens via the local datastore cache.
> 
> [...]

Applied, thanks!

Might be something to consider when refactoring the whole datastore/chunkstore
interaction with a proper locked wrapper and integrating the backend - this
could then be derived from the backend?

[1/1] chunk store: s3: mute warning on overwrite empty chunk in local cache
      commit: fa9fa9b82d768b912987f78df313cea6fd145078

Best regards,
-- 
Fabian Grünbichler <f.gruenbichler@proxmox.com>

Re: applied: [PATCH proxmox-backup] chunk store: s3: mute warning on overwrite empty chunk in local cache

[Christian Ebner]

On 3/5/26 9:51 AM, Fabian Grünbichler wrote:
> 
> On Wed, 04 Mar 2026 11:36:00 +0100, Christian Ebner wrote:
>> For regular datastores overwriting chunks which have 0 size is
>> unexpected and cannot happen under regular operation. For datastores
>> backed by an S3 object storage, this is however expected as regular
>> operation if the cache reaches its capacity, since chunks get evicted
>> from the cache by leaving an empty file as chunk marker behind.
>>
>> While the warning is benign, it causes false alerts in the task logs.
>> Therefore, silence the warning if the chunk insert overwriting the
>> file happens via the local datastore cache.
>>
>> [...]
> 
> Applied, thanks!
> 
> Might be something to consider when refactoring the whole datastore/chunkstore
> interaction with a proper locked wrapper and integrating the backend - this
> could then be derived from the backend?

Yes, although if we do keep the working principle for the cache as is, a 
chunk insert overwriting an empty file will be regular operation, 
independent on the actual chunk store backend.

Backend implementation and local cache should not be intertwined to much 
I think? We might reuse this for other backend implementations?