From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id B1B9F1FF183 for ; Wed, 8 Oct 2025 17:22:10 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 26B52C545; Wed, 8 Oct 2025 17:22:15 +0200 (CEST) From: Christian Ebner To: pbs-devel@lists.proxmox.com Date: Wed, 8 Oct 2025 17:21:20 +0200 Message-ID: <20251008152125.849216-8-c.ebner@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20251008152125.849216-1-c.ebner@proxmox.com> References: <20251008152125.849216-1-c.ebner@proxmox.com> MIME-Version: 1.0 X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1759936867814 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.043 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: [pbs-devel] [PATCH proxmox-backup v2 07/12] local store cache: rework access cache fetching and insert logic X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" The local datastore cache has both, an in-memory LRU cache only storing the digests and the chunk marks on the filesystem. Chunks in the LRU cache have recently been accessed, therefore the chunk contents are expected to be present in the local chunk file, while no payload is present for evicted ones. The current implementation relied on the cacher to fetch the chunk data on cache misses, but required to re-read the chunk file after the download, as the cacher interface does not allow to return a payload value other than the one defined for the LRU cache, which is however none. Therefore, instead of using the LRU cache access method and in turn the S3Cacher, rather try to access the local filesystem chunks directly. They need to be accessed anyways, and further this avoids possible races with download and insert, as now the held filehandle either has a chunk with valid content and can bypass the backend, or the chunk must be downloaded, serving the chunk from the fetched data instead after inserting into the cache. By unconditional re-insertion, it is assured that the chunk will be marked as recently used in all cases and the least recently used one is evicted. Signed-off-by: Christian Ebner --- .../src/local_datastore_lru_cache.rs | 50 ++++++++----------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/pbs-datastore/src/local_datastore_lru_cache.rs b/pbs-datastore/src/local_datastore_lru_cache.rs index ea92bc9b3..f03265a5b 100644 --- a/pbs-datastore/src/local_datastore_lru_cache.rs +++ b/pbs-datastore/src/local_datastore_lru_cache.rs @@ -102,42 +102,36 @@ impl LocalDatastoreLruCache { digest: &[u8; 32], cacher: &mut S3Cacher, ) -> Result, Error> { - if self - .cache - .access(*digest, cacher, |digest| self.store.clear_chunk(&digest)) - .await? - .is_some() - { - let (path, _digest_str) = self.store.chunk_path(digest); - let mut file = match std::fs::File::open(&path) { - Ok(file) => file, - Err(err) => { - // Expected chunk to be present since LRU cache has it, but it is missing - // locally, try to fetch again - if err.kind() == std::io::ErrorKind::NotFound { - let chunk = self.fetch_and_insert(cacher.client.clone(), digest).await?; - return Ok(Some(chunk)); - } else { - return Err(Error::from(err)); - } + let (path, _digest_str) = self.store.chunk_path(digest); + match std::fs::File::open(&path) { + Ok(mut file) => match DataBlob::load_from_reader(&mut file) { + // File was still cached with contents, load response from file + Ok(chunk) => { + self.cache + .insert(*digest, (), |digest| self.store.clear_chunk(&digest))?; + Ok(Some(chunk)) } - }; - let chunk = match DataBlob::load_from_reader(&mut file) { - Ok(chunk) => chunk, + // File was empty, might have been evicted since Err(err) => { use std::io::Seek; // Check if file is empty marker file, try fetching content if so if file.seek(std::io::SeekFrom::End(0))? == 0 { let chunk = self.fetch_and_insert(cacher.client.clone(), digest).await?; - return Ok(Some(chunk)); + Ok(Some(chunk)) } else { - return Err(err); + Err(err) } } - }; - Ok(Some(chunk)) - } else { - Ok(None) + }, + Err(err) => { + // Failed to open file, missing + if err.kind() == std::io::ErrorKind::NotFound { + let chunk = self.fetch_and_insert(cacher.client.clone(), digest).await?; + Ok(Some(chunk)) + } else { + Err(Error::from(err)) + } + } } } @@ -159,7 +153,7 @@ impl LocalDatastoreLruCache { Some(response) => { let bytes = response.content.collect().await?.to_bytes(); let chunk = DataBlob::from_raw(bytes.to_vec())?; - self.store.insert_chunk(&chunk, digest)?; + self.insert(digest, &chunk)?; Ok(chunk) } } -- 2.47.3 _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel