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: implement takeover for nydusd fusedev daemon #1434

Closed
wants to merge 1 commit into from

Conversation

loheagn
Copy link
Contributor

@loheagn loheagn commented Sep 26, 2023

Relevant Issue (if applicable)

#1421

Details

This patch implements the save and restore functions in the fusedev_upgrade in the service crate.

To do this,

  • This patch adds a new crate named nydus-upgrade into the workspace. The nydus-upgrade crate has some util functions help to do serialization and deserialization for the rust structs using the versionize and snapshot crates. The crate also has a trait named StorageBackend which can be used to store and restore fuse session fds and state data for the upgrade action, and there's also an implementation named UdsStorageBackend which uses unix domain socket to do this.
  • as we have to use the same fuse session connection, backend file system mount commands, Vfs to re-mount the rafs for the new daemon (created for "hot upgrade" or failover), this patch add a new struct named FusedevState to hold these information. The FusedevState is serialized and stored into the UdsStorageBackend (which happens in the save function in the fusedev_upgrade module) before the new daemon is created, and the FusedevState is deserialized and restored from the UdsStorageBackend (which happens in the restore function in the fusedev_upgrade module) when the new daemon is triggered by takeover.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation Update (if none of the other choices apply)

Checklist

Go over all the following points, and put an x in all the boxes that apply.

  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.

@loheagn
Copy link
Contributor Author

loheagn commented Sep 26, 2023

As this patch depend on the cloud-hypervisor/fuse-backend-rs#149 ,
I keep the fuse-backend-rs crate use the local repo in my local devbox, and this pr may keep draft until the cloud-hypervisor/fuse-backend-rs#149 merged.

@imeoer
Copy link
Collaborator

imeoer commented Sep 27, 2023

@ccx1024cc Can we test the compatibility with the internal nydus implementation?

@codecov
Copy link

codecov bot commented Sep 27, 2023

Codecov Report

Merging #1434 (ff79cdb) into master (d7b1851) will decrease coverage by 0.21%.
The diff coverage is 19.60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1434      +/-   ##
==========================================
- Coverage   46.41%   46.20%   -0.21%     
==========================================
  Files         123      127       +4     
  Lines       38745    39033     +288     
  Branches    38745    39033     +288     
==========================================
+ Hits        17983    18036      +53     
- Misses      19789    20024     +235     
  Partials      973      973              
Files Coverage Δ
api/src/http.rs 0.00% <ø> (ø)
service/src/fusedev.rs 0.00% <ø> (ø)
upgrade/src/lib.rs 100.00% <100.00%> (ø)
upgrade/src/backend/mod.rs 98.30% <98.30%> (ø)
service/src/lib.rs 75.64% <0.00%> (ø)
service/src/fs_service.rs 50.21% <0.00%> (-2.47%) ⬇️
upgrade/src/backend/unix_domain_socket.rs 0.00% <0.00%> (ø)
upgrade/src/persist.rs 0.00% <0.00%> (ø)
service/src/upgrade.rs 0.00% <0.00%> (ø)

... and 3 files with indirect coverage changes

@loheagn loheagn force-pushed the takeover-1 branch 3 times, most recently from 2a8ab23 to ed7df0c Compare September 28, 2023 07:05
This patch implements the `save` and `restore` functions in the `fusedev_upgrade` in the service create.
To do this,
- This patch add a new create named `nydus-upgrade` into the workspace. The `nydus-upgrade` create has some util functions help to do serialization and deserialization for the rust structs using the versionize and snapshot crates. The crate also has a trait named `StorageBackend` which can be used to store and restore fuse session fds and state data for the upgrade action, and there's also an implementation named `UdsStorageBackend` which uses unix domain socket to do this.
- as we have to use the same fuse session connection, backend file system mount commands, Vfs to re-mount the rafs for the new daemon (created for "hot upgrade" or failover), this patch add a new struct named `FusedevState` to hold these information. The `FusedevState` is serialized and stored into the `UdsStorageBackend` (which happens in the `save` function in the `fusedev_upgrade` module) before the new daemon is created, and the `FusedevState` is deserialized and restored from the `UdsStorageBackend` (which happens in the `restore` function in the `fusedev_upgrade` module) when the new daemon is triggered by `takeover`.

Signed-off-by: Nan Li <loheagn@icloud.com>
Signed-off-by: linan.loheagn3 <linan.loheagn3@bytedance.com>
@kevinXYin
Copy link
Contributor

Thanks for the nice job! Can this work with the nydus-snapshotter ? I also have plans to support takeover feature for fscache mode, I think there are some process that can be reused.

.session
.lock()
.unwrap()
.get_fuse_file()
Copy link
Contributor

Choose a reason for hiding this comment

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

If nydusd exit when there are inflight fuse requests, IO hung occurs because the mainline kernel does not support resending inflight fuse requests. If we want to support takeover for fusedev, how about store the fuse fd created by ioctl FUSE_DEV_IOC_CLONE, then when nydusd exit, the inflight requests will be dropped instead of blocking IO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Get.

How about implementing this function which clones the fuse fd in fuse-backend-rs crate? For example, add a method named clone_fuse_file to FuseSession?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if macos can support clone, maybe we need to keep the API more generic.
Any suggestions? @imeoer @jiangliu

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe make it as a built-in feature in fuse-backend-rs? WDYT cc @jiangliu ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure, let's get fuse-backend-rs ready for this:)

Copy link
Contributor Author

@loheagn loheagn Nov 13, 2023

Choose a reason for hiding this comment

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

Sure, let's get fuse-backend-rs ready for this:)

@imeoer @jiangliu I might be able to help with this :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@loheagn
Copy link
Contributor Author

loheagn commented Oct 6, 2023

Thanks for the nice job! Can this work with the nydus-snapshotter ? I also have plans to support takeover feature for fscache mode, I think there are some process that can be reused.

Here is a pr containerd/nydus-snapshotter#520 for nydus-snapshotter to do failover and hot upgrade. This pr adds some check code to ensure the upgrade process be done for all the old version nydus daemons, especially the deamons created during the upgrade stage.

I have tested the failover feature and more test cases should be added.

@kevinXYin
Copy link
Contributor

Thanks for the nice job! Can this work with the nydus-snapshotter ? I also have plans to support takeover feature for fscache mode, I think there are some process that can be reused.

Here is a pr containerd/nydus-snapshotter#520 for nydus-snapshotter to do failover and hot upgrade. This pr adds some check code to ensure the upgrade process be done for all the old version nydus daemons, especially the deamons created during the upgrade stage.

I have tested the failover feature and more test cases should be added.

As I know , the nydus-snapshotter already support failover and hot upgrade workflow.

@loheagn loheagn marked this pull request as ready for review November 10, 2023 05:31
@loheagn loheagn requested a review from a team as a code owner November 10, 2023 05:31
@loheagn loheagn requested review from bergwolf, liubin and adamqqqplay and removed request for a team November 10, 2023 05:31
@kevinXYin
Copy link
Contributor

I will send another pr also support takeover for fscache , which fold your patch , then we can start reviewing full takeover feature.

@bergwolf
Copy link
Member

bergwolf commented Dec 6, 2023

@kevinXYin The feature has been pending for too long. Let's do it step by step, by merging the fuse takeover first, and then you can proceed with fscache takeover.

@loheagn
Copy link
Contributor Author

loheagn commented Dec 8, 2023

@kevinXYin The feature has been pending for too long. Let's do it step by step, by merging the fuse takeover first, and then you can proceed with fscache takeover.

@bergwolf #1487 has already contain the commits in this pr. This pr can be closed.

@loheagn loheagn closed this Dec 8, 2023
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.

5 participants