Fix ddl kv mgr deadlock with get_rec_scn

src/storage/ddl/ob_ddl_redo_log_writer.cpp

@@ -697,7 +697,7 @@ int ObDDLRedoLogWriter::write(
   if (OB_FAIL(ret)) {
   } else if (OB_FAIL(ddl_kv_mgr_handle.get_obj()->rdlock(ObDDLRedoLogHandle::DDL_REDO_LOG_TIMEOUT, lock_tid))) {
     LOG_WARN("failed to rdlock", K(ret));
-  } else if (ddl_kv_mgr_handle.get_obj()->get_commit_scn_nolock(tablet_handle.get_obj()->get_tablet_meta()).is_valid_and_not_min()) {
+  } else if (ddl_kv_mgr_handle.get_obj()->get_commit_scn(tablet_handle.get_obj()->get_tablet_meta()).is_valid_and_not_min()) {
     ret = OB_TRANS_COMMITED;
     LOG_WARN("already commit", K(ret));
   } else if (ddl_kv_mgr_handle.get_obj()->get_start_scn() != log.get_redo_info().start_scn_) {
@@ -774,7 +774,7 @@ int ObDDLRedoLogWriter::write_ddl_start_log(ObTabletHandle &tablet_handle,
   } else if (ddl_kv_mgr_handle.get_obj()->is_execution_id_older(log.get_execution_id())) {
     ret = OB_TASK_EXPIRED;
     LOG_INFO("receive a old execution id, don't do ddl start", K(ret), K(log));
-  } else if (ddl_kv_mgr_handle.get_obj()->get_commit_scn_nolock(tablet_handle.get_obj()->get_tablet_meta()).is_valid_and_not_min()) {
+  } else if (ddl_kv_mgr_handle.get_obj()->get_commit_scn(tablet_handle.get_obj()->get_tablet_meta()).is_valid_and_not_min()) {
     start_scn = ddl_kv_mgr_handle.get_obj()->get_start_scn();
     if (!start_scn.is_valid_and_not_min()) {
       start_scn = tablet_handle.get_obj()->get_tablet_meta().ddl_start_scn_;
@@ -782,8 +782,6 @@ int ObDDLRedoLogWriter::write_ddl_start_log(ObTabletHandle &tablet_handle,
     if (!start_scn.is_valid_and_not_min()) {
       ret = OB_ERR_UNEXPECTED;
       LOG_WARN("start scn must be valid after commit", K(ret), K(start_scn));
-    } else if (OB_FAIL(ddl_kv_mgr_handle.get_obj()->set_execution_id_nolock(log.get_execution_id()))) {
-      LOG_WARN("failed to set execution id", K(ret));
     } else {
       LOG_INFO("already committed, use previous start scn", K(ret), K(tablet_handle.get_obj()->get_tablet_meta()));
     }
@@ -888,7 +886,7 @@ int ObDDLRedoLogWriter::write_ddl_commit_log(ObTabletHandle &tablet_handle,
   } else if (ddl_kv_mgr_handle.get_obj()->get_start_scn() != log.get_start_scn()) {
     ret = OB_TASK_EXPIRED;
     LOG_WARN("restarted", K(ret));
-  } else if (ddl_kv_mgr_handle.get_obj()->get_commit_scn_nolock(tablet_handle.get_obj()->get_tablet_meta()).is_valid_and_not_min()) {
+  } else if (ddl_kv_mgr_handle.get_obj()->get_commit_scn(tablet_handle.get_obj()->get_tablet_meta()).is_valid_and_not_min()) {
     ret = OB_TRANS_COMMITED;
     LOG_WARN("already committed", K(ret), K(log));
   } else if (OB_ISNULL(buffer = static_cast<char *>(ob_malloc(buffer_size, ObMemAttr(MTL_ID(), "DDL_COMMIT_LOG"))))) {

src/storage/ddl/ob_tablet_ddl_kv_mgr.cpp

@@ -125,7 +125,7 @@ int ObTabletDDLKvMgr::ddl_start_nolock(const ObITable::TableKey &table_key,
       table_key_ = table_key;
       data_format_version_ = data_format_version;
       execution_id_ = execution_id;
-      start_scn_ = start_scn;
+      start_scn_.atomic_store(start_scn);
       max_freeze_scn_ = SCN::max(start_scn, checkpoint_scn);
     }
   }
@@ -158,10 +158,10 @@ int ObTabletDDLKvMgr::ddl_start(ObTablet &tablet,
       // save variables under lock
       saved_start_scn = start_scn_;
       saved_snapshot_version = table_key_.get_snapshot_version();
-      commit_scn_ = get_commit_scn_nolock(tablet.get_tablet_meta());
+      commit_scn_.atomic_store(get_commit_scn(tablet.get_tablet_meta()));
       if (checkpoint_scn.is_valid_and_not_min()) {
         if (tablet.get_tablet_meta().table_store_flag_.with_major_sstable() && tablet.get_tablet_meta().ddl_commit_scn_.is_valid_and_not_min()) {
-          success_start_scn_ = tablet.get_tablet_meta().ddl_start_scn_;
+          success_start_scn_.atomic_store(tablet.get_tablet_meta().ddl_start_scn_);
         }
       }
     }
@@ -185,7 +185,7 @@ int ObTabletDDLKvMgr::ddl_commit(const SCN &start_scn, const SCN &commit_scn)
   } else if (!is_started()) {
     ret = OB_STATE_NOT_MATCH;
     LOG_WARN("ddl not started", K(ret));
-  } else if (start_scn < start_scn_) {
+  } else if (start_scn < get_start_scn()) {
     ret = OB_TASK_EXPIRED;
     LOG_INFO("skip ddl commit log", K(start_scn), K(*this));
   } else if (OB_FAIL(set_commit_scn(commit_scn))) {
@@ -236,7 +236,7 @@ int ObTabletDDLKvMgr::schedule_ddl_merge_task(const SCN &start_scn, const SCN &c
     LOG_WARN("not init", K(ret), K(is_inited_));
   } else if (is_commit_success()) {
     FLOG_INFO("ddl commit already succeed", K(start_scn), K(commit_scn), K(*this));
-  } else if (start_scn < start_scn_) {
+  } else if (start_scn < get_start_scn()) {
     ret = OB_TASK_EXPIRED;
     LOG_INFO("skip ddl commit log", K(start_scn), K(commit_scn), K(*this));
   } else if (OB_FAIL(MTL(ObLSService *)->get_ls(ls_id_, ls_handle, ObLSGetMod::DDL_MOD))) {
@@ -275,7 +275,7 @@ int ObTabletDDLKvMgr::wait_ddl_merge_success(const SCN &start_scn, const SCN &co
   } else if (!is_started()) {
     ret = OB_STATE_NOT_MATCH;
     LOG_WARN("ddl not started", K(ret));
-  } else if (start_scn > start_scn_) {
+  } else if (start_scn > get_start_scn()) {
     ret = OB_ERR_SYS;
     LOG_WARN("start log ts not match", K(ret), K(start_scn), K(start_scn_), K(ls_id_), K(tablet_id_));
   } else {
@@ -305,17 +305,21 @@ int ObTabletDDLKvMgr::wait_ddl_merge_success(const SCN &start_scn, const SCN &co
 int ObTabletDDLKvMgr::get_ddl_major_merge_param(const ObTabletMeta &tablet_meta, ObDDLTableMergeDagParam &param)
 {
   int ret = OB_SUCCESS;
-  ObLatchRGuard state_guard(state_lock_, ObLatchIds::TABLET_DDL_KV_MGR_LOCK);
+  uint32_t lock_tid = 0;
-  ObLatchRGuard guard(lock_, ObLatchIds::TABLET_DDL_KV_MGR_LOCK);
+  if (OB_FAIL(rdlock(TRY_LOCK_TIMEOUT, lock_tid))) {
-  if (can_schedule_major_compaction_nolock(tablet_meta)) {
+    LOG_WARN("failed to rdlock", K(ret), KPC(this));
+  } else if (can_schedule_major_compaction_nolock(tablet_meta)) {
     param.ls_id_ = ls_id_;
     param.tablet_id_ = tablet_id_;
-    param.rec_scn_ = get_commit_scn_nolock(tablet_meta);
+    param.rec_scn_ = get_commit_scn(tablet_meta);
     param.is_commit_ = true;
     param.start_scn_ = start_scn_;
   } else {
     ret = OB_EAGAIN;
   }
+  if (0 != lock_tid) {
+    unlock(lock_tid);
+  }
   return ret;
 }
 
@@ -342,10 +346,10 @@ int ObTabletDDLKvMgr::get_rec_scn(SCN &rec_scn)
     // rec scn of ddl start log
     if (OB_SUCC(ret)) {
       const ObTabletMeta &tablet_meta = tablet_handle.get_obj()->get_tablet_meta();
-      ObLatchRGuard state_guard(state_lock_, ObLatchIds::TABLET_DDL_KV_MGR_LOCK);
+      const SCN start_scn = get_start_scn();
-      if (start_scn_.is_valid_and_not_min() && start_scn_ != tablet_meta.ddl_start_scn_) {
+      if (start_scn.is_valid_and_not_min() && start_scn != tablet_meta.ddl_start_scn_) {
         // has a latest start log and not flushed to tablet meta, keep it
-        rec_scn = SCN::min(rec_scn, start_scn_);
+        rec_scn = SCN::min(rec_scn, start_scn);
       }
     }
 
@@ -403,12 +407,12 @@ int ObTabletDDLKvMgr::set_commit_scn(const SCN &commit_scn)
     LOG_WARN("get tablet handle failed", K(ret), K(ls_id_), K(tablet_id_));
   } else {
     ObLatchWGuard state_guard(state_lock_, ObLatchIds::TABLET_DDL_KV_MGR_LOCK);
-    const SCN old_commit_scn = get_commit_scn_nolock(tablet_handle.get_obj()->get_tablet_meta());
+    const SCN old_commit_scn = get_commit_scn(tablet_handle.get_obj()->get_tablet_meta());
     if (old_commit_scn.is_valid_and_not_min() && old_commit_scn != commit_scn) {
       ret = OB_ERR_UNEXPECTED;
       LOG_WARN("already committed by others", K(ret), K(commit_scn), K(*this));
     } else {
-      commit_scn_ = commit_scn;
+      commit_scn_.atomic_store(commit_scn);
     }
   }
   return ret;
@@ -416,18 +420,13 @@ int ObTabletDDLKvMgr::set_commit_scn(const SCN &commit_scn)
 
 SCN ObTabletDDLKvMgr::get_commit_scn(const ObTabletMeta &tablet_meta)
 {
-  ObLatchRGuard state_guard(state_lock_, ObLatchIds::TABLET_DDL_KV_MGR_LOCK);
+  SCN mgr_commit_scn = commit_scn_.atomic_load();
-  return get_commit_scn_nolock(tablet_meta);
-}
-
-SCN ObTabletDDLKvMgr::get_commit_scn_nolock(const ObTabletMeta &tablet_meta)
-{
   SCN commit_scn = SCN::min_scn();
-  if (tablet_meta.ddl_commit_scn_.is_valid_and_not_min() || commit_scn_.is_valid_and_not_min()) {
+  if (tablet_meta.ddl_commit_scn_.is_valid_and_not_min() || mgr_commit_scn.is_valid_and_not_min()) {
     if (tablet_meta.ddl_commit_scn_.is_valid_and_not_min()) {
       commit_scn = tablet_meta.ddl_commit_scn_;
     } else {
-      commit_scn = commit_scn_;
+      commit_scn = mgr_commit_scn;
     }
   } else {
     commit_scn = SCN::min_scn();
@@ -460,7 +459,7 @@ int ObTabletDDLKvMgr::set_commit_success(const SCN &start_scn)
         }
       }
     } else {
-      success_start_scn_ = start_scn;
+      success_start_scn_.atomic_store(start_scn);
     }
   }
   return ret;
@@ -468,24 +467,20 @@ int ObTabletDDLKvMgr::set_commit_success(const SCN &start_scn)
 
 bool ObTabletDDLKvMgr::is_commit_success()
 {
-  ObLatchRGuard state_guard(state_lock_, ObLatchIds::TABLET_DDL_KV_MGR_LOCK);
+  const SCN success_start_scn = success_start_scn_.atomic_load();
-  return is_commit_success_unlock();
+  const SCN start_scn = start_scn_.atomic_load();
-}
+  return success_start_scn > SCN::min_scn() && success_start_scn == start_scn;
-
-bool ObTabletDDLKvMgr::is_commit_success_unlock() const
-{
-  return success_start_scn_ > SCN::min_scn() && success_start_scn_ == start_scn_;
 }
 
 void ObTabletDDLKvMgr::reset_commit_success()
 {
   ObLatchWGuard state_guard(state_lock_, ObLatchIds::TABLET_DDL_KV_MGR_LOCK);
-  success_start_scn_.set_min();
+  success_start_scn_.atomic_store(SCN::min_scn());
 }
 
 bool ObTabletDDLKvMgr::can_schedule_major_compaction_nolock(const ObTabletMeta &tablet_meta)
 {
-  return get_commit_scn_nolock(tablet_meta).is_valid_and_not_min() && !is_commit_success_unlock();
+  return get_commit_scn(tablet_meta).is_valid_and_not_min() && !is_commit_success();
 }
 
 int ObTabletDDLKvMgr::cleanup()
@@ -516,11 +511,11 @@ void ObTabletDDLKvMgr::cleanup_unlock()
   }
   table_key_.reset();
   data_format_version_ = 0;
-  start_scn_.set_min();
+  start_scn_.atomic_store(SCN::min_scn());
-  commit_scn_.set_min();
+  commit_scn_.atomic_store(SCN::min_scn());
   max_freeze_scn_.set_min();
   execution_id_ = -1;
-  success_start_scn_.set_min();
+  success_start_scn_.atomic_store(SCN::min_scn());
 }
 
 bool ObTabletDDLKvMgr::is_execution_id_older(const int64_t execution_id)
@@ -528,33 +523,6 @@ bool ObTabletDDLKvMgr::is_execution_id_older(const int64_t execution_id)
   return execution_id < execution_id_;
 }
 
-int ObTabletDDLKvMgr::set_execution_id_nolock(const int64_t execution_id)
-{
-  int ret = OB_SUCCESS;
-  if (execution_id < execution_id_) {
-    ret = OB_TASK_EXPIRED;
-    LOG_WARN("ddl task expired", K(ret), K(execution_id), K(*this));
-  } else {
-    execution_id_ = execution_id;
-  }
-  return ret;
-}
-
-int ObTabletDDLKvMgr::set_execution_id(const int64_t execution_id)
-{
-  int ret = OB_SUCCESS;
-  if (OB_UNLIKELY(!is_inited_)) {
-    ret = OB_NOT_INIT;
-    LOG_WARN("not init", K(ret), K(is_inited_));
-  } else {
-    ObLatchWGuard state_guard(state_lock_, ObLatchIds::TABLET_DDL_KV_MGR_LOCK);
-    if (OB_FAIL(set_execution_id_nolock(execution_id))) {
-      LOG_WARN("failed to set execution id", K(ret));
-    }
-  }
-  return ret;
-}
-
 int ObTabletDDLKvMgr::online()
 {
   int ret = OB_SUCCESS;
@@ -809,7 +777,7 @@ int ObTabletDDLKvMgr::update_ddl_major_sstable()
                                   storage_schema,
                                   ls_handle.get_ls()->get_rebuild_seq());
     param.ddl_info_.keep_old_ddl_sstable_ = true;
-    param.ddl_info_.ddl_commit_scn_ = commit_scn_;
+    param.ddl_info_.ddl_commit_scn_ = get_commit_scn(tablet_handle.get_obj()->get_tablet_meta());
     if (OB_FAIL(ls_handle.get_ls()->update_tablet_table_store(tablet_id_, param, new_tablet_handle))) {
       LOG_WARN("failed to update tablet table store", K(ret), K(ls_id_), K(tablet_id_), K(param));
     }

src/storage/ddl/ob_tablet_ddl_kv_mgr.h

@@ -50,11 +50,10 @@ public:
   int release_ddl_kvs(const share::SCN &rec_scn); // release persistent ddl kv, used in ddl merge task for free ddl kv
   int check_has_effective_ddl_kv(bool &has_ddl_kv); // used in ddl log handler for checkpoint
   int get_ddl_kv_min_scn(share::SCN &min_scn); // for calculate rec_scn of ls
-  share::SCN get_start_scn() const { return start_scn_; }
+  share::SCN get_start_scn() const { return start_scn_.atomic_load(); }
   bool is_started() const { return share::SCN::min_scn() != start_scn_; }
   void set_commit_scn_nolock(const share::SCN &scn) { commit_scn_ = scn; }
   int set_commit_scn(const share::SCN &scn);
-  share::SCN get_commit_scn_nolock(const ObTabletMeta &tablet_meta);
   share::SCN get_commit_scn(const ObTabletMeta &tablet_meta);
   int set_commit_success(const share::SCN &start_scn);
   bool is_commit_success();
@@ -64,8 +63,6 @@ public:
   int cleanup();
   int online();
   bool is_execution_id_older(const int64_t execution_id);
-  int set_execution_id_nolock(const int64_t execution_id);
-  int set_execution_id(const int64_t execution_id);
   int register_to_tablet(const share::SCN &ddl_start_scn, ObDDLKvMgrHandle &kv_mgr_handle);
   int unregister_from_tablet(const share::SCN &ddl_start_scn, ObDDLKvMgrHandle &kv_mgr_handle);
   int rdlock(const int64_t timeout_us, uint32_t &lock_tid);
@@ -96,7 +93,6 @@ private:
   int create_empty_ddl_sstable(common::ObArenaAllocator &allocator, blocksstable::ObSSTable &sstable);
   void cleanup_unlock();
   void destroy();
-  bool is_commit_success_unlock() const;
 public:
   static const int64_t MAX_DDL_KV_CNT_IN_STORAGE = 16;
   static const int64_t TRY_LOCK_TIMEOUT = 10 * 1000000; // 10s