Skip to content

Commit

Permalink
FlushProcess
Browse files Browse the repository at this point in the history
Summary:
Abstract out FlushProcess and take it out of DBImpl.
This also includes taking DeletionState outside of DBImpl.

Currently this diff is only doing the refactoring. Future work includes:
1. Decoupling flush_process.cc, make it depend on less state
2. Write flush_process_test, which will mock out everything that FlushProcess depends on and test it in isolation

Test Plan: make check

Reviewers: rven, yhchiang, sdong, ljin

Reviewed By: ljin

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D27561
  • Loading branch information
igorcanadi committed Oct 28, 2014
1 parent efa2fb3 commit a39e931
Show file tree
Hide file tree
Showing 13 changed files with 689 additions and 431 deletions.
6 changes: 5 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ TESTS = \
cuckoo_table_builder_test \
cuckoo_table_reader_test \
cuckoo_table_db_test \
write_batch_with_index_test
write_batch_with_index_test \
flush_job_test

TOOLS = \
sst_dump \
Expand Down Expand Up @@ -412,6 +413,9 @@ ttl_test: utilities/ttl/ttl_test.o $(LIBOBJECTS) $(TESTHARNESS)
write_batch_with_index_test: utilities/write_batch_with_index/write_batch_with_index_test.o $(LIBOBJECTS) $(TESTHARNESS)
$(CXX) utilities/write_batch_with_index/write_batch_with_index_test.o $(LIBOBJECTS) $(TESTHARNESS) $(EXEC_LDFLAGS) -o $@ $(LDFLAGS) $(COVERAGEFLAGS)

flush_job_test: db/flush_job_test.o $(LIBOBJECTS) $(TESTHARNESS)
$(CXX) db/flush_job_test.o $(LIBOBJECTS) $(TESTHARNESS) $(EXEC_LDFLAGS) -o $@ $(LDFLAGS) $(COVERAGEFLAGS)

dbformat_test: db/dbformat_test.o $(LIBOBJECTS) $(TESTHARNESS)
$(CXX) db/dbformat_test.o $(LIBOBJECTS) $(TESTHARNESS) $(EXEC_LDFLAGS) -o $@ $(LDFLAGS) $(COVERAGEFLAGS)

Expand Down
9 changes: 5 additions & 4 deletions db/column_family.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include <limits>

#include "db/db_impl.h"
#include "db/job_context.h"
#include "db/version_set.h"
#include "db/internal_stats.h"
#include "db/compaction_picker.h"
Expand Down Expand Up @@ -71,15 +72,15 @@ ColumnFamilyHandleImpl::ColumnFamilyHandleImpl(ColumnFamilyData* cfd,

ColumnFamilyHandleImpl::~ColumnFamilyHandleImpl() {
if (cfd_ != nullptr) {
DBImpl::DeletionState deletion_state;
JobContext job_context;
mutex_->Lock();
if (cfd_->Unref()) {
delete cfd_;
}
db_->FindObsoleteFiles(deletion_state, false, true);
db_->FindObsoleteFiles(&job_context, false, true);
mutex_->Unlock();
if (deletion_state.HaveSomethingToDelete()) {
db_->PurgeObsoleteFiles(deletion_state);
if (job_context.HaveSomethingToDelete()) {
db_->PurgeObsoleteFiles(job_context);
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions db/db_filesnapshot.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Status DBImpl::DisableFileDeletions() {
}

Status DBImpl::EnableFileDeletions(bool force) {
DeletionState deletion_state;
JobContext job_context;
bool should_purge_files = false;
{
MutexLock l(&mutex_);
Expand All @@ -55,15 +55,15 @@ Status DBImpl::EnableFileDeletions(bool force) {
if (disable_delete_obsolete_files_ == 0) {
Log(db_options_.info_log, "File Deletions Enabled");
should_purge_files = true;
FindObsoleteFiles(deletion_state, true);
FindObsoleteFiles(&job_context, true);
} else {
Log(db_options_.info_log,
"File Deletions Enable, but not really enabled. Counter: %d",
disable_delete_obsolete_files_);
}
}
if (should_purge_files) {
PurgeObsoleteFiles(deletion_state);
PurgeObsoleteFiles(job_context);
}
LogFlush(db_options_.info_log);
return Status::OK();
Expand Down
Loading

0 comments on commit a39e931

Please sign in to comment.