Arvados

21223-arv-mount-nofile @ a5ea331da0b2e58c686c490f4274ecadccc67355

developer-run-tests: #3925

21223-arv-mount-nofile @ 7982e6ae73cc314954a86514bf54b10c38ee592d

developer-run-tests: #3926

Peter Amstutz wrote in #note-5:

21223-arv-mount-nofile @ 7982e6ae73cc314954a86514bf54b10c38ee592d

developer-run-tests: #3926

re-run failed: developer-run-tests-remainder: #4128

21223-arv-mount-nofile @ 7982e6ae73cc314954a86514bf54b10c38ee592d

Tests linked above

• All agreed upon points are implemented / addressed.
  • comments
• Anything not implemented (discovered or discussed during work) has a follow-up story.
  • chooses RLIMIT_NOFILE based on --file-cache
• Code is tested and passing, both automated and manual, what manual testing was done is described
  • tested manually, fixed a minor logging issue and made it so that RLIMIT_NOFILE is logged on normal operation and confirmed the value was set as expected.
• Documentation has been updated.
  • n/a
• Behaves appropriately at the intended scale (describe intended scale).
  • makes it possible to have more than 1280 cache blocks, so that we can cache inputs in the 100s of gigabytes range
• Considered backwards and forwards compatibility issues between client and server.
  • n/a
• Follows our coding standards and GUI style guidelines.
  • yes

This is very simple, if an explicit --file-cache is provided, ensure that RLIMIT_NOFILE is set appropriately so that the desired size of --file-cache is actually possible.

Peter Amstutz wrote in #note-7:

21223-arv-mount-nofile @ 7982e6ae73cc314954a86514bf54b10c38ee592d

Is it possible to please get a few tests for this? It looks like it should be straightforward to mock resource.setrlimit and check it's called correctly for no/low/high --file-cache.

Should we add more handling the case where the NOFILE limit we want exceeds the hard limit? The current code takes care not to request more than that, but it seems like that situation would be confusing for the user the way exceeding the soft limit is now: they're not actually gonna get the cache size they requested. We could at least warn about it or something.

Casting to int is fine but for the future if you want it, you can get C-style integer division with the // operator.

Thanks.

Brett Smith wrote in #note-8:

Peter Amstutz wrote in #note-7:

21223-arv-mount-nofile @ 7982e6ae73cc314954a86514bf54b10c38ee592d

Is it possible to please get a few tests for this? It looks like it should be straightforward to mock resource.setrlimit and check it's called correctly for no/low/high --file-cache.

Added test_default_file_cache, test_small_file_cache, test_large_file_cache and test_file_cache_hard_limit.

Should we add more handling the case where the NOFILE limit we want exceeds the hard limit? The current code takes care not to request more than that, but it seems like that situation would be confusing for the user the way exceeding the soft limit is now: they're not actually gonna get the cache size they requested. We could at least warn about it or something.

I added a warning.

Casting to int is fine but for the future if you want it, you can get C-style integer division with the // operator.

Thanks.

21223-arv-mount-nofile @ a05e443dbfcde94651afe783e633b08d79e2b6d1

developer-run-tests: #3950

Peter Amstutz wrote in #note-10:

21223-arv-mount-nofile @ a05e443dbfcde94651afe783e633b08d79e2b6d1

This LGTM, thank you.