fix(validator_client): raise soft fd limit #4796
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
|
|
michaelsproul
added
ready-for-review
dapplion
previously requested changes
Jan 19, 2024
There was a problem hiding this comment.
I have tested the fslimit crate on a separate repo on my local M2 mac on a regular user and it could raise the fd limit from 256 -> 10240.
However I have mixed feelings about this feature. It seems to only be useful to large operators or devnet operators, two parties who can manually raise the fd limit on their own.
I would recommend mantainers to take a look at the fdlimit crate briefly since it's just one short file https://github.com/paritytech/fdlimit/blob/master/src/lib.rs
To scope its usage better the validator client could only try to raise the fdlimit if needs too i.e. if the user is loading more than N keys.
validator_client/Cargo.toml
Outdated
| @@ -62,3 +62,4 @@ malloc_utils = { workspace = true } | |||
| sysinfo = { workspace = true } | |||
| system_health = { path = "../common/system_health" } | |||
| logging = { workspace = true } | |||
| fdlimit = "0.2.1" | |||
There was a problem hiding this comment.
Can you use version 0.3.0? It includes an improvement to not panic on syscall errors, paritytech/fdlimit@eee618f
rkrasiuk
force-pushed
the
rkrasiuk/raise-vc-fdlimit
branch
from
d689d0a
to
47671b2
Compare
|
@dapplion upgraded the crate. re limiting this behavior - fair enough, we could raise fdlimit based on the current limit and the number of user keys loaded. alternatively, we could expose a CLI flag, that users could pass in case they hit the limit |
This feature is maximally useful when it allows users to not have to think about file limits. Adding a flag defeats the purpose, as after an error the user now has to add a new flag, might as well manually bump the file limit |
|
I've pushed this to v5.1.0 since it doesn't seem critical for Deneb. |
|
Pushed back to 5.2 since we want to get networking improvements in the 5.1 release out soon. |
dapplion
approved these changes
Apr 2, 2024
…rasiuk/raise-vc-fdlimit
…rasiuk/raise-vc-fdlimit
|
@mergify queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at c8ffafb |
ethDreamer
added a commit
that referenced
this pull request
May 2, 2024
* Fix execution integration tests (#5647) * update waiting status * revert to old nethermind version * Add electra presets to beacon API (#5630) * add presets to API * add extra fields to config spec in beacon API * remove unused * add mainnet presets for gnosis and fix minimal preset default values * Rename `Merge` to `Bellatrix` (#5601) * Rename Merge to Bellatrix * Remove tree-hash-cache which got readded from the rebase * Deterministic block generation for tests (#5654) * Deterministic block generation for tests * Electra other containers (#5652) * add new fields to execution payload and header * beacon state changes * partial beacon state * safe arith in upgrade to electra * initialize balances cache in interop genesis state * Revert "initialize balances cache in interop genesis state" This reverts commit c60b522. * always initialize balances cache if necessary in electra upgrade * build cache earlier * fix block test * per fork NUM_FIELDS_POW2 * Merge branch 'unstable' of https://github.com/sigp/lighthouse into electra-other-containers * fix lints * get fields based on post state, as is spec'd * fix type and move cache build * Add more electra helpers (#5653) * Add new helpers * Fix some stuff * Fix compilation errors * lint * Address review * Ignore gossip blob already imported (#5656) * Ignore gossip blob already imported * Beta compiler fix (#5659) * fix beta compiler compilation * remove unused import * Revert "remove unused import" This reverts commit 0bef36b. * Revert "fix beta compiler compilation" This reverts commit 23152cf. * rename ununsed fields * allow dead code on some error variants * remove unused blob download queue * add back debug to backfill error * more allow dead code on errors * fix(validator_client): raise soft fd limit (#4796) * fix(validator_client): raise soft fd limit * Merge branch 'unstable' of https://github.com/sigp/lighthouse into rkrasiuk/raise-vc-fdlimit * cargo lock * Merge branch 'unstable' of https://github.com/sigp/lighthouse into rkrasiuk/raise-vc-fdlimit * Proposer and attester slashing sse events (#5327) * default vc to block v3 endpoint and deprecate block-v3 flag * Merge branch 'unstable' of https://github.com/sigp/lighthouse into unstable * add proposer and attester event variants * add TOOOs * add tests, event triggers * Merge branch 'unstable' of https://github.com/sigp/lighthouse into proposer-and-attester-slashing-sse-events * revert * revert * remove double event tracking * Merge branch 'unstable' into proposer-and-attester-slashing-sse-events * remove todo, fix test * resolve merge conflicts * Merge branch 'proposer-and-attester-slashing-sse-events' of https://github.com/eserilev/lighthouse into proposer-and-attester-slashing-sse-events * leftover debugging * Merge branch 'unstable' of https://github.com/sigp/lighthouse into proposer-and-attester-slashing-sse-events * Merge branch 'unstable' of https://github.com/sigp/lighthouse into proposer-and-attester-slashing-sse-events * pin macos release runner to `macos-13` (#5665) * pin macos release runner to `macos-13` * Update .github/workflows/release.yml * Remove snapshot cache related code (#5661) * Remove snapshot cache and other references. * Fix default state cache size in docs * Remove cache miss comment entirely * Add state cache CLI tests * Uncomment self_hosted_runner after PR Merge #5137 (#5291) * Uncomment self_hosted_runner after PR Merge #5137 * Merge branch 'unstable' into fix_todo * Merge branch 'unstable' of https://github.com/sigp/lighthouse into fix_todo * Only `portable` builds (binaries) (#5615) * release workflow: portable builds by default * Delete outdated comment * Merge branch 'unstable' into portable-builds-binaries # Conflicts: # .github/workflows/release.yml * Merge parent and current sync lookups (#5655) * Drop lookup type trait for a simple arg * Drop reconstructed for processing * Send parent blocks one by one * Merge current and parent lookups * Merge current and parent lookups clean up todos * Merge current and parent lookups tests * Merge remote-tracking branch 'origin/unstable' into sync-merged-lookup * Merge branch 'unstable' of https://github.com/sigp/lighthouse into sync-merged-lookup * fix compile after merge * #5655 pr review (#26) * fix compile after merge * remove todos, fix typos etc * fix compile * stable rng * delete TODO and unfilled out test * make download result a struct * enums instead of bools as params * fix comment * Various fixes * Track ignored child components * Track dropped lookup reason as metric * fix test * add comment describing behavior of avail check error * update ordering * delete spammy log (#5672) * delete spammy log * Ensure block only range requests don't fail on download (#5675) * ensure pruned blobs don't fail on download * Typo * Improve ENR updates (#5483) * Improve ENR updates * forever fmt * Appease my old friend clippy * Merge network unstable * Check da_checker before doing a block lookup request (#5681) * Check da_checker before doing a block lookup request * Ensure consistent handling of lookup result * use req resp pre import cache rather than da checker * Update Cargo.lock (#5670) * update rust-yamux * update Cargo.lock * Merge branch 'unstable' of github.com:jxs/lighthouse into update-cargo * Merge branch 'unstable' of github.com:sigp/lighthouse into update-cargo * update to new libp2p versions * Add metric for current epoch total balance (#5688) * Add metric for current epoch total balance --------- Co-authored-by: realbigsean <sean@sigmaprime.io> Co-authored-by: Mac L <mjladson@pm.me> Co-authored-by: Lion - dapplion <35266934+dapplion@users.noreply.github.com> Co-authored-by: Pawan Dhananjay <pawandhananjay@gmail.com> Co-authored-by: Roman Krasiuk <rokrassyuk@gmail.com> Co-authored-by: Eitan Seri-Levi <eserilev@ucsc.edu> Co-authored-by: antondlr <anton@delaruelle.net> Co-authored-by: Jimmy Chen <jchen.tc@gmail.com> Co-authored-by: Ærvin <arvin.morawej@gmail.com> Co-authored-by: Age Manning <Age@AgeManning.com> Co-authored-by: João Oliveira <hello@jxs.pt> Co-authored-by: Michael Sproul <michael@sigmaprime.io>
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.
Issue Addressed
Closes #3468
Proposed Changes
Attempt to raise validator client's fdlimit. OS specific behavior is described in the comments