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 <[email protected]>
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 <[email protected]>
---
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
