Skip to content

Commit

Permalink
Do not panic if the fdlimit call to increase the file descriptor li…
Browse files Browse the repository at this point in the history
…mit fails (paritytech#2155)

Sometimes changing file descriptor limits is not allowed, but there is
no need to crash the node if/when this happens. Since `fdlimit`'s author
decided to use panics instead of returning `Result`, we need to catch
it.

- [x] My PR includes a detailed description as outlined in the
"Description" section above
- [ ] My PR follows the [labeling requirements](CONTRIBUTING.md#Process)
of this project (at minimum one label for `T`
  required)
- [ ] I have made corresponding changes to the documentation (if
applicable)
- [ ] I have added tests that prove my fix is effective or that my
feature works (if applicable)

---------

Co-authored-by: Koute <[email protected]>
  • Loading branch information
2 people authored and ParthDesai committed Dec 1, 2023
1 parent 4c82ce2 commit ce02703
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 15 deletions.
13 changes: 7 additions & 6 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion substrate/client/cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ array-bytes = "6.1"
atomic = "0.5.3"
chrono = "0.4.27"
clap = { version = "4.4.3", features = ["derive", "string"] }
fdlimit = "0.2.1"
fdlimit = "0.3.0"
futures = "0.3.21"
libp2p-identity = { version = "0.1.3", features = ["peerid", "ed25519"]}
log = "0.4.17"
Expand Down
23 changes: 17 additions & 6 deletions substrate/client/cli/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -614,14 +614,25 @@ pub trait CliConfiguration<DCV: DefaultConfigurationValues = ()>: Sized {

logger.init()?;

if let Some(new_limit) = fdlimit::raise_fd_limit() {
if new_limit < RECOMMENDED_OPEN_FILE_DESCRIPTOR_LIMIT {
match fdlimit::raise_fd_limit() {
Ok(fdlimit::Outcome::LimitRaised { to, .. }) =>
if to < RECOMMENDED_OPEN_FILE_DESCRIPTOR_LIMIT {
warn!(
"Low open file descriptor limit configured for the process. \
Current value: {:?}, recommended value: {:?}.",
to, RECOMMENDED_OPEN_FILE_DESCRIPTOR_LIMIT,
);
},
Ok(fdlimit::Outcome::Unsupported) => {
// Unsupported platform (non-Linux)
},
Err(error) => {
warn!(
"Low open file descriptor limit configured for the process. \
Current value: {:?}, recommended value: {:?}.",
new_limit, RECOMMENDED_OPEN_FILE_DESCRIPTOR_LIMIT,
"Failed to configure file descriptor limit for the process: \
{}, recommended value: {:?}.",
error, RECOMMENDED_OPEN_FILE_DESCRIPTOR_LIMIT,
);
}
},
}

Ok(())
Expand Down
2 changes: 1 addition & 1 deletion substrate/client/service/test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ targets = ["x86_64-unknown-linux-gnu"]
[dependencies]
async-channel = "1.8.0"
array-bytes = "6.1"
fdlimit = "0.2.1"
fdlimit = "0.3.0"
futures = "0.3.21"
log = "0.4.17"
parity-scale-codec = "3.6.1"
Expand Down
2 changes: 1 addition & 1 deletion substrate/client/service/test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ where
base_port: u16,
) -> TestNet<G, E, F, U> {
sp_tracing::try_init_simple();
fdlimit::raise_fd_limit();
fdlimit::raise_fd_limit().unwrap();
let runtime = Runtime::new().expect("Error creating tokio runtime");
let mut net = TestNet {
runtime,
Expand Down

0 comments on commit ce02703

Please sign in to comment.