Use protobufs to store confirmed blocks in BigTable #12526
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
TODO:
|
mvines
reviewed
Sep 28, 2020
jstarry
force-pushed
the
bigtable-proto
branch
3 times, most recently
from
eb51be5 to
39f5ade
Compare
|
Here are the bigtable rows read out using I'm not sure how to translate this to raw bytes, but length of the string encoded values is roughly the same 🤷 |
mvines
reviewed
Sep 29, 2020
| @@ -0,0 +1,67 @@ | |||
| syntax = "proto3"; | |||
There was a problem hiding this comment.
I think I'd prefer this layout:
storage-bigtable/src/confirmed_block.proto
storage-bigtable/proto/confirmed_block.rs
- keeping generated files out of
src/in general feels best, as we unleash linters and formatters onsrc/. - No need for
src/proto/blah.protoin this case,src/blah.protois just as good since the number of files in this crate is small - I'm not a superfan of
storage-bigtable/proto/...as the location for generated protobuf source files, but I guess it works and it's notsrc/
mvines
reviewed
Sep 29, 2020
|
|
||
| message Reward { | ||
| string pubkey = 1; | ||
| int64 lamports = 2; |
There was a problem hiding this comment.
fun fact: I'm already thinking about adding a new field to this struct
There was a problem hiding this comment.
post_balance from this PR: #12561
Codecov Report
@@ Coverage Diff @@
## master #12526 +/- ##
========================================
Coverage 82.0% 82.0%
========================================
Files 356 357 +1
Lines 83140 83390 +250
========================================
+ Hits 68197 68461 +264
+ Misses 14943 14929 -14 |
CriesofCarrots
force-pushed
the
bigtable-proto
branch
from
39f5ade to
661eff3
Compare
CriesofCarrots
force-pushed
the
bigtable-proto
branch
from
661eff3 to
3ae2d33
Compare
CriesofCarrots
force-pushed
the
bigtable-proto
branch
from
3ae2d33 to
0625a9e
Compare
mvines
previously approved these changes
Sep 30, 2020
storage-bigtable/src/utils.rs
Outdated
| @@ -0,0 +1,291 @@ | |||
| use solana_sdk::{ | |||
There was a problem hiding this comment.
Generic file/directory names like "utils" are pretty triggering for me. Maybe this can be called from.rs? convert.rs?
There was a problem hiding this comment.
Way better. Thanks!
mergify
bot
dismissed
mvines’s stale review
Pull request has been modified.
CriesofCarrots
added
the
automerge
mergify bot
pushed a commit
that referenced
this pull request
Sep 30, 2020
* Use protobufs to store confirmed blocks in BigTable * Cleanup * Reorganize proto * Clean up use statements * Split out function for unit testing * s/utils/convert Co-authored-by: Tyera Eulberg <[email protected]> (cherry picked from commit ce598c5)
mergify bot
added a commit
that referenced
this pull request
Sep 30, 2020
* Use protobufs to store confirmed blocks in BigTable * Cleanup * Reorganize proto * Clean up use statements * Split out function for unit testing * s/utils/convert Co-authored-by: Tyera Eulberg <[email protected]> (cherry picked from commit ce598c5) Co-authored-by: Justin Starry <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Data is stored in BigTable using bincode serialization which is not versioned and does not allow extending nested structs.
Summary of Changes
storage-bigtableto store protobuf serialized data"proto""bin"cell data if"proto"was not foundFixes #12494