Update SstFileManager to also smooth deletes for WAL files #2768
Conversation
|
@IslamAbdelRahman has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
| @@ -2300,7 +2298,8 @@ Status DestroyDB(const std::string& dbname, const Options& options) { | |||
| // Delete log files in the WAL dir | |||
| for (const auto& file : walDirFiles) { | |||
| if (ParseFileName(file, &number, &type) && type == kLogFile) { | |||
| Status del = env->DeleteFile(LogFileName(soptions.wal_dir, number)); | |||
| Status del = DeleteDBFile( | |||
| &soptions, LogFileName(soptions.wal_dir, number), 0, type); | |||
There was a problem hiding this comment.
I'm not sure about using path_id == 0 when wal_dir is set. Feels to me we should treat files that aren't in db_paths[0] directory the same. It might be required for correctness, too, as wal_dir might not be on the same filesystem as db_paths[0] and trash_dir.
IslamAbdelRahman
force-pushed
the
wal_sst_file_mgr
branch
from
932771d
to
953a13c
Compare
|
@IslamAbdelRahman has updated the pull request. |
There was a problem hiding this comment.
@IslamAbdelRahman has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
updated and rebased |
|
sorry I missed it |
| if (sfm && path_id == 0) { | ||
|
|
||
| if (sfm != nullptr && | ||
| ((type == kTableFile && path_id == 0) || type == kLogFile)) { |
There was a problem hiding this comment.
can you get rid of the path_id == 0 restriction now that the trash files don't need to be moved?
|
Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours has expired. Before we can review or merge your code, we need you to email cla@fb.com with your details so we can update your status. |
|
@IslamAbdelRahman Do you plan to continue with this PR? If not, I can update and land it if you don't mind |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
|
@anand1976, Yes please take it over |
Summary: WAL files are currently not subject to deletion rate limiting by DeleteScheduler. If the size of the WAL files is significant, this can cause a high delete rate on SSDs that may affect other operations. To fix it, force WAL file deletions to go through the SstFileManager. Original PR for this is #2768 Pull Request resolved: #5116 Differential Revision: D14669437 Pulled By: anand1976 fbshipit-source-id: c5f62d0640cebaa1574de841a1d01e4ce2faadf0
Summary: WAL files are currently not subject to deletion rate limiting by DeleteScheduler. If the size of the WAL files is significant, this can cause a high delete rate on SSDs that may affect other operations. To fix it, force WAL file deletions to go through the SstFileManager. Original PR for this is facebook#2768 Pull Request resolved: facebook#5116 Differential Revision: D14669437 Pulled By: anand1976 fbshipit-source-id: c5f62d0640cebaa1574de841a1d01e4ce2faadf0
|
I believe @anand1976 has already done this functionality. Assign to him so that he can verify and close it if it is the case. |
WAL files could be huge which will cause stalls if they are deleted quickly.
This patch update SstFileManager to also rate limit the deletes for WAL files