Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bigtable: update google proto files and allow configuration of max_message_size #34740

Merged
merged 7 commits into from
Jan 11, 2024

Conversation

CriesofCarrots
Copy link
Contributor

Problem

Blocks with rewards are not returning from bigtable on v1.17+. This is because we bumped tonic (#32994), which now limits the size of decoded messages to 4MB by default.

There is a Grpc client method to customize this limit, but since we didn't regenerate the google/bigtable protobuf files, current code doesn't support that method.

Summary of Changes

Regenerate the google/bigtable proto files, which includes the Grpc client and new method
Add our own default max_message_size value of 64MB (currently, the first block of an epoch is ~30MB encoded)
Add solana-validator CLI arg to allow tuning max_message_size

@CriesofCarrots CriesofCarrots added the v1.17 PRs that should be backported to v1.17 label Jan 11, 2024
Copy link
Contributor

mergify bot commented Jan 11, 2024

Backports to the beta branch are to be avoided unless absolutely necessary for fixing bugs, security issues, and perf regressions. Changes intended for backport should be structured such that a minimum effective diff can be committed separately from any refactoring, plumbing, cleanup, etc that are not strictly necessary to achieve the goal. Any of the latter should go only into master and ride the normal stabilization schedule. Exceptions include CI/metrics changes, CLI improvements and documentation updates on a case by case basis.

Copy link
Contributor

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have i ever mentioned how much i hate code generators?

Copy link

codecov bot commented Jan 11, 2024

Codecov Report

Attention: 156 lines in your changes are missing coverage. Please review.

Comparison is base (e9a6bb3) 81.8% compared to head (010677d) 81.7%.
Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff            @@
##           master   #34740     +/-   ##
=========================================
- Coverage    81.8%    81.7%   -0.1%     
=========================================
  Files         823      823             
  Lines      222713   222865    +152     
=========================================
- Hits       182337   182255     -82     
- Misses      40376    40610    +234     

@CriesofCarrots CriesofCarrots merged commit 166be29 into solana-labs:master Jan 11, 2024
35 checks passed
mergify bot pushed a commit that referenced this pull request Jan 11, 2024
…ssage_size (#34740)

* Update proto files with tonic-build v0.9.2

* Manually ignore invalid doc-tests

* Add new ReadRowsRequest fields

* Add LedgerStorageConfig::max_message_size and default value

* Add BigtableConnection::max_message_size and use on client creation

* Add max_message_size to RpcBigtableConfig and make const pub

* Add solana-validator cli arg

(cherry picked from commit 166be29)
mergify bot added a commit that referenced this pull request Jan 11, 2024
… max_message_size (backport of #34740) (#34741)

Bigtable: update google proto files and allow configuration of max_message_size (#34740)

* Update proto files with tonic-build v0.9.2

* Manually ignore invalid doc-tests

* Add new ReadRowsRequest fields

* Add LedgerStorageConfig::max_message_size and default value

* Add BigtableConnection::max_message_size and use on client creation

* Add max_message_size to RpcBigtableConfig and make const pub

* Add solana-validator cli arg

(cherry picked from commit 166be29)

Co-authored-by: Tyera <tyera@solana.com>
@fanatid
Copy link
Contributor

fanatid commented Jan 17, 2024

Add our own default max_message_size value of 64MB (currently, the first block of an epoch is ~30MB encoded)

Probably would be good to have a higher default value than 64MB, iirc I already had seen blocks around 48MB 😕

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v1.17 PRs that should be backported to v1.17
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants