Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce bind mounting (and thus --bind args) in containers. #1568

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

DailyDreaming
Copy link
Contributor

@DailyDreaming DailyDreaming commented Nov 17, 2021

There is a memory limit on the length of bind arguments one can pass in the CLI and we can hit that limit (particularly Toil, which handles directories differently and tries to export files individually). This PR stages individual input files into one directory if STAGE_SRC_DIR is specified, so that only one bind mount arg needs to be supplied on the commandline for all individual files.

This attempts to hardlink first, and, failing that, will attempt to copy the files into the staging directory.

I need to make a test. Currently os.environ.get('STAGE_SRC_DIR', os.path.join(tempfile.gettempdir(), 'cwl-stg-src-dir')) is set just to have this run through CI/CD. I'll change to os.environ.get('STAGE_SRC_DIR', None) once I make a test.

Copy link
Member

@mr-c mr-c left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @DailyDreaming ; are the tests passing for you? They aren't for me or on CI

@mr-c
Copy link
Member

mr-c commented Nov 18, 2021

@DailyDreaming I recommend using the following test to guide you

pytest -n 0 tests/test_iwdr.py::test_iwdr_permutations -s -vv

From

def test_iwdr_permutations(tmp_path_factory: Any) -> None:
and https://github.com/common-workflow-language/cwltool/blob/352daf6d53f12df4cd960c0343ef659924aa9712/tests/wf/iwdr_permutations.cwl

Here is the expected contents of log.txt

.
./eleventh
./fifth_writable_directory
./first_writable_file
./log.txt
./nineth_writable_directory_literal
./second_read_only_file
./sixth_read_only_directory
./twelfth
/my_path
/my_path/my_file_literal
/my_path/seventh_writable_directory
/my_path/tenth_writable_directory_literal
/my_path/third_writable_file
/my_other_path
/my_other_path/eighth_read_only_directory
/my_other_path/fourth_read_only_file
.
./eleventh
./fifth_writable_directory
./fifth_writable_directory/c
./first_writable_file
./log.txt
./nineth_writable_directory_literal
./second_read_only_file
./sixth_read_only_directory
./twelfth
/my_path
/my_path/my_file_literal
/my_path/seventh_writable_directory
/my_path/seventh_writable_directory/d
/my_path/tenth_writable_directory_literal
/my_path/third_writable_file
/my_other_path
/my_other_path/eighth_read_only_directory
/my_other_path/fourth_read_only_file

Right now I'm seeing the following

.
./fifth_writable_directory
./log.txt
./nineth_writable_directory_literal
./sixth_read_only_directory
/my_path
/my_path/my_file_literal
/my_path/seventh_writable_directory
/my_path/tenth_writable_directory_literal
/my_other_path
/my_other_path/eighth_read_only_directory
.
./fifth_writable_directory
./fifth_writable_directory/c
./first_writable_file
./log.txt
./nineth_writable_directory_literal
./sixth_read_only_directory
/my_path
/my_path/my_file_literal
/my_path/seventh_writable_directory
/my_path/seventh_writable_directory/d
/my_path/tenth_writable_directory_literal
/my_other_path
/my_other_path/eighth_read_only_directory

@mr-c
Copy link
Member

mr-c commented Nov 24, 2021

@tetron recommends doing this primarily only for the non-InitialWorkDirRequirement inputs (which are always read-only), and then once for each top-level InitialWorkDirRequirement entry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants