From b20901e8c84d3ea774beeaca963c67d7802e4b4e Mon Sep 17 00:00:00 2001
From: kongfy <njukongfy@gmail.com>
Date: Wed, 4 Aug 2021 17:57:29 +0800
Subject: [PATCH] fix rowlock double locked problem

---
 src/storage/memtable/mvcc/ob_mvcc_row.cpp |  1 -
 src/storage/memtable/ob_memtable.cpp      | 35 ++++++++++++++++++-----
 src/storage/ob_sstable.cpp                |  5 ++--
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/src/storage/memtable/mvcc/ob_mvcc_row.cpp b/src/storage/memtable/mvcc/ob_mvcc_row.cpp
index a0367acb1..0a142215a 100644
--- a/src/storage/memtable/mvcc/ob_mvcc_row.cpp
+++ b/src/storage/memtable/mvcc/ob_mvcc_row.cpp
@@ -1130,7 +1130,6 @@ int ObMvccRow::check_row_locked(
   }
   // locked by other
   if (is_locked && lock_descriptor != ctx.get_ctx_descriptor()) {
-    lock_descriptor = row_lock_.get_exclusive_uid();
     int64_t lock_wait_start_ts =
         ctx.get_lock_wait_start_ts() > 0 ? ctx.get_lock_wait_start_ts() : common::ObClockGenerator::getClock();
     int64_t query_abs_lock_wait_timeout = ctx.get_query_abs_lock_wait_timeout(lock_wait_start_ts);
diff --git a/src/storage/memtable/ob_memtable.cpp b/src/storage/memtable/ob_memtable.cpp
index b2ee48c05..1d1445829 100644
--- a/src/storage/memtable/ob_memtable.cpp
+++ b/src/storage/memtable/ob_memtable.cpp
@@ -1912,15 +1912,18 @@ int ObMemtable::lock_row_on_frozen_stores(const ObStoreCtx& ctx, const ObMemtabl
     ret = OB_ERR_UNEXPECTED;
     TRANS_LOG(ERROR, "stores in context is null", K(ret));
   } else {
+    bool locked_by_self = false;
     int64_t max_trans_version = 0;
     const ObIArray<ObITable*>* stores = ctx.tables_;
+    ObStoreRowLockState lock_state;
 
     // ignore active memtable
     for (int64_t i = stores->count() - 2; OB_SUCC(ret) && i >= 0; i--) {
       int64_t current_version = 0;
       bool is_locked = false;
       uint32_t lock_descriptor = 0;
-      ObStoreRowLockState lock_state;
+
+      lock_state.reset();
 
       if (NULL == stores->at(i)) {
         ret = OB_ERR_UNEXPECTED;
@@ -1929,7 +1932,13 @@ int ObMemtable::lock_row_on_frozen_stores(const ObStoreCtx& ctx, const ObMemtabl
         ObMemtable* memtable = static_cast<ObMemtable*>(stores->at(i));
         if (OB_FAIL(memtable->get_mvcc_engine().check_row_locked(
                 *ctx.mem_ctx_, key, is_locked, lock_descriptor, current_version))) {
-          TRANS_LOG(WARN, "mvcc engine check row lock fail", K(ret), K(is_locked), K(lock_descriptor));
+          TRANS_LOG(WARN,
+              "mvcc engine check row lock fail, may be locked by other",
+              K(ret),
+              K(is_locked),
+              K(lock_descriptor));
+        } else {
+          locked_by_self |= is_locked && lock_descriptor == ctx.mem_ctx_->get_ctx_descriptor();
         }
       } else if (stores->at(i)->is_sstable()) {
         ObSSTable* sstable = static_cast<ObSSTable*>(stores->at(i));
@@ -1937,6 +1946,7 @@ int ObMemtable::lock_row_on_frozen_stores(const ObStoreCtx& ctx, const ObMemtabl
           TRANS_LOG(WARN, "failed to check row lock by other", K(ret), K(*key), K(lock_state));
         } else {
           current_version = lock_state.trans_version_;
+          locked_by_self |= lock_state.is_locked_ && ctx.trans_id_ == lock_state.lock_trans_id_;
         }
         TRANS_LOG(DEBUG, "check_row_locked meet sstable", K(ret), K(*key), K(*sstable), K(current_version));
       } else {
@@ -1945,16 +1955,27 @@ int ObMemtable::lock_row_on_frozen_stores(const ObStoreCtx& ctx, const ObMemtabl
       }
 
       max_trans_version = max(max_trans_version, current_version);
-      TRANS_LOG(DEBUG, "check_row_locked", K(i), K(stores->count()), K(stores->at(i)));
+      TRANS_LOG(DEBUG,
+          "check_row_locked",
+          K(i),
+          K(stores->count()),
+          K(stores->at(i)),
+          K(locked_by_self),
+          K(current_version),
+          K(max_trans_version));
     }
 
     if (OB_SUCC(ret)) {
       value->update_max_trans_version(max_trans_version);
-      value->set_lower_lock_scaned();
+      if (!locked_by_self) {
+        // there is no locks on frozen stores
+        if (max_trans_version > ctx.mem_ctx_->get_read_snapshot()) {
+          ret = OB_TRANSACTION_SET_VIOLATION;
+          TRANS_LOG(WARN, "TRANS_SET_VIOLATION", K(ret), K(max_trans_version), "ctx", ctx.mem_ctx_);
+        }
 
-      if (max_trans_version > ctx.mem_ctx_->get_read_snapshot()) {
-        ret = OB_TRANSACTION_SET_VIOLATION;
-        TRANS_LOG(WARN, "TRANS_SET_VIOLATION", K(ret), K(max_trans_version), "ctx", ctx.mem_ctx_);
+        value->set_lower_lock_scaned();
+        TRANS_LOG(DEBUG, "lower lock check finish", K(*value), K(*stores));
       }
     }
   }
diff --git a/src/storage/ob_sstable.cpp b/src/storage/ob_sstable.cpp
index e330749c5..13a3f0fad 100644
--- a/src/storage/ob_sstable.cpp
+++ b/src/storage/ob_sstable.cpp
@@ -3033,9 +3033,10 @@ int ObSSTable::check_row_locked(const ObStoreCtx& ctx, const common::ObStoreRowk
         STORAGE_LOG(WARN, "failed to open getter, ", K(ret), K(iter_param), K(access_context), K(ext_rowkey));
       } else if (OB_SUCC(row_iterator->get_next_row(store_row))) {
         lock_state.trans_version_ = store_row->snapshot_version_;
-        lock_state.is_locked_ = ((ObSSTableRowLockChecker*)row_iterator)->is_curr_row_locked();
-        lock_state.lock_trans_id_ = ((ObSSTableRowLockChecker*)row_iterator)->get_lock_trans_id();
       }
+
+      lock_state.is_locked_ = ((ObSSTableRowLockChecker*)row_iterator)->is_curr_row_locked();
+      lock_state.lock_trans_id_ = ((ObSSTableRowLockChecker*)row_iterator)->get_lock_trans_id();
     }
     row_iterator->~ObISSTableRowIterator();
   }
-- 
GitLab