Fall back to 1 core when # CPUs unavailable

[tjni]

Changelog Entry

To be copied to the draft changelog by merger:

• Fall back to assuming machine has 1 core when CPU count is unavailable.

Reviewer Checklist

• Make sure it is coming from issues/XXXX-fix-the-thing in the Toil repo, or from an external repo.
  • If it is coming from an external repo, make sure to pull it in for CI with:

    contrib/admin/test-pr otheruser theirbranchname issues/XXXX-fix-the-thing
    

  • If there is no associated issue, create one.
• Read through the code changes. Make sure that it doesn't have:
  • Addition of trailing whitespace.
  • New variable or member names in camelCase that want to be in snake_case.
  • New functions without type hints.
  • New functions or classes without informative docstrings.
  • Changes to semantics not reflected in the relevant docstrings.
  • New or changed command line options for Toil workflows that are not reflected in docs/running/{cliOptions,cwl,wdl}.rst
  • New features without tests.
• Comment on the lines of code where problems exist with a review comment. You can shift-click the line numbers in the diff to select multiple lines.
• Finish the review with an overall description of your opinion.

Merger Checklist

• Make sure the PR passes tests.
• Make sure the PR has been reviewed since its last modification. If not, review it.
• Merge with the Github "Squash and merge" feature.
  • If there are multiple authors' commits, add Co-authored-by to give credit to all contributing authors.
• Copy its recommended changelog entry to the Draft Changelog.
• Append the issue number in parentheses to the changelog entry.

[adamnovak]

I have pulled this in for testing in our branch issues/4545-cpu-count-unavailable.

@tjni What environment do you have where psutil.cpu_count(logical=True) is unavailable? Are you sure we want to fall straight back to 1 and we shouldn't try checking e.g. os.cpu_count() first?

[adamnovak]

At the end of this function we do:

    # Return the smaller of the actual thread count and the cgroup's limit, minimum 1.
    result = cast(int, max(1, min(min(affinity_size, cgroup_size), total_machine_size)))

So I think if we let total_machine_size fall back to 1, we won't ever be able to get an answer above 1, even of e.g. os.sched_getaffinity() is available and working properly.

If we really want to properly support cases where psutil.cpu_count(logical=True) is unavailable, we need to also change the way we combine the values so that we don't use the CPU count as an upper limit when it's not available.

[adamnovak]

@tjni Do you want to change they way the values get combined, or should we do it?

[tjni]

You're correct, I'm sorry I missed that case. I can work on it given that it's my odd environment triggering this corner case, and I'll add a test for this too.

My environment is building this package bundled in nixpkgs using the nix package manager. This package manager has a sandbox mode, which prevents access to a lot of system functionality (in my case, it uses the Apple sandbox). This includes access to sysctlbyname unless explicitly allowed. Aside from that, one way to fix this in nixpkgs is just to disable the tests that require going outside of this sandbox, but I noticed from psutil docs that this function could return None so thought it would be nice to fill that gap.

[adamnovak]

@tjni Are you still planning to revise this, or should we take over?

[adamnovak]

I'm going to close this in favor of the same commits in #4780 from within the repo.