Parcourir la source

Propagate errors from accounts scan buffered reader (#6822)

* Make it compile

* Unify messages.

* Update test code.

* more test updates

* Update accounts-db/src/accounts_db/scan_account_storage.rs

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

* Update accounts-db/src/accounts_file.rs

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

* Update accounts-db/src/accounts_db/geyser_plugin_utils.rs

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

* Fixes

* Clippy

* Only print error instead of panicking.

* Use eprintln.

---------

Co-authored-by: Brooks <brooks@prumo.org>
Kamil Skalski il y a 4 mois
Parent
commit
1985cf4c88

+ 2 - 2
accounts-db/benches/bench_accounts_file.rs

@@ -165,14 +165,14 @@ fn bench_scan_pubkeys(c: &mut Criterion) {
         group.bench_function(BenchmarkId::new("append_vec_mmap", accounts_count), |b| {
             b.iter(|| {
                 let mut count = 0;
-                append_vec_mmap.scan_pubkeys(|_| count += 1);
+                append_vec_mmap.scan_pubkeys(|_| count += 1).unwrap();
                 assert_eq!(count, accounts_count);
             });
         });
         group.bench_function(BenchmarkId::new("append_vec_file", accounts_count), |b| {
             b.iter(|| {
                 let mut count = 0;
-                append_vec_file.scan_pubkeys(|_| count += 1);
+                append_vec_file.scan_pubkeys(|_| count += 1).unwrap();
                 assert_eq!(count, accounts_count);
             });
         });

+ 62 - 38
accounts-db/src/accounts_db.rs

@@ -2425,7 +2425,8 @@ impl AccountsDb {
                                 let pubkey = *account.pubkey();
                                 let is_zero_lamport = account.is_zero_lamport();
                                 insert_candidate(pubkey, is_zero_lamport);
-                            });
+                            })
+                            .expect("must scan accounts storage");
                     });
                     oldest_dirty_slot
                 })
@@ -2509,19 +2510,22 @@ impl AccountsDb {
                 return;
             }
             if let Some(storage) = self.storage.get_slot_storage_entry(slot) {
-                storage.accounts.scan_accounts(|_offset, account| {
-                    let pk = account.pubkey();
-                    match pubkey_refcount.entry(*pk) {
-                        dashmap::mapref::entry::Entry::Occupied(mut occupied_entry) => {
-                            if !occupied_entry.get().iter().any(|s| s == &slot) {
-                                occupied_entry.get_mut().push(slot);
+                storage
+                    .accounts
+                    .scan_accounts(|_offset, account| {
+                        let pk = account.pubkey();
+                        match pubkey_refcount.entry(*pk) {
+                            dashmap::mapref::entry::Entry::Occupied(mut occupied_entry) => {
+                                if !occupied_entry.get().iter().any(|s| s == &slot) {
+                                    occupied_entry.get_mut().push(slot);
+                                }
+                            }
+                            dashmap::mapref::entry::Entry::Vacant(vacant_entry) => {
+                                vacant_entry.insert(vec![slot]);
                             }
                         }
-                        dashmap::mapref::entry::Entry::Vacant(vacant_entry) => {
-                            vacant_entry.insert(vec![slot]);
-                        }
-                    }
-                });
+                    })
+                    .expect("must scan accounts storage");
             }
         });
         let total = pubkey_refcount.len();
@@ -3369,7 +3373,8 @@ impl AccountsDb {
                     pubkey: *account.pubkey(),
                     data_len: account.data_len as u64,
                 });
-            });
+            })
+            .expect("must scan accounts storage");
 
         // sort by pubkey to keep account index lookups close
         let num_duplicated_accounts = Self::sort_and_remove_dups(&mut stored_accounts);
@@ -4427,15 +4432,16 @@ impl AccountsDb {
                 ScanAccountStorageData::NoData => {
                     storage.scan_accounts_without_data(|_offset, account_without_data| {
                         storage_scan_func(retval, &account_without_data, None);
-                    });
+                    })
                 }
                 ScanAccountStorageData::DataRefForStorage => {
                     storage.scan_accounts(|_offset, account| {
                         let account_without_data = StoredAccountInfoWithoutData::new_from(&account);
                         storage_scan_func(retval, &account_without_data, Some(account.data));
-                    });
+                    })
                 }
-            };
+            }
+            .expect("must scan accounts storage");
         })
     }
 
@@ -5337,9 +5343,12 @@ impl AccountsDb {
             .storage
             .get_slot_storage_entry_shrinking_in_progress_ok(remove_slot)
         {
-            storage.accounts.scan_pubkeys(|pk| {
-                stored_keys.insert((*pk, remove_slot));
-            });
+            storage
+                .accounts
+                .scan_pubkeys(|pk| {
+                    stored_keys.insert((*pk, remove_slot));
+                })
+                .expect("must scan accounts storage");
         }
         scan_storages_elapsed.stop();
         purge_stats
@@ -6250,10 +6259,13 @@ impl AccountsDb {
         let mut lt_hash = storages
             .par_iter()
             .fold(LtHash::identity, |mut accum, storage| {
-                storage.accounts.scan_accounts(|_offset, account| {
-                    let account_lt_hash = Self::lt_hash_account(&account, account.pubkey());
-                    accum.mix_in(&account_lt_hash.0);
-                });
+                storage
+                    .accounts
+                    .scan_accounts(|_offset, account| {
+                        let account_lt_hash = Self::lt_hash_account(&account, account.pubkey());
+                        accum.mix_in(&account_lt_hash.0);
+                    })
+                    .expect("must scan accounts storage");
                 accum
             })
             .reduce(LtHash::identity, |mut accum, elem| {
@@ -6879,9 +6891,11 @@ impl AccountsDb {
             slot,
             |loaded_account| Some(*loaded_account.pubkey()),
             |accum: &mut HashSet<Pubkey>, storage| {
-                storage.scan_pubkeys(|pubkey| {
-                    accum.insert(*pubkey);
-                });
+                storage
+                    .scan_pubkeys(|pubkey| {
+                        accum.insert(*pubkey);
+                    })
+                    .expect("must scan accounts storage");
             },
         );
         match scan_result {
@@ -7424,9 +7438,12 @@ impl AccountsDb {
                     .map(|store| {
                         let slot = store.slot();
                         let mut pubkeys = Vec::with_capacity(store.count());
-                        store.accounts.scan_pubkeys(|pubkey| {
-                            pubkeys.push((slot, *pubkey));
-                        });
+                        store
+                            .accounts
+                            .scan_pubkeys(|pubkey| {
+                                pubkeys.push((slot, *pubkey));
+                            })
+                            .expect("must scan accounts storage");
                         pubkeys
                     })
                     .flatten()
@@ -7877,7 +7894,7 @@ impl AccountsDb {
                         &account,
                         &self.account_indexes,
                     );
-                });
+                })
             } else {
                 // withOUT secondary indexes -- scan accounts withOUT account data
                 storage
@@ -7896,8 +7913,9 @@ impl AccountsDb {
                             },
                         };
                         itemizer(info);
-                    });
+                    })
             }
+            .expect("must scan accounts storage");
             let items = items_local.into_iter().map(|info| {
                 (
                     info.pubkey,
@@ -8082,7 +8100,8 @@ impl AccountsDb {
                                         }
                                     }
                                     assert_eq!(1, count);
-                                });
+                                })
+                                .expect("must scan accounts storage");
                             lookup_time.stop();
                             lookup_time.as_us()
                         };
@@ -8616,9 +8635,11 @@ pub(crate) enum UpdateIndexThreadSelection {
 impl AccountStorageEntry {
     fn accounts_count(&self) -> usize {
         let mut count = 0;
-        self.accounts.scan_pubkeys(|_| {
-            count += 1;
-        });
+        self.accounts
+            .scan_pubkeys(|_| {
+                count += 1;
+            })
+            .expect("must scan accounts storage");
         count
     }
 }
@@ -8755,9 +8776,12 @@ impl AccountsDb {
     pub fn sizes_of_accounts_in_storage_for_tests(&self, slot: Slot) -> Vec<usize> {
         let mut sizes = Vec::default();
         if let Some(storage) = self.storage.get_slot_storage_entry(slot) {
-            storage.accounts.scan_accounts_stored_meta(|account| {
-                sizes.push(account.stored_size());
-            });
+            storage
+                .accounts
+                .scan_accounts_stored_meta(|account| {
+                    sizes.push(account.stored_size());
+                })
+                .expect("must scan accounts storage");
         }
         sizes
     }

+ 15 - 12
accounts-db/src/accounts_db/geyser_plugin_utils.rs

@@ -103,18 +103,21 @@ impl AccountsDb {
         let mut pure_notify_time = Duration::ZERO;
         let mut i = 0;
         let notifying_start = Instant::now();
-        storage.accounts.scan_accounts_for_geyser(|account| {
-            i += 1;
-            // later entries in the same slot are more recent and override earlier accounts for the same pubkey
-            // We can pass an incrementing number here for write_version in the future, if the storage does not have a write_version.
-            // As long as all accounts for this slot are in 1 append vec that can be iterated oldest to newest.
-            let (_, notify_dur) = meas_dur!(notifier.notify_account_restore_from_snapshot(
-                storage.slot(),
-                i as u64,
-                &account
-            ));
-            pure_notify_time += notify_dur;
-        });
+        storage
+            .accounts
+            .scan_accounts_for_geyser(|account| {
+                i += 1;
+                // later entries in the same slot are more recent and override earlier accounts for the same pubkey
+                // We can pass an incrementing number here for write_version in the future, if the storage does not have a write_version.
+                // As long as all accounts for this slot are in 1 append vec that can be iterated oldest to newest.
+                let (_, notify_dur) = meas_dur!(notifier.notify_account_restore_from_snapshot(
+                    storage.slot(),
+                    i as u64,
+                    &account
+                ));
+                pure_notify_time += notify_dur;
+            })
+            .expect("must scan accounts storage");
         let notifying_time = notifying_start.elapsed();
 
         GeyserPluginNotifyAtSnapshotRestoreStats {

+ 23 - 12
accounts-db/src/accounts_db/scan_account_storage.rs

@@ -1,6 +1,7 @@
 use {
     super::{AccountStorageEntry, AccountsDb, BinnedHashData, LoadedAccount, SplitAncientStorages},
     crate::{
+        accounts_file::AccountsFileError,
         accounts_hash::{
             AccountHash, CalcAccountsHashConfig, CalculateHashIntermediate, HashStats,
         },
@@ -340,7 +341,8 @@ impl AccountsDb {
                                 }
                                 scanner.set_slot(slot, ancient, storage);
 
-                                Self::scan_single_account_storage(storage, &mut scanner);
+                                Self::scan_single_account_storage(storage, &mut scanner)
+                                    .expect("must scan accounts storage");
                             });
                             if ancient {
                                 stats
@@ -373,7 +375,10 @@ impl AccountsDb {
     }
 
     /// iterate over a single storage, calling scanner on each item
-    fn scan_single_account_storage<S>(storage: &AccountStorageEntry, scanner: &mut S)
+    fn scan_single_account_storage<S>(
+        storage: &AccountStorageEntry,
+        scanner: &mut S,
+    ) -> Result<(), AccountsFileError>
     where
         S: AppendVecScan,
     {
@@ -381,7 +386,7 @@ impl AccountsDb {
             if scanner.filter(account.pubkey()) {
                 scanner.found_account(&LoadedAccount::Stored(account))
             }
-        });
+        })
     }
 }
 
@@ -629,7 +634,7 @@ mod tests {
             accum: Vec::default(),
             calls: calls.clone(),
         };
-        AccountsDb::scan_single_account_storage(&storage, &mut scanner);
+        AccountsDb::scan_single_account_storage(&storage, &mut scanner).unwrap();
         let accum = scanner.scanning_complete();
         assert_eq!(calls.load(Ordering::Relaxed), 1);
         assert_eq!(
@@ -675,7 +680,7 @@ mod tests {
                 value_to_use_for_lamports: expected,
             };
 
-            AccountsDb::scan_single_account_storage(&storage, &mut test_scan);
+            AccountsDb::scan_single_account_storage(&storage, &mut test_scan).unwrap();
             let accum = test_scan.scanning_complete();
             assert_eq!(calls.load(Ordering::Relaxed), 1);
             assert_eq!(
@@ -704,9 +709,12 @@ mod tests {
                     let slot = storage.slot();
                     let copied_storage = accounts_db.create_and_insert_store(slot, 10000, "test");
                     let mut all_accounts = Vec::default();
-                    storage.accounts.scan_accounts(|_offset, acct| {
-                        all_accounts.push((*acct.pubkey(), acct.to_account_shared_data()));
-                    });
+                    storage
+                        .accounts
+                        .scan_accounts(|_offset, acct| {
+                            all_accounts.push((*acct.pubkey(), acct.to_account_shared_data()));
+                        })
+                        .expect("must scan accounts storage");
                     let accounts = all_accounts
                         .iter()
                         .map(|stored| (&stored.0, &stored.1))
@@ -738,9 +746,12 @@ mod tests {
                 let slot = storage.slot() + max_slot;
                 let copied_storage = accounts_db.create_and_insert_store(slot, 10000, "test");
                 let mut all_accounts = Vec::default();
-                storage.accounts.scan_accounts(|_offset, acct| {
-                    all_accounts.push((*acct.pubkey(), acct.to_account_shared_data()));
-                });
+                storage
+                    .accounts
+                    .scan_accounts(|_offset, acct| {
+                        all_accounts.push((*acct.pubkey(), acct.to_account_shared_data()));
+                    })
+                    .expect("must scan accounts storage");
                 let accounts = all_accounts
                     .iter()
                     .map(|stored| (&stored.0, &stored.1))
@@ -1135,7 +1146,7 @@ mod tests {
             };
             scanner.set_slot(max_slot as Slot, false, &storage);
 
-            AccountsDb::scan_single_account_storage(&storage, &mut scanner);
+            AccountsDb::scan_single_account_storage(&storage, &mut scanner).unwrap();
             scanner.scanning_complete();
             assert_eq!(calls.load(Ordering::Relaxed), expected_count as u64);
         }

+ 39 - 26
accounts-db/src/accounts_db/tests.rs

@@ -2677,9 +2677,12 @@ fn test_shrink_candidate_slots_with_dead_ancient_account() {
     // used. More generally the pubkey of the smallest account
     // shouldn't be present in the shrunk storage, which is
     // validated by the following scan of the storage accounts.
-    storage.accounts.scan_pubkeys(|pubkey| {
-        assert_ne!(pubkey, &modified_account_pubkey);
-    });
+    storage
+        .accounts
+        .scan_pubkeys(|pubkey| {
+            assert_ne!(pubkey, &modified_account_pubkey);
+        })
+        .expect("must scan accounts storage");
 }
 
 #[test]
@@ -3899,7 +3902,8 @@ define_accounts_db_test!(test_alive_bytes, |accounts_db| {
                     num_obsolete_accounts
                 );
             }
-        });
+        })
+        .expect("must scan accounts storage");
 });
 
 // Test alive_bytes_exclude_zero_lamport_single_ref_accounts calculation
@@ -6679,15 +6683,21 @@ fn get_all_accounts_from_storages<'a>(
     storages
         .flat_map(|storage| {
             let mut vec = Vec::default();
-            storage.accounts.scan_accounts(|_offset, account| {
-                vec.push((*account.pubkey(), account.to_account_shared_data()));
-            });
+            storage
+                .accounts
+                .scan_accounts(|_offset, account| {
+                    vec.push((*account.pubkey(), account.to_account_shared_data()));
+                })
+                .expect("must scan accounts storage");
             // make sure scan_pubkeys results match
             // Note that we assume traversals are both in the same order, but this doesn't have to be true.
             let mut compare = Vec::default();
-            storage.accounts.scan_pubkeys(|k| {
-                compare.push(*k);
-            });
+            storage
+                .accounts
+                .scan_pubkeys(|k| {
+                    compare.push(*k);
+                })
+                .expect("must scan accounts storage");
             assert_eq!(compare, vec.iter().map(|(k, _)| *k).collect::<Vec<_>>());
             vec
         })
@@ -6907,22 +6917,25 @@ pub fn get_account_from_account_from_storage(
 fn populate_index(db: &AccountsDb, slots: Range<Slot>) {
     slots.into_iter().for_each(|slot| {
         if let Some(storage) = db.get_storage_for_slot(slot) {
-            storage.accounts.scan_accounts_stored_meta(|account| {
-                let info = AccountInfo::new(
-                    StorageLocation::AppendVec(storage.id(), account.offset()),
-                    account.is_zero_lamport(),
-                );
-                db.accounts_index.upsert(
-                    slot,
-                    slot,
-                    account.pubkey(),
-                    &account,
-                    &AccountSecondaryIndexes::default(),
-                    info,
-                    &mut Vec::default(),
-                    UpsertReclaim::IgnoreReclaims,
-                );
-            })
+            storage
+                .accounts
+                .scan_accounts_stored_meta(|account| {
+                    let info = AccountInfo::new(
+                        StorageLocation::AppendVec(storage.id(), account.offset()),
+                        account.is_zero_lamport(),
+                    );
+                    db.accounts_index.upsert(
+                        slot,
+                        slot,
+                        account.pubkey(),
+                        &account,
+                        &AccountSecondaryIndexes::default(),
+                        info,
+                        &mut Vec::default(),
+                        UpsertReclaim::IgnoreReclaims,
+                    );
+                })
+                .expect("must scan accounts storage");
         }
     })
 }

+ 11 - 8
accounts-db/src/accounts_file.rs

@@ -301,13 +301,14 @@ impl AccountsFile {
     pub fn scan_accounts_without_data(
         &self,
         callback: impl for<'local> FnMut(Offset, StoredAccountInfoWithoutData<'local>),
-    ) {
+    ) -> Result<()> {
         match self {
             Self::AppendVec(av) => av.scan_accounts_without_data(callback),
             Self::TieredStorage(ts) => {
                 if let Some(reader) = ts.reader() {
-                    _ = reader.scan_accounts_without_data(callback);
+                    reader.scan_accounts_without_data(callback)?;
                 }
+                Ok(())
             }
         }
     }
@@ -323,13 +324,14 @@ impl AccountsFile {
     pub fn scan_accounts(
         &self,
         callback: impl for<'local> FnMut(Offset, StoredAccountInfo<'local>),
-    ) {
+    ) -> Result<()> {
         match self {
             Self::AppendVec(av) => av.scan_accounts(callback),
             Self::TieredStorage(ts) => {
                 if let Some(reader) = ts.reader() {
-                    _ = reader.scan_accounts(callback);
+                    reader.scan_accounts(callback)?;
                 }
+                Ok(())
             }
         }
     }
@@ -342,7 +344,7 @@ impl AccountsFile {
     pub fn scan_accounts_stored_meta(
         &self,
         callback: impl for<'local> FnMut(StoredAccountMeta<'local>),
-    ) {
+    ) -> Result<()> {
         match self {
             Self::AppendVec(av) => av.scan_accounts_stored_meta(callback),
             Self::TieredStorage(_) => {
@@ -356,7 +358,7 @@ impl AccountsFile {
     pub fn scan_accounts_for_geyser(
         &self,
         mut callback: impl for<'local> FnMut(AccountForGeyser<'local>),
-    ) {
+    ) -> Result<()> {
         self.scan_accounts(|_offset, account| {
             let account_for_geyser = AccountForGeyser {
                 pubkey: account.pubkey(),
@@ -394,13 +396,14 @@ impl AccountsFile {
     }
 
     /// iterate over all pubkeys
-    pub fn scan_pubkeys(&self, callback: impl FnMut(&Pubkey)) {
+    pub fn scan_pubkeys(&self, callback: impl FnMut(&Pubkey)) -> Result<()> {
         match self {
             Self::AppendVec(av) => av.scan_pubkeys(callback),
             Self::TieredStorage(ts) => {
                 if let Some(reader) = ts.reader() {
-                    _ = reader.scan_pubkeys(callback);
+                    reader.scan_pubkeys(callback)?;
                 }
+                Ok(())
             }
         }
     }

+ 22 - 11
accounts-db/src/ancient_append_vecs.rs

@@ -2088,7 +2088,8 @@ pub mod tests {
                 .accounts
                 .scan_accounts(|_, _| {
                     count += 1;
-                });
+                })
+                .expect("must scan accounts storage");
             assert_eq!(count, 1);
             let account = shrink_in_progress
                 .new_storage()
@@ -2247,9 +2248,12 @@ pub mod tests {
                 })
                 .unwrap();
             let mut count = 0;
-            storage.accounts.scan_accounts(|_, _| {
-                count += 1;
-            });
+            storage
+                .accounts
+                .scan_accounts(|_, _| {
+                    count += 1;
+                })
+                .expect("must scan accounts storage");
             assert_eq!(count, 2);
             assert_eq!(accounts_shrunk_same_slot.0, *pk_with_2_refs);
             assert_eq!(accounts_shrunk_same_slot.1, account_shared_data_with_2_refs);
@@ -3108,9 +3112,12 @@ pub mod tests {
                             .iter()
                             .map(|storage| {
                                 let mut accounts = Vec::default();
-                                storage.accounts.scan_accounts_stored_meta(|account| {
-                                    accounts.push(AccountFromStorage::new(&account));
-                                });
+                                storage
+                                    .accounts
+                                    .scan_accounts_stored_meta(|account| {
+                                        accounts.push(AccountFromStorage::new(&account));
+                                    })
+                                    .expect("must scan accounts storage");
                                 (storage.slot(), accounts)
                             })
                             .collect::<Vec<_>>();
@@ -3182,11 +3189,15 @@ pub mod tests {
                             );
                             // make sure the single new append vec contains all the same accounts
                             let mut two = Vec::default();
-                            one.first().unwrap().1.new_storage().accounts.scan_accounts(
-                                |_offset, meta| {
+                            one.first()
+                                .unwrap()
+                                .1
+                                .new_storage()
+                                .accounts
+                                .scan_accounts(|_offset, meta| {
                                     two.push((*meta.pubkey(), meta.to_account_shared_data()));
-                                },
-                            );
+                                })
+                                .expect("must scan accounts storage");
 
                             compare_all_accounts(&initial_accounts, &two[..]);
                         }

+ 56 - 52
accounts-db/src/append_vec.rs

@@ -431,13 +431,7 @@ impl AppendVec {
         let path = path.into();
         let new = Self::new_from_file_unchecked(path, current_len, storage_access)?;
 
-        let (sanitized, num_accounts) = new.sanitize_layout_and_length();
-        if !sanitized {
-            return Err(AccountsFileError::AppendVecError(
-                AppendVecError::IncorrectLayout(new.path.clone()),
-            ));
-        }
-
+        let num_accounts = new.sanitize_layout_and_length()?;
         Ok((new, num_accounts))
     }
 
@@ -476,14 +470,8 @@ impl AppendVec {
                 current_len,
                 new.path.display(),
             );
-            let (sanitized, _num_accounts) = new.sanitize_layout_and_length();
-            if sanitized {
-                Ok(new)
-            } else {
-                Err(AccountsFileError::AppendVecError(
-                    AppendVecError::IncorrectLayout(new.path.clone()),
-                ))
-            }
+            let _num_accounts = new.sanitize_layout_and_length()?;
+            Ok(new)
         }
     }
 
@@ -558,7 +546,8 @@ impl AppendVec {
         Self::new_from_file_unchecked(path, file_size as usize, StorageAccess::default())
     }
 
-    fn sanitize_layout_and_length(&self) -> (bool, usize) {
+    /// Checks that all accounts layout is correct and returns the number of accounts.
+    fn sanitize_layout_and_length(&self) -> Result<usize> {
         // This discards allocated accounts immediately after check at each loop iteration.
         //
         // This code should not reuse AppendVec.accounts() method as the current form or
@@ -574,13 +563,16 @@ impl AppendVec {
             }
             last_offset = account.offset() + account.stored_size();
             num_accounts += 1;
-        });
-        if !matches {
-            return (false, num_accounts);
-        }
+        })?;
         let aligned_current_len = u64_align!(self.current_len.load(Ordering::Acquire));
 
-        (last_offset == aligned_current_len, num_accounts)
+        if !matches || last_offset != aligned_current_len {
+            return Err(AccountsFileError::AppendVecError(
+                AppendVecError::IncorrectLayout(self.path.clone()),
+            ));
+        }
+
+        Ok(num_accounts)
     }
 
     /// Get a reference to the data at `offset` of `size` bytes if that slice
@@ -996,7 +988,7 @@ impl AppendVec {
     pub fn scan_accounts_without_data(
         &self,
         mut callback: impl for<'local> FnMut(Offset, StoredAccountInfoWithoutData<'local>),
-    ) {
+    ) -> Result<()> {
         self.scan_stored_accounts_no_data(|stored_account| {
             let offset = stored_account.offset();
             let account = StoredAccountInfoWithoutData {
@@ -1022,7 +1014,7 @@ impl AppendVec {
     pub fn scan_accounts(
         &self,
         mut callback: impl for<'local> FnMut(Offset, StoredAccountInfo<'local>),
-    ) {
+    ) -> Result<()> {
         self.scan_accounts_stored_meta(|stored_account_meta| {
             let offset = stored_account_meta.offset();
             let account = StoredAccountInfo {
@@ -1045,7 +1037,7 @@ impl AppendVec {
     pub fn scan_accounts_stored_meta(
         &self,
         mut callback: impl for<'local> FnMut(StoredAccountMeta<'local>),
-    ) {
+    ) -> Result<()> {
         match &self.backing {
             AppendVecFileBacking::Mmap(_mmap) => {
                 let mut offset = 0;
@@ -1074,7 +1066,7 @@ impl AppendVec {
                 // Buffer for account data that doesn't fit within the stack allocated buffer.
                 // This will be re-used for each account that doesn't fit within the stack allocated buffer.
                 let mut data_overflow_buffer = vec![];
-                while let Ok(BufferedReaderStatus::Success) = reader.read() {
+                while let BufferedReaderStatus::Success = reader.read()? {
                     let (offset, bytes_subset) = reader.get_offset_and_data();
                     let (meta, next) = Self::get_type::<StoredMeta>(bytes_subset, 0).unwrap();
                     let (account_meta, next) =
@@ -1147,6 +1139,7 @@ impl AppendVec {
                 }
             }
         }
+        Ok(())
     }
 
     /// Calculate the amount of storage required for an account with the passed
@@ -1212,14 +1205,17 @@ impl AppendVec {
     /// `data` is completely ignored, for example.
     /// Also, no references have to be maintained/returned from an iterator function.
     /// This fn can operate on a batch of data at once.
-    pub fn scan_pubkeys(&self, mut callback: impl FnMut(&Pubkey)) {
+    pub fn scan_pubkeys(&self, mut callback: impl FnMut(&Pubkey)) -> Result<()> {
         self.scan_stored_accounts_no_data(|account| {
             callback(account.pubkey());
-        });
+        })
     }
 
     /// Iterate over all accounts and call `callback` with the fixed sized portion of each account.
-    fn scan_stored_accounts_no_data(&self, mut callback: impl FnMut(StoredAccountNoData)) {
+    fn scan_stored_accounts_no_data(
+        &self,
+        mut callback: impl FnMut(StoredAccountNoData),
+    ) -> Result<()> {
         let self_len = self.len();
         match &self.backing {
             AppendVecFileBacking::Mmap(mmap) => {
@@ -1262,7 +1258,7 @@ impl AppendVec {
                     file,
                     mem::size_of::<StoredMeta>() + mem::size_of::<AccountMeta>(),
                 );
-                while let Ok(BufferedReaderStatus::Success) = reader.read() {
+                while let BufferedReaderStatus::Success = reader.read()? {
                     let (offset, bytes) = reader.get_offset_and_data();
                     let (stored_meta, next) = Self::get_type::<StoredMeta>(bytes, 0).unwrap();
                     let (account_meta, _) = Self::get_type::<AccountMeta>(bytes, next).unwrap();
@@ -1288,6 +1284,7 @@ impl AppendVec {
                 }
             }
         }
+        Ok(())
     }
 
     /// Copy each account metadata, account and hash to the internal buffer.
@@ -1663,7 +1660,8 @@ pub mod tests {
             let mut count = 0;
             self.scan_stored_accounts_no_data(|_| {
                 count += 1;
-            });
+            })
+            .expect("must scan accounts storage");
             count
         }
     }
@@ -1739,7 +1737,8 @@ pub mod tests {
                 assert_eq!(&recovered, account);
                 assert_eq!(v.pubkey(), pubkey);
                 index += 1;
-            });
+            })
+            .expect("must scan accounts storage");
         }
     }
 
@@ -1786,7 +1785,8 @@ pub mod tests {
             let recovered = v.to_account_shared_data();
             assert_eq!(recovered, account.1);
             sample += 1;
-        });
+        })
+        .expect("must scan accounts storage");
         trace!("sequential read time: {} ms", now.elapsed().as_millis());
     }
 
@@ -2196,10 +2196,12 @@ pub mod tests {
             modify_fn,
             |append_vec, pubkeys, _account_offsets, _accounts| {
                 let mut i = 0;
-                append_vec.scan_pubkeys(|pubkey| {
-                    assert_eq!(pubkey, pubkeys.get(i).unwrap());
-                    i += 1;
-                });
+                append_vec
+                    .scan_pubkeys(|pubkey| {
+                        assert_eq!(pubkey, pubkeys.get(i).unwrap());
+                        i += 1;
+                    })
+                    .expect("must scan accounts storage");
                 assert_eq!(i, pubkeys.len());
             },
         )
@@ -2282,22 +2284,24 @@ pub mod tests {
             modify_fn,
             |append_vec, pubkeys, account_offsets, accounts| {
                 let mut i = 0;
-                append_vec.scan_stored_accounts_no_data(|stored_account| {
-                    let pubkey = pubkeys.get(i).unwrap();
-                    let account = accounts.get(i).unwrap();
-                    let offset = account_offsets.get(i).unwrap();
-
-                    assert_eq!(
-                        stored_account.stored_size,
-                        aligned_stored_size(account.data().len()),
-                    );
-                    assert_eq!(stored_account.offset(), *offset);
-                    assert_eq!(stored_account.pubkey(), pubkey);
-                    assert_eq!(stored_account.lamports(), account.lamports());
-                    assert_eq!(stored_account.data_len(), account.data().len() as u64);
-
-                    i += 1;
-                });
+                append_vec
+                    .scan_stored_accounts_no_data(|stored_account| {
+                        let pubkey = pubkeys.get(i).unwrap();
+                        let account = accounts.get(i).unwrap();
+                        let offset = account_offsets.get(i).unwrap();
+
+                        assert_eq!(
+                            stored_account.stored_size,
+                            aligned_stored_size(account.data().len()),
+                        );
+                        assert_eq!(stored_account.offset(), *offset);
+                        assert_eq!(stored_account.pubkey(), pubkey);
+                        assert_eq!(stored_account.lamports(), account.lamports());
+                        assert_eq!(stored_account.data_len(), account.data().len() as u64);
+
+                        i += 1;
+                    })
+                    .expect("must scan accounts storage");
                 assert_eq!(i, accounts.len());
             },
         )

+ 8 - 2
accounts-db/store-tool/src/main.rs

@@ -153,7 +153,12 @@ fn do_inspect(file: impl AsRef<Path>, verbose: bool) -> Result<(), String> {
         num_accounts += 1;
         stored_accounts_size += account.stored_size();
         lamports += account.lamports();
-    });
+    }).map_err(|err| {
+        format!(
+            "failed to scan accounts in file '{}': {err}",
+            file.as_ref().display(),
+        )
+    })?;
 
     println!(
         "number of accounts: {}, stored accounts size: {}, file size: {}, lamports: {}",
@@ -229,7 +234,8 @@ fn do_search(
                     );
                 }
             }
-        });
+        }).unwrap_or_else(|err| eprintln!("failed to scan accounts in file '{}': {err}",
+                         file.display()));
     });
 
     Ok(())

+ 11 - 8
runtime/src/bank/accounts_lt_hash.rs

@@ -922,14 +922,17 @@ mod tests {
         // get all the lt hashes for each version of all accounts
         let mut stored_accounts_map = HashMap::<_, Vec<_>>::new();
         for storage in &storages {
-            storage.accounts.scan_accounts(|_offset, account| {
-                let pubkey = account.pubkey();
-                let account_lt_hash = AccountsDb::lt_hash_account(&account, pubkey);
-                stored_accounts_map
-                    .entry(*pubkey)
-                    .or_default()
-                    .push(account_lt_hash)
-            });
+            storage
+                .accounts
+                .scan_accounts(|_offset, account| {
+                    let pubkey = account.pubkey();
+                    let account_lt_hash = AccountsDb::lt_hash_account(&account, pubkey);
+                    stored_accounts_map
+                        .entry(*pubkey)
+                        .or_default()
+                        .push(account_lt_hash)
+                })
+                .expect("must scan accounts storage");
         }
 
         // calculate the duplicates lt hash by skipping the first version (latest) of each account,

+ 6 - 3
runtime/src/snapshot_minimizer.rs

@@ -659,9 +659,12 @@ mod tests {
 
         let mut account_count = 0;
         snapshot_storages.into_iter().for_each(|storage| {
-            storage.accounts.scan_pubkeys(|_| {
-                account_count += 1;
-            });
+            storage
+                .accounts
+                .scan_pubkeys(|_| {
+                    account_count += 1;
+                })
+                .expect("must scan accounts storage");
         });
 
         assert_eq!(