From 8a01819087c8189244a215df826f34e1d659b058 Mon Sep 17 00:00:00 2001 From: Nobuhiro Ban Date: Mon, 4 Nov 2024 04:35:22 +0900 Subject: [PATCH 1/3] add fail test (modify_by_upsert_must_change_tidword) before fix --- .../tsurugi_issues/tsurugi_issue1016_test.cpp | 92 +++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 test/tsurugi_issues/tsurugi_issue1016_test.cpp diff --git a/test/tsurugi_issues/tsurugi_issue1016_test.cpp b/test/tsurugi_issues/tsurugi_issue1016_test.cpp new file mode 100644 index 00000000..1c657da2 --- /dev/null +++ b/test/tsurugi_issues/tsurugi_issue1016_test.cpp @@ -0,0 +1,92 @@ + +#include "test_tool.h" + +#include "concurrency_control/include/epoch.h" +#include "concurrency_control/include/garbage.h" +#include "concurrency_control/include/session.h" + +#include "shirakami/interface.h" +#include "index/yakushima/include/interface.h" + +#include "glog/logging.h" +#include "gtest/gtest.h" + +using namespace shirakami; + +// tsurugi issue #1016: upserting the Record (which is written in same epoch) may not change tid_word + +namespace shirakami::testing { + +class tsurugi_issue1016_test : public ::testing::Test { +public: + static void call_once_f() { + google::InitGoogleLogging("shirakami-test-tsurugi_issues-tsurugi_issue1016_test"); + // FLAGS_stderrthreshold = 0; + } + + void SetUp() override { + std::call_once(init_, call_once_f); + init(); + } + + void TearDown() override { fin(); } + +private: + static inline std::once_flag init_; +}; + +TEST_F(tsurugi_issue1016_test, modify_by_upsert_must_change_tidword) { + // expect + // OCC1: begin -> OK + // OCC2: begin -> OK + // OCC3: begin -> OK + // wait epoch; to do next 3 lines (OCC1 commit - OCC3 commit) in the same epoch + // OCC1: upsert A 1, commit -> OK + // OCC2: search_key A -> OK, reads A=1 + // OCC3: upsert A 3, commit -> OK + // OCC2: commit -> ERR_CC by read verify + + // before ti#1016 fix + // OCC1: begin -> OK + // OCC2: begin -> OK + // OCC3: begin -> OK + // wait epoch; to do next 3 lines (OCC1 commit - OCC3 commit) in the same epoch + // OCC1: upsert A 1, commit -> OK + // OCC2: search_key A -> OK, reads A=1 + // OCC3: upsert A 3 -> OK + // OCC3: commit -> OK <- sometime use same tid as previous + // OCC2: commit -> OK <- wrong, read_verify doesn't work + + Storage st{}; + ASSERT_OK(create_storage("", st)); + Token s1{}; + Token s2{}; + Token s3{}; + ASSERT_OK(enter(s1)); + ASSERT_OK(enter(s2)); + ASSERT_OK(enter(s3)); + + ASSERT_OK(tx_begin({s1, transaction_options::transaction_type::SHORT})); + ASSERT_OK(tx_begin({s2, transaction_options::transaction_type::SHORT})); + ASSERT_OK(tx_begin({s3, transaction_options::transaction_type::SHORT})); + + wait_epoch_update(); + + ASSERT_OK(upsert(s1, st, "A", "1")); + ASSERT_OK(commit(s1)); + + std::string buf{}; + ASSERT_OK(search_key(s2, st, "A", buf)); + ASSERT_EQ(buf, "1"); + + ASSERT_OK(upsert(s3, st, "A", "3")); + ASSERT_OK(commit(s3)); + + ASSERT_EQ(commit(s2), Status::ERR_CC); + + ASSERT_OK(leave(s1)); + ASSERT_OK(leave(s2)); + ASSERT_OK(leave(s3)); +} + +} // namespace shirakami::testing From f72da54d2a316ce3a6c434a057c1a61dc943b3d8 Mon Sep 17 00:00:00 2001 From: Nobuhiro Ban Date: Wed, 6 Nov 2024 16:27:47 +0900 Subject: [PATCH 2/3] check tid_word oreder around flag bits --- test/tid_word/tid_word_test.cpp | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/tid_word/tid_word_test.cpp b/test/tid_word/tid_word_test.cpp index d9bfcf72..9889d2f3 100644 --- a/test/tid_word/tid_word_test.cpp +++ b/test/tid_word/tid_word_test.cpp @@ -78,4 +78,31 @@ TEST_F(tid_word_test, tid_keeps_ordered) { } } +TEST_F(tid_word_test, tid_is_stronger_than_lock) { + tid_word t1{}, t2{}; + t1.set_tid(1); + t1.set_lock(true); + t2.set_tid(2); + t2.set_lock(false); + EXPECT_LT(t1, t2); +} + +TEST_F(tid_word_test, tid_is_stronger_than_absent) { + tid_word t1{}, t2{}; + t1.set_tid(1); + t1.set_absent(true); + t2.set_tid(2); + t2.set_absent(false); + EXPECT_LT(t1, t2); +} + +TEST_F(tid_word_test, absent_is_stronger_than_lock) { + tid_word t1{}, t2{}; + t1.set_lock(true); + t1.set_absent(false); + t2.set_lock(false); + t2.set_absent(true); + EXPECT_LT(t1, t2); +} + } // namespace shirakami::testing From b618f002ce79231867169058173cf1a56cf63d3a Mon Sep 17 00:00:00 2001 From: Nobuhiro Ban Date: Wed, 6 Nov 2024 18:15:48 +0900 Subject: [PATCH 3/3] see updated Record's vesion (epoch/tid) at upsert --- src/concurrency_control/interface/short_tx/termination.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/concurrency_control/interface/short_tx/termination.cpp b/src/concurrency_control/interface/short_tx/termination.cpp index 6f9be6b8..e154395f 100644 --- a/src/concurrency_control/interface/short_tx/termination.cpp +++ b/src/concurrency_control/interface/short_tx/termination.cpp @@ -395,6 +395,10 @@ Status write_lock(session* ti, tid_word& commit_tid) { return Status::ERR_KVS; } } + // check Record version to update + if (!rec_ptr->get_tidw_ref().get_absent()) { + commit_tid = std::max(commit_tid, rec_ptr->get_tidw_ref()); + } return Status::OK; } if (rc == Status::ERR_CC) {