add defense & unittest for ls_reserved_snapshot

src/storage/ls/ob_ls_reserved_snapshot_mgr.cpp

@@ -113,7 +113,9 @@ int ObLSReservedSnapshotMgr::del_dependent_medium_tablet(const ObTabletID tablet
     if (OB_FAIL(dependent_tablet_set_.erase_refactored(tablet_id.id()))) {
       LOG_WARN("failed to erase tablet id", K(ret), "ls_id", ls_->get_ls_id(),
           K(tablet_id), K(dependent_tablet_set_.size()), KP(this));
-    } else if (0 == dependent_tablet_set_.size() && next_reserved_snapshot_ > 0) {
+    } else if (0 == dependent_tablet_set_.size()
+        && next_reserved_snapshot_ > 0
+        && next_reserved_snapshot_ > min_reserved_snapshot_) {
       min_reserved_snapshot_ = next_reserved_snapshot_;
       new_snapshot_version = next_reserved_snapshot_;
       next_reserved_snapshot_ = 0;
@@ -154,6 +156,7 @@ int ObLSReservedSnapshotMgr::submit_log(
   return ret;
 }
 
+// called by ObTenantFreezeInfoMgr, sync clog in Timer
 int ObLSReservedSnapshotMgr::update_min_reserved_snapshot_for_leader(const int64_t new_snapshot_version)
 {
   int ret = OB_SUCCESS;
@@ -163,19 +166,19 @@ int ObLSReservedSnapshotMgr::update_min_reserved_snapshot_for_leader(const int64
     LOG_WARN("ObLSReservedSnapshotMgr is not inited", K(ret), KP(ls_));
   } else {
     common::TCWLockGuard lock_guard(snapshot_lock_);
-    if (0 == dependent_tablet_set_.size()) {
     if (new_snapshot_version < min_reserved_snapshot_) {
-        ret = OB_INVALID_ARGUMENT;
+      ret = OB_SNAPSHOT_DISCARDED;
       LOG_WARN("failed to update min reserved snapshot", K(ret), "ls_id", ls_->get_ls_id(),
         K(new_snapshot_version), K(min_reserved_snapshot_));
-      } else if (new_snapshot_version > min_reserved_snapshot_) {
+    } else if (0 == dependent_tablet_set_.size()) { // no dependent tablet, can push snapshot forward
+      if (new_snapshot_version > min_reserved_snapshot_) {
         // update min_reserved_snapshot and send clog
         min_reserved_snapshot_ = new_snapshot_version;
         next_reserved_snapshot_ = 0;
         send_log_flag = true;
       }
     } else if (new_snapshot_version > next_reserved_snapshot_) {
-      // wait for next call
+      // have dependent tablet, record in next_reserved_snapshot_, to sync later
       next_reserved_snapshot_ = new_snapshot_version;
     }
   } // end of lock
@@ -204,8 +207,10 @@ int ObLSReservedSnapshotMgr::try_sync_reserved_snapshot(
     LOG_WARN("invalid argument", K(ret), K(new_reserved_snapshot));
   } else if (update_flag) {
     if (OB_FAIL(update_min_reserved_snapshot_for_leader(new_reserved_snapshot))) {
+      if (OB_SNAPSHOT_DISCARDED != ret) {
         LOG_WARN("failed to update min_reserved_snapshot", K(ret), "ls_id", ls_->get_ls_id(), K(new_reserved_snapshot));
       }
+    }
   } else if (OB_FAIL(sync_clog(new_reserved_snapshot))) {
     LOG_WARN("failed to send update reserved snapshot log", K(ret), K(new_reserved_snapshot));
   } else if (need_print_log()) {

src/storage/tablet/ob_tablet.cpp

@@ -5290,8 +5290,8 @@ int ObTablet::get_kept_snapshot_info(
     ret = OB_ERR_UNEXPECTED;
     LOG_WARN("snapshot info is invalid", KR(ret), K(snapshot_info));
   }
-  LOG_TRACE("get multi version start", "ls_id", get_tablet_meta().ls_id_, K(tablet_id),
-      K(snapshot_info), K(old_min_reserved_snapshot), K(get_tablet_meta()),
+  LOG_INFO("get multi version start", "ls_id", get_tablet_meta().ls_id_, K(tablet_id),
+      K(snapshot_info), K(old_min_reserved_snapshot),
       K(min_medium_snapshot), K(min_reserved_snapshot_on_ls), K(max_merged_snapshot));
   return ret;
 }

unittest/storage/CMakeLists.txt

@@ -121,6 +121,7 @@ storage_dml_unittest(test_partition_range_splite)
 storage_dml_unittest(test_major_rows_merger)
 storage_dml_unittest(test_tablet tablet/test_tablet.cpp)
 storage_unittest(test_medium_list_checker compaction/test_medium_list_checker.cpp)
+storage_dml_unittest(test_ls_reserved_snapshot_mgr compaction/test_ls_reserved_snapshot_mgr.cpp)
 storage_unittest(test_protected_memtable_mgr_handle test_protected_memtable_mgr_handle.cpp)
 storage_unittest(test_choose_migration_source_policy migration/test_choose_migration_source_policy.cpp)
 

unittest/storage/compaction/test_ls_reserved_snapshot_mgr.cpp

@@ -0,0 +1,166 @@
+/**
+ * Copyright (c) 2023 OceanBase
+ * OceanBase CE is licensed under Mulan PubL v2.
+ * You can use this software according to the terms and conditions of the Mulan PubL v2.
+ * You may obtain a copy of Mulan PubL v2 at:
+ *          http://license.coscl.org.cn/MulanPubL-2.0
+ * THIS SOFTWARE IS PROVIDED ON AN "AS IS" BASIS, WITHOUT WARRANTIES OF ANY KIND,
+ * EITHER EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO NON-INFRINGEMENT,
+ * MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE.
+ * See the Mulan PubL v2 for more details.
+ */
+#define USING_LOG_PREFIX STORAGE
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+#define private public
+#define protected public
+#include <string.h>
+#include "storage/ls/ob_ls_reserved_snapshot_mgr.h"
+#include "mittest/mtlenv/mock_tenant_module_env.h"
+#include "unittest/storage/test_dml_common.h"
+namespace oceanbase
+{
+using namespace common;
+using namespace compaction;
+using namespace storage;
+
+namespace storage
+{
+int ObLSReservedSnapshotMgr::sync_clog(const int64_t new_reserved_snapshot)
+{
+  LOG_INFO("mock sync clog", K(new_reserved_snapshot));
+  return OB_SUCCESS;
+}
+}
+
+namespace unittest
+{
+
+class TestLSReservedSnapshotMgr : public ::testing::Test
+{
+public:
+  TestLSReservedSnapshotMgr()
+  : ls_id_(TEST_LS_ID)
+  {}
+  ~TestLSReservedSnapshotMgr() = default;
+  static void SetUpTestCase();
+  static void TearDownTestCase();
+  static constexpr int64_t TEST_TENANT_ID = 1;
+  static constexpr int64_t TEST_LS_ID = 9001;
+  share::ObLSID ls_id_;
+};
+
+void TestLSReservedSnapshotMgr::SetUpTestCase()
+{
+  int ret = OB_SUCCESS;
+  ret = MockTenantModuleEnv::get_instance().init();
+  ASSERT_EQ(OB_SUCCESS, ret);
+
+  // ls service cannot service before ObServerCheckpointSlogHandler starts
+  // running
+  ObServerCheckpointSlogHandler::get_instance().is_started_ = true;
+  // create ls
+  ObLSHandle ls_handle;
+  ret = TestDmlCommon::create_ls(TEST_TENANT_ID, ObLSID(TEST_LS_ID), ls_handle);
+  if (OB_SUCCESS != ret) {
+    LOG_ERROR("[FATAL ERROR] failed to create ls", K(ret));
+  }
+  ASSERT_EQ(OB_SUCCESS, ret);
+}
+
+void TestLSReservedSnapshotMgr::TearDownTestCase()
+{
+  int ret = OB_SUCCESS;
+  ret = MTL(ObLSService*)->remove_ls(ObLSID(TEST_LS_ID));
+  ASSERT_EQ(OB_SUCCESS, ret);
+
+  MockTenantModuleEnv::get_instance().destroy();
+}
+
+TEST_F(TestLSReservedSnapshotMgr, test_add_del_dependent_tablet)
+{
+  int ret = OB_SUCCESS;
+  ObLSHandle ls_handle;
+  ObLSService *ls_svr = MTL(ObLSService*);
+  ret = ls_svr->get_ls(ls_id_, ls_handle, ObLSGetMod::STORAGE_MOD);
+  ASSERT_EQ(OB_SUCCESS, ret);
+
+  ObLSReservedSnapshotMgr &reserved_snapshot_mgr = ls_handle.get_ls()->reserved_snapshot_mgr_;
+  ASSERT_EQ(OB_SUCCESS, reserved_snapshot_mgr.try_sync_reserved_snapshot(100, true/*update_flag*/));
+  ASSERT_EQ(100, reserved_snapshot_mgr.min_reserved_snapshot_);
+
+  const ObTabletID tablet_id(100);
+
+  ASSERT_EQ(OB_SUCCESS, reserved_snapshot_mgr.add_dependent_medium_tablet(tablet_id));
+  ASSERT_EQ(OB_ENTRY_EXIST, reserved_snapshot_mgr.add_dependent_medium_tablet(tablet_id));
+
+  ASSERT_EQ(OB_SUCCESS, reserved_snapshot_mgr.del_dependent_medium_tablet(tablet_id));
+  ASSERT_EQ(OB_HASH_NOT_EXIST, reserved_snapshot_mgr.del_dependent_medium_tablet(tablet_id));
+
+  ASSERT_EQ(100, reserved_snapshot_mgr.min_reserved_snapshot_);
+
+  const ObTabletID tablet_id2(200);
+  ASSERT_EQ(OB_SUCCESS, reserved_snapshot_mgr.add_dependent_medium_tablet(tablet_id));
+  ASSERT_EQ(OB_SUCCESS, reserved_snapshot_mgr.add_dependent_medium_tablet(tablet_id2));
+
+  // have dependent tablet, can not push min_reserved_snapshot_
+  ASSERT_EQ(OB_SUCCESS, reserved_snapshot_mgr.try_sync_reserved_snapshot(200, true/*update_flag*/));
+  ASSERT_EQ(100, reserved_snapshot_mgr.min_reserved_snapshot_);
+  ASSERT_EQ(200, reserved_snapshot_mgr.next_reserved_snapshot_);
+
+  ASSERT_EQ(OB_SUCCESS, reserved_snapshot_mgr.try_sync_reserved_snapshot(300, true/*update_flag*/));
+  ASSERT_EQ(100, reserved_snapshot_mgr.min_reserved_snapshot_);
+  ASSERT_EQ(300, reserved_snapshot_mgr.next_reserved_snapshot_);
+
+  ASSERT_EQ(OB_SUCCESS, reserved_snapshot_mgr.del_dependent_medium_tablet(tablet_id));
+  ASSERT_EQ(100, reserved_snapshot_mgr.min_reserved_snapshot_);
+  ASSERT_EQ(300, reserved_snapshot_mgr.next_reserved_snapshot_);
+  // no dependent tablet, push min_reserved_snapshot_
+  ASSERT_EQ(OB_SUCCESS, reserved_snapshot_mgr.del_dependent_medium_tablet(tablet_id2));
+  ASSERT_EQ(300, reserved_snapshot_mgr.min_reserved_snapshot_);
+  ASSERT_EQ(0, reserved_snapshot_mgr.next_reserved_snapshot_);
+
+  reserved_snapshot_mgr.min_reserved_snapshot_ = 0;
+  reserved_snapshot_mgr.next_reserved_snapshot_ = 0;
+}
+
+TEST_F(TestLSReservedSnapshotMgr, test_update_with_smaller_snapshot)
+{
+  int ret = OB_SUCCESS;
+  ObLSHandle ls_handle;
+  ObLSService *ls_svr = MTL(ObLSService*);
+  ret = ls_svr->get_ls(ls_id_, ls_handle, ObLSGetMod::STORAGE_MOD);
+  ASSERT_EQ(OB_SUCCESS, ret);
+
+  ObLSReservedSnapshotMgr &reserved_snapshot_mgr = ls_handle.get_ls()->reserved_snapshot_mgr_;
+  ASSERT_EQ(OB_SUCCESS, reserved_snapshot_mgr.try_sync_reserved_snapshot(100, true/*update_flag*/));
+  ASSERT_EQ(100, reserved_snapshot_mgr.min_reserved_snapshot_);
+
+  // can't update with smaller snapshot
+  ASSERT_EQ(OB_SNAPSHOT_DISCARDED, reserved_snapshot_mgr.try_sync_reserved_snapshot(50, true/*update_flag*/));
+
+  const ObTabletID tablet_id(100);
+
+  ASSERT_EQ(OB_SUCCESS, reserved_snapshot_mgr.add_dependent_medium_tablet(tablet_id));
+  ASSERT_EQ(OB_SNAPSHOT_DISCARDED, reserved_snapshot_mgr.try_sync_reserved_snapshot(50, true/*update_flag*/));
+
+  ASSERT_EQ(OB_SUCCESS, reserved_snapshot_mgr.del_dependent_medium_tablet(tablet_id));
+  ASSERT_EQ(OB_SNAPSHOT_DISCARDED, reserved_snapshot_mgr.try_sync_reserved_snapshot(50, true/*update_flag*/));
+
+  ASSERT_EQ(100, reserved_snapshot_mgr.min_reserved_snapshot_);
+
+  ASSERT_EQ(OB_SUCCESS, reserved_snapshot_mgr.try_sync_reserved_snapshot(150, true/*update_flag*/));
+  ASSERT_EQ(150, reserved_snapshot_mgr.min_reserved_snapshot_);
+}
+
+}//end namespace unittest
+}//end namespace oceanbase
+
+int main(int argc, char **argv)
+{
+  system("rm -f test_ls_reserved_snapshot.log*");
+  OB_LOGGER.set_file_name("test_ls_reserved_snapshot.log");
+  OB_LOGGER.set_log_level("DEBUG");
+  testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}

unittest/storage/test_compaction_policy.cpp

@@ -233,7 +233,6 @@ void TestCompactionPolicy::TearDownTestCase()
   ret = MTL(ObLSService*)->remove_ls(ObLSID(TEST_LS_ID));
   ASSERT_EQ(OB_SUCCESS, ret);
   ObTenantMetaMemMgr *t3m = MTL(ObTenantMetaMemMgr*);
-  ASSERT_EQ(OB_SUCCESS, ret);
 
   ObLSID ls_id = ObLSID(TEST_LS_ID);
   ObTabletID tablet_id = ObTabletID(TEST_TABLET_ID);