Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Pass around --max-genesis-archive-unpacked-size#9161

Merged
solana-grimes merged 7 commits intosolana-labs:masterfrom
ryoqun:pass-around-genesis-limit
Apr 30, 2020
Merged

Pass around --max-genesis-archive-unpacked-size#9161
solana-grimes merged 7 commits intosolana-labs:masterfrom
ryoqun:pass-around-genesis-limit

Conversation

@ryoqun
Copy link
Copy Markdown
Contributor

@ryoqun ryoqun commented Mar 30, 2020 •
edited
Loading

Problem

#9106 (comment)

solution

... How about adding a --max-genesis-archive-unpack-size argument to solana-validator, solana-ledger-tool and solana-genesis instead of an env var? We can add this flag into net/.
I say solana-genesis because we should have a check in there that ensures it didn't just produce a genesis archive that would not be accepted by solana-validator/solana-ledger-tool

Also revert #9133

fixes #9106

@ryoqun ryoqun added the work in progress This isn't quite right yet label Mar 30, 2020
ryoqun
ryoqun commented Mar 30, 2020
View reviewed changes
Comment thread ledger-tool/src/main.rs
poh_verify: !arg_matches.is_present("skip_poh_verify"),
..ProcessOptions::default()
};
println!("{}", open_genesis_config(&ledger_path).hash());
Copy link
Copy Markdown
Contributor Author

@ryoqun ryoqun Mar 30, 2020

Choose a reason for hiding this comment

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

I always wondered what is this base-58 thing on the terminal output...

ryoqun
ryoqun commented Mar 30, 2020
View reviewed changes
Comment thread ledger/src/hardened_unpack.rs
#[error("IO error: {0}")]
IO(#[from] std::io::Error),
#[error("Archive error")]
#[error("Archive error: {0}")]
Copy link
Copy Markdown
Contributor Author

@ryoqun ryoqun Mar 30, 2020

Choose a reason for hiding this comment

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

print original error!

ryoqun
ryoqun commented Mar 30, 2020
View reviewed changes
Comment thread ledger/src/hardened_unpack.rs
let total_size = total_size.saturating_add(entry_size);
if total_size > limit_size {
return Err(UnpackError::Archive(format!(
"too large snapshot: {:?}",
Copy link
Copy Markdown
Contributor Author

@ryoqun ryoqun Mar 30, 2020

Choose a reason for hiding this comment

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

snapshot for genesis codepath is plain wrong...

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 30, 2020 •
edited
Loading

Codecov Report

Merging #9161 into master will increase coverage by 0.0%.
The diff coverage is 42.8%.

@@          Coverage Diff           @@
##           master   #9161   +/-   ##
======================================
  Coverage    80.4%   80.4%           
======================================
  Files         281     281           
  Lines       64644   64683   +39     
======================================
+ Hits        51975   52019   +44     
+ Misses      12669   12664    -5

@ryoqun ryoqun mentioned this pull request Apr 1, 2020
Closed
@stale
Copy link
Copy Markdown

stale Bot commented Apr 7, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale Bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Apr 7, 2020
@stale
Copy link
Copy Markdown

stale Bot commented Apr 14, 2020

This stale pull request has been automatically closed. Thank you for your contributions.

@stale stale Bot closed this Apr 14, 2020
@ryoqun ryoqun reopened this Apr 28, 2020
@stale stale Bot removed the stale [bot only] Added to stale content; results in auto-close after a week. label Apr 28, 2020
@ryoqun
Copy link
Copy Markdown
Contributor Author

ryoqun commented Apr 28, 2020

After long queue of interrupts, I've finally got back to the original normal-priority work... ;)

@ryoqun ryoqun force-pushed the pass-around-genesis-limit branch from 40bb6c7 to 2e2e72f Compare April 28, 2020 09:45
ryoqun
ryoqun commented Apr 28, 2020
View reviewed changes
Comment thread genesis/src/main.rs
add_genesis_accounts(&mut genesis_config, issued_lamports - faucet_lamports);

create_new_ledger(&ledger_path, &genesis_config)?;
solana_logger::setup();
Copy link
Copy Markdown
Contributor Author

@ryoqun ryoqun Apr 28, 2020

Choose a reason for hiding this comment

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

just like solana cli.

I think there should be no harm enabling this?

Copy link
Copy Markdown
Contributor Author

@ryoqun ryoqun Apr 28, 2020

Choose a reason for hiding this comment

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

btw, logging makes debugging life easier. :)

ryoqun
ryoqun commented Apr 28, 2020
View reviewed changes
Comment thread validator/src/main.rs
)
.arg(
clap::Arg::with_name("gossip_port")
Arg::with_name("gossip_port")
Copy link
Copy Markdown
Contributor Author

@ryoqun ryoqun Apr 28, 2020

Choose a reason for hiding this comment

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

Follow this newly added line: https://github.com/solana-labs/solana/pull/9161/files#diff-1f422d7e73618aca7bce9935356de012R807

@ryoqun ryoqun changed the title [wip] Pass around --max-genesis-archive-unpacked-size Pass around --max-genesis-archive-unpacked-size Apr 28, 2020
@ryoqun ryoqun requested a review from mvines April 28, 2020 15:44
@ryoqun ryoqun removed the work in progress This isn't quite right yet label Apr 28, 2020
@ryoqun
Copy link
Copy Markdown
Contributor Author

ryoqun commented Apr 28, 2020

@mvines I think this is ready for review. Sorry about being a bit dated pr.... I've also checked ./net failed only with the limit revert and succeeded with all the patch on buildkite.

@ryoqun ryoqun marked this pull request as ready for review April 28, 2020 15:48
mvines
mvines previously approved these changes Apr 28, 2020
View reviewed changes
Comment thread ledger/src/blockstore.rs Outdated
@mergify mergify Bot dismissed mvines’s stale review April 30, 2020 00:16

Pull request has been modified.

@ryoqun ryoqun added the automerge Merge this Pull Request automatically once CI passes label Apr 30, 2020
@solana-grimes
Copy link
Copy Markdown
Contributor

solana-grimes commented Apr 30, 2020

💔 Unable to automerge due to merge conflict

@solana-grimes solana-grimes removed the automerge Merge this Pull Request automatically once CI passes label Apr 30, 2020
@ryoqun ryoqun added the automerge Merge this Pull Request automatically once CI passes label Apr 30, 2020
@solana-grimes solana-grimes removed the automerge Merge this Pull Request automatically once CI passes label Apr 30, 2020
@solana-grimes
Copy link
Copy Markdown
Contributor

solana-grimes commented Apr 30, 2020

💔 Unable to automerge due to merge conflict

@ryoqun ryoqun force-pushed the pass-around-genesis-limit branch from 6c3176d to ebc801c Compare April 30, 2020 00:26
@ryoqun ryoqun added the automerge Merge this Pull Request automatically once CI passes label Apr 30, 2020
@solana-grimes solana-grimes merged commit a912360 into solana-labs:master Apr 30, 2020
t-nelson pushed a commit to t-nelson/solana that referenced this pull request May 29, 2020
@ryoqun @t-nelson
Backport of solana-labs#9161 to v1.1 branch
1f16840 
Pass around --max-genesis-archive-unpacked-size
@t-nelson t-nelson mentioned this pull request May 29, 2020
Merged
solana-grimes pushed a commit that referenced this pull request May 29, 2020
@t-nelson
Backport of #9161 to v1.1 branch (#10327)
b128087 
automerge
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@mvines mvines mvines left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

automerge Merge this Pull Request automatically once CI passes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

/net testnet scripts are failing to start validators due to failed genesis opening

3 participants

@ryoqun @solana-grimes @mvines