Adjust nofile limit recommendations

[jaroeichler]

Description

A few examples like nix run github:TraceMachina/nativelink ./basic_cas.json5 default to lower than recommended max_open_files. This commit increases their value and adjusts set_open_file_limit() to higher defaults.

Type of change

Please delete options that aren't relevant.

• Bug fix (non-breaking change which fixes an issue)
• This change requires a documentation update

How Has This Been Tested?

Please also list any relevant details for your test configuration

Checklist

• Updated documentation if needed
• Tests added/amended
• bazel test //... passes locally
• PR is contained in a single commit, using git amend see some docs

---

This change is 

[aaronmondal]

Looks really good!

Since we're at it, let's fix some latent issues with the set_open_file_limit function:

• It's currently lacking code coverage, i.e. there are no tests for it. You can see the current code coverage here (also available via a tab at the top of the docs page) https://tracemachina.github.io/nativelink/coverage/build/source/nativelink-util/src/fs.rs.html#L142. See also https://www.nativelink.com/docs/contribute/guidelines#generating-code-coverage
• The clippy::pedantic warning set currently triggers on this code due to a lack of documentation for the panic. The expect call marked above causes this panic: https://doc.rust-lang.org/std/result/enum.Result.html#method.expect. We have two options here.
  • One would be to document the panic (If you put ![warn(clippy::all, clippy::nursery, clippy::pedantic)] at the top of a single file and run bazel test you'll see the warnings. You can also put -Dclippy::pedantic here to get this globally:

    nativelink/.bazelrc

    Line 57 in 07bd27a

    build --@rules_rust//:clippy_flags=-Dwarnings,-Dclippy::missing_const_for_fn,-Dclippy::uninlined_format_args,-Dclippy::manual_string_new,-Dclippy::manual_let_else,-Dclippy::single_match_else,-Dclippy::redundant_closure_for_method_calls,-Dclippy::semicolon_if_nothing_returned,-Dclippy::unreadable_literal,-Dclippy::range_plus_one,-Dclippy::inconsistent_struct_constructor,-Dclippy::match_wildcard_for_single_variants,-Dclippy::implicit_clone,-Dclippy::needless_pass_by_value,-Dclippy::explicit_deref_methods,-Dclippy::trivially_copy_pass_by_ref,-Dclippy::unnecessary_wraps,-Dclippy::cast_lossless,-Dclippy::map_unwrap_or,-Dclippy::ref_as_ptr,-Dclippy::inline_always,-Dclippy::redundant_else,-Dclippy::return_self_not_must_use,-Dclippy::match_same_arms,-Dclippy::explicit_iter_loop,-Dclippy::items_after_statements,-Dclippy::explicit_into_iter_loop,-Dclippy::stable_sort_primitive,-Dclippy::ptr_as_ptr,-Dclippy::needless_raw_string_hashes,-Dclippy::default_trait_access,-Dclippy::ignored_unit_patterns,-Dclippy::needless_continue,-Dclippy::wildcard_imports,-Dclippy::doc_markdown,-Dclippy::struct_field_names,-Dclippy::implicit_hasher

    )
  • I'd actually lean towards the other option which is to not use expect at all but "proper" error handling as outlined in the docs for expect.

Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: 0 of 2 LGTMs obtained, and all files reviewed, and pending CI: Bazel Dev / macos-15, Coverage, Installation / macos-13, Installation / macos-14, Installation / macos-15, Remote / lre-cc / large-ubuntu-22.04, Remote / lre-rs / large-ubuntu-22.04, and 1 discussions need to be resolved

---

toolchain-examples/nativelink-config.json line 144 at r1 (raw file):

  }],
  "global": {
    // Defaults to 24576 = 24 * 1024.

nit: While we're at it, consider changing the file extension to json5. This gives better syntax highlighting in GitHub.

[jaroeichler]

I'd suggest that we keep this pull request as it is and discuss how we want to do error handling separately.
I also agree that tests would be desirable. In this particular case they aren't easy to implement since open file descriptors are handled differently per system.

So todos after this pull request: add tests and adjust error handling

Reviewable status: 0 of 1 LGTMs obtained, and 5 of 8 files reviewed, and pending CI: Analyze (javascript-typescript), Analyze (python), Bazel Dev / macos-15, Bazel Dev / ubuntu-24.04, Cargo Dev / macos-15, Cargo Dev / ubuntu-24.04, Coverage, Installation / macos-13, Installation / macos-14, Installation / macos-15, Installation / ubuntu-22.04, Installation / ubuntu-24.04, Local / lre-cc / ubuntu-24.04, Local / lre-rs / macos-15, Local / lre-rs / ubuntu-24.04, NativeLink.com Cloud / Remote Cache / macos-15, NativeLink.com Cloud / Remote Cache / ubuntu-24.04, Publish image, Publish nativelink-worker-init, Publish nativelink-worker-lre-cc, Remote / lre-cc / large-ubuntu-22.04, Remote / lre-rs / large-ubuntu-22.04, Web Platform Deployment / macos-15, Web Platform Deployment / ubuntu-24.04, asan / ubuntu-24.04, integration-tests (24.04), macos-15, pre-commit-checks, ubuntu-24.04, ubuntu-24.04 / stable, vale, windows-2022 / stable

[aaronmondal]

Reviewed 3 of 3 files at r2, all commit messages.
Reviewable status: 1 of 1 LGTMs obtained, and all files reviewed, and 2 discussions need to be resolved

---

nativelink-util/src/fs.rs line 143 at r2 (raw file):

/// Sets the soft nofile limit to `desired_open_file_limit` and adjusts
/// `OPEN_FILE_SEMAPHORE` accorgingly.

nit : "accordingly"

---

nativelink-util/src/fs.rs line 172 at r2 (raw file):

        }
    };
    // Can we give a better estimate?

nit: Make this "TODO(jaroeichler): Can we. .."

[aaronmondal]

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: complete! 1 of 1 LGTMs obtained, and all files reviewed