[multiple-commpaths] Allows colon-separated commpath list - #196
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. |
| continue-on-error: true | ||
| run: | | ||
| failed=0 | ||
| for node in slurm-head slurm-node-1 slurm-node-2 slurm-node-3 slurm-node-4; do |
There was a problem hiding this comment.
| for node in slurm-head slurm-node-1 slurm-node-2 slurm-node-3 slurm-node-4; do | |
| for node in slurm-plugin-head slurm-plugin-node-1 slurm-plugin-node-2 slurm-plugin-node-3 slurm-plugin-node-4; do |
I think this will need to use the container name as defined in the docker-compose.yml, which for the plugin variant includes -plugin- in the name (see for example docker exec slurm-plugin-head above)
|
|
||
| static char* exitSocketPath(spindle_args_t *params) | ||
| { | ||
| char *realized_dir, *socket_path = NULL; |
There was a problem hiding this comment.
| char *realized_dir = NULL, *socket_path = NULL; |
With the change below, if getFirstValidPath returns -1, we goto done without ever assigning to realized_dir; before these changes, we would always assign to realized_dir. Since the done block checks the value of realized_dir, we need to initialize it to NULL.
| snprintf(options_str, 32, "%lu", (unsigned long) params->opts); | ||
| string options(options_str); | ||
|
|
||
| getFirstValidPath( params->commpaths, &( params->commpath ), params->number ); |
There was a problem hiding this comment.
Need to handle the case where getFirstValidPath returns -1
| [COMMPATH=${withval}], | ||
| [COMMPATH=$DEFAULT_LOC]) | ||
| AC_ARG_WITH(commpaths, | ||
| [AS_HELP_STRING([--with-compaths=DIR],[Colon-separated list of candidate paths for back-end communication and housekeeping])], |
There was a problem hiding this comment.
| [AS_HELP_STRING([--with-compaths=DIR],[Colon-separated list of candidate paths for back-end communication and housekeeping])], | |
| [AS_HELP_STRING([--with-commpaths=DIR],[Colon-separated list of candidate paths for back-end communication and housekeeping])], |
The name of the argument is commpaths but the help string is --with-compaths, with one m instead of two.
| #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
| return NULL; | ||
| } | ||
|
|
||
| result = spindle_mkdir(dir); |
There was a problem hiding this comment.
The old code called spindle_mkdir to ensure that the session directory exists, but the new code doesn't. Do we still need the spindle_mkdir?
| [AS_HELP_STRING([--with-compath=DIR],[Back-end directory for communication and housekeeping])], | ||
| [COMMPATH=${withval}], | ||
| [COMMPATH=$DEFAULT_LOC]) | ||
| AC_ARG_WITH(commpaths, |
There was a problem hiding this comment.
We might want to still accept the old commpath (singular) form so that existing configure arguments continue to work
There was a problem hiding this comment.
The above line increased the size of the buffer, but this line still null-terminates at the original size; is that correct?
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.