kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [kudu] branch master updated: mvcc: some locking cleanup
Date Tue, 03 Dec 2019 23:13:23 GMT
This is an automated email from the ASF dual-hosted git repository.

adar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 7917a81  mvcc: some locking cleanup
7917a81 is described below

commit 7917a819c15f94bdaa070682a47d60f47821f45d
Author: Adar Dembo <adar@cloudera.com>
AuthorDate: Mon Dec 2 11:12:19 2019 -0800

    mvcc: some locking cleanup
    
    Close was notifying waiters before erasing them from the waiters_ list. In
    practice this didn't matter: lock_ was always held around any manipulation
    of waiters_. But for consistency, let's do what AdjustCleanTime does.
    
    AdjustCleanTime required lock_ to be held, so let's enforce that.
    
    Change-Id: I189553d0f589794ad830a30b3c0b4ce5fd5569ba
    Reviewed-on: http://gerrit.cloudera.org:8080/14816
    Tested-by: Kudu Jenkins
    Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
---
 src/kudu/tablet/mvcc.cc | 17 ++++++++++-------
 src/kudu/tablet/mvcc.h  |  4 +++-
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/kudu/tablet/mvcc.cc b/src/kudu/tablet/mvcc.cc
index f80d387..4edec6c 100644
--- a/src/kudu/tablet/mvcc.cc
+++ b/src/kudu/tablet/mvcc.cc
@@ -142,7 +142,7 @@ void MvccManager::CommitTransaction(Timestamp timestamp) {
   if (was_earliest && safe_time_ >= timestamp) {
     // If this transaction was the earliest in-flight, we might have to adjust
     // the "clean" timestamp.
-    AdjustCleanTime();
+    AdjustCleanTimeUnlocked();
   }
 }
 
@@ -206,7 +206,7 @@ void MvccManager::AdjustSafeTime(Timestamp safe_time) {
     return;
   }
 
-  AdjustCleanTime();
+  AdjustCleanTimeUnlocked();
 }
 
 // Remove any elements from 'v' which are < the given watermark.
@@ -226,12 +226,15 @@ void MvccManager::Close() {
   std::lock_guard<LockType> l(lock_);
   auto iter = waiters_.begin();
   while (iter != waiters_.end()) {
-    (*iter)->latch->CountDown();
+    auto* waiter = *iter;
     iter = waiters_.erase(iter);
+    waiter->latch->CountDown();
   }
 }
 
-void MvccManager::AdjustCleanTime() {
+void MvccManager::AdjustCleanTimeUnlocked() {
+  DCHECK(lock_.is_locked());
+
   // There are two possibilities:
   //
   // 1) We still have an in-flight transaction earlier than 'safe_time_'.
@@ -269,7 +272,7 @@ void MvccManager::AdjustCleanTime() {
   if (PREDICT_FALSE(!waiters_.empty())) {
     auto iter = waiters_.begin();
     while (iter != waiters_.end()) {
-      WaitingState* waiter = *iter;
+      auto* waiter = *iter;
       if (IsDoneWaitingUnlocked(*waiter)) {
         iter = waiters_.erase(iter);
         waiter->latch->CountDown();
@@ -305,8 +308,8 @@ Status MvccManager::WaitUntil(WaitFor wait_for, Timestamp ts, const MonoTime&
de
   // We timed out. We need to clean up our entry in the waiters_ array.
 
   std::lock_guard<LockType> l(lock_);
-  // It's possible that while we were re-acquiring the lock, we did not get
-  // notified. In that case, we have no cleanup to do.
+  // It's possible that we got notified while we were re-acquiring the lock. In
+  // that case, we have no cleanup to do.
   if (waiting_state.latch->count() == 0) {
     return CheckOpen();
   }
diff --git a/src/kudu/tablet/mvcc.h b/src/kudu/tablet/mvcc.h
index 5928af1..62ccf13 100644
--- a/src/kudu/tablet/mvcc.h
+++ b/src/kudu/tablet/mvcc.h
@@ -354,7 +354,9 @@ class MvccManager {
   // Adjusts the clean time, i.e. the timestamp such that all transactions with
   // lower timestamps are committed or aborted, based on which transactions are
   // currently in flight and on what is the latest value of 'safe_time_'.
-  void AdjustCleanTime();
+  //
+  // Must be called with lock_ held.
+  void AdjustCleanTimeUnlocked();
 
   // Advances the earliest in-flight timestamp, based on which transactions are
   // currently in-flight. Usually called when the previous earliest transaction


Mime
View raw message