Code Review Updates #5

speedb-io · Sep 5, 2022 · 6cc4a50 · 6cc4a50
1 parent e20340e
commit 6cc4a50
Show file tree

Hide file tree

Showing 6 changed files with 16 additions and 10 deletions.
diff --git a/db/memtable.h b/db/memtable.h
@@ -480,8 +480,12 @@ class MemTable {
   uint64_t GetID() const { return id_; }
 
   void SetFlushCompleted(bool completed) {
+    // Flush Can't complete twice
+    if (completed) {
+      assert(!flush_completed_);
+    }
     // In case flush is aborted, notify the memory tracker
-    if (flush_in_progress_ && (completed == false)) {
+    if (flush_completed_ && (completed == false)) {
       mem_tracker_.FreeMemAborted();
     }
     flush_completed_ = completed;
@@ -542,8 +546,8 @@ class MemTable {
   std::atomic<size_t> write_buffer_size_;
 
   // These are used to manage memtable flushes to storage
-  bool flush_in_progress_; // started the flush
-  bool flush_completed_;   // finished the flush
+  bool flush_in_progress_;  // started the flush
+  bool flush_completed_;    // finished the flush
   uint64_t file_number_;    // filled up after flush is complete
 
   // The updates to be applied to the transaction log when this

diff --git a/db/memtable_list.cc b/db/memtable_list.cc
@@ -701,8 +701,8 @@ void MemTableList::RemoveMemTablesOrRestoreFlags(
                          m->edit_.GetBlobFileAdditions().size(), mem_id);
       }
 
-      m->SetFlushInProgress(false);
       m->SetFlushCompleted(false);
+      m->SetFlushInProgress(false);
 
       m->edit_.Clear();
       num_flush_not_started_++;
@@ -894,8 +894,8 @@ Status InstallMemtableAtomicFlushResults(
                            edit->GetBlobFileAdditions().size(), mem_id);
         }
 
-        m->SetFlushInProgress(false);
         m->SetFlushCompleted(false);
+        m->SetFlushInProgress(false);
         m->GetEdits()->Clear();
         m->SetFileNumber(0);
         imm->num_flush_not_started_++;

diff --git a/include/rocksdb/write_buffer_manager.h b/include/rocksdb/write_buffer_manager.h
@@ -166,7 +166,7 @@ class WriteBufferManager final {
   // The process may complete successfully and FreeMem() will be called to
   // notifiy successfull completion, or, aborted, and FreeMemCancelled() will be
   // called to notify that.
-  void FreeMemStarted(size_t mem);
+  void FreeMemBegin(size_t mem);
 
   // Freeing 'mem' bytes was aborted and that memory is no longer in the process
   // of being freed

diff --git a/memtable/alloc_tracker.cc b/memtable/alloc_tracker.cc
@@ -52,7 +52,7 @@ void AllocTracker::FreeMemStarted() {
 
   if (state_ == State::kDoneAllocating) {
     if (ShouldUpdateWriteBufferManager()) {
-      write_buffer_manager_->FreeMemStarted(
+      write_buffer_manager_->FreeMemBegin(
           bytes_allocated_.load(std::memory_order_relaxed));
     }
     state_ = State::kFreeMemStarted;
@@ -61,7 +61,9 @@ void AllocTracker::FreeMemStarted() {
 
 void AllocTracker::FreeMemAborted() {
   assert(write_buffer_manager_ != nullptr);
-  assert(state_ == State::kFreeMemStarted);
+  // May be called without actually starting to free memory
+  assert((state_ == State::kDoneAllocating) ||
+         (state_ == State::kFreeMemStarted));
 
   if (state_ == State::kFreeMemStarted) {
     if (ShouldUpdateWriteBufferManager()) {

diff --git a/memtable/write_buffer_manager.cc b/memtable/write_buffer_manager.cc
@@ -95,7 +95,7 @@ void WriteBufferManager::ScheduleFreeMem(size_t mem) {
   }
 }
 
-void WriteBufferManager::FreeMemStarted(size_t mem) {
+void WriteBufferManager::FreeMemBegin(size_t mem) {
   if (enabled()) {
     memory_being_freed_.fetch_add(mem, std::memory_order_relaxed);
   }

diff --git a/memtable/write_buffer_manager_test.cc b/memtable/write_buffer_manager_test.cc
@@ -18,7 +18,7 @@ const size_t kSizeDummyEntry = 256 * 1024;
 
 namespace {
 void BeginAndFree(WriteBufferManager& wbf, size_t size) {
-  wbf.FreeMemStarted(size);
+  wbf.FreeMemBegin(size);
   wbf.FreeMem(size);
 }