Add test for get multi ids

Signed-off-by: JaySon-Huang <tshent@qq.com>
pingcap · Dec 13, 2021 · ccb060a · ccb060a
1 parent 782da35
commit ccb060a
Show file tree

Hide file tree

Showing 6 changed files with 152 additions and 25 deletions.
diff --git a/dbms/src/Common/ErrorCodes.cpp b/dbms/src/Common/ErrorCodes.cpp
@@ -406,8 +406,8 @@ extern const int DIVIDED_BY_ZERO = 10011;
 extern const int INVALID_TIME = 10012;
 extern const int DEADLOCK_AVOIDED = 10013;
 extern const int PTHREAD_ERROR = 10014;
-extern const int PS_PAGE_NOT_EXISTS = 10015;
-extern const int PS_PAGE_NO_VALID_VERSION = 10016;
+extern const int PS_ENTRY_NOT_EXISTS = 10015;
+extern const int PS_ENTRY_NO_VALID_VERSION = 10016;
 } // namespace ErrorCodes
 
 } // namespace DB
diff --git a/dbms/src/Storages/Page/PageStorage.h b/dbms/src/Storages/Page/PageStorage.h
@@ -7,8 +7,8 @@
 #include <Storages/Page/Page.h>
 #include <Storages/Page/PageDefines.h>
 #include <Storages/Page/PageUtil.h>
-#include <Storages/Page/WriteBatch.h>
 #include <Storages/Page/Snapshot.h>
+#include <Storages/Page/WriteBatch.h>
 #include <fmt/format.h>
 
 #include <condition_variable>

diff --git a/dbms/src/Storages/Page/V3/PageDirectory.cpp b/dbms/src/Storages/Page/V3/PageDirectory.cpp
@@ -1,5 +1,6 @@
 #include <Common/Exception.h>
 #include <Storages/Page/V3/PageDirectory.h>
+#include <Storages/Page/V3/PageEntry.h>
 
 #include <mutex>
 
@@ -8,8 +9,8 @@ namespace DB
 namespace ErrorCodes
 {
 extern const int NOT_IMPLEMENTED;
-extern const int PS_PAGE_NOT_EXISTS;
-extern const int PS_PAGE_NO_VALID_VERSION;
+extern const int PS_ENTRY_NOT_EXISTS;
+extern const int PS_ENTRY_NO_VALID_VERSION;
 } // namespace ErrorCodes
 namespace PS::V3
 {
@@ -37,20 +38,50 @@ PageIDAndEntryV3 PageDirectory::get(PageId page_id, const PageDirectorySnapshotP
         std::shared_lock read_lock(table_rw_mutex);
         iter = mvcc_table_directory.find(page_id);
         if (iter == mvcc_table_directory.end())
-            throw Exception(fmt::format("Page{} not exist!", page_id), ErrorCodes::PS_PAGE_NOT_EXISTS);
+            throw Exception(fmt::format("Entry [id={}] not exist!", page_id), ErrorCodes::PS_ENTRY_NOT_EXISTS);
     }
 
     if (auto entry = iter->second->getEntry(snap->sequence); entry)
     {
         return PageIDAndEntryV3(page_id, *entry);
     }
 
-    throw Exception(fmt::format("Page{} with seq={} not exist!", page_id, snap->sequence), ErrorCodes::PS_PAGE_NO_VALID_VERSION);
+    throw Exception(fmt::format("Entry [id={}] [seq={}] not exist!", page_id, snap->sequence), ErrorCodes::PS_ENTRY_NO_VALID_VERSION);
 }
 
-PageIDAndEntriesV3 PageDirectory::get(const PageIds & /*read_ids*/, const PageDirectorySnapshotPtr & /*snap*/) const
+PageIDAndEntriesV3 PageDirectory::get(const PageIds & page_ids, const PageDirectorySnapshotPtr & snap) const
 {
-    throw Exception("Not implemented", ErrorCodes::NOT_IMPLEMENTED);
+    std::vector<MVCCMapType::const_iterator> iters;
+    iters.reserve(page_ids.size());
+    {
+        std::shared_lock read_lock(table_rw_mutex);
+        for (size_t idx = 0; idx < page_ids.size(); ++idx)
+        {
+            if (auto iter = mvcc_table_directory.find(page_ids[idx]);
+                iter != mvcc_table_directory.end())
+            {
+                iters.emplace_back(iter);
+            }
+            else
+            {
+                throw Exception(fmt::format("Entry [id={}] at [idx={}] not exist!", page_ids[idx], idx), ErrorCodes::PS_ENTRY_NOT_EXISTS);
+            }
+        }
+    }
+
+    PageIDAndEntriesV3 id_entries;
+    for (size_t idx = 0; idx < page_ids.size(); ++idx)
+    {
+        const auto & iter = iters[idx];
+        if (auto entry = iter->second->getEntry(snap->sequence); entry)
+        {
+            id_entries.emplace_back(page_ids[idx], *entry);
+        }
+        else
+            throw Exception(fmt::format("Entry [id={}] [seq={}] at [idx={}] not exist!", page_ids[idx], snap->sequence, idx), ErrorCodes::PS_ENTRY_NO_VALID_VERSION);
+    }
+
+    return id_entries;
 }
 
 void PageDirectory::apply(PageEntriesEdit && edit)

diff --git a/dbms/src/Storages/Page/V3/PageDirectory.h b/dbms/src/Storages/Page/V3/PageDirectory.h
@@ -31,7 +31,7 @@ class PageDirectory
     PageDirectorySnapshotPtr createSnapshot() const;
 
     PageIDAndEntryV3 get(PageId page_id, const PageDirectorySnapshotPtr & snap) const;
-    PageIDAndEntriesV3 get(const PageIds & read_ids, const PageDirectorySnapshotPtr & snap) const;
+    PageIDAndEntriesV3 get(const PageIds & page_ids, const PageDirectorySnapshotPtr & snap) const;
 
     void apply(PageEntriesEdit && edit);
 

diff --git a/dbms/src/Storages/Page/V3/tests/gtest_map_utils.cpp b/dbms/src/Storages/Page/V3/tests/gtest_map_utils.cpp
@@ -5,8 +5,9 @@
 #include <map>
 namespace DB::PS::V3::tests
 {
-
-struct Empty{};
+struct Empty
+{
+};
 
 ::testing::AssertionResult MapIterCompare(
     const char * lhs_expr,

diff --git a/dbms/src/Storages/Page/V3/tests/gtest_page_directory.cpp b/dbms/src/Storages/Page/V3/tests/gtest_page_directory.cpp
@@ -1,4 +1,6 @@
 #include <Common/Exception.h>
+#include <Common/FmtUtils.h>
+#include <Common/joinStr.h>
 #include <IO/WriteHelpers.h>
 #include <Storages/Page/Page.h>
 #include <Storages/Page/V3/PageDirectory.h>
@@ -7,15 +9,13 @@
 #include <TestUtils/TiFlashTestBasic.h>
 #include <fmt/format.h>
 
-#include "gtest/gtest.h"
-
 
 namespace DB
 {
 namespace ErrorCodes
 {
-extern const int PS_PAGE_NOT_EXISTS;
-extern const int PS_PAGE_NO_VALID_VERSION;
+extern const int PS_ENTRY_NOT_EXISTS;
+extern const int PS_ENTRY_NO_VALID_VERSION;
 } // namespace ErrorCodes
 namespace PS::V3::tests
 {
@@ -45,16 +45,15 @@ ::testing::AssertionResult getEntryCompare(
         String err_msg;
         if (pid != expected_id_entry.first)
         {
-            err_msg = fmt::format("Try to get Page{} but get Page{}", page_id_expr, pid);
+            err_msg = fmt::format("Try to get entry [id={}] but get [id={}]", page_id_expr, pid);
             return ::testing::AssertionFailure(::testing::Message(err_msg.c_str()));
         }
         if (isSameEntry(entry, expected_entry))
         {
-            // Do not need a further check on the unsafe
             return ::testing::AssertionSuccess();
         }
         // else not the expected entry we want
-        auto actual_expr = fmt::format("Get Page{} from {} with snap{}", page_id_expr, dir_expr, snap_expr);
+        auto actual_expr = fmt::format("Get entry [id={}] from {} with snap{}", page_id_expr, dir_expr, snap_expr);
         return testing::internal::EqFailure(
             expected_entry_expr,
             actual_expr.c_str(),
@@ -70,10 +69,10 @@ ::testing::AssertionResult getEntryCompare(
     }
     catch (DB::Exception & ex)
     {
-        if (ex.code() == ErrorCodes::PS_PAGE_NOT_EXISTS)
-            error = fmt::format("Try to get Page{} but not exists. Err message: {}", page_id_expr, ex.message());
-        else if (ex.code() == ErrorCodes::PS_PAGE_NO_VALID_VERSION)
-            error = fmt::format("Try to get Page{} with version {} from {} but failed. Err message: {}", page_id_expr, snap->sequence, snap_expr, ex.message());
+        if (ex.code() == ErrorCodes::PS_ENTRY_NOT_EXISTS)
+            error = fmt::format("Try to get entry [id={}] but not exists. Err message: {}", page_id_expr, ex.message());
+        else if (ex.code() == ErrorCodes::PS_ENTRY_NO_VALID_VERSION)
+            error = fmt::format("Try to get entry [id={}] with version {} from {} but failed. Err message: {}", page_id_expr, snap->sequence, snap_expr, ex.message());
         else
             error = ex.displayText();
         return ::testing::AssertionFailure(::testing::Message(error.c_str()));
@@ -89,6 +88,96 @@ ::testing::AssertionResult getEntryCompare(
 #define EXPECT_ENTRY_EQ(expected_entry, dir, pid, snap) \
     EXPECT_PRED_FORMAT4(getEntryCompare, expected_entry, dir, pid, snap)
 
+String toString(const PageIDAndEntriesV3 & entries)
+{
+    FmtBuffer buf;
+    buf.append("[");
+    joinStr(
+        entries.begin(),
+        entries.end(),
+        buf,
+        [](const PageIDAndEntryV3 & id_entry, FmtBuffer & buf) {
+            buf.fmtAppend("<{},{}>", id_entry.first, toString(id_entry.second));
+        });
+    buf.append("]");
+    return buf.toString();
+}
+::testing::AssertionResult getEntriesCompare(
+    const char * expected_entries_expr,
+    const char * dir_expr,
+    const char * page_ids_expr,
+    const char * snap_expr,
+    const PageIDAndEntriesV3 & expected_entries,
+    const PageDirectory & dir,
+    const PageIds page_ids,
+    const PageDirectorySnapshotPtr & snap)
+{
+    auto check_id_entries = [&](const PageIDAndEntriesV3 & expected_id_entries, const PageIDAndEntriesV3 & actual_id_entries) -> ::testing::AssertionResult {
+        if (expected_id_entries.size() == actual_id_entries.size())
+        {
+            for (size_t idx = 0; idx == expected_id_entries.size(); ++idx)
+            {
+                const auto & expected_id_entry = expected_id_entries[idx];
+                const auto & actual_id_entry = expected_id_entries[idx];
+                if (actual_id_entry.first != expected_id_entry.first)
+                {
+                    auto err_msg = fmt::format("Try to get entry [id={}] but get [id={}] at [index={}]", expected_id_entry.first, actual_id_entry.first, idx);
+                    return ::testing::AssertionFailure(::testing::Message(err_msg.c_str()));
+                }
+                if (!isSameEntry(expected_id_entry.second, actual_id_entry.second))
+                {
+                    // not the expected entry we want
+                    String err_msg;
+                    auto expect_expr = fmt::format("Entry at {} [index={}]", idx);
+                    auto actual_expr = fmt::format("Get entries {} from {} with snap {} [index={}", page_ids_expr, dir_expr, snap_expr, idx);
+                    return testing::internal::EqFailure(
+                        expect_expr.c_str(),
+                        actual_expr.c_str(),
+                        toString(expected_id_entry.second),
+                        toString(actual_id_entry.second),
+                        false);
+                }
+            }
+            return ::testing::AssertionSuccess();
+        }
+
+        // else not the expected entry we want
+        auto expected_expr = fmt::format("Entries from {} [size={}]", expected_entries_expr, expected_entries.size());
+        auto actual_expr = fmt::format("Get entries {} from {} with snap {}, [size={}]", page_ids_expr, dir_expr, snap_expr, actual_id_entries.size());
+        return testing::internal::EqFailure(
+            expected_expr.c_str(),
+            actual_expr.c_str(),
+            toString(expected_entries),
+            toString(actual_id_entries),
+            false);
+    };
+    String error;
+    try
+    {
+        auto id_entries = dir.get(page_ids, snap);
+        return check_id_entries(expected_entries, id_entries);
+    }
+    catch (DB::Exception & ex)
+    {
+        if (ex.code() == ErrorCodes::PS_ENTRY_NOT_EXISTS)
+            error = fmt::format("Try to get entries with [ids={}] but not exists. Err message: {}", page_ids_expr, ex.message());
+        else if (ex.code() == ErrorCodes::PS_ENTRY_NO_VALID_VERSION)
+            error = fmt::format("Try to get entries with [ids={}] with version {} from {} but failed. Err message: {}", page_ids_expr, snap->sequence, snap_expr, ex.message());
+        else
+            error = ex.displayText();
+        return ::testing::AssertionFailure(::testing::Message(error.c_str()));
+    }
+    catch (...)
+    {
+        error = getCurrentExceptionMessage(true);
+    }
+    return ::testing::AssertionFailure(::testing::Message(error.c_str()));
+}
+#define ASSERT_ENTRIES_EQ(expected_entries, dir, pid, snap) \
+    ASSERT_PRED_FORMAT4(getEntriesCompare, expected_entries, dir, pid, snap)
+#define EXPECT_ENTRIES_EQ(expected_entries, dir, pid, snap) \
+    EXPECT_PRED_FORMAT4(getEntriesCompare, expected_entries, dir, pid, snap)
+
 ::testing::AssertionResult getEntryNotExist(
     const char * dir_expr,
     const char * page_id_expr,
@@ -102,7 +191,7 @@ ::testing::AssertionResult getEntryNotExist(
     {
         auto id_entry = dir.get(page_id, snap);
         error = fmt::format(
-            "Expect Page{} from {} with snap{} not exist, but got <{}, {}>",
+            "Expect entry [id={}] from {} with snap{} not exist, but got <{}, {}>",
             page_id_expr,
             dir_expr,
             snap_expr,
@@ -111,7 +200,7 @@ ::testing::AssertionResult getEntryNotExist(
     }
     catch (DB::Exception & ex)
     {
-        if (ex.code() == ErrorCodes::PS_PAGE_NOT_EXISTS || ex.code() == ErrorCodes::PS_PAGE_NO_VALID_VERSION)
+        if (ex.code() == ErrorCodes::PS_ENTRY_NOT_EXISTS || ex.code() == ErrorCodes::PS_ENTRY_NO_VALID_VERSION)
             return ::testing::AssertionSuccess();
         else
             error = ex.displayText();
@@ -158,6 +247,12 @@ try
     auto snap2 = dir.createSnapshot();
     EXPECT_ENTRY_NOT_EXIST(dir, 2, snap1);
     EXPECT_ENTRY_EQ(entry2, dir, 2, snap2);
+    EXPECT_ENTRY_EQ(entry1, dir, 1, snap2);
+    {
+        PageIds ids{1, 2};
+        PageIDAndEntriesV3 expected_entries{{1, entry1}, {2, entry2}};
+        EXPECT_ENTRIES_EQ(expected_entries, dir, ids, snap2);
+    }
 }
 CATCH