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] curvefs/client: warmup list and stop #2652

Conversation

ken90242
Copy link
Contributor

@ken90242 ken90242 commented Jul 28, 2023

What problem does this PR solve?

Issue Number:

#2244

Problem Summary:

1. warmup add file list: not overwriting the providing file list file

The original design for the "add warmup file list" will modify the contents of the provided file by replacing the mount point with the root path in every path mentioned in the file.

For example, if you execute the command curve fs warmup add --filelist /warmup/0809.list, with the mount point being /mnt, and the root path in the file system being /x, and the content of the file /warmup/0809.list being:

/mnt/abc
/mnt/def

After running the command, /warmup/0809.list will be updated to

/x/abc
/x/def

Afterward, the handling process can access the file content directly and utilize the modified paths to locate the corresponding dentries.

However, This change is non-reversible, and it can be unexpected for clients who may not anticipate such modifications. It would be better to achieve the same goal without altering the provided file content.

2. warmup stop

A functionality that allows users to cancel a warmup job and its derivatives.

3. warmup list

A functionality that allows users to list out all the running warmup jobs under the specified curvefs.


What is changed, and how does it work?

What's Changed:

  • CLI command tool (add.go)
  • All functions on the call path related to the "add warmup file list"
  • Impacted test files

How it Works:

1. warmup add file list: not overwriting the providing file list file

The CLI command tool no longer modifies the file directly. It passes the mount point and root to the fuse-client, and path replacement is handled on-the-fly by the "ScanWarmupFilelist" procedure.

2. warmup stop

According to the activity diagram below, the data structure requires cleanup with a specific dependency sequence:

File List: inode2Progress_ <- warmupFilelistDeque_ <- inode2FetchDentryPool_ <- warmupInodeDeque_ <- inode2FetchS3ObjectsPool
File: inode2Progress_ <- inode2FetchDentryPool_ <- warmupInodeDeque_ <- inode2FetchS3ObjectsPool

image
My implementation is erasing the inode entry key in the sequence of
inode2Progress_, warmupFilelistDeque_ (if file list), inode2FetchDentryPool_, warmupInodeDeque_, inode2FetchS3ObjectsPool

3. warmup list

I iterate through all warmup jobs in inode2Progress_ and use the path stored inside the inode2Progress_ to form a map (i.e., { path: progress }) solely serving the CLI output purposes.


Side effects(Breaking backward compatibility? Performance regression?):

This PR might break backward compatibility since now the user has to upgrade both their curve toolset and curvefs client at the same time.
None


Demo

  • curve fs warmup list <curvefs mount path> & curve fs warmup cancel <file>
    Kapture 2023-09-27 at 23 15 51

  • curve fs warmup list <curvefs mount path> & curve fs warmup cancel --filelist <filelist>
    Kapture 2023-09-28 at 00 17 17

  • curve fs warmup add --filelist <filelist> (not change the file list content)
    Kapture 2023-09-28 at 00 19 05

  • curve fs warmup query <file> (query still works)
    Kapture 2023-09-28 at 00 21 17


Check List

  • Relevant documentation/comments is changed or added
  • I acknowledge that all my contributions will be made under the project's license

@ken90242 ken90242 changed the title [feat] curvefs/client: warmup list and stop [WIP DO NOT MERGE] [feat] curvefs/client: warmup list and stop Jul 28, 2023
@ken90242 ken90242 force-pushed the feature/curve-fs-client-warmup-list-stop branch from 5bdf0c8 to 4afb55a Compare July 28, 2023 07:03
@ken90242 ken90242 marked this pull request as draft July 28, 2023 07:09
@ken90242 ken90242 force-pushed the feature/curve-fs-client-warmup-list-stop branch from 4afb55a to 2201c4c Compare August 7, 2023 07:51
@ken90242
Copy link
Contributor Author

ken90242 commented Aug 7, 2023

cicheck

@ken90242 ken90242 marked this pull request as ready for review August 7, 2023 08:36
@ken90242 ken90242 changed the title [WIP DO NOT MERGE] [feat] curvefs/client: warmup list and stop [feat] curvefs/client: warmup list and stop Aug 7, 2023
@ken90242 ken90242 force-pushed the feature/curve-fs-client-warmup-list-stop branch from 2201c4c to d058e72 Compare August 7, 2023 09:02
@ken90242
Copy link
Contributor Author

ken90242 commented Aug 7, 2023

cicheck

@ken90242
Copy link
Contributor Author

ken90242 commented Aug 7, 2023

@Cyber-SiKu Could you help review my PR? Thanks

Copy link
Contributor

@Cyber-SiKu Cyber-SiKu left a comment

Choose a reason for hiding this comment

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

Is this a pre-pr? Doesn't seem to be finished?

tools-v2/pkg/cli/command/curvefs/warmup/add/add.go Outdated Show resolved Hide resolved
tools-v2/pkg/cli/command/curvefs/warmup/add/add.go Outdated Show resolved Hide resolved
tools-v2/pkg/cli/command/curvefs/warmup/add/add.go Outdated Show resolved Hide resolved
curvefs/src/client/warmup/warmup_manager.h Outdated Show resolved Hide resolved
curvefs/src/client/curve_fuse_op.cpp Outdated Show resolved Hide resolved
@ken90242 ken90242 force-pushed the feature/curve-fs-client-warmup-list-stop branch from d7edd91 to 34a9d19 Compare August 8, 2023 09:20
@ken90242
Copy link
Contributor Author

ken90242 commented Aug 8, 2023

Is this a pre-pr? Doesn't seem to be finished?

@Cyber-SiKu

This is actually a completed pull request.
Any specific concerns you have about this PR?

@ken90242
Copy link
Contributor Author

ken90242 commented Aug 8, 2023

cicheck

@ken90242
Copy link
Contributor Author

ken90242 commented Aug 8, 2023

cicheck

@Cyber-SiKu
Copy link
Contributor

Is this a pre-pr? Doesn't seem to be finished?

@Cyber-SiKu

This is actually a completed pull request. Any specific concerns you have about this PR?

Haven't seen the implementation of list and stop?

@Cyber-SiKu
Copy link
Contributor

Cyber-SiKu commented Aug 8, 2023

@ken90242
Copy link
Contributor Author

ken90242 commented Aug 9, 2023

@ken90242
Copy link
Contributor Author

ken90242 commented Aug 9, 2023

Is this a pre-pr? Doesn't seem to be finished?

@Cyber-SiKu
This is actually a completed pull request. Any specific concerns you have about this PR?

Haven't seen the implementation of list and stop?

This is what I have gathered from the original issue - there don't seem to be any requirements concerning the list and stop functionalities. Is there anything I might be overlooking?

@ken90242 ken90242 force-pushed the feature/curve-fs-client-warmup-list-stop branch from c6b2605 to d4dd8de Compare August 10, 2023 08:36
@ken90242
Copy link
Contributor Author

ken90242 commented Aug 10, 2023

@Cyber-SiKu

Could you check out my latest commit?
fec1b41
I have added the warmup list functionality. Let me know if it meets the requirements. Thanks!

p.s. I haven't finalized it yet.
Im just presenting my idea so that we can identify any aspects that shouldn't be implemented this way early on.

@ken90242 ken90242 force-pushed the feature/curve-fs-client-warmup-list-stop branch 4 times, most recently from 3023e6a to c12fbac Compare August 14, 2023 08:31
@Cyber-SiKu
Copy link
Contributor

@Cyber-SiKu

Could you check out my latest commit? fec1b41 I have added the warmup list functionality. Let me know if it meets the requirements. Thanks!

p.s. I haven't finalized it yet. Im just presenting my idea so that we can identify any aspects that shouldn't be implemented this way early on.

Thanks for working on this!

@ken90242
Copy link
Contributor Author

ken90242 commented Aug 15, 2023

@Cyber-SiKu

I have finished my updates; you can find the corresponding commits below:

@ken90242
Copy link
Contributor Author

cicheck

@ken90242 ken90242 force-pushed the feature/curve-fs-client-warmup-list-stop branch from e1db6ff to d182c77 Compare August 15, 2023 17:40
@ken90242
Copy link
Contributor Author

cicheck

1 similar comment
@ken90242
Copy link
Contributor Author

cicheck

@ken90242
Copy link
Contributor Author

@Cyber-SiKu the test is passed

@ken90242 ken90242 force-pushed the feature/curve-fs-client-warmup-list-stop branch 3 times, most recently from 4dc928f to 2a69423 Compare September 28, 2023 07:26
@ken90242
Copy link
Contributor Author

cicheck

2 similar comments
@ken90242
Copy link
Contributor Author

cicheck

@ken90242
Copy link
Contributor Author

cicheck

@ken90242
Copy link
Contributor Author

cicheck

3 similar comments
@ken90242
Copy link
Contributor Author

cicheck

@ken90242
Copy link
Contributor Author

cicheck

@ken90242
Copy link
Contributor Author

cicheck

@ken90242 ken90242 force-pushed the feature/curve-fs-client-warmup-list-stop branch from 2a69423 to 163aec7 Compare October 8, 2023 08:34
@Cyber-SiKu
Copy link
Contributor

pls fix dco
rebase commits to one

@ken90242
Copy link
Contributor Author

ken90242 commented Oct 8, 2023

cicheck

one

which doc are you referring to?

Signed-off-by: ken90242 <ken90242@gmail.com>
@ken90242 ken90242 force-pushed the feature/curve-fs-client-warmup-list-stop branch from 163aec7 to f5217d3 Compare October 8, 2023 09:13
@Cyber-SiKu
Copy link
Contributor

cicheck

@Cyber-SiKu
Copy link
Contributor

cicheck

one

which doc are you referring to?

image
has passed

@ken90242
Copy link
Contributor Author

ken90242 commented Oct 8, 2023

cicheck

@ken90242
Copy link
Contributor Author

ken90242 commented Oct 9, 2023

@Cyber-SiKu anyone you would suggest to be another reviewer of this PR?

@wuhongsong
Copy link
Contributor

What problem does this PR solve?

Issue Number:

#2244

Problem Summary:

1. warmup add file list: not overwriting the providing file list file

The original design for the "add warmup file list" will modify the contents of the provided file by replacing the mount point with the root path in every path mentioned in the file.

For example, if you execute the command curve fs warmup add --filelist /warmup/0809.list, with the mount point being /mnt, and the root path in the file system being /x, and the content of the file /warmup/0809.list being:

/mnt/abc
/mnt/def

After running the command, /warmup/0809.list will be updated to

/x/abc
/x/def

Afterward, the handling process can access the file content directly and utilize the modified paths to locate the corresponding dentries.

However, This change is non-reversible, and it can be unexpected for clients who may not anticipate such modifications. It would be better to achieve the same goal without altering the provided file content.

2. warmup stop

A functionality that allows users to cancel a warmup job and its derivatives.

3. warmup list

A functionality that allows users to list out all the running warmup jobs under the specified curvefs.

What is changed, and how does it work?

What's Changed:

  • CLI command tool (add.go)
  • All functions on the call path related to the "add warmup file list"
  • Impacted test files

How it Works:

1. warmup add file list: not overwriting the providing file list file

The CLI command tool no longer modifies the file directly. It passes the mount point and root to the fuse-client, and path replacement is handled on-the-fly by the "ScanWarmupFilelist" procedure.

2. warmup stop

According to the activity diagram below, the data structure requires cleanup with a specific dependency sequence:

File List: inode2Progress_ <- warmupFilelistDeque_ <- inode2FetchDentryPool_ <- warmupInodeDeque_ <- inode2FetchS3ObjectsPool
File: inode2Progress_ <- inode2FetchDentryPool_ <- warmupInodeDeque_ <- inode2FetchS3ObjectsPool

image My implementation is erasing the inode entry key in the sequence of inode2Progress_, warmupFilelistDeque_ (if file list), inode2FetchDentryPool_, warmupInodeDeque_, inode2FetchS3ObjectsPool

3. warmup list

I iterate through all warmup jobs in inode2Progress_ and use the path stored inside the inode2Progress_ to form a map (i.e., { path: progress }) solely serving the CLI output purposes.

Side effects(Breaking backward compatibility? Performance regression?):

This PR might break backward compatibility since now the user has to upgrade both their curve toolset and curvefs client at the same time. None

Demo

  • curve fs warmup list <curvefs mount path> & curve fs warmup cancel <file>
    Kapture 2023-09-27 at 23 15 51

        [
          
        
            ![Kapture 2023-09-27 at 23 15 51](https://user-images.githubusercontent.com/3871037/271190221-95bb96c1-7dc2-4176-99be-7d43a9690e2e.gif)
          ](https://user-images.githubusercontent.com/3871037/271190221-95bb96c1-7dc2-4176-99be-7d43a9690e2e.gif)
        
        
          
            
              
            
            
              
              
            
          
          [
            
              
            
          ](https://user-images.githubusercontent.com/3871037/271190221-95bb96c1-7dc2-4176-99be-7d43a9690e2e.gif)
    
  • curve fs warmup list <curvefs mount path> & curve fs warmup cancel --filelist <filelist>
    Kapture 2023-09-28 at 00 17 17

        [
          
        
            ![Kapture 2023-09-28 at 00 17 17](https://user-images.githubusercontent.com/3871037/271202892-f4296fdc-9f68-4be9-8679-4e34149bb25e.gif)
          ](https://user-images.githubusercontent.com/3871037/271202892-f4296fdc-9f68-4be9-8679-4e34149bb25e.gif)
        
        
          
            
              
            
            
              
              
            
          
          [
            
              
            
          ](https://user-images.githubusercontent.com/3871037/271202892-f4296fdc-9f68-4be9-8679-4e34149bb25e.gif)
    
  • curve fs warmup add --filelist <filelist> (not change the file list content)
    Kapture 2023-09-28 at 00 19 05

        [
          
        
            ![Kapture 2023-09-28 at 00 19 05](https://user-images.githubusercontent.com/3871037/271203221-9ae4471b-3109-4817-ac1e-578fd23fbca8.gif)
          ](https://user-images.githubusercontent.com/3871037/271203221-9ae4471b-3109-4817-ac1e-578fd23fbca8.gif)
        
        
          
            
              
            
            
              
              
            
          
          [
            
              
            
          ](https://user-images.githubusercontent.com/3871037/271203221-9ae4471b-3109-4817-ac1e-578fd23fbca8.gif)
    
  • curve fs warmup query <file> (query still works)
    Kapture 2023-09-28 at 00 21 17

        [
          
        
            ![Kapture 2023-09-28 at 00 21 17](https://user-images.githubusercontent.com/3871037/271203716-ae8ff623-52a8-40b4-a04b-081ca010707e.gif)
          ](https://user-images.githubusercontent.com/3871037/271203716-ae8ff623-52a8-40b4-a04b-081ca010707e.gif)
        
        
          
            
              
            
            
              
              
            
          
          [
            
              
            
          ](https://user-images.githubusercontent.com/3871037/271203716-ae8ff623-52a8-40b4-a04b-081ca010707e.gif)
    

Check List

  • Relevant documentation/comments is changed or added
  • I acknowledge that all my contributions will be made under the project's license

good job.

@wuhongsong wuhongsong merged commit 874c098 into opencurve:master Oct 9, 2023
4 checks passed
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.

None yet

3 participants