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

Validator CLI can specify accounts hash cache path #33117

Merged

Conversation

brooksprumo
Copy link
Contributor

@brooksprumo brooksprumo commented Sep 1, 2023

Problem

The accounts hash cache is stored in the ledger directory, and does not have a way for a node operator to move it. Since the accounts hash cache has a decent amount of I/O, if a disk is overwhelmed, there's no way to move the accounts hash cache somewhere else.

Summary of Changes

Add a CLI arg, --accounts-hash-cache-path, to solana-validator to specify the accounts hash cache path.

@brooksprumo brooksprumo added work in progress This isn't quite right yet noCI Suppress CI on this Pull Request labels Sep 1, 2023
@brooksprumo brooksprumo self-assigned this Sep 1, 2023
@brooksprumo brooksprumo force-pushed the accounts-hash-cache-dir/cli branch from 9de5168 to 4d27c5a Compare September 1, 2023 19:30
@brooksprumo brooksprumo added CI Pull Request is ready to enter CI and removed work in progress This isn't quite right yet noCI Suppress CI on this Pull Request labels Sep 1, 2023
@solana-grimes solana-grimes removed the CI Pull Request is ready to enter CI label Sep 1, 2023
@brooksprumo brooksprumo marked this pull request as ready for review September 1, 2023 19:51
Copy link
Contributor

@jeffwashington jeffwashington left a comment

Choose a reason for hiding this comment

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

lgtm

@brooksprumo brooksprumo added the automerge Merge this Pull Request automatically once CI passes label Sep 1, 2023
@mergify mergify bot removed the automerge Merge this Pull Request automatically once CI passes label Sep 1, 2023
@mergify
Copy link
Contributor

mergify bot commented Sep 1, 2023

automerge label removed due to a CI failure

@brooksprumo brooksprumo merged commit 88d3c8c into solana-labs:master Sep 1, 2023
@brooksprumo brooksprumo deleted the accounts-hash-cache-dir/cli branch September 2, 2023 02:46
@t-nelson t-nelson added the v1.16 PRs that should be backported to v1.16 label Sep 2, 2023
mergify bot pushed a commit that referenced this pull request Sep 2, 2023
(cherry picked from commit 88d3c8c)

# Conflicts:
#	validator/src/main.rs
let accounts_hash_cache_path = matches
.value_of("accounts_hash_cache_path")
.map(Into::into)
.unwrap_or_else(|| ledger_path.join(AccountsDb::DEFAULT_ACCOUNTS_HASH_CACHE_DIR));
Copy link
Contributor

Choose a reason for hiding this comment

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

the help makes no mention of a default

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What would be the preferred way to indicate the default?

Copy link
Contributor

Choose a reason for hiding this comment

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

since we can't resolve the path at help generation time, something like...

format!("[default: LEDGER_DIR/{}]", AccountsDb::DEFAULT_ACCOUNTS_HASH_CACHE_DIR)

at the end of the help message

t-nelson pushed a commit that referenced this pull request Sep 2, 2023
t-nelson pushed a commit that referenced this pull request Sep 2, 2023
mergify bot added a commit that referenced this pull request Sep 2, 2023
…#33117) (#33127)

Validator CLI can specify accounts hash cache path (#33117)

(cherry picked from commit 88d3c8c)

Co-authored-by: Brooks <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v1.16 PRs that should be backported to v1.16
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants