[multiple-commpaths] Allows colon-separated commpath list - #196
[multiple-commpaths] Allows colon-separated commpath list#196rountree wants to merge 10 commits into
Conversation
2d223fe to
8c7c63f
Compare
Replaces --commpath with --commpaths. Takes a colon-separated list of commpaths. The choice of which of those to use is made per-server. The first workable path is selected. The paths may differ from server to server: unlike --cachepaths, there is no consensus process.
b184a51 to
1072473
Compare
|
@rountree - Are you working on fixing the CI failures? |
07e1a40 to
1072473
Compare
|
@mplegendre Yes. What's queued up for this coming Friday is:
|
See ci-artifacts branch/PR for description. The workflows need be added here as the files are required to be both in the default branch as well as the branch using the workflows.
44a204f to
bd856fa
Compare
|
Debugging notes: Running Running Running Running Running At this point GitHub actions became unusably slow. The good news is that the podman port can replicate the problem on rzadams with a 4% failure rate. |
| #if !defined(COMMPATH) | ||
| #error COMMPATH must be defined in config.h | ||
| #endif | ||
| #define SESSIONPATH "/tmp/spindle/session" |
There was a problem hiding this comment.
I'm not sure we should hard-code a path for the session directory
There was a problem hiding this comment.
This one got complicated quickly. The naive implementation (picking a path from --commpaths) requires the same --commpaths parameter used for the session initialization job be present in every job to be run under that session, and also requires that the state of the candidate paths doesn't change of the course of the session (e.g., if the mount described by the first path in the list was initially down but came back online later, jobs fail).
Possible fix is to have a dedicated --sessionpaths parameter that's evaluated only at session start, and the SESSION_ID returned by the initializing job embeds the path to use. If no --sessionpaths are working, then fall back to commpaths, and eventually $TMPDIR.
Will take this up in a separate PR.
For now, change the hard-coded path to be based on $TMPDIR.
For the slurm-plugin case, fix copy/paste error by updating container names to those found in docker-compose.yml. (h/t Nick) Co-authored-by: Nicholas Chaimov <nchaimov@gmail.com>
Typo fix. h/t Nick. Co-authored-by: Nicholas Chaimov <nchaimov@gmail.com>
h/t Nick. Co-authored-by: Nicholas Chaimov <nchaimov@gmail.com>
src/client/client_comlib/client_api.c message.data (via buffer) is allocated 2*(MAX_PATH_LEN+1) bytes on the stack. message.data now truncated to 2*(MAX_PATH_LEN+1)-1 after the message is populated. src/fe/startup/parse_launcher.cc Both getFirstValidPath() and getApplicationArgsFE() were ignoring return values. Both now immediately return -1 on error, and the enclosing funciton signature has been converted from void to int.
Handles configure-, build-, and runtime options.
|
@nchaimov Ready for review. |
nchaimov
left a comment
nchaimov
left a comment
There was a problem hiding this comment.
It looks like this addresses all of my comments from the previous review
4 participants
Per-server selection, first path in
--commpathsthat works for that server gets picked. No consensus process, as the commpath is not visible outside of the server and its particular clients.