Correct missing entry handling to avoid bad warns by ryoqun · Pull Request #8339 · solana-labs/solana

ryoqun · 2020-02-20T00:40:40Z

Problem

Bogus warnings is introduced by #7281. Also this is needed for #8337 because compacting at generate_index causes a lot of missing storage entries.

Summary of Changes

~~Put empty dummy files for missing storage entry's AppendVec in the snapshot file.~~

=> Changed the design strategy to change AccountStorageSerialize as discussed on discord.

~~This changes ABI for snapshots.~~

=> this is not true anymore; this shouldn't introduce any ABI changes.

ryoqun · 2020-02-20T16:48:01Z

-            .filter(|store| accounts_index.is_root(store.slot_id))
+            .iter()
+            .filter(|(slot, storage)| {
+                **slot <= snapshot_slot && accounts_index.is_root(**slot) && !storage.is_empty()


slot <= snapshot_slot logic is moved from here.

ryoqun · 2020-02-20T16:49:19Z

@sakridge Does this look better?Still needs to ~~add tests~~ (added) and ~~more local testing with TdS snapshots~~ (tested). ~~Also CI might not pass....~~ (CI passed), also I checked ABI as well.

codecov · 2020-02-20T17:36:47Z

Codecov Report

Merging #8339 into master will increase coverage by <.1%.
The diff coverage is 86.3%.

@@           Coverage Diff            @@
##           master   #8339     +/-   ##
========================================
+ Coverage    80.5%   80.5%   +<.1%     
========================================
  Files         253     253             
  Lines       55412   55465     +53     
========================================
+ Hits        44637   44681     +44     
- Misses      10775   10784      +9

sakridge · 2020-02-20T19:21:19Z

@sakridge Does this look better?Still needs to add tests and more local testing with TdS snapshots. Also CI might not pass....

Yea. This looks really good. Thanks for the updates.

sakridge

lgtm

ryoqun · 2020-02-21T03:54:12Z

For the record, this PR doesn't change the ABI:

$ diff -U20 /tmp/abi8/accounts_db__test_accounts_db_serialize__AccountsDBSerializeAbiTestWrapper_frozen_abi__test_digest_DAT7VpkAnXSw7r9pDHppJSqKCRYC8YAF4yCDAfkCbVE5 /tmp/abi8/accounts_db__test_accounts_db_serialize__AccountsDBSerializeAbiTestWrapper_frozen_abi__test_digest_2zC1Q4tpzCGw88khu3Aab13zM4cQzeH9kq2SHtMyrhvc
--- /tmp/abi8/accounts_db__test_accounts_db_serialize__AccountsDBSerializeAbiTestWrapper_frozen_abi__test_digest_DAT7VpkAnXSw7r9pDHppJSqKCRYC8YAF4yCDAfkCbVE5   2020-02-21 12:41:47.146055549 +0900
+++ /tmp/abi8/accounts_db__test_accounts_db_serialize__AccountsDBSerializeAbiTestWrapper_frozen_abi__test_digest_2zC1Q4tpzCGw88khu3Aab13zM4cQzeH9kq2SHtMyrhvc   2020-02-21 12:48:42.452915738 +0900
@@ -1,30 +1,30 @@
 struct AccountsDBSerializeAbiTestWrapper (fields = 1)
     field owned: solana_runtime::accounts_db::test_accounts_db_serialize::_IMPL_SERIALIZE_FOR_AccountsDBSerializeAbiTestWrapper::<impl serde::ser::Serialize for solana_runtime::accounts_db::test_accounts_db_serialize::AccountsDBSerializeAbiTestWrapper>::serialize::__SerializeWith
         tuple (elements = 4)
             element solana_runtime::accounts_db::AccountStorageSerialize
                 map (entries = 1)
-                    key &u64
+                    key u64
                         primitive u64
-                    value alloc::vec::Vec<&alloc::sync::Arc<solana_runtime::accounts_db::AccountStorageEntry>>
+                    value &alloc::vec::Vec<alloc::sync::Arc<solana_runtime::accounts_db::AccountStorageEntry>>
                         seq (elements = 1)
-                            element &&alloc::sync::Arc<solana_runtime::accounts_db::AccountStorageEntry>
+                            element &alloc::sync::Arc<solana_runtime::accounts_db::AccountStorageEntry>
                                 struct AccountStorageEntry (fields = 3)
                                     field id: usize
                                         primitive u64
                                     field accounts: solana_runtime::append_vec::AppendVec
                                         bytes [u8] (len = 8)
                                     field count_and_status: std::sync::rwlock::RwLock<(usize, solana_runtime::accounts_db::AccountStorageStatus)>
                                         tuple (elements = 2)
                                             element usize
                                                 primitive u64
                                             element solana_runtime::accounts_db::AccountStorageStatus
                                                 enum AccountStorageStatus (variants = 3)
                                                     variant(0) Available (unit)
                                                     variant(1) Full (unit)
                                                     variant(2) Candidate (unit)
             element u64
                 primitive u64
             element u64
                 primitive u64
             element &solana_runtime::accounts_db::BankHashInfo
                 struct BankHashInfo (fields = 2)

Generated by this branch: https://github.com/ryoqun/solana/commits/abi-management-implementation-for-missing-storage-entry-handling

Pull request has been modified.

ryoqun · 2020-02-21T06:22:27Z

@sakridge Does this look better?Still needs to ~~add tests~~ (added) and ~~more local testing with TdS snapshots~~ (tested). ~~Also CI might not pass....~~ (CI passed), also I checked ABI as well.

All, prudent testing and preparation is done; now it's time to merge!

@sakridge Thanks for quick review!!

* Correct missing entry handling to avoid bad warns * Pass storage entries to AccountStorageSerialize * Fix CI..... * Add tests and reorder condition for cheapest first * Remove unneeded reference (cherry picked from commit d238371) # Conflicts: # runtime/src/bank.rs

ryoqun · 2020-02-21T06:35:27Z

@mvines Btw, snapshots created after this PR doesn't anymore produce bad snapshots, which cause these scary warnings to be printed when loading from it as some validators are scared of:

[2020-02-16T17:20:43.188504734Z INFO  solana_runtime::accounts_db] Err(Custom { kind: Other, error: "Unable to move \"/home/vk/validator-ledger/snapshot/.tmpBQUoxR/accounts/1400272.5601273\" to \"/home/vk/validator-ledger/accounts\": entity not found" })
[2020-02-16T17:20:43.188544129Z WARN  solana_runtime::accounts_db] AccountsDB error: "Unable to move \"/home/vk/validator-ledger/snapshot/.tmpBQUoxR/accounts/1400272.5601280\" to \"/home/vk/validator-ledger/accounts\": entity not found"
[2020-02-16T17:20:43.188557641Z INFO  solana_runtime::accounts_db] Err(Custom { kind: Other, error: "Unable to move \"/home/vk/validator-ledger/snapshot/.tmpBQUoxR/accounts/1400272.5601280\" to \"/home/vk/validator-ledger/accounts\": entity not found" })
[2020-02-16T17:20:43.188608874Z WARN  solana_runtime::accounts_db] AccountsDB error: "Unable to move \"/home/vk/validator-ledger/snapshot/.tmpBQUoxR/accounts/1400272.5601283\" to \"/home/vk/validator-ledger/accounts\": entity not found"
[2020-02-16T17:20:43.188624977Z INFO  solana_runtime::accounts_db] Err(Custom { kind: Other, error: "Unable to move \"/home/vk/validator-ledger/snapshot/.tmpBQUoxR/accounts/1400272.5601283\" to \"/home/vk/validator-ledger/accounts\": entity not found" })

mvines · 2020-02-21T06:36:14Z

Awesome, thanks :)

automerge

$ git diff --no-index /tmp/abi8/*{8HonGUuN4veVMzn6JBsUd2o7AkTRkkrKxy1bQeB5rnvE,EpX1HbzgTKh1jki8aVJKzTL3dWoA5cknxn6qr6WDrwm}* diff --git a/tmp/abi8/accounts_db__test_accounts_db_serialize__AccountsDBSerializeAbiTestWrapper_frozen_abi__test_digest_8HonGUuN4veVMzn6JBsUd2o7AkTRkkrKxy1bQeB5rnvE b/tmp/abi8/accounts_db__test_accounts_db_serialize__AccountsDBSerializeAbiTestWrapper_frozen_abi__test_digest_EpX1HbzgTKh1jki8aVJKzTL3dWoA5cknxn6qr6WDrwm index 2607110e2..92dbb9206 100644 --- a/tmp/abi8/accounts_db__test_accounts_db_serialize__AccountsDBSerializeAbiTestWrapper_frozen_abi__test_digest_8HonGUuN4veVMzn6JBsUd2o7AkTRkkrKxy1bQeB5rnvE +++ b/tmp/abi8/accounts_db__test_accounts_db_serialize__AccountsDBSerializeAbiTestWrapper_frozen_abi__test_digest_EpX1HbzgTKh1jki8aVJKzTL3dWoA5cknxn6qr6WDrwm @@ -1,3 +1,3 @@ struct AccountsDBSerializeAbiTestWrapper (fields = 1) field owned: solana_runtime::accounts_db::test_accounts_db_serialize::_IMPL_SERIALIZE_FOR_AccountsDBSerializeAbiTestWrapper::<impl serde::ser::Serialize for solana_runtime::accounts_db::test_accounts_db_serialize::AccountsDBSerializeAbiTestWrapper>::serialize::__SerializeWith - bytes [u8] (len = 572) + bytes [u8] (len = 180)

ryoqun · 2021-05-28T10:46:39Z

+    pub fn get_snapshot_storages(&self) -> SnapshotStorages {
+        self.rc
+            .get_snapshot_storages(self.slot())
+            .into_iter()


redundant me #17559

Correct missing entry handling to avoid bad warns

20d1918

ryoqun added the work in progress This isn't quite right yet label Feb 20, 2020

ryoqun requested a review from sakridge February 20, 2020 00:40

ryoqun added the v0.23 label Feb 20, 2020

Pass storage entries to AccountStorageSerialize

bab03a8

ryoqun changed the title ~~[wip] Correct missing entry handling to avoid bad warns~~ Correct missing entry handling to avoid bad warns Feb 20, 2020

Fix CI.....

9336659

ryoqun commented Feb 20, 2020

View reviewed changes

ryoqun removed the work in progress This isn't quite right yet label Feb 20, 2020

sakridge previously approved these changes Feb 20, 2020

View reviewed changes

ryoqun added 2 commits February 21, 2020 14:02

Add tests and reorder condition for cheapest first

91ff1ab

Remove unneeded reference

a197cb4

ryoqun merged commit d238371 into solana-labs:master Feb 21, 2020

mergify Bot mentioned this pull request Feb 21, 2020

Correct missing entry handling to avoid bad warns (bp #8339) #8378

Merged

ryoqun mentioned this pull request Feb 21, 2020

Introduce automatic ABI maintenance mechanism (2/2; rollout) #8012

Merged

solana-grimes pushed a commit that referenced this pull request Feb 21, 2020

Correct missing entry handling to avoid bad warns (bp #8339) (#8378)

c2b17c7

automerge

ryoqun mentioned this pull request Feb 21, 2020

Convert snapshot #8380

Closed

ryoqun mentioned this pull request Jun 17, 2020

Cleanup error type mapping in serde_snapshot #10580

Merged

ryoqun commented May 28, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Correct missing entry handling to avoid bad warns#8339

Correct missing entry handling to avoid bad warns#8339
ryoqun merged 5 commits intosolana-labs:masterfrom
ryoqun:missing-storage-entry-handling

ryoqun commented Feb 20, 2020 •

edited

Loading

Uh oh!

ryoqun Feb 20, 2020

Uh oh!

ryoqun commented Feb 20, 2020 •

edited

Loading

Uh oh!

codecov Bot commented Feb 20, 2020 •

edited

Loading

Uh oh!

sakridge commented Feb 20, 2020

Uh oh!

sakridge left a comment

Uh oh!

ryoqun commented Feb 21, 2020 •

edited

Loading

Uh oh!

ryoqun commented Feb 21, 2020

Uh oh!

ryoqun commented Feb 21, 2020

Uh oh!

mvines commented Feb 21, 2020

Uh oh!

ryoqun May 28, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

ryoqun commented Feb 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Summary of Changes

Uh oh!

ryoqun Feb 20, 2020

Choose a reason for hiding this comment

Uh oh!

ryoqun commented Feb 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Feb 20, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sakridge commented Feb 20, 2020

Uh oh!

sakridge left a comment

Choose a reason for hiding this comment

Uh oh!

ryoqun commented Feb 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryoqun commented Feb 21, 2020

Uh oh!

ryoqun commented Feb 21, 2020

Uh oh!

mvines commented Feb 21, 2020

Uh oh!

ryoqun May 28, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ryoqun commented Feb 20, 2020 •

edited

Loading

ryoqun commented Feb 20, 2020 •

edited

Loading

codecov Bot commented Feb 20, 2020 •

edited

Loading

ryoqun commented Feb 21, 2020 •

edited

Loading