diff --git a/crates/transaction-pool/src/blobstore/disk.rs b/crates/transaction-pool/src/blobstore/disk.rs index 964d28e7d0..d1d9e54901 100644 --- a/crates/transaction-pool/src/blobstore/disk.rs +++ b/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, BlobStoreError> { diff --git a/crates/transaction-pool/src/blobstore/mem.rs b/crates/transaction-pool/src/blobstore/mem.rs index 7cf4cd894a..c276efa696 100644 --- a/crates/transaction-pool/src/blobstore/mem.rs +++ b/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, BlobStoreError> { diff --git a/crates/transaction-pool/src/blobstore/mod.rs b/crates/transaction-pool/src/blobstore/mod.rs index 4a69d1fb94..af2d7efb52 100644 --- a/crates/transaction-pool/src/blobstore/mod.rs +++ b/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) -> 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, 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::*; diff --git a/crates/transaction-pool/src/blobstore/noop.rs b/crates/transaction-pool/src/blobstore/noop.rs index 16c949e9da..7714c864c7 100644 --- a/crates/transaction-pool/src/blobstore/noop.rs +++ b/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, BlobStoreError> { Ok(None) diff --git a/crates/transaction-pool/src/pool/mod.rs b/crates/transaction-pool/src/pool/mod.rs index 017e0e594a..02dfb064ff 100644 --- a/crates/transaction-pool/src/pool/mod.rs +++ b/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) { - 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(); }