Add multi-mount parallelstore support #3256

harshthakkar01 · 2024-11-13T21:41:57Z

This PR,

excludes GPU interfaces in daos config file
Add support for multiple mount for single parallelstore instance (creates systemd service for each mount)

Submission Checklist

NOTE: Community submissions can take up to 2 weeks to be reviewed.

Please take the following actions before submitting this pull request.

Fork your PR branch from the Toolkit "develop" branch (not main)
Test all changes with pre-commit in a local branch #
Confirm that "make tests" passes all tests
Add or modify unit tests to cover code changes
Ensure that unit test coverage remains above 80%
Update all applicable documentation
Follow Cluster Toolkit Contribution guidelines #

mounts

wiktorn · 2024-11-16T13:07:12Z

modules/file-system/parallelstore/scripts/mount-daos.sh

@@ -41,6 +41,17 @@ sed -i "s/#.*transport_config/transport_config/g" $daos_config
 sed -i "s/#.*allow_insecure:.*false/  allow_insecure: true/g" $daos_config
 sed -i "s/.*access_points.*/access_points: $access_points/g" $daos_config


Does the goal here is only to support multiple mounts from the server Parallelstore instance, or to support mounts from multiple Parallelstore instances?

I fear, that with multiple Parallelstore instances, the last script that will run will setup access_points in the config, and it will be the only instance mounted (but multiple times)

The goal is to have 1 parallelstore instance but able to mount to multiple mount points (possible with different mount options).

Ok, perfect. I think it is worth mentioning in the README, that module supports mounting only from Parallelstore instance, but it's possibile to mount it multiple times with different options using pre-existing-network-storage.

Probably worth mentioning in both modules.

tpdownes

I would like to know a bit more about what attempting to support multiple Parallelstore instances. In the meantime, the changes I suggest will improve reliability.

tpdownes · 2024-11-18T19:37:27Z

modules/file-system/parallelstore/scripts/mount-daos.sh

@@ -41,6 +41,17 @@ sed -i "s/#.*transport_config/transport_config/g" $daos_config
 sed -i "s/#.*allow_insecure:.*false/  allow_insecure: true/g" $daos_config
 sed -i "s/.*access_points.*/access_points: $access_points/g" $daos_config

+# Get interface names with "s0f0" suffix
+if ifconfig -a | grep 's0f0'; then


I believe that you are attempting to identify devices that are on the same PCI bus as GPUs in the A3 VM types (a3-highgpu-8g and a3-megagpu-8g). This is not a reliable mechanism by which to do that.

I believe ifconfig is not uniformly installed across OS clients. (e.g. Ubuntu 22.04)

The "tail" of the device name is the least predictable element of the automated device name.

The following command strictly identifies ethernet devices not attached to PCI slot 0, which is guaranteed to be nic0 as seen by the Cloud Console. It is also the interface guaranteed to route to the metadata server

find /sys/class/net/ -not -name 'enp0s*' -regextype posix-extended -regex '.*/enp[0-9]+s.*' -printf "%f\n" | paste -s -d ','

I believe you could this command and then test for whether it has zero length.

tpdownes · 2024-11-18T20:05:55Z

modules/file-system/parallelstore/scripts/mount-daos.sh

+mount_command="if mountpoint -q '$local_mount'; then fusermount3 -u '$local_mount'; fi; for i in {1..10}; do /bin/dfuse -m '$local_mount' --pool default-pool --container default-container --multi-user $mount_options --foreground && break; sleep 1; done"
+
+# Construct the service name with the local_mount suffix
+service_name="mount_parallelstore_${local_mount//\//_}.service"


Use systemd-escape -p "${local_mount}" to generate that part of the unit name.

tpdownes · 2024-11-18T20:09:06Z

modules/file-system/parallelstore/scripts/mount-daos.sh

 [Unit]
 Description=DAOS Mount Service
 After=network-online.target daos_agent.service


Suggested change

[Unit]

Description=DAOS Mount Service

After=network-online.target daos_agent.service

[Unit]

Description=DAOS Mount Service

After=network-online.target daos_agent.service

Before=slurmd.service

ConditionPathIsMountPoint=!${local_mount}

tpdownes · 2024-11-18T20:22:46Z

modules/file-system/parallelstore/scripts/mount-daos.sh

 # Store the mounting logic in a variable
-mount_command='for i in {1..10}; do /bin/dfuse -m '$local_mount' --pool default-pool --container default-container --multi-user '$mount_options' --foreground && break; echo \"dfuse, failed, retrying in 1 second (attempt '$i'/10)\"; sleep 1; done'
+mount_command="if mountpoint -q '$local_mount'; then fusermount3 -u '$local_mount'; fi; for i in {1..10}; do /bin/dfuse -m '$local_mount' --pool default-pool --container default-container --multi-user $mount_options --foreground && break; sleep 1; done"


I would remove this line entirely. The command should not include sleep logic.

tpdownes · 2024-11-18T20:27:40Z

modules/file-system/parallelstore/scripts/mount-daos.sh

+Restart=always
+RestartSec=1
+ExecStart=/bin/bash -c "$mount_command"
+ExecStop=fusermount3 -u '$local_mount'


Suggested change

Restart=always

RestartSec=1

ExecStart=/bin/bash -c "$mount_command"

ExecStop=fusermount3 -u '$local_mount'

Restart=on-failure

RestartSec=10

ExecStart=/bin/dfuse -m $local_mount --pool default-pool --container default-container --multi-user $mount_options --foreground

ExecStop=/usr/bin/fusermount3 -u $local_mount

After you make these changes and we discuss how multi-Parallelstore instance suport would look like, I would like to consider using systemd unit templates to avoid re-creation of multiple service files.

https://fedoramagazine.org/systemd-template-unit-files/

harshthakkar01 added the release-improvements Added to release notes under the "Improvements" heading. label Nov 13, 2024

harshthakkar01 force-pushed the ps-fix-2 branch 4 times, most recently from 727d580 to f607f80 Compare November 15, 2024 06:50

harshthakkar01 changed the title ~~Update mount parallelstore script to support multiple parallelstore~~ Add multi-mount parallelstore support Nov 15, 2024

harshthakkar01 force-pushed the ps-fix-2 branch from f607f80 to 3f01fcb Compare November 15, 2024 07:18

harshthakkar01 requested a review from tpdownes November 15, 2024 07:20

harshthakkar01 assigned tpdownes Nov 15, 2024

Update mount parallelstore script to support multiple parallelstore

02c7962

mounts

harshthakkar01 force-pushed the ps-fix-2 branch from 3f01fcb to 02c7962 Compare November 15, 2024 20:07

harshthakkar01 mentioned this pull request Nov 15, 2024

Allow customization of Parallelstore mounts #3144

Open

6 tasks

mr0re1 mentioned this pull request Nov 16, 2024

Update parallelstore mount service from oneshot to simple #3245

Closed

wiktorn reviewed Nov 16, 2024

View reviewed changes

tpdownes requested changes Nov 18, 2024

View reviewed changes

tpdownes assigned harshthakkar01 and unassigned tpdownes Nov 18, 2024

mr0re1 assigned mr0re1 and unassigned harshthakkar01 Nov 19, 2024

tpdownes assigned tpdownes and unassigned mr0re1 Nov 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add multi-mount parallelstore support #3256

Add multi-mount parallelstore support #3256

harshthakkar01 commented Nov 13, 2024 •

edited

Loading

wiktorn Nov 16, 2024

harshthakkar01 Nov 17, 2024

wiktorn Nov 18, 2024

tpdownes left a comment

tpdownes Nov 18, 2024

tpdownes Nov 18, 2024 •

edited

Loading

tpdownes Nov 18, 2024

tpdownes Nov 18, 2024

tpdownes Nov 18, 2024

tpdownes Nov 18, 2024

		@@ -41,6 +41,17 @@ sed -i "s/#.*transport_config/transport_config/g" $daos_config
		sed -i "s/#.allow_insecure:.false/ allow_insecure: true/g" $daos_config
		sed -i "s/.access_points./access_points: $access_points/g" $daos_config

Add multi-mount parallelstore support #3256

Are you sure you want to change the base?

Add multi-mount parallelstore support #3256

Conversation

harshthakkar01 commented Nov 13, 2024 • edited Loading

Submission Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tpdownes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tpdownes Nov 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

harshthakkar01 commented Nov 13, 2024 •

edited

Loading

tpdownes Nov 18, 2024 •

edited

Loading