Browse Source

Allow obsolete accounts structure to be passed in to reconstruct_single_storage and then recreating the account storage entry (#8098)

* Add obsolete accounts as a passed in parameter

* Update to pass around Vec, rather than slice

* No longer passing in obsolete accounts to accounts file

Added SerdesObsoleteAccounts Structure

* Updating in reponse to review feedback

* Updating formatting

* Switching to id

* Update runtime/src/serde_snapshot.rs

Co-authored-by: Brooks <brooks@prumo.org>

---------

Co-authored-by: Brooks <brooks@prumo.org>
Rory Harris 1 month ago
parent
commit
f3f9b083a0

+ 13 - 2
accounts-db/src/account_storage_reader.rs

@@ -137,6 +137,7 @@ mod tests {
         crate::{
         crate::{
             accounts_db::{get_temp_accounts_paths, AccountStorageEntry},
             accounts_db::{get_temp_accounts_paths, AccountStorageEntry},
             accounts_file::{AccountsFile, AccountsFileProvider, StorageAccess},
             accounts_file::{AccountsFile, AccountsFileProvider, StorageAccess},
+            ObsoleteAccounts,
         },
         },
         log::*,
         log::*,
         rand::{rngs::StdRng, seq::SliceRandom, SeedableRng},
         rand::{rngs::StdRng, seq::SliceRandom, SeedableRng},
@@ -329,7 +330,12 @@ mod tests {
             );
             );
 
 
             // Create a new AccountStorageEntry from the output file
             // Create a new AccountStorageEntry from the output file
-            let new_storage = AccountStorageEntry::new_existing(slot, 0, accounts_file);
+            let new_storage = AccountStorageEntry::new_existing(
+                slot,
+                0,
+                accounts_file,
+                ObsoleteAccounts::default(),
+            );
 
 
             // Verify that the new storage has the same length as the reader
             // Verify that the new storage has the same length as the reader
             assert_eq!(new_storage.accounts.len(), reader.len());
             assert_eq!(new_storage.accounts.len(), reader.len());
@@ -431,7 +437,12 @@ mod tests {
                     .unwrap();
                     .unwrap();
 
 
             // Create a new AccountStorageEntry from the output file
             // Create a new AccountStorageEntry from the output file
-            let new_storage = AccountStorageEntry::new_existing(slot, 0, accounts_file);
+            let new_storage = AccountStorageEntry::new_existing(
+                slot,
+                0,
+                accounts_file,
+                ObsoleteAccounts::default(),
+            );
 
 
             // Verify that the new storage has the same length as the reader
             // Verify that the new storage has the same length as the reader
             assert_eq!(new_storage.accounts.len(), reader.len());
             assert_eq!(new_storage.accounts.len(), reader.len());

+ 7 - 2
accounts-db/src/accounts_db.rs

@@ -958,7 +958,12 @@ impl AccountStorageEntry {
         })
         })
     }
     }
 
 
-    pub fn new_existing(slot: Slot, id: AccountsFileId, accounts: AccountsFile) -> Self {
+    pub fn new_existing(
+        slot: Slot,
+        id: AccountsFileId,
+        accounts: AccountsFile,
+        obsolete_accounts: ObsoleteAccounts,
+    ) -> Self {
         Self {
         Self {
             id,
             id,
             slot,
             slot,
@@ -966,7 +971,7 @@ impl AccountStorageEntry {
             count: AtomicUsize::new(0),
             count: AtomicUsize::new(0),
             alive_bytes: AtomicUsize::new(0),
             alive_bytes: AtomicUsize::new(0),
             zero_lamport_single_ref_offsets: RwLock::default(),
             zero_lamport_single_ref_offsets: RwLock::default(),
-            obsolete_accounts: RwLock::default(),
+            obsolete_accounts: RwLock::new(obsolete_accounts),
         }
         }
     }
     }
 
 

+ 4 - 1
accounts-db/src/lib.rs

@@ -45,7 +45,10 @@ pub mod tiered_storage;
 pub mod utils;
 pub mod utils;
 pub mod waitable_condvar;
 pub mod waitable_condvar;
 
 
-pub use {buffered_reader::large_file_buf_reader, file_io::validate_memlock_limit_for_disk_io};
+pub use {
+    buffered_reader::large_file_buf_reader, file_io::validate_memlock_limit_for_disk_io,
+    obsolete_accounts::ObsoleteAccounts,
+};
 
 
 #[macro_use]
 #[macro_use]
 extern crate solana_metrics;
 extern crate solana_metrics;

+ 2 - 0
runtime/src/bank/serde_snapshot.rs

@@ -22,6 +22,7 @@ mod tests {
                 ACCOUNTS_DB_CONFIG_FOR_TESTING,
                 ACCOUNTS_DB_CONFIG_FOR_TESTING,
             },
             },
             accounts_file::{AccountsFile, AccountsFileError, StorageAccess},
             accounts_file::{AccountsFile, AccountsFileError, StorageAccess},
+            ObsoleteAccounts,
         },
         },
         solana_epoch_schedule::EpochSchedule,
         solana_epoch_schedule::EpochSchedule,
         solana_genesis_config::create_genesis_config,
         solana_genesis_config::create_genesis_config,
@@ -68,6 +69,7 @@ mod tests {
                 storage_entry.slot(),
                 storage_entry.slot(),
                 storage_entry.id(),
                 storage_entry.id(),
                 accounts_file,
                 accounts_file,
+                ObsoleteAccounts::default(),
             );
             );
             next_append_vec_id = next_append_vec_id.max(new_storage_entry.id());
             next_append_vec_id = next_append_vec_id.max(new_storage_entry.id());
             storage.insert(new_storage_entry.slot(), Arc::new(new_storage_entry));
             storage.insert(new_storage_entry.slot(), Arc::new(new_storage_entry));

+ 25 - 2
runtime/src/serde_snapshot.rs

@@ -6,6 +6,7 @@ use {
         epoch_stakes::VersionedEpochStakes,
         epoch_stakes::VersionedEpochStakes,
         rent_collector::RentCollector,
         rent_collector::RentCollector,
         runtime_config::RuntimeConfig,
         runtime_config::RuntimeConfig,
+        serde_snapshot::storage::SerdeObsoleteAccounts,
         snapshot_utils::{SnapshotError, StorageAndNextAccountsFileId},
         snapshot_utils::{SnapshotError, StorageAndNextAccountsFileId},
         stake_account::StakeAccount,
         stake_account::StakeAccount,
         stakes::{serialize_stake_accounts_to_delegation_format, Stakes},
         stakes::{serialize_stake_accounts_to_delegation_format, Stakes},
@@ -24,6 +25,7 @@ use {
         accounts_update_notifier_interface::AccountsUpdateNotifier,
         accounts_update_notifier_interface::AccountsUpdateNotifier,
         ancestors::AncestorsForSerialization,
         ancestors::AncestorsForSerialization,
         blockhash_queue::BlockhashQueue,
         blockhash_queue::BlockhashQueue,
+        ObsoleteAccounts,
     },
     },
     solana_clock::{Epoch, Slot, UnixTimestamp},
     solana_clock::{Epoch, Slot, UnixTimestamp},
     solana_epoch_schedule::EpochSchedule,
     solana_epoch_schedule::EpochSchedule,
@@ -852,15 +854,35 @@ pub(crate) fn reconstruct_single_storage(
     slot: &Slot,
     slot: &Slot,
     append_vec_path: &Path,
     append_vec_path: &Path,
     current_len: usize,
     current_len: usize,
-    append_vec_id: AccountsFileId,
+    id: AccountsFileId,
     storage_access: StorageAccess,
     storage_access: StorageAccess,
+    obsolete_accounts: Option<SerdeObsoleteAccounts>,
 ) -> Result<Arc<AccountStorageEntry>, SnapshotError> {
 ) -> Result<Arc<AccountStorageEntry>, SnapshotError> {
+    // When restoring from an archive, obsolete accounts will always be `None`
+    // When restoring from fastboot, obsolete accounts will be 'Some' if the storage contained
+    // accounts marked obsolete at the time the snapshot was taken.
+    let (current_len, obsolete_accounts) = if let Some(obsolete_accounts) = obsolete_accounts {
+        let updated_len = current_len + obsolete_accounts.bytes as usize;
+        let id = id as SerializedAccountsFileId;
+        if obsolete_accounts.id != id {
+            return Err(SnapshotError::MismatchedAccountsFileId(
+                id,
+                obsolete_accounts.id,
+            ));
+        }
+
+        (updated_len, obsolete_accounts.accounts)
+    } else {
+        (current_len, ObsoleteAccounts::default())
+    };
+
     let accounts_file =
     let accounts_file =
         AccountsFile::new_for_startup(append_vec_path, current_len, storage_access)?;
         AccountsFile::new_for_startup(append_vec_path, current_len, storage_access)?;
     Ok(Arc::new(AccountStorageEntry::new_existing(
     Ok(Arc::new(AccountStorageEntry::new_existing(
         *slot,
         *slot,
-        append_vec_id,
+        id,
         accounts_file,
         accounts_file,
+        obsolete_accounts,
     )))
     )))
 }
 }
 
 
@@ -959,6 +981,7 @@ pub(crate) fn remap_and_reconstruct_single_storage(
         current_len,
         current_len,
         remapped_append_vec_id,
         remapped_append_vec_id,
         storage_access,
         storage_access,
+        None,
     )?;
     )?;
     Ok(storage)
     Ok(storage)
 }
 }

+ 15 - 1
runtime/src/serde_snapshot/storage.rs

@@ -1,6 +1,6 @@
 use {
 use {
     serde::{Deserialize, Serialize},
     serde::{Deserialize, Serialize},
-    solana_accounts_db::accounts_db::AccountStorageEntry,
+    solana_accounts_db::{accounts_db::AccountStorageEntry, ObsoleteAccounts},
     solana_clock::Slot,
     solana_clock::Slot,
 };
 };
 
 
@@ -49,3 +49,17 @@ impl SerializableStorage for SerializableAccountStorageEntry {
 
 
 #[cfg(feature = "frozen-abi")]
 #[cfg(feature = "frozen-abi")]
 impl solana_frozen_abi::abi_example::TransparentAsHelper for SerializableAccountStorageEntry {}
 impl solana_frozen_abi::abi_example::TransparentAsHelper for SerializableAccountStorageEntry {}
+
+/// This structure handles the load/store of obsolete accounts during snapshot restoration.
+#[derive(Debug, Default)]
+pub(crate) struct SerdeObsoleteAccounts {
+    /// The ID of the associated account file. Used for verification to ensure the restored
+    /// obsolete accounts correspond to the correct account file
+    pub id: SerializedAccountsFileId,
+    /// The number of obsolete bytes in the storage. These bytes are removed during archive
+    /// serialization/deserialization but are present when restoring from directories. This value
+    /// is used to validate the size when creating the accounts file.
+    pub bytes: u64,
+    /// A list of accounts that are obsolete in the storage being restored.
+    pub accounts: ObsoleteAccounts,
+}

+ 2 - 0
runtime/src/serde_snapshot/tests.rs

@@ -23,6 +23,7 @@ mod serde_snapshot_tests {
             },
             },
             accounts_file::{AccountsFile, AccountsFileError, StorageAccess},
             accounts_file::{AccountsFile, AccountsFileError, StorageAccess},
             ancestors::Ancestors,
             ancestors::Ancestors,
+            ObsoleteAccounts,
         },
         },
         solana_clock::Slot,
         solana_clock::Slot,
         solana_epoch_schedule::EpochSchedule,
         solana_epoch_schedule::EpochSchedule,
@@ -144,6 +145,7 @@ mod serde_snapshot_tests {
                 storage_entry.slot(),
                 storage_entry.slot(),
                 storage_entry.id(),
                 storage_entry.id(),
                 accounts_file,
                 accounts_file,
+                ObsoleteAccounts::default(),
             );
             );
             next_append_vec_id = next_append_vec_id.max(new_storage_entry.id());
             next_append_vec_id = next_append_vec_id.max(new_storage_entry.id());
             storage.insert(new_storage_entry.slot(), Arc::new(new_storage_entry));
             storage.insert(new_storage_entry.slot(), Arc::new(new_storage_entry));

+ 8 - 1
runtime/src/snapshot_utils.rs

@@ -5,7 +5,8 @@ use {
         bank::{BankFieldsToDeserialize, BankFieldsToSerialize, BankHashStats, BankSlotDelta},
         bank::{BankFieldsToDeserialize, BankFieldsToSerialize, BankHashStats, BankSlotDelta},
         serde_snapshot::{
         serde_snapshot::{
             self, AccountsDbFields, ExtraFieldsToSerialize, SerializableAccountStorageEntry,
             self, AccountsDbFields, ExtraFieldsToSerialize, SerializableAccountStorageEntry,
-            SnapshotAccountsDbFields, SnapshotBankFields, SnapshotStreams,
+            SerializedAccountsFileId, SnapshotAccountsDbFields, SnapshotBankFields,
+            SnapshotStreams,
         },
         },
         snapshot_archive_info::{
         snapshot_archive_info::{
             FullSnapshotArchiveInfo, IncrementalSnapshotArchiveInfo, SnapshotArchiveInfo,
             FullSnapshotArchiveInfo, IncrementalSnapshotArchiveInfo, SnapshotArchiveInfo,
@@ -337,6 +338,12 @@ pub enum SnapshotError {
     #[error("snapshot hash mismatch: deserialized bank: {0:?}, snapshot archive: {1:?}")]
     #[error("snapshot hash mismatch: deserialized bank: {0:?}, snapshot archive: {1:?}")]
     MismatchedHash(SnapshotHash, SnapshotHash),
     MismatchedHash(SnapshotHash, SnapshotHash),
 
 
+    #[error(
+        "snapshot accounts file id mismatch: deserialized obsolete accounts file id: {0}, \
+         snapshot archive: {1}"
+    )]
+    MismatchedAccountsFileId(SerializedAccountsFileId, SerializedAccountsFileId),
+
     #[error("snapshot slot deltas are invalid: {0}")]
     #[error("snapshot slot deltas are invalid: {0}")]
     VerifySlotDeltas(#[from] VerifySlotDeltasError),
     VerifySlotDeltas(#[from] VerifySlotDeltasError),
 
 

+ 1 - 0
runtime/src/snapshot_utils/snapshot_storage_rebuilder.rs

@@ -252,6 +252,7 @@ impl SnapshotStorageRebuilder {
                         current_len,
                         current_len,
                         old_append_vec_id as AccountsFileId,
                         old_append_vec_id as AccountsFileId,
                         self.storage_access,
                         self.storage_access,
+                        None,
                     )?,
                     )?,
                 };
                 };