From adc086bafc74b6d7b0109d652748db9d75562208 Mon Sep 17 00:00:00 2001 From: Takayuki Tanabe Date: Thu, 22 Feb 2024 14:07:30 +0900 Subject: [PATCH] fix: to keep epoch and tid at converting inserting to deleted project-tsurugi/tsurugi-issues#361 --- .../interface/short_tx/termination.cpp | 6 ++- test/tsurugi_issues/tsurugi_issue361_test.cpp | 51 +++++++++++++++++-- 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/src/concurrency_control/interface/short_tx/termination.cpp b/src/concurrency_control/interface/short_tx/termination.cpp index 661e8a6db..3b531ddf5 100644 --- a/src/concurrency_control/interface/short_tx/termination.cpp +++ b/src/concurrency_control/interface/short_tx/termination.cpp @@ -87,6 +87,9 @@ void unlock_records(session* const ti, std::size_t num_locked) { } } +/** + * This is called by only abort function +*/ void change_inserting_records_state(session* const ti) { auto process = [ti](write_set_obj* wso_ptr) { Record* rec_ptr = wso_ptr->get_rec_ptr(); @@ -113,7 +116,8 @@ void change_inserting_records_state(session* const ti) { tid.set_absent(true); tid.set_latest(false); tid.set_lock(false); - tid.set_epoch(ti->get_step_epoch()); + tid.set_epoch(check.get_epoch()); + tid.set_tid(check.get_tid()); rec_ptr->set_tid(tid); // and unlock } else { // some operations interrupt and this is normal state. diff --git a/test/tsurugi_issues/tsurugi_issue361_test.cpp b/test/tsurugi_issues/tsurugi_issue361_test.cpp index bbf23c427..6cdadcbe6 100644 --- a/test/tsurugi_issues/tsurugi_issue361_test.cpp +++ b/test/tsurugi_issues/tsurugi_issue361_test.cpp @@ -17,8 +17,6 @@ using namespace shirakami; -#define ASSERT_OK(expr) ASSERT_EQ(expr, shirakami::Status::OK) - namespace shirakami::testing { class tsurugi_issue361 : public ::testing::Test { // NOLINT @@ -27,6 +25,7 @@ class tsurugi_issue361 : public ::testing::Test { // NOLINT google::InitGoogleLogging("shirakami-test-concurrency_control-" "complicated-tsurugi_issue361"); // FLAGS_stderrthreshold = 0; + init_for_test(); } void SetUp() override { @@ -40,7 +39,53 @@ class tsurugi_issue361 : public ::testing::Test { // NOLINT static inline std::once_flag init_; // NOLINT }; -TEST_F(tsurugi_issue361, occ_must_not_read_uncommitted_record) { // NOLINT +TEST_F(tsurugi_issue361, comment_20240220) { // NOLINT + // prepare + Storage st{}; + ASSERT_OK(create_storage("tb1", st)); + Token t1{}; + ASSERT_OK(enter(t1)); + ASSERT_OK(tx_begin({t1, transaction_options::transaction_type::SHORT})); + ASSERT_OK(upsert(t1, st, "1", "100")); + ASSERT_OK(commit(t1)); + + // test + ASSERT_OK(tx_begin({t1, transaction_options::transaction_type::SHORT})); + ScanHandle shd1{}; + ASSERT_OK(open_scan(t1, st, "", scan_endpoint::INF, "", scan_endpoint::INF, + shd1)); + std::string buf{}; + ASSERT_OK(read_key_from_scan(t1, shd1, buf)); + ASSERT_EQ(buf, "1"); + ASSERT_EQ(Status::WARN_SCAN_LIMIT, next(t1, shd1)); + ASSERT_OK(close_scan(t1, shd1)); + ASSERT_OK(insert(t1, st, "2", "100")); + + Token t2{}; + ASSERT_OK(enter(t2)); + ASSERT_OK(tx_begin({t2, transaction_options::transaction_type::SHORT})); + ScanHandle shd2{}; + ASSERT_OK(open_scan(t2, st, "", scan_endpoint::INF, "", scan_endpoint::INF, + shd1)); + ASSERT_OK(read_key_from_scan(t2, shd2, buf)); + ASSERT_EQ(buf, "1"); + ASSERT_EQ(Status::OK, next(t2, shd2)); + ASSERT_EQ(Status::WARN_CONCURRENT_INSERT, + read_key_from_scan(t2, shd2, buf)); // for "2" + ASSERT_EQ(Status::WARN_SCAN_LIMIT, next(t2, shd2)); + ASSERT_OK(close_scan(t2, shd2)); + ASSERT_OK(insert(t2, st, "3", "100")); + + ASSERT_EQ(Status::ERR_CC, commit(t1)); + ASSERT_OK(commit(t2)); + + // cleanup + ASSERT_OK(leave(t1)); + ASSERT_OK(leave(t2)); +} + +TEST_F(tsurugi_issue361, + occ_must_not_read_uncommitted_record) { // NOLINT std::size_t inserting_num = 0; std::size_t ok_num = 0; auto count = [&inserting_num, &ok_num](Storage& st) {