From 5b5d21223f6c66c377e3a7a2dbb5acc585aa26c5 Mon Sep 17 00:00:00 2001 From: Steven Czabaniuk Date: Tue, 3 Oct 2023 12:52:54 -0500 Subject: [PATCH 1/4] Remove dummy entries in Blockstore special columns These entries were found to improve compaction performance when ledger compaction was done by LedgerCleanupService on a primary index. Cleaning by primary index has been deprecated for a while, and these dummy entries are no longer needed. --- ledger/src/blockstore.rs | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index b4426aa3678501..e496f24a9b5b19 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -2127,14 +2127,27 @@ impl Blockstore { .put(0, &TransactionStatusIndexMeta::default())?; self.transaction_status_index_cf .put(1, &TransactionStatusIndexMeta::default())?; - // This dummy status improves compaction performance - let default_status = TransactionStatusMeta::default().into(); - self.transaction_status_cf - .put_protobuf(cf::TransactionStatus::as_index(2), &default_status)?; - self.address_signatures_cf.put( - cf::AddressSignatures::as_index(2), - &AddressSignatureMeta::default(), - ) + + // If present, delete dummy entries inserted by old software + // https://github.com/solana-labs/solana/blob/bc2b372/ledger/src/blockstore.rs#L2130-L2137 + let transaction_status_dummy_key = cf::TransactionStatus::as_index(2); + if let Some(_) = self + .transaction_status_cf + .get_protobuf_or_bincode::(transaction_status_dummy_key)? + { + self.transaction_status_cf + .delete(transaction_status_dummy_key)?; + }; + let address_signatures_dummy_key = cf::AddressSignatures::as_index(2); + if let Some(_) = self + .address_signatures_cf + .get(address_signatures_dummy_key)? + { + self.address_signatures_cf + .delete(address_signatures_dummy_key)?; + }; + + Ok(()) } /// Toggles the active primary index between `0` and `1`, and clears the From 1f231d71d7adb39f8be13f6877f4b870494a583e Mon Sep 17 00:00:00 2001 From: Steven Czabaniuk Date: Tue, 3 Oct 2023 12:56:13 -0500 Subject: [PATCH 2/4] Remove comment that will now be outdated --- ledger/src/blockstore.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index e496f24a9b5b19..60a3252d09d59f 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -7682,8 +7682,6 @@ pub mod tests { fn test_get_transaction_status() { let ledger_path = get_tmp_ledger_path_auto_delete!(); let blockstore = Blockstore::open(ledger_path.path()).unwrap(); - - // TransactionStatus column opens initialized with one entry at index 2 let transaction_status_cf = &blockstore.transaction_status_cf; let pre_balances_vec = vec![1, 2, 3]; @@ -7876,8 +7874,6 @@ pub mod tests { let ledger_path = get_tmp_ledger_path_auto_delete!(); let blockstore = Blockstore::open(ledger_path.path()).unwrap(); - - // TransactionStatus column opens initialized with one entry at index 2 let transaction_status_cf = &blockstore.transaction_status_cf; let pre_balances_vec = vec![1, 2, 3]; From 26f0b3639370bd22d1a99d950bd37994b13bb7f9 Mon Sep 17 00:00:00 2001 From: Steven Czabaniuk Date: Tue, 3 Oct 2023 13:25:08 -0500 Subject: [PATCH 3/4] clippy + fmt --- ledger/src/blockstore.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index 60a3252d09d59f..c5338b9a0a76d7 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -2131,17 +2131,19 @@ impl Blockstore { // If present, delete dummy entries inserted by old software // https://github.com/solana-labs/solana/blob/bc2b372/ledger/src/blockstore.rs#L2130-L2137 let transaction_status_dummy_key = cf::TransactionStatus::as_index(2); - if let Some(_) = self + if self .transaction_status_cf .get_protobuf_or_bincode::(transaction_status_dummy_key)? + .is_some() { self.transaction_status_cf .delete(transaction_status_dummy_key)?; }; let address_signatures_dummy_key = cf::AddressSignatures::as_index(2); - if let Some(_) = self + if self .address_signatures_cf .get(address_signatures_dummy_key)? + .is_some() { self.address_signatures_cf .delete(address_signatures_dummy_key)?; From e9c602615c499e50997e12236fd687c08e618898 Mon Sep 17 00:00:00 2001 From: Steven Czabaniuk Date: Tue, 3 Oct 2023 14:32:31 -0500 Subject: [PATCH 4/4] Update unit tests to no longer expect dummy entry --- ledger/src/blockstore.rs | 4 ++-- ledger/src/blockstore/blockstore_purge.rs | 13 +++---------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index c5338b9a0a76d7..93de196cc10a85 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -7862,13 +7862,13 @@ pub mod tests { .get_transaction_status_with_counter(signature7, &[].into()) .unwrap(); assert_eq!(status, None); - assert_eq!(counter, 2); + assert_eq!(counter, 1); let (status, counter) = blockstore .get_transaction_status_with_counter(signature7, &[3].into()) .unwrap(); assert_eq!(status, None); - assert_eq!(counter, 2); + assert_eq!(counter, 1); } fn do_test_lowest_cleanup_slot_and_special_cfs(simulate_ledger_cleanup_service: bool) { diff --git a/ledger/src/blockstore/blockstore_purge.rs b/ledger/src/blockstore/blockstore_purge.rs index 71d20720ff4dcc..473e0de1f5927e 100644 --- a/ledger/src/blockstore/blockstore_purge.rs +++ b/ledger/src/blockstore/blockstore_purge.rs @@ -649,9 +649,6 @@ pub mod tests { IteratorDirection::Forward, )) .unwrap(); - let padding_entry = status_entry_iterator.next().unwrap().0; - assert_eq!(padding_entry.0, 2); - assert_eq!(padding_entry.2, 0); assert!(status_entry_iterator.next().is_none()); let mut address_transactions_iterator = blockstore .db @@ -660,10 +657,8 @@ pub mod tests { IteratorDirection::Forward, )) .unwrap(); - let padding_entry = address_transactions_iterator.next().unwrap().0; - assert_eq!(padding_entry.0, 2); - assert_eq!(padding_entry.2, 0); assert!(address_transactions_iterator.next().is_none()); + assert_eq!( transaction_status_index_cf.get(0).unwrap().unwrap(), TransactionStatusIndexMeta { @@ -1095,8 +1090,6 @@ pub mod tests { frozen: false, } ); - let entry = status_entry_iterator.next().unwrap().0; - assert_eq!(entry.0, 2); // Buffer entry, no index 1 entries remaining drop(status_entry_iterator); // Purge up to but not including index0_max_slot @@ -1135,6 +1128,8 @@ pub mod tests { IteratorDirection::Forward, )) .unwrap(); + assert!(status_entry_iterator.next().is_none()); + assert_eq!( blockstore .transaction_status_index_cf @@ -1157,8 +1152,6 @@ pub mod tests { frozen: false, } ); - let entry = status_entry_iterator.next().unwrap().0; - assert_eq!(entry.0, 2); // Buffer entry, no index 0 or index 1 entries remaining } #[test]