[C] fix checksum iterator bug: may 'eat' ret_errno if occur memory_failed [4_0_0_release < master]

src/observer/omt/ob_tenant_config.cpp

@@ -37,7 +37,7 @@ ObTenantConfig::ObTenantConfig(uint64_t tenant_id)
     : tenant_id_(tenant_id), current_version_(1),
       mutex_(),
       update_task_(), system_config_(), config_mgr_(nullptr),
-      lock_(), is_deleting_(false)
+      lock_(ObLatchIds::CONFIG_LOCK), is_deleting_(false)
 {
 }
 
@@ -55,7 +55,7 @@ int ObTenantConfig::init(ObTenantConfigMgr *config_mgr)
 
 void ObTenantConfig::print() const
 {
-  ObLatchRGuard rd_guard(const_cast<ObLatch&>(lock_), ObLatchIds::CONFIG_LOCK);
+  DRWLock::RDLockGuard guard(lock_);
   OB_LOG(INFO, "===================== * begin tenant config report * =====================", K(tenant_id_));
   ObConfigContainer::const_iterator it = container_.begin();
   for (; it != container_.end(); ++it) {
@@ -71,7 +71,7 @@ void ObTenantConfig::print() const
 int ObTenantConfig::check_all() const
 {
   int ret = OB_SUCCESS;
-  ObLatchRGuard rd_guard(const_cast<ObLatch&>(lock_), ObLatchIds::CONFIG_LOCK);
+  DRWLock::RDLockGuard guard(lock_);
   ObConfigContainer::const_iterator it = container_.begin();
   for (; OB_SUCC(ret) && it != container_.end(); ++it) {
     if (OB_ISNULL(it->second)) {
@@ -90,31 +90,32 @@ int ObTenantConfig::check_all() const
 
 int ObTenantConfig::rdlock()
 {
-  return lock_.rdlock(ObLatchIds::CONFIG_LOCK) == OB_SUCCESS
-      ? OB_SUCCESS : OB_EAGAIN;
+  return lock_.rdlock() == OB_SUCCESS ? OB_SUCCESS : OB_EAGAIN;
 }
 
 int ObTenantConfig::wrlock()
 {
-  return lock_.wrlock(ObLatchIds::CONFIG_LOCK) == OB_SUCCESS
-      ? OB_SUCCESS : OB_EAGAIN;
+  return lock_.wrlock() == OB_SUCCESS ? OB_SUCCESS : OB_EAGAIN;
 }
 
 int ObTenantConfig::try_rdlock()
 {
-  return lock_.try_rdlock(ObLatchIds::CONFIG_LOCK) == OB_SUCCESS
-      ? OB_SUCCESS : OB_EAGAIN;
+  return lock_.try_rdlock() == OB_SUCCESS ? OB_SUCCESS : OB_EAGAIN;
 }
 
 int ObTenantConfig::try_wrlock()
 {
-  return lock_.try_wrlock(ObLatchIds::CONFIG_LOCK) == OB_SUCCESS
-      ? OB_SUCCESS : OB_EAGAIN;
+  return lock_.try_wrlock() == OB_SUCCESS ? OB_SUCCESS : OB_EAGAIN;
 }
 
 int ObTenantConfig::unlock()
 {
-  return lock_.unlock() == OB_SUCCESS ? OB_SUCCESS : OB_EAGAIN;
+  return lock_.rdunlock() == OB_SUCCESS ? OB_SUCCESS : OB_EAGAIN;
+}
+
+int ObTenantConfig::wrunlock()
+{
+  return lock_.wrunlock() == OB_SUCCESS ? OB_SUCCESS : OB_EAGAIN;
 }
 
 int ObTenantConfig::read_config()
@@ -124,7 +125,7 @@ int ObTenantConfig::read_config()
   ObAddr server;
   char local_ip[OB_MAX_SERVER_ADDR_SIZE] = "";
   DRWLock::RDLockGuard lguard(ObConfigManager::get_serialize_lock());
-  ObLatchWGuard wr_guard(lock_, ObLatchIds::CONFIG_LOCK);
+  DRWLock::WRLockGuard guard(lock_);
   server = GCTX.self_addr();
   if (OB_UNLIKELY(true != server.ip_to_string(local_ip, sizeof(local_ip)))) {
     ret = OB_CONVERT_ERROR;
@@ -364,7 +365,7 @@ int ObTenantConfig::add_extra_config(char *config_str,
   char *saveptr = NULL;
   char *token = NULL;
   DRWLock::RDLockGuard lguard(ObConfigManager::get_serialize_lock());
-  ObLatchWGuard wr_guard(lock_, ObLatchIds::CONFIG_LOCK);
+  DRWLock::WRLockGuard guard(lock_);
   token = STRTOK_R(config_str, ",\n", &saveptr);
   while (OB_SUCC(ret) && OB_LIKELY(NULL != token)) {
     char *saveptr_one = NULL;
@@ -422,7 +423,7 @@ OB_DEF_SERIALIZE(ObTenantConfig)
   int ret = OB_SUCCESS;
   int64_t expect_data_len = get_serialize_size_();
   int64_t saved_pos = pos;
-  ObLatchRGuard rd_guard(const_cast<ObLatch&>(lock_), ObLatchIds::CONFIG_LOCK);
+  DRWLock::RDLockGuard guard(lock_);
   if (OB_FAIL(databuff_printf(buf, buf_len, pos, "[%lu]\n", tenant_id_))) {
   } else {
     ret = ObCommonConfig::serialize(buf, buf_len, pos);
@@ -440,7 +441,7 @@ OB_DEF_SERIALIZE(ObTenantConfig)
 OB_DEF_DESERIALIZE(ObTenantConfig)
 {
   int ret = OB_SUCCESS;
-  ObLatchWGuard wr_guard(lock_, ObLatchIds::CONFIG_LOCK);
+  DRWLock::WRLockGuard guard(lock_);
   if ('[' != *(buf + pos)) {
     ret = OB_INVALID_DATA;
     LOG_ERROR("invalid tenant config", K(ret));
@@ -483,7 +484,7 @@ OB_DEF_SERIALIZE_SIZE(ObTenantConfig)
   int64_t len = 0, tmp_pos = 0;
   int ret = OB_SUCCESS;
   char tenant_str[100] = {'\0'};
-  ObLatchRGuard rd_guard(const_cast<ObLatch&>(lock_), ObLatchIds::CONFIG_LOCK);
+  DRWLock::RDLockGuard guard(lock_);
   if (OB_FAIL(databuff_printf(tenant_str, 100, tmp_pos, "[%lu]\n", tenant_id_))) {
     LOG_WARN("write data buff failed", K(ret));
   } else {

src/observer/omt/ob_tenant_config.h

@@ -17,6 +17,7 @@
 #include "share/config/ob_system_config.h"
 #include "share/config/ob_common_config.h"
 #include "share/config/ob_config_helper.h"
+#include "lib/lock/ob_drw_lock.h"
 
 namespace oceanbase {
 
@@ -79,6 +80,7 @@ public:
   int try_rdlock();
   int try_wrlock();
   int unlock();
+  int wrunlock();
 
   int read_config();
   uint64_t get_tenant_id() const { return tenant_id_; }
@@ -100,7 +102,7 @@ private:
   common::ObSystemConfig system_config_;
   ObTenantConfigMgr *config_mgr_;
   // protect this object from being deleted in OTC_MGR.del_tenant_config
-  common::ObLatch lock_;
+  mutable common::DRWLock lock_;
   bool is_deleting_;
 
 public:

src/observer/omt/ob_tenant_config_mgr.cpp

@@ -314,7 +314,7 @@ int ObTenantConfigMgr::del_tenant_config(uint64_t tenant_id)
         LOG_INFO("tenant config deleted", K(tenant_id), K(ret));
       }
       if (OB_FAIL(ret)) {
-        config->unlock();
+        config->wrunlock();
       }
     }
   }

src/share/ob_tablet_replica_checksum_iterator.cpp

@@ -73,12 +73,10 @@ int ObTabletReplicaChecksumIterator::next(ObTabletReplicaChecksumItem &item)
     ret = OB_ITER_END;
   } else {
     while (OB_SUCC(ret)) {
-      ObTabletReplicaChecksumItem tmp_item;
       if (cur_idx_ < checksum_items_.count()) {
-        if (OB_FAIL(checksum_items_.at(cur_idx_, tmp_item))) {
-          LOG_WARN("fail to get checksum item", KR(ret), K_(cur_idx));
-        } else {
-          item = tmp_item;
+        if (OB_FAIL(item.assign(checksum_items_.at(cur_idx_)))) {
+          LOG_WARN("fail to assign tablet replica checksum item", KR(ret), K_(cur_idx), "target_item",
+            checksum_items_.at(cur_idx_), "total cnt", checksum_items_.count());
         }
         ++cur_idx_;
         break;

src/share/ob_tablet_replica_checksum_operator.cpp

@@ -189,12 +189,17 @@ int ObTabletReplicaReportColumnMeta::check_checksum(
 {
   int ret = OB_SUCCESS;
   is_equal = true;
-  if ((pos < 0) || (pos > column_checksums_.count()) || (pos > other.column_checksums_.count())) {
+  const int64_t col_ckm_cnt = column_checksums_.count();
+  const int64_t other_col_ckm_cnt = other.column_checksums_.count();
+  if ((pos < 0) || (pos > col_ckm_cnt) || (pos > other_col_ckm_cnt)) {
     ret = OB_INVALID_ARGUMENT;
-    LOG_WARN("get invalid args", KR(ret), K(pos), K(column_checksums_), K(other.column_checksums_));
+    LOG_WARN("get invalid args", KR(ret), K(pos), K(col_ckm_cnt), K(other_col_ckm_cnt),
+      K(column_checksums_), K(other.column_checksums_));
   } else if (column_checksums_.at(pos) != other.column_checksums_.at(pos)) {
     is_equal = false;
-    LOG_WARN("column checksum is not equal!", K(pos), K(column_checksums_), K(other.column_checksums_));
+    LOG_WARN("column checksum is not equal!", K(pos), "col_ckm", column_checksums_.at(pos),
+      "other_col_ckm", other.column_checksums_.at(pos), K(col_ckm_cnt), K(other_col_ckm_cnt),
+      K(column_checksums_), K(other.column_checksums_));
   }
   return ret;
 }
@@ -207,11 +212,13 @@ int ObTabletReplicaReportColumnMeta::check_all_checksums(
   is_equal = true;
   if (column_checksums_.count() != other.column_checksums_.count()) {
     is_equal = false;
-    LOG_WARN("column cnt is not equal!", K(*this), K(other));
+    LOG_WARN("column cnt is not equal!", "cur_cnt", column_checksums_.count(), "other_cnt",
+      other.column_checksums_.count(), K(*this), K(other));
   } else {
-    for (int64_t i = 0; OB_SUCC(ret) && is_equal && i < column_checksums_.count(); ++i) {
+    const int64_t column_ckm_cnt = column_checksums_.count();
+    for (int64_t i = 0; OB_SUCC(ret) && is_equal && (i < column_ckm_cnt); ++i) {
       if (OB_FAIL(check_checksum(other, i, is_equal))) {
-        LOG_WARN("fail to check checksum", KR(ret), K(i));
+        LOG_WARN("fail to check checksum", KR(ret), K(i), K(column_ckm_cnt));
       }
     }
   }
@@ -333,16 +340,29 @@ int ObTabletReplicaChecksumItem::assign_key(const ObTabletReplicaChecksumItem &o
   return ret;
 }
 
+int ObTabletReplicaChecksumItem::assign(const ObTabletReplicaChecksumItem &other)
+{
+  int ret = OB_SUCCESS;
+  if (this != &other) {
+    reset();
+    if (OB_FAIL(column_meta_.assign(other.column_meta_))) {
+      LOG_WARN("fail to assign column meta", KR(ret), K(other));
+    } else {
+      tenant_id_ = other.tenant_id_;
+      tablet_id_ = other.tablet_id_;
+      ls_id_ = other.ls_id_;
+      server_ = other.server_;
+      row_count_ = other.row_count_;
+      snapshot_version_ = other.snapshot_version_;
+      data_checksum_ = other.data_checksum_;
+    }
+  }
+  return ret;
+}
+
 ObTabletReplicaChecksumItem &ObTabletReplicaChecksumItem::operator=(const ObTabletReplicaChecksumItem &other)
 {
-  tenant_id_ = other.tenant_id_;
-  tablet_id_ = other.tablet_id_;
-  ls_id_ = other.ls_id_;
-  server_ = other.server_;
-  row_count_ = other.row_count_;
-  snapshot_version_ = other.snapshot_version_;
-  data_checksum_ = other.data_checksum_;
-  column_meta_.assign(other.column_meta_);
+  assign(other);
   return *this;
 }
 

src/share/ob_tablet_replica_checksum_operator.h

@@ -82,6 +82,7 @@ public:
   bool is_same_tablet(const ObTabletReplicaChecksumItem &other) const;
   int verify_checksum(const ObTabletReplicaChecksumItem &other) const;
   int assign_key(const ObTabletReplicaChecksumItem &other);
+  int assign(const ObTabletReplicaChecksumItem &other);
   ObTabletReplicaChecksumItem &operator =(const ObTabletReplicaChecksumItem &other);
 
   TO_STRING_KV(K_(tenant_id), K_(ls_id), K_(tablet_id), K_(server), K_(row_count),