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

Add resource manager support for Flux #798

Merged
merged 2 commits into from
Sep 25, 2023
Merged

Add resource manager support for Flux #798

merged 2 commits into from
Sep 25, 2023

Conversation

wangvsa
Copy link
Collaborator

@wangvsa wangvsa commented Sep 19, 2023

Description

Tioga uses Flux to schedule jobs and has limited support for srun. Also Tioga does not have the scontrol command, which we use to retrieve the allocated node list.

This PR includes native support for Flux. It uses flux run to run clients and servers and uses flux resource to retrieve the number of nodes and the node list.
PS: flux resource returns a condensed node list, e.g., tioga[3-10, 12, 14]. The existing parse_hostfile() function can't handle this format, I added some code to parse it manually.

How Has This Been Tested?

Tested on Tioga with 1, 2, 4 nodes. Also tested unifyfs-ls, unifyfs-stage and stage-in/out features.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Testing (addition of new tests or update to current tests)
  • Documentation (a change to man pages or other documentation)

Checklist:

  • My code follows the UnifyFS code style requirements.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All commit messages are properly formatted.

TODO

Unlike slurm where SLURM_JOBID can be used to determine a slurm allocation, flux only sets environment variables such as FLUX_JOB_ID for each flux run job (a flux job is similar to a slurm step). At the time of executing unifyfs (batch level), those variables have not been set yet.

A short flux script example:

#!/bin/bash
# flux: -N4 -n256 -t 5m
# flux: --job-name="UnifyFS"
# flux: --queue="pdebug"

export UNIFYFS_LOGIO_SPILL_DIR=/tmp
export UNIFYFS_LOG_DIR=`pwd`
export UNIFYFS_LOG_VERBOSITY=3
export UNIFYFS_MOUNTPOINT=/unifyfs

# Here FLUX_JOB_ID are FLUX_JOB_NNODES are not set

unifyfs start -d --stage-in=`pwd`/manifest-in.txt --share-dir=`pwd`
flux run -n128 -N4 -c1 $UNIFYFS_DIR/libexec/write-static -m /unifyfs -f myTestFile
flux run -n128 -N4 -c1 $UNIFYFS_DIR/libexec/write-gotcha -f workflowTestFile
flux run -n128 -N4 -c1 $UNIFYFS_DIR/libexec/read-gotcha  -f workflowTestFile
flux run -n16 -N4 -c1 unifyfs-stage --parallel --status-file=/tmp/stage-out-status.txt `pwd`/manifest-out.txt 
unifyfs-ls 
unifyfs terminate --cleanup

As a result, currently I use FLUX_EXEC_PATH to determine if the system has flux scheduler. I feel this is not optimal but I couldn't figure out a better way.

Signed-off-by: Chen Wang <wangvsa@gmail.com>
pclose(pipe_fp);

// remove the trailing ']'
nodelist_str[strlen(nodelist_str)-1] = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the trailing ']' still here when only allocating one node?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's missing, actually:

>>: flux alloc -q pdebug -N 1
>>: flux resource list --states=free -no '{nodelist}'
tioga20

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you want to avoid parsing directly, another option is to rely on /bin/hostlist.

>>: /bin/hostlist -e 'tioga[18-19,21,32]'
tioga18,tioga19,tioga21,tioga32
>>:  /bin/hostlist -e 'tioga20'
tioga20

Though I think that command might just be available on LLNL systems and is not distributed with flux.

I think I've seen that flux has some python packages that parse hostlists, too. I can dig that up if you're interested.

I'm also fine with parsing the hostlist directly.

Copy link
Collaborator Author

@wangvsa wangvsa Sep 25, 2023

Choose a reason for hiding this comment

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

It turns out the last character of the buffer returned from fgets is either \n or EOF. So my code actually removes that instead of ], and it just happens that the rest of the code still works fine even with ] in the buffer. Anyway, I have fixed this and now I will remove the last two characters all together.

Signed-off-by: Chen Wang <wangvsa@gmail.com>
@adammoody adammoody merged commit 5884ff1 into LLNL:dev Sep 25, 2023
6 checks passed
@adammoody
Copy link
Collaborator

Thanks, @wangvsa

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