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

feat(csi driver): make mount and unmounts non-blocking #888

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Abhinandan-Purkait
Copy link
Member

@Abhinandan-Purkait Abhinandan-Purkait commented Nov 23, 2024

  • Adds the mount utils to frontend and backend sys mount calls via a child process
  • Makes the currently mount and umount calls async

@Abhinandan-Purkait
Copy link
Member Author

bors try

bors-openebs-mayastor bot pushed a commit that referenced this pull request Nov 23, 2024
@bors-openebs-mayastor
Copy link

try

Build failed:

@Abhinandan-Purkait
Copy link
Member Author

bors try

bors-openebs-mayastor bot pushed a commit that referenced this pull request Nov 23, 2024
@bors-openebs-mayastor
Copy link

try

Build failed:

@Abhinandan-Purkait
Copy link
Member Author

bors try

bors-openebs-mayastor bot pushed a commit that referenced this pull request Nov 23, 2024
@Abhinandan-Purkait
Copy link
Member Author

Abhinandan-Purkait commented Nov 23, 2024

Testing:

  1. 150 Volumes creation followed deletion all at once.
  2. Randomly killed(force) the csi node pods, while operations were in flight.

Observations:

  1. All operations completed eventually.
  2. No stale entries found eventually.
  3. Did not see zombie processes :-;

@bors-openebs-mayastor
Copy link

try

Build failed:

mount = mount.data(data_str);
}

mount
Copy link
Contributor

Choose a reason for hiding this comment

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

wonder if we need this at all anymore, if it's not safe to call ioctl directly, maybe we can simply call the mount binary? Though it might be safer to do this first until we figure out how to migrate from this to mount/unmount arguments properly.

Copy link
Member Author

Choose a reason for hiding this comment

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

With using binaries directly, the errors become a bit vague, but anyhow yeah we can do that. For now kept it as it was to keep the same behaviour.

use sys_mount::{FilesystemType, Mount, MountFlags, UnmountFlags};

/// Calls sys mount's mount in a spawn blocking.
pub(crate) async fn mount(
Copy link
Contributor

Choose a reason for hiding this comment

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

If the mount/unmount calls get stuck due to the device being in a bad state, how are you ensuring you don't keep stacking up new processes trying to the same operation?

Copy link
Member Author

Choose a reason for hiding this comment

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

In my testing I did not see any process stacking up.

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't the ioctl get stuck? Which hangs the server. I thought this was the issue causing the the lockups.

Copy link
Contributor

Choose a reason for hiding this comment

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

When the problem happens(repro), have you checked the host's dmesg if it shows any softlockup or oops message from kernel? In that case the host is already unreliable.

@Abhinandan-Purkait
Copy link
Member Author

bors try

bors-openebs-mayastor bot pushed a commit that referenced this pull request Nov 25, 2024
@bors-openebs-mayastor
Copy link

try

Build succeeded:

@Abhinandan-Purkait Abhinandan-Purkait marked this pull request as draft November 30, 2024 15:00
@Abhinandan-Purkait Abhinandan-Purkait force-pushed the non-blocking-mnt branch 3 times, most recently from 2111c90 to ae92ce5 Compare December 3, 2024 06:39
@Abhinandan-Purkait Abhinandan-Purkait marked this pull request as ready for review December 3, 2024 06:39
…ount calls by spwaning process

Signed-off-by: Abhinandan Purkait <purkaitabhinandan@gmail.com>
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.

3 participants