Rocks db erasure decoding by carllin · Pull Request #1900 · solana-labs/solana

carllin · 2018-11-24T12:38:27Z

Problem

Erasure decoding was relying on the old windows instead of RocksDb.

Summary of Changes

Update decoding of windows to use new RocksDb style ledger. Needs some elements of
#1888 to build, but tests should be passing after that.

Fixes #

rob-solana · 2018-11-24T17:40:08Z

-pub fn recover(id: &Pubkey, window: &mut [WindowSlot], start_idx: u64, start: usize) -> Result<()> {
-    let block_start = start - (start % NUM_DATA);
+pub fn recover(
+    id: &Pubkey,


this can be removed by now, I think

rob-solana · 2018-11-24T17:42:36Z

-    assert!(!corrupt, " {} ", id);

-    Ok(())
+    assert!(!corrupt, " {} ", id);


we have an opportunity to do something about this, now. if we end up with a corrupt blob, let's not put it in the ledger/window?

rob-solana · 2018-11-25T19:43:10Z

    }

-    assert!(!corrupt);
+    if corrupt {


cool, but we want to remove or mark as bogus the coding blobs from the ledger? maybe we need a way to say "this is a bogus blob" in the ledger/window...

Yeah we can do that. I don't know how you would be able to tell which of the coding blobs/data blobs were the one that caused the decoding to fail though. We could remove all the coding blobs, and then say that sending bad erasures is a penalizable transgression.

sakridge · 2018-11-26T17:11:43Z

+    db_ledger: &mut DbLedger,
+    slot: u64,
+    start_idx: u64,
+) -> Result<(Vec<SharedBlob>, Vec<SharedBlob>)> {


What is the result?

also, the function comment doesn't seem accurate, it doesn't take a window anymore.

The result is a list of reconstructed data and coding blobs returned to the caller for processing.

Ok, yea just could be indicated with a comment or something, I think a reader of the the function wouldn't realize it just reading the signature.

For sure, updated the comment

sakridge · 2018-11-26T17:14:53Z

-            for i in b_wl.meta.size..size {
-                b_wl.data[i] = 0;
+    // Add the coding blobs we have into the recovery vector, mark the missing ones
+    for i in coding_start_idx..block_end_idx {


Seems like a lot of logic to duplicate, can be moved into a function?

rob-solana · 2018-11-27T19:07:21Z

 use db_ledger::DbLedger;
 use db_window::{find_missing_coding_indexes, find_missing_data_indexes};
 use packet::{Blob, SharedBlob, BLOB_DATA_SIZE, BLOB_HEADER_SIZE};
+use result::Result as SolanaResult;


what happened here?

The original erasure module used its own version of Result, so I had to import our more generic result under a different name (SolanaResult) in an earlier commit in order to work with it. I ultimately replaced the erasure-specific Result type with the one in result.rs, so this import from the earlier commit can now be deleted.

rob-solana · 2018-11-27T19:09:47Z

+            return Err(ErasureError::InvalidBlobData);
+        }
+        Ok(Some(b)) => {
+            if b.len() <= BLOB_HEADER_SIZE {


I guess we don't support zero-length blobs...

rob-solana · 2018-11-27T19:11:49Z

+            // Mark the missing memory
+            erasures.push(erasure_index);
+            blobs.push(SharedBlob::default());
+            missing.push(blobs.last_mut().unwrap().clone());


is this more or less code, compiled, than:

let b = Default::default(); blobs.push(b.clone()); missing.push(b);

?

That's definitely a lot better, yay!

rob-solana · 2018-12-03T23:51:39Z

 impl Blob {
+    pub fn new(data: &[u8]) -> Self {
+        let mut blob = Self::default();
+        let data_len = cmp::min(data.len(), blob.data.len());


this won't always be zero?

blob.data is a fixed size array of size 65408, so it shouldn't be

ok, I see. we're gonna assert!() that data is small enough to fit?

Added a check where new() is called in erasure to make sure the data fits.

…ex in the ledger

…ob structure in erasure

…olana-labs#1888) (solana-labs#1900) * Refactor and additional metrics for cost tracking (solana-labs#1888) * Refactor and add metrics: - Combine remove_* and update_* functions to reduce locking on cost-tracker and iteration. - Add method to calculate executed transaction cost by directly using actual execution cost and loaded accounts size; - Wireup histogram to report loaded accounts size; - Report time of block limits checking; - Move account counters from ExecuteDetailsTimings to ExecuteAccountsDetails; * Move committed transactions adjustment into its own function (cherry picked from commit c3fadac) * rename cost_tracker.account_data_size to better describe its purpose is to tracker per-block new account allocation --------- Co-authored-by: Tao Zhu <82401714+tao-stones@users.noreply.github.com> Co-authored-by: Tao Zhu <tao@solana.com>

…port of solana-labs#1888) (solana-labs#1900)" This reverts commit 0aef62e.

…port of solana-labs#1888) (solana-labs#1900) (solana-labs#1937) Revert "v2.0: Refactor and additional metrics for cost tracking (backport of solana-labs#1888) (solana-labs#1900)" This reverts commit 0aef62e.

carllin added the work in progress This isn't quite right yet label Nov 24, 2018

carllin requested review from rob-solana and sakridge November 24, 2018 12:38

carllin added the noCI Suppress CI on this Pull Request label Nov 24, 2018

rob-solana reviewed Nov 24, 2018

View reviewed changes

carllin force-pushed the RocksDbErasure branch from 94b6cf0 to 25415d6 Compare November 25, 2018 05:25

carllin removed the noCI Suppress CI on this Pull Request label Nov 25, 2018

rob-solana reviewed Nov 25, 2018

View reviewed changes

carllin force-pushed the RocksDbErasure branch 5 times, most recently from 0f23ddb to 967a0b6 Compare November 26, 2018 04:59

sakridge reviewed Nov 26, 2018

View reviewed changes

carllin force-pushed the RocksDbErasure branch from 199ced2 to 9b9f901 Compare November 26, 2018 23:58

rob-solana reviewed Nov 27, 2018

View reviewed changes

carllin force-pushed the RocksDbErasure branch from 1503315 to 308aa83 Compare November 29, 2018 06:01

rob-solana approved these changes Nov 29, 2018

View reviewed changes

rob-solana mentioned this pull request Nov 29, 2018

merge window and ledger #1717

Closed

carllin force-pushed the RocksDbErasure branch 2 times, most recently from a447149 to 848f8e6 Compare December 3, 2018 23:44

rob-solana reviewed Dec 3, 2018

View reviewed changes

carllin force-pushed the RocksDbErasure branch 2 times, most recently from 3f43a6f to 9562b6b Compare December 4, 2018 00:57

carllin added 2 commits December 5, 2018 11:11

Change erasure to consume new RocksDb window

271b178

Change test for erasure

d324b5f

carllin added 13 commits December 5, 2018 11:11

remove erasure from window

bb3d069

Working erasure

4ca3d18

Remove id from recover()

4d8bccc

If corrupt, don't return any recovered blobs

2bf0540

Integrate erasure decoding back into window

eaa1a30

Fix find_missing() to account for end_index greater than the last ind…

8aebeaf

…ex in the ledger

Factor out categorize_blobs() function

a641916

Remove corrupted blobs from ledger

da571f9

Replace Erasure result with result module's Result

17054dc

Comment recover() function

b5048ae

Cleanup

7a07ea2

Add test for erasure integration into db_ledger

8112244

Add Check that blob is smaller than max BLOB_SIZE when consructing Bl…

419ed35

…ob structure in erasure

carllin force-pushed the RocksDbErasure branch from b4d445d to 419ed35 Compare December 5, 2018 19:11

sakridge approved these changes Dec 5, 2018

View reviewed changes

carllin merged commit 9c30bdd into solana-labs:master Dec 5, 2018

carllin removed the work in progress This isn't quite right yet label Dec 6, 2018

tao-stones added a commit to tao-stones/solana that referenced this pull request Jul 1, 2024

Revert "v2.0: Refactor and additional metrics for cost tracking (back…

5695539

…port of solana-labs#1888) (solana-labs#1900)" This reverts commit 0aef62e.

Conversation

carllin commented Nov 24, 2018

Problem

Summary of Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carllin Nov 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rob-solana Nov 27, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

carllin Nov 26, 2018 •

edited

Loading

rob-solana Nov 27, 2018 •

edited

Loading