txpool: track failed delete blobs in cleanup (#6952)

Signed-off-by: jsvisa <delweng@gmail.com>

crates/transaction-pool/src/blobstore/disk.rs

@@ -1,6 +1,6 @@
 //! A simple diskstore for blobs
 
-use crate::{blobstore::BlobStoreSize, BlobStore, BlobStoreError};
+use crate::blobstore::{BlobStore, BlobStoreCleanupStat, BlobStoreError, BlobStoreSize};
 use alloy_rlp::{Decodable, Encodable};
 use parking_lot::{Mutex, RwLock};
 use reth_primitives::{BlobTransactionSidecar, TxHash, B256};
@@ -71,12 +71,12 @@ impl BlobStore for DiskFileBlobStore {
         Ok(())
     }
 
-    fn cleanup(&self) {
+    fn cleanup(&self) -> BlobStoreCleanupStat {
         let txs_to_delete = {
             let mut txs_to_delete = self.inner.txs_to_delete.write();
             std::mem::take(&mut *txs_to_delete)
         };
-        let mut deleted_blobs = txs_to_delete.len();
+        let mut stat = BlobStoreCleanupStat::default();
         let mut subsize = 0;
         debug!(target:"txpool::blob", num_blobs=%txs_to_delete.len(), "Removing blobs from disk");
         for tx in txs_to_delete {
@@ -84,14 +84,18 @@ impl BlobStore for DiskFileBlobStore {
             let _ = fs::metadata(&path).map(|meta| {
                 subsize += meta.len();
             });
-            let _ = fs::remove_file(&path).map_err(|e| {
-                deleted_blobs -= 1;
-                let err = DiskFileBlobStoreError::DeleteFile(tx, path, e);
-                debug!(target:"txpool::blob", %err);
-            });
+            match fs::remove_file(&path) {
+                Ok(_) => stat.delete_succeed += 1,
+                Err(e) => {
+                    stat.delete_failed += 1;
+                    let err = DiskFileBlobStoreError::DeleteFile(tx, path, e);
+                    debug!(target:"txpool::blob", %err);
+                }
+            };
         }
         self.inner.size_tracker.sub_size(subsize as usize);
-        self.inner.size_tracker.sub_len(deleted_blobs);
+        self.inner.size_tracker.sub_len(stat.delete_succeed);
+        stat
     }
 
     fn get(&self, tx: B256) -> Result<Option<BlobTransactionSidecar>, BlobStoreError> {

crates/transaction-pool/src/blobstore/mem.rs

@@ -1,4 +1,6 @@
-use crate::blobstore::{BlobStore, BlobStoreError, BlobStoreSize, BlobTransactionSidecar};
+use crate::blobstore::{
+    BlobStore, BlobStoreCleanupStat, BlobStoreError, BlobStoreSize, BlobTransactionSidecar,
+};
 use parking_lot::RwLock;
 use reth_primitives::B256;
 use std::{collections::HashMap, sync::Arc};
@@ -67,7 +69,9 @@ impl BlobStore for InMemoryBlobStore {
         Ok(())
     }
 
-    fn cleanup(&self) {}
+    fn cleanup(&self) -> BlobStoreCleanupStat {
+        BlobStoreCleanupStat::default()
+    }
 
     // Retrieves the decoded blob data for the given transaction hash.
     fn get(&self, tx: B256) -> Result<Option<BlobTransactionSidecar>, BlobStoreError> {

crates/transaction-pool/src/blobstore/mod.rs

@@ -34,11 +34,12 @@ pub trait BlobStore: fmt::Debug + Send + Sync + 'static {
     /// Deletes multiple blob sidecars from the store
     fn delete_all(&self, txs: Vec<B256>) -> Result<(), BlobStoreError>;
 
-    /// A maintenance function that can be called periodically to clean up the blob store.
+    /// A maintenance function that can be called periodically to clean up the blob store, returns
+    /// the number of successfully deleted blobs and the number of failed deletions.
     ///
     /// This is intended to be called in the background to clean up any old or unused data, in case
     /// the store uses deferred cleanup: [DiskFileBlobStore]
-    fn cleanup(&self);
+    fn cleanup(&self) -> BlobStoreCleanupStat;
 
     /// Retrieves the decoded blob data for the given transaction hash.
     fn get(&self, tx: B256) -> Result<Option<BlobTransactionSidecar>, BlobStoreError>;
@@ -139,6 +140,15 @@ impl PartialEq for BlobStoreSize {
     }
 }
 
+/// Statistics for the cleanup operation.
+#[derive(Debug, Clone, Default)]
+pub struct BlobStoreCleanupStat {
+    /// the number of successfully deleted blobs
+    pub delete_succeed: usize,
+    /// the number of failed deletions
+    pub delete_failed: usize,
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

crates/transaction-pool/src/blobstore/noop.rs

@@ -1,4 +1,4 @@
-use crate::blobstore::{BlobStore, BlobStoreError, BlobTransactionSidecar};
+use crate::blobstore::{BlobStore, BlobStoreCleanupStat, BlobStoreError, BlobTransactionSidecar};
 use reth_primitives::B256;
 
 /// A blobstore implementation that does nothing
@@ -23,7 +23,9 @@ impl BlobStore for NoopBlobStore {
         Ok(())
     }
 
-    fn cleanup(&self) {}
+    fn cleanup(&self) -> BlobStoreCleanupStat {
+        BlobStoreCleanupStat::default()
+    }
 
     fn get(&self, _tx: B256) -> Result<Option<BlobTransactionSidecar>, BlobStoreError> {
         Ok(None)

crates/transaction-pool/src/pool/mod.rs

@@ -784,26 +784,18 @@ where
 
     /// Delete a blob from the blob store
     pub(crate) fn delete_blob(&self, blob: TxHash) {
-        if let Err(err) = self.blob_store.delete(blob) {
-            warn!(target: "txpool", %err, "[{:?}] failed to delete blobs", blob);
-            self.blob_store_metrics.blobstore_failed_deletes.increment(1);
-        }
-        self.update_blob_store_metrics();
+        let _ = self.blob_store.delete(blob);
     }
 
     /// Delete all blobs from the blob store
     pub(crate) fn delete_blobs(&self, txs: Vec<TxHash>) {
-        let num = txs.len();
-        if let Err(err) = self.blob_store.delete_all(txs) {
-            warn!(target: "txpool", %err,?num, "failed to delete blobs");
-            self.blob_store_metrics.blobstore_failed_deletes.increment(num as u64);
-        }
-        self.update_blob_store_metrics();
+        let _ = self.blob_store.delete_all(txs);
     }
 
     /// Cleans up the blob store
     pub(crate) fn cleanup_blobs(&self) {
-        self.blob_store.cleanup();
+        let stat = self.blob_store.cleanup();
+        self.blob_store_metrics.blobstore_failed_deletes.increment(stat.delete_failed as u64);
         self.update_blob_store_metrics();
     }