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.

Sign up for GitHub

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

Jump to bottom

Update SstFileManager to also smooth deletes for WAL files #2768

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

Update SstFileManager to also smooth deletes for WAL files #2768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 7 additions & 7 deletions db/db_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2320,10 +2320,8 @@ Status DestroyDB(const std::string& dbname, const Options& options) {
std::string path_to_delete = dbname + "/" + filenames[i];
if (type == kMetaDatabase) {
del = DestroyDB(path_to_delete, options);
} else if (type == kTableFile) {
del = DeleteSSTFile(&soptions, path_to_delete, 0);
} else {
del = env->DeleteFile(path_to_delete);
del = DeleteDBFile(&soptions, path_to_delete, 0, type);
}
if (result.ok() && !del.ok()) {
result = del;
Expand All @@ -2338,8 +2336,8 @@ Status DestroyDB(const std::string& dbname, const Options& options) {
if (ParseFileName(filenames[i], &number, &type) &&
type == kTableFile) { // Lock file will be deleted at end
std::string table_path = db_path.path + "/" + filenames[i];
Status del = DeleteSSTFile(&soptions, table_path,
static_cast<uint32_t>(path_id));
Status del = DeleteDBFile(&soptions, table_path,
static_cast<uint32_t>(path_id), type);
if (result.ok() && !del.ok()) {
result = del;
}
Expand All @@ -2357,7 +2355,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);
Copy link
Contributor

@ajkr ajkr Aug 21, 2017 •
edited

Choose a reason for hiding this comment

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

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.

if (result.ok() && !del.ok()) {
result = del;
}
Expand All @@ -2370,7 +2369,8 @@ Status DestroyDB(const std::string& dbname, const Options& options) {
for (size_t i = 0; i < archiveFiles.size(); ++i) {
if (ParseFileName(archiveFiles[i], &number, &type) &&
type == kLogFile) {
Status del = env->DeleteFile(archivedir + "/" + archiveFiles[i]);
Status del = DeleteDBFile(&soptions, archivedir + "/" + archiveFiles[i],
0, type);
if (result.ok() && !del.ok()) {
result = del;
}
Expand Down
9 changes: 2 additions & 7 deletions db/db_impl_files.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,13 +305,8 @@ bool CompareCandidateFile(const JobContext::CandidateFileInfo& first,
void DBImpl::DeleteObsoleteFileImpl(int job_id, const std::string& fname,
FileType type, uint64_t number,
uint32_t path_id) {
Status file_deletion_status;
if (type == kTableFile) {
file_deletion_status =
DeleteSSTFile(&immutable_db_options_, fname, path_id);
} else {
file_deletion_status = env_->DeleteFile(fname);
}
Status file_deletion_status =
DeleteDBFile(&immutable_db_options_, fname, path_id, type);
if (file_deletion_status.ok()) {
ROCKS_LOG_DEBUG(immutable_db_options_.info_log,
"[JOB %d] Delete %s type=%d #%" PRIu64 " -- %s\n", job_id,
Expand Down
36 changes: 27 additions & 9 deletions db/db_sst_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,14 +336,16 @@ TEST_F(DBSSTTest, RateLimitedDelete) {
auto sfm = static_cast<SstFileManagerImpl*>(options.sst_file_manager.get());
sfm->delete_scheduler()->TEST_SetMaxTrashDBRatio(1.1);

WriteOptions wo;
wo.disableWAL = true;
ASSERT_OK(TryReopen(options));
// Create 4 files in L0
for (char v = 'a'; v <= 'd'; v++) {
ASSERT_OK(Put("Key2", DummyString(1024, v)));
ASSERT_OK(Put("Key3", DummyString(1024, v)));
ASSERT_OK(Put("Key4", DummyString(1024, v)));
ASSERT_OK(Put("Key1", DummyString(1024, v)));
ASSERT_OK(Put("Key4", DummyString(1024, v)));
ASSERT_OK(Put("Key2", DummyString(1024, v), wo));
ASSERT_OK(Put("Key3", DummyString(1024, v), wo));
ASSERT_OK(Put("Key4", DummyString(1024, v), wo));
ASSERT_OK(Put("Key1", DummyString(1024, v), wo));
ASSERT_OK(Put("Key4", DummyString(1024, v), wo));
ASSERT_OK(Flush());
}
// We created 4 sst files in L0
Expand Down Expand Up @@ -403,9 +405,12 @@ TEST_F(DBSSTTest, DeleteSchedulerMultipleDBPaths) {

DestroyAndReopen(options);

WriteOptions wo;
wo.disableWAL = true;

// Create 4 files in L0
for (int i = 0; i < 4; i++) {
ASSERT_OK(Put("Key" + ToString(i), DummyString(1024, 'A')));
ASSERT_OK(Put("Key" + ToString(i), DummyString(1024, 'A'), wo));
ASSERT_OK(Flush());
}
// We created 4 sst files in L0
Expand All @@ -421,7 +426,7 @@ TEST_F(DBSSTTest, DeleteSchedulerMultipleDBPaths) {

// Create 4 files in L0
for (int i = 4; i < 8; i++) {
ASSERT_OK(Put("Key" + ToString(i), DummyString(1024, 'B')));
ASSERT_OK(Put("Key" + ToString(i), DummyString(1024, 'B'), wo));
ASSERT_OK(Flush());
}
ASSERT_EQ("4,1", FilesPerLevel(0));
Expand Down Expand Up @@ -474,14 +479,27 @@ TEST_F(DBSSTTest, DestroyDBWithRateLimitedDelete) {
// Close DB and destroy it using DeleteScheduler
Close();

int num_sst_files = 0;
int num_wal_files = 0;
std::vector<std::string> db_files;
env_->GetChildren(dbname_, &db_files);
for (std::string f : db_files) {
if (f.substr(f.find_last_of(".") + 1) == "sst") {
num_sst_files++;
} else if (f.substr(f.find_last_of(".") + 1) == "log") {
num_wal_files++;
}
}
ASSERT_GT(num_sst_files, 0);
ASSERT_GT(num_wal_files, 0);

auto sfm = static_cast<SstFileManagerImpl*>(options.sst_file_manager.get());

sfm->SetDeleteRateBytesPerSecond(1024 * 1024);
sfm->delete_scheduler()->TEST_SetMaxTrashDBRatio(1.1);
ASSERT_OK(DestroyDB(dbname_, options));
sfm->WaitForEmptyTrash();
// We have deleted the 4 sst files in the delete_scheduler
ASSERT_EQ(bg_delete_file, 4);
ASSERT_EQ(bg_delete_file, num_sst_files + num_wal_files);
}

TEST_F(DBSSTTest, DBWithMaxSpaceAllowed) {
Expand Down
8 changes: 5 additions & 3 deletions db/wal_manager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include "util/cast_util.h"
#include "util/coding.h"
#include "util/file_reader_writer.h"
#include "util/file_util.h"
#include "util/filename.h"
#include "util/logging.h"
#include "util/mutexlock.h"
Expand Down Expand Up @@ -181,7 +182,7 @@ void WalManager::PurgeObsoleteWALFiles() {
continue;
}
if (now_seconds - file_m_time > db_options_.wal_ttl_seconds) {
s = env_->DeleteFile(file_path);
s = DeleteDBFile(&db_options_, file_path, 0, kLogFile);
if (!s.ok()) {
ROCKS_LOG_WARN(db_options_.info_log, "Can't delete file: %s: %s",
file_path.c_str(), s.ToString().c_str());
Expand All @@ -207,7 +208,7 @@ void WalManager::PurgeObsoleteWALFiles() {
log_file_size = std::max(log_file_size, file_size);
++log_files_num;
} else {
s = env_->DeleteFile(file_path);
s = DeleteDBFile(&db_options_, file_path, 0, kLogFile);
if (!s.ok()) {
ROCKS_LOG_WARN(db_options_.info_log,
"Unable to delete file: %s: %s", file_path.c_str(),
Expand Down Expand Up @@ -246,7 +247,8 @@ void WalManager::PurgeObsoleteWALFiles() {

for (size_t i = 0; i < files_del_num; ++i) {
std::string const file_path = archived_logs[i]->PathName();
s = env_->DeleteFile(db_options_.wal_dir + "/" + file_path);
s = DeleteDBFile(&db_options_, db_options_.wal_dir + "/" + file_path, 0,
kLogFile);
if (!s.ok()) {
ROCKS_LOG_WARN(db_options_.info_log, "Unable to delete file: %s: %s",
file_path.c_str(), s.ToString().c_str());
Expand Down
3 changes: 3 additions & 0 deletions include/rocksdb/sst_file_manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ class SstFileManager {

// Create a new SstFileManager that can be shared among multiple RocksDB
// instances to track SST file and control there deletion rate.
// Even though SstFileManager don't track WAL files but it still control
// there deletion rate.
//
// @param env: Pointer to Env object, please see "rocksdb/env.h".
// @param info_log: If not nullptr, info_log will be used to log errors.
Expand All @@ -71,6 +73,7 @@ class SstFileManager {
// this value is set to 1024 (1 Kb / sec) and we deleted a file of size 4 Kb
// in 1 second, we will wait for another 3 seconds before we delete other
// files, Set to 0 to disable deletion rate limiting.
// This option also affect the delete rate of WAL files in the DB.
// @param delete_existing_trash: Deprecated, this argument have no effect, but
// if user provide trash_dir we will schedule deletes for files in the dir
// @param status: If not nullptr, status will contain any errors that happened
Expand Down
27 changes: 19 additions & 8 deletions util/file_util.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,21 +82,32 @@ Status CreateFile(Env* env, const std::string& destination,
return dest_writer->Append(Slice(contents));
}

Status DeleteSSTFile(const ImmutableDBOptions* db_options,
const std::string& fname, uint32_t path_id) {
// TODO(tec): support sst_file_manager for multiple path_ids
#ifndef ROCKSDB_LITE
auto sfm =
Status DeleteDBFile(const ImmutableDBOptions* db_options,
const std::string& fname, uint32_t path_id, FileType type) {
SstFileManagerImpl* sfm =
static_cast<SstFileManagerImpl*>(db_options->sst_file_manager.get());
if (sfm && path_id == 0) {

if (sfm != nullptr &&
((type == kTableFile && path_id == 0) || type == kLogFile)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you get rid of the path_id == 0 restriction now that the trash files don't need to be moved?

// Pass files to SstFileManager if
// - SstFileManager is enabled
// - File type is
// - SST file that live in Path id 0
// - WAL file

return sfm->ScheduleFileDeletion(fname);
} else {
return db_options->env->DeleteFile(fname);
}

return db_options->env->DeleteFile(fname);
}
#else
Status DeleteDBFile(const ImmutableDBOptions* db_options,
const std::string& fname, uint32_t path_id, FileType type) {
// SstFileManager is not supported in ROCKSDB_LITE
// Delete file immediately
return db_options->env->DeleteFile(fname);
#endif
}
#endif

} // namespace rocksdb
6 changes: 4 additions & 2 deletions util/file_util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "rocksdb/env.h"
#include "rocksdb/status.h"
#include "rocksdb/types.h"
#include "util/filename.h"

namespace rocksdb {
// use_fsync maps to options.use_fsync, which determines the way that
Expand All @@ -21,7 +22,8 @@ extern Status CopyFile(Env* env, const std::string& source,
extern Status CreateFile(Env* env, const std::string& destination,
const std::string& contents);

extern Status DeleteSSTFile(const ImmutableDBOptions* db_options,
const std::string& fname, uint32_t path_id);
extern Status DeleteDBFile(const ImmutableDBOptions* db_options,
const std::string& fname, uint32_t path_id,
FileType type);

} // namespace rocksdb