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

curvefs/client: cancel the warm-up task in the warm-up queue #2019

Open
wuhongsong opened this issue Nov 3, 2022 · 31 comments
Open

curvefs/client: cancel the warm-up task in the warm-up queue #2019

wuhongsong opened this issue Nov 3, 2022 · 31 comments
Labels

Comments

@wuhongsong
Copy link
Contributor

wuhongsong commented Nov 3, 2022

Is your feature request related to a problem? (你需要的功能是否与某个问题有关?)

cancel the warm-up task in the warm-up queue

Describe the solution you'd like (描述你期望的解决方法)

  1. what is warmup?

Before we are ready to access the file, we can pre-load the data locally

  1. how to warmup

Iterate over all files and download all s3 objects according to the chunk information in the inode

  1. What are we going to do with this task

all warm-up task is store in the warmUpTasks_ queue which is the member of class FuseClient, so if we had received the cmd to cancel the tash, we can remove it

@wuhongsong
Copy link
Contributor Author

anybody want do it?

@Tom-CaoZH
Copy link

Hello, Can I try this one?

@ilixiaocui
Copy link
Contributor

Hello, Can I try this one?

Okay, thanks for contributing to curve.

@Cyber-SiKu
Copy link
Contributor

Hello, Can I try this one?

you can take a look:
#2234

@Tom-CaoZH
Copy link

Hello, Can I try this one?

you can take a look: #2234

Thanks for your suggestions.

@Tom-CaoZH
Copy link

@Cyber-SiKu Hi, It seems that #2234 is still under construction, so maybe I need to wait until you done, then I will implement this feature based on your code.

After viewing the related code(mainly the modified version), I roughly know how to implement this and let me give you a quick idea here so that you can correct me timely. In your code(the modified version), the WarmupFileList represents the task and the WarmupInodes represnts the needed warm-up files in a task. So If I need to cancel the warm-up task in the warm-up queue, maybe I just need to add a function in 'WarmupManager' and its derived class(now here is one: 'WarmupManagerS3Impl'). This function is used to remove the canceled task. And Also maybe I need to add a wrapper in 'FuseClient' class so that it can be more convenient. The upper-layer application needs provide the inode of the needed-cancel task.

@Cyber-SiKu
Copy link
Contributor

Cyber-SiKu commented Feb 7, 2023

@Cyber-SiKu Hi, It seems that #2234 is still under construction, so maybe I need to wait until you done, then I will implement this feature based on your code.

After viewing the related code(mainly the modified version), I roughly know how to implement this and let me give you a quick idea here so that you can correct me timely. In your code(the modified version), the WarmupFileList represents the task and the WarmupInodes represnts the needed warm-up files in a task. So If I need to cancel the warm-up task in the warm-up queue, maybe I just need to add a function in 'WarmupManager' and its derived class(now here is one: 'WarmupManagerS3Impl'). This function is used to remove the canceled task. And Also maybe I need to add a wrapper in 'FuseClient' class so that it can be more convenient. The upper-layer application needs provide the inode of the needed-cancel task.

heroes see alike.
The warm-up task is added by setting xattr, and functions such as cancellation can also be implemented in the same way.

@wuhongsong
Copy link
Contributor Author

@Cyber-SiKu

@Cyber-SiKu
Copy link
Contributor

Cyber-SiKu commented Mar 13, 2023

@Tom-CaoZH 2234 has been merged, can you continue?

@Cyber-SiKu
Copy link
Contributor

@Tom-CaoZH Are you still interested in this issue?

@Tom-CaoZH
Copy link

@Cyber-SiKu Yes, but unfortunately, I have to finish my paper first as it is due on 3.30. I have been pretty busy lately , before the paper, I just finished my final exam, so I didn't have enough time to work on the project these days. But I believe I can come back as I finish the paper.

@Cyber-SiKu
Copy link
Contributor

@Cyber-SiKu Yes, but unfortunately, I have to finish my paper first as it is due on 3.30. I have been pretty busy lately , before the paper, I just finished my final exam, so I didn't have enough time to work on the project these days. But I believe I can come back as I finish the paper.

ok

@Tom-CaoZH
Copy link

Hello @Cyber-SiKu , I have returned and reviewed the latest code. I have some ideas to implement it. Firstly, I plan to implement RemoveWarmupFilelist and RemoveWarmupFile in WarmupManagerS3Impl. Next, I will create an interface in fuse_client.h to support this feature and also write a test in test_fuse_s3_client.cpp to ensure its correctness. Then, I will proceed with implementing the related functions in curve_fuse_op.cpp. I estimate that it may take me several days to complete this work, and I will keep you updated on my progress.

@Cyber-SiKu
Copy link
Contributor

Hello @Cyber-SiKu , I have returned and reviewed the latest code. I have some ideas to implement it. Firstly, I plan to implement RemoveWarmupFilelist and RemoveWarmupFile in WarmupManagerS3Impl. Next, I will create an interface in fuse_client.h to support this feature and also write a test in test_fuse_s3_client.cpp to ensure its correctness. Then, I will proceed with implementing the related functions in curve_fuse_op.cpp. I estimate that it may take me several days to complete this work, and I will keep you updated on my progress.

Thanks for your work, mainly want to ask if you are still continuing or have any difficulties.

@Tom-CaoZH
Copy link

@Cyber-SiKu Yes, I am still continuing, I am almost done. I am a bit confused about what kind of tests I need to write in test_fuse_s3_client.cpp, can you give me some hints?

@Tom-CaoZH
Copy link

Also, I want to ask when I implement RemoveWarmupFile, it seems that I need to implement two types. one for fuse_ino_t which will remove the whole thread for the fuse_inode. another for only certain path or task which I just remove the task from the related thread. I want to verify whether this is right?

@Cyber-SiKu
Copy link
Contributor

@Cyber-SiKu Yes, I am still continuing, I am almost done. I am a bit confused about what kind of tests I need to write in test_fuse_s3_client.cpp, can you give me some hints?

Add a warm-up task (and then block the warm-up task), and then delete and recycle resources normally.
In order to block the warmup task, you can inherit the WarmupManager class, and then rewrite the method of adding the warmup task.

@Cyber-SiKu
Copy link
Contributor

Also, I want to ask when I implement RemoveWarmupFile, it seems that I need to implement two types. one for fuse_ino_t which will remove the whole thread for the fuse_inode. another for only certain path or task which I just remove the task from the related thread. I want to verify whether this is right?

The warm-up task takes the inode that initiated the warm-up task as the primary key, and all resources (including thread pools and other resources) that match the primary key need to be deleted. If you only delete the corresponding thread pool, other resources will be resolved by the scanning thread and generate a new thread pool.

@zhanghuidinah
Copy link
Member

@xzt1590
Copy link

xzt1590 commented Apr 20, 2023

我可以试试吗

@Cyber-SiKu
Copy link
Contributor

我可以试试吗

yep

@xzt1590
Copy link

xzt1590 commented Apr 21, 2023

hello,can I change the issue?

@Xinlong-Chen
Copy link
Contributor

I am interested in this task, please assign it to me, thanks. @Cyber-SiKu

@Cyber-SiKu
Copy link
Contributor

Cyber-SiKu commented May 17, 2023

hello,can I change the issue?

@xzt1590 What are you trying to change?

@Cyber-SiKu
Copy link
Contributor

I am interested in this task, please assign it to me, thanks. @Cyber-SiKu

welcome

@opencurveadmin
Copy link
Collaborator

ping @Xinlong-Chen @Tom-CaoZH

What progress has been made in solving this issue?

@Xinlong-Chen
Copy link
Contributor

ping @Xinlong-Chen @Tom-CaoZH

What progress has been made in solving this issue?

be busy with ospp task, will do it in this month~

@ken90242
Copy link
Contributor

ken90242 commented Jul 24, 2023

@Cyber-SiKu Yes, I am still continuing, I am almost done. I am a bit confused about what kind of tests I need to write in test_fuse_s3_client.cpp, can you give me some hints?

Add a warm-up task (and then block the warm-up task), and then delete and recycle resources normally. In order to block the warmup task, you can inherit the WarmupManager class, and then rewrite the method of adding the warmup task.

How does the "overwrite method of adding the warmup task" enable us to block the thread pool?
Asking because it's essential to block,
and check if the cancellation effectively removes the stuck task related to the inode.

Take a test case for a warmup file cancellation operation as an example:
(1) Add the warmup file.
(2) Enqueue tasks into the thread pool.
(3) Verify if tasks existed in the thread pool
(Tasks are stuck in the thread pool)
(4) Cancel the warmup file.
(5) Verify that the tasks no longer exist in the thread pool.

@Cyber-SiKu

@Cyber-SiKu
Copy link
Contributor

@Cyber-SiKu Yes, I am still continuing, I am almost done. I am a bit confused about what kind of tests I need to write in test_fuse_s3_client.cpp, can you give me some hints?

Add a warm-up task (and then block the warm-up task), and then delete and recycle resources normally. In order to block the warmup task, you can inherit the WarmupManager class, and then rewrite the method of adding the warmup task.

How does the "overwrite method of adding the warmup task" enable us to block the thread pool? Asking because it's essential to block, and check if the cancellation effectively removes the stuck task related to the inode.

Take a test case for a warmup file cancellation operation as an example: (1) Add the warmup file. (2) Enqueue tasks into the thread pool. (3) Verify if tasks existed in the thread pool (Tasks are stuck in the thread pool) (4) Cancel the warmup file. (5) Verify that the tasks no longer exist in the thread pool.

@Cyber-SiKu

I am just giving an example, blocking is to ensure that the task is still there when it is canceled.
You can also use the mock function to set the return value without blocking.

@ken90242
Copy link
Contributor

ken90242 commented Jul 24, 2023

@Cyber-SiKu Yes, I am still continuing, I am almost done. I am a bit confused about what kind of tests I need to write in test_fuse_s3_client.cpp, can you give me some hints?

Add a warm-up task (and then block the warm-up task), and then delete and recycle resources normally. In order to block the warmup task, you can inherit the WarmupManager class, and then rewrite the method of adding the warmup task.

How does the "overwrite method of adding the warmup task" enable us to block the thread pool? Asking because it's essential to block, and check if the cancellation effectively removes the stuck task related to the inode.
Take a test case for a warmup file cancellation operation as an example: (1) Add the warmup file. (2) Enqueue tasks into the thread pool. (3) Verify if tasks existed in the thread pool (Tasks are stuck in the thread pool) (4) Cancel the warmup file. (5) Verify that the tasks no longer exist in the thread pool.
@Cyber-SiKu

I am just giving an example, blocking is to ensure that the task is still there when it is canceled. You can also use the mock function to set the return value without blocking.

@Cyber-SiKu I am not quite sure how to apply the mock function to the testing (in terms of ensuring that the task is still in the thread pool).
Could you please provide an example to help me understand better?


Update here is my application link: https://ask.opencurve.io/t/topic/168

@caoxianfei1
Copy link
Contributor

@ken90242 Got it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests