From 3aedbe385586eae7ea54d8216ee77ac9e3b85d8f Mon Sep 17 00:00:00 2001 From: Ryoji Kurosawa Date: Wed, 4 Dec 2024 16:45:20 +0900 Subject: [PATCH] improve error handling for DROP TABLE https://github.com/project-tsurugi/tsurugi-issues/issues/1030 --- src/jogasaki/api/impl/database.cpp | 17 +- src/jogasaki/executor/common/create_table.cpp | 3 +- src/jogasaki/executor/common/drop_table.cpp | 19 +- src/jogasaki/executor/sequence/manager.cpp | 22 +- src/jogasaki/executor/sequence/manager.h | 18 +- .../executor/wrt/fill_record_fields.cpp | 10 +- .../api/transaction_fail_ddl_test.cpp | 494 ++++++++++++++++++ .../sequence/sequence_manager_test.cpp | 12 +- 8 files changed, 568 insertions(+), 27 deletions(-) create mode 100644 test/jogasaki/api/transaction_fail_ddl_test.cpp diff --git a/src/jogasaki/api/impl/database.cpp b/src/jogasaki/api/impl/database.cpp index 8e1d8d88..2cc2568c 100644 --- a/src/jogasaki/api/impl/database.cpp +++ b/src/jogasaki/api/impl/database.cpp @@ -980,10 +980,21 @@ status database::initialize_from_providers() { } try { sequence_manager_->load_id_map(tx.get()); - sequence_manager_->register_sequences(tx.get(), tables_); + sequence_manager_->register_sequences(tx.get(), tables_, false); } catch(executor::sequence::exception& e) { - LOG_LP(ERROR) << "registering sequences failed:" << e.get_status() << " " << e.what(); - return e.get_status(); + if(e.get_status() != status::err_not_found) { + LOG_LP(ERROR) << "registering sequences failed:" << e.get_status() << " " << e.what(); + return e.get_status(); + } + // missing sequence entry in __sequences table + // The situation possibly occurs by aborting transaction used for CREATE TABLE. + // Dropping the table and recreating it will fix the issue. + // We do not raise error in the start-up here. Allow users to read/dump the data for backup or + // to drop the table to fix the situation. + LOG_LP(WARNING) << "sequence '" << e.what() + << "' not found on the system table. Possibly the table definition did not " + "complete successfully. Inserting new records using the sequence is likely to hit an " + "error. Re-creating the table that owns the sequence may fix the issue"; } if(auto res = tx->commit(); res != status::ok) { LOG_LP(ERROR) << "committing table schema entries failed"; diff --git a/src/jogasaki/executor/common/create_table.cpp b/src/jogasaki/executor/common/create_table.cpp index ef1c4339..0633d378 100644 --- a/src/jogasaki/executor/common/create_table.cpp +++ b/src/jogasaki/executor/common/create_table.cpp @@ -96,7 +96,8 @@ bool create_generated_sequence( p.min_value(), p.max_value(), p.cycle(), - true + true, + true // create new seq_id ); // provider.add_sequence(p); // sequence definition is added in serializer, no need to add it here } catch (sequence::exception& e) { diff --git a/src/jogasaki/executor/common/drop_table.cpp b/src/jogasaki/executor/common/drop_table.cpp index bb8f034b..8e5a5294 100644 --- a/src/jogasaki/executor/common/drop_table.cpp +++ b/src/jogasaki/executor/common/drop_table.cpp @@ -76,13 +76,18 @@ bool remove_generated_sequences( auto def_id = s->definition_id().value(); try { if(! context.sequence_manager()->remove_sequence(def_id, context.transaction()->object().get())) { - VLOG_LP(log_error) << "sequence '" << sequence_name << "' not found"; - context.status_code(status::err_not_found); - return false; + // even on error, continue clean-up as much as possible + VLOG_LP(log_info) << "sequence '" << sequence_name << "' not found"; } } catch(executor::sequence::exception const& e) { - VLOG_LP(log_error) << "removing sequence '" << sequence_name << "' not found"; - context.status_code(e.get_status(), e.what()); + // unrecoverable error - transaction aborted and we cannot continue + VLOG_LP(log_error) << "removing sequence '" << sequence_name << "' failed"; + set_error( + context, + error_code::sql_execution_exception, + e.what(), + e.get_status() + ); return false; } } @@ -131,6 +136,10 @@ bool drop_table::operator()(request_context& context) const { return false; } + // note on error handling: now regular check for existence of table has passed. Going forward, if part of the table + // dependencies (e.g. secondary indices or sequence entry on system table) are missing, drop table should clean-up + // those dependencies as much as possible in order to avoid left garbage becoming a road block for other operations. + std::vector generated_sequences{}; // drop auto-generated sequences first because accessing system table causes cc error if(! drop_auto_generated_sequences(context, *t, generated_sequences)) { diff --git a/src/jogasaki/executor/sequence/manager.cpp b/src/jogasaki/executor/sequence/manager.cpp index 6624f2e5..c5b021ad 100644 --- a/src/jogasaki/executor/sequence/manager.cpp +++ b/src/jogasaki/executor/sequence/manager.cpp @@ -96,10 +96,14 @@ sequence* manager::register_sequence( sequence_value minimum_value, sequence_value maximum_value, bool enable_cycle, - bool save_id_map_entry + bool save_id_map_entry, + bool assign_new_seq_id_if_not_found ) { sequence_id seq_id{}; if(sequences_.count(def_id) == 0) { + if(! assign_new_seq_id_if_not_found) { + return {}; + } if(auto rc = db_->create_sequence(seq_id); rc != status::ok) { (void) tx->abort(); throw_exception(exception{rc}); @@ -142,15 +146,16 @@ sequence* manager::register_sequence( void manager::register_sequences( kvs::transaction* tx, - maybe_shared_ptr const& provider + maybe_shared_ptr const& provider, + bool assign_new_seq_id_if_not_found ) { provider->each_sequence( - [this, &tx]( + [this, &tx, assign_new_seq_id_if_not_found]( std::string_view, std::shared_ptr const& entry ) { auto def_id = *entry->definition_id(); - register_sequence( + if(auto p = register_sequence( tx, def_id, entry->simple_name(), @@ -159,8 +164,11 @@ void manager::register_sequences( entry->min_value(), entry->max_value(), entry->cycle(), - false - ); + false, + assign_new_seq_id_if_not_found + ); p == nullptr) { + throw_exception(exception{status::err_not_found, entry->simple_name()}); + } } ); save_id_map(tx); @@ -200,7 +208,7 @@ bool manager::remove_sequence( if (sequences_.count(def_id) == 0) { return false; } - if (auto rc = db_->delete_sequence(sequences_[def_id].id()); rc != status::ok) { + if (auto rc = db_->delete_sequence(sequences_[def_id].id()); rc != status::ok && rc != status::err_not_found) { // status::err_not_found never comes here because the sequence is already in sequences_ (void) tx->abort(); throw_exception(exception{rc}); diff --git a/src/jogasaki/executor/sequence/manager.h b/src/jogasaki/executor/sequence/manager.h index c2a980b4..b93c396d 100644 --- a/src/jogasaki/executor/sequence/manager.h +++ b/src/jogasaki/executor/sequence/manager.h @@ -120,7 +120,8 @@ class manager { /** * @brief register the sequence properties for the definition id * @details using the id_map currently held, create the in-memory sequence object with the given spec. - * If the id_map doesn't has the given def_id, ask kvs to assign new sequence id. Optionally save the id map. + * If the id_map doesn't has the given def_id, ask kvs to assign new sequence id + * (or exit if `assign_new_seq_id_if_not_found=false`). Optionally save the id map. * This function is not thread-safe. Only a thread can call this member function at a time. * @param def_id the key to uniquely identify the sequence * @param name the name of the sequence @@ -132,6 +133,8 @@ class manager { * min or max value (corresponding to the boundary that is went over.) * @param save_id_map_entry indicates whether the id_map entry for registered sequence should be saved now. * Set false if you are registering multiples sequences and they should be saved later. + * @param assign_new_seq_id_if_not_found if false, the function returns nullptr when the def_id is not found + * in the id_map. If true, the function creates new sequence id for such def_id. * @return the in-memory sequence object just registered * @throws sequence::exception if any error occurs, then passed transaction is aborted. */ @@ -144,7 +147,8 @@ class manager { sequence_value minimum_value = 0, sequence_value maximum_value = std::numeric_limits::max(), bool enable_cycle = true, - bool save_id_map_entry = true + bool save_id_map_entry = true, + bool assign_new_seq_id_if_not_found = true ); /** @@ -152,11 +156,17 @@ class manager { * @details this function retrieves sequence definitions from provider and register one by one. * This function is not thread-safe. Only a thread can call this member function at a time. * @param provider the config. provider that gives sequences definitions. - * @throws sequence::exception if any error occurs, then passed transaction is aborted. + * @param assign_new_seq_id_if_not_found if true, the function creates new sequence id for the def_id that + * is not on the id_map. If false, exception below is thrown for such def_id. + * @throws sequence::exception with status::err_not_found and what() containing sequence name if + * `assign_new_seq_id_if_not_found` is false and def_id is + * not found on id_map. The passed transaction is not touched. + * @throws sequence::exception if any other error occurs, then passed transaction is aborted. */ void register_sequences( kvs::transaction* tx, - maybe_shared_ptr const& provider + maybe_shared_ptr const& provider, + bool assign_new_seq_id_if_not_found ); /** diff --git a/src/jogasaki/executor/wrt/fill_record_fields.cpp b/src/jogasaki/executor/wrt/fill_record_fields.cpp index 8f0c8255..a9724c95 100644 --- a/src/jogasaki/executor/wrt/fill_record_fields.cpp +++ b/src/jogasaki/executor/wrt/fill_record_fields.cpp @@ -69,7 +69,15 @@ status next_sequence_value(request_context& ctx, sequence_definition_id def_id, auto& mgr = *ctx.sequence_manager(); auto* seq = mgr.find_sequence(def_id); if(seq == nullptr) { - throw_exception(std::logic_error{""}); + auto rc = status::err_unknown; + auto msg = string_builder{} << "sequence not found def_id:" << def_id + << " possibly due to DDL failure. Recreating " + "the table may fix the situation" + << string_builder::to_string; + set_error(ctx, error_code::sql_execution_exception, msg, rc); + // system level errornous situation - table can be read, but write is not reliable until recration + LOG_LP(ERROR) << msg; + return rc; } auto ret = seq->next(*ctx.transaction()->object()); // even if there is an error with next(), it mark the sequence as used by the tx, so call notify_updates first diff --git a/test/jogasaki/api/transaction_fail_ddl_test.cpp b/test/jogasaki/api/transaction_fail_ddl_test.cpp new file mode 100644 index 00000000..40203c81 --- /dev/null +++ b/test/jogasaki/api/transaction_fail_ddl_test.cpp @@ -0,0 +1,494 @@ +/* + * Copyright 2018-2023 Project Tsurugi. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include +#include +#include + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "../api/api_test_base.h" + +namespace jogasaki::testing { + +using namespace std::literals::string_literals; +using namespace jogasaki; +using namespace jogasaki::model; +using namespace jogasaki::executor; +using namespace jogasaki::scheduler; + +using takatori::util::unsafe_downcast; + +/** + * @brief test the sisuation that transaction used for DDL is aborted + */ +class transaction_fail_ddl_test : + public ::testing::Test, + public api_test_base { + +public: + // change this flag to debug with explain + bool to_explain() override { + return false; + } + + void SetUp() override { + auto cfg = std::make_shared(); + db_setup(cfg); + utils::set_global_tx_option(utils::create_tx_option{false, true}); + } + + void TearDown() override { + db_teardown(); + } + std::size_t seq_count(); + std::unordered_map seq_list(); + bool exists_seq(std::size_t seq_id); + bool remove_seq(std::size_t seq_id); +}; + +using namespace std::string_view_literals; + +std::size_t transaction_fail_ddl_test::seq_count() { + auto tx = utils::create_transaction(*db_); + auto tctx = get_transaction_context(*tx); + executor::sequence::metadata_store ms{*tctx->object()}; + return ms.size(); +} + +std::unordered_map transaction_fail_ddl_test::seq_list() { + auto tx = utils::create_transaction(*db_); + auto tctx = get_transaction_context(*tx); + executor::sequence::metadata_store ms{*tctx->object()}; + std::unordered_map ret{}; + ms.scan([&ret](std::size_t def_id, std::size_t id) { + ret[def_id] = id; + }); + return ret; +} + +bool transaction_fail_ddl_test::exists_seq(std::size_t seq_id) { + sequence_versioned_value value{}; + return api::impl::get_impl(*db_).kvs_db()->read_sequence(seq_id, value) != status::err_not_found; +} + +bool transaction_fail_ddl_test::remove_seq(std::size_t seq_id) { + return api::impl::get_impl(*db_).kvs_db()->delete_sequence(seq_id) == status::ok; +} + +TEST_F(transaction_fail_ddl_test, create_simple_table) { + // simple table with primary key and no identity column will not use sequence or sequences system table, so + // aborting transaction will not affect the table creation + if (jogasaki::kvs::implementation_id() == "memory") { + GTEST_SKIP() << "jogasaki-memory cannot rollback by abort"; + } + api::transaction_option opts{}; + { + auto tx = utils::create_transaction(*db_, opts); + execute_statement("CREATE TABLE t (c0 int primary key)", *tx); + ASSERT_EQ(status::ok, tx->abort()); + execute_statement("INSERT INTO t VALUES (1)"); + } + ASSERT_EQ(0, seq_count()); + + ASSERT_EQ(status::ok, db_->stop()); + ASSERT_EQ(status::ok, db_->start()); + execute_statement("INSERT INTO t VALUES (2)"); + ASSERT_EQ(0, seq_count()); + { + std::vector result{}; + execute_query("SELECT * FROM t", result); + ASSERT_EQ(2, result.size()); + } + execute_statement("DROP TABLE t"); + execute_statement("CREATE TABLE t (c0 int primary key)"); + ASSERT_EQ(0, seq_count()); + execute_statement("INSERT INTO t VALUES (1)"); + execute_statement("DROP TABLE t"); +} + +TEST_F(transaction_fail_ddl_test, create_pkless_table) { + // verify aborting transaction causes failure in storing sequence system table + // it appears DML works as in-memory object is used just after the DDL, but it fails to work after re-start. + if (jogasaki::kvs::implementation_id() == "memory") { + GTEST_SKIP() << "jogasaki-memory cannot rollback by abort"; + } + api::transaction_option opts{}; + { + auto tx = utils::create_transaction(*db_, opts); + execute_statement("CREATE TABLE t (c0 int)", *tx); + ASSERT_EQ(status::ok, tx->abort()); + } + execute_statement("INSERT INTO t VALUES (1)"); // using in-memory sequence, this dml won't fail + ASSERT_EQ(0, seq_count()); // no entry due to abort + + ASSERT_EQ(status::ok, db_->stop()); + ASSERT_EQ(status::ok, db_->start()); // warning message should be shown + test_stmt_err("INSERT INTO t VALUES (1)", error_code::sql_execution_exception); // sequence not found and DML should fail unexpectedly + ASSERT_EQ(0, seq_count()); + { + // even if sequence does not work, query should work + std::vector result{}; + execute_query("SELECT * FROM t", result); + ASSERT_EQ(1, result.size()); + } + // verify drop completely clean-up and recreation is successful + execute_statement("DROP TABLE t"); + execute_statement("CREATE TABLE t (c0 int)"); + ASSERT_EQ(1, seq_count()); + execute_statement("INSERT INTO t VALUES (1)"); + execute_statement("INSERT INTO t VALUES (1)"); + execute_statement("DROP TABLE t"); +} + +TEST_F(transaction_fail_ddl_test, create_table_with_identity_column) { + // similar as create_pkless_table, but using identity column instead of generated primary key + if (jogasaki::kvs::implementation_id() == "memory") { + GTEST_SKIP() << "jogasaki-memory cannot rollback by abort"; + } + api::transaction_option opts{}; + { + auto tx = utils::create_transaction(*db_, opts); + execute_statement("CREATE TABLE t (c0 int primary key, c1 int generated always as identity)", *tx); + ASSERT_EQ(status::ok, tx->abort()); + } + execute_statement("INSERT INTO t(c0) VALUES (1)"); // using in-memory sequence, this dml won't fail + ASSERT_EQ(0, seq_count()); // no entry due to abort + + ASSERT_EQ(status::ok, db_->stop()); + ASSERT_EQ(status::ok, db_->start()); // warning message should be shown + test_stmt_err("INSERT INTO t(c0) VALUES (2)", error_code::sql_execution_exception); // sequence not found and DML should fail unexpectedly + ASSERT_EQ(0, seq_count()); + { + // even if sequence does not work, query should work + std::vector result{}; + execute_query("SELECT * FROM t", result); + ASSERT_EQ(1, result.size()); + } + // verify drop completely clean-up and recreation is successful + execute_statement("DROP TABLE t"); + execute_statement("CREATE TABLE t (c0 int primary key, c1 int generated always as identity)"); + ASSERT_EQ(1, seq_count()); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + execute_statement("INSERT INTO t(c0) VALUES (2)"); + execute_statement("DROP TABLE t"); +} + +TEST_F(transaction_fail_ddl_test, create_pkless_table_with_identity_column) { + // same as create_pkless_table, but with identity column to add more generated column + if (jogasaki::kvs::implementation_id() == "memory") { + GTEST_SKIP() << "jogasaki-memory cannot rollback by abort"; + } + api::transaction_option opts{}; + { + auto tx = utils::create_transaction(*db_, opts); + execute_statement("CREATE TABLE t (c0 int, c1 int generated by default as identity)", *tx); + ASSERT_EQ(status::ok, tx->abort()); + } + execute_statement("INSERT INTO t(c0) VALUES (1)"); // using in-memory sequence, this dml won't fail + ASSERT_EQ(0, seq_count()); + + ASSERT_EQ(status::ok, db_->stop()); + ASSERT_EQ(status::ok, db_->start()); // warning message should be shown + test_stmt_err("INSERT INTO t(c0) VALUES (2)", error_code::sql_execution_exception); // sequence not found and DML should fail unexpectedly + ASSERT_EQ(0, seq_count()); + { + std::vector result{}; + execute_query("SELECT * FROM t", result); + ASSERT_EQ(1, result.size()); + } + execute_statement("DROP TABLE t"); + execute_statement("CREATE TABLE t (c0 int, c1 int generated by default as identity)"); + ASSERT_EQ(2, seq_count()); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + execute_statement("DROP TABLE t"); +} + +TEST_F(transaction_fail_ddl_test, drop_simple_table) { + // verify aborting transaction to drop table with primary key with no identity colum does no harm + if (jogasaki::kvs::implementation_id() == "memory") { + GTEST_SKIP() << "jogasaki-memory cannot rollback by abort"; + } + api::transaction_option opts{}; + execute_statement("CREATE TABLE t (c0 int primary key)"); + ASSERT_EQ(0, seq_count()); + execute_statement("INSERT INTO t VALUES (1)"); + execute_statement("INSERT INTO t VALUES (2)"); + { + auto tx = utils::create_transaction(*db_, opts); + execute_statement("DROP TABLE t", *tx); + ASSERT_EQ(status::ok, tx->abort()); + } + ASSERT_EQ(0, seq_count()); + execute_statement("CREATE TABLE t (c0 int primary key)"); + ASSERT_EQ(0, seq_count()); + execute_statement("INSERT INTO t VALUES (1)"); + execute_statement("INSERT INTO t VALUES (2)"); + execute_statement("DROP TABLE t"); + ASSERT_EQ(0, seq_count()); +} + +TEST_F(transaction_fail_ddl_test, drop_pkless_table) { + // verify aborting transaction to drop leaves sequence entry on the system table, but it does no harm (just leak) + if (jogasaki::kvs::implementation_id() == "memory") { + GTEST_SKIP() << "jogasaki-memory cannot rollback by abort"; + } + api::transaction_option opts{}; + execute_statement("CREATE TABLE t (c0 int)"); + ASSERT_EQ(1, seq_count()); + auto seqs = seq_list(); + ASSERT_TRUE(exists_seq(seqs[0])); + execute_statement("INSERT INTO t VALUES (1)"); + execute_statement("INSERT INTO t VALUES (1)"); + { + auto tx = utils::create_transaction(*db_, opts); + execute_statement("DROP TABLE t", *tx); + ASSERT_EQ(status::ok, tx->abort()); + } + ASSERT_EQ(1, seq_count()); // left entry due to abort + ASSERT_TRUE(! exists_seq(seqs[0])); // though table entry is left, sequence is removed correctly + execute_statement("CREATE TABLE t (c0 int)"); + ASSERT_EQ(2, seq_count()); + execute_statement("INSERT INTO t VALUES (1)"); + execute_statement("INSERT INTO t VALUES (1)"); + execute_statement("DROP TABLE t"); + ASSERT_EQ(1, seq_count()); +} + +TEST_F(transaction_fail_ddl_test, drop_pkless_table_with_restart) { + // same as drop_pkless_table, but with restart + if (jogasaki::kvs::implementation_id() == "memory") { + GTEST_SKIP() << "jogasaki-memory cannot rollback by abort"; + } + api::transaction_option opts{}; + execute_statement("CREATE TABLE t (c0 int)"); + ASSERT_EQ(1, seq_count()); + auto seqs = seq_list(); + ASSERT_TRUE(exists_seq(seqs[0])); + execute_statement("INSERT INTO t VALUES (1)"); + execute_statement("INSERT INTO t VALUES (1)"); + { + auto tx = utils::create_transaction(*db_, opts); + execute_statement("DROP TABLE t", *tx); + ASSERT_EQ(status::ok, tx->abort()); + } + ASSERT_EQ(1, seq_count()); // left entry due to abort + ASSERT_TRUE(! exists_seq(seqs[0])); // though table entry is left, sequence is removed correctly + + ASSERT_EQ(status::ok, db_->stop()); + ASSERT_EQ(status::ok, db_->start()); // warning message should be shown + + execute_statement("CREATE TABLE t (c0 int)"); + ASSERT_EQ(2, seq_count()); + execute_statement("INSERT INTO t VALUES (1)"); + execute_statement("INSERT INTO t VALUES (1)"); + execute_statement("DROP TABLE t"); + ASSERT_EQ(1, seq_count()); +} + +TEST_F(transaction_fail_ddl_test, drop_table_with_identity_column) { + // same as drop_pkless_table, but with identity column + if (jogasaki::kvs::implementation_id() == "memory") { + GTEST_SKIP() << "jogasaki-memory cannot rollback by abort"; + } + api::transaction_option opts{}; + execute_statement("CREATE TABLE t (c0 int, c1 int generated always as identity)"); + ASSERT_EQ(2, seq_count()); + auto seqs = seq_list(); + ASSERT_TRUE(exists_seq(seqs[0])); + ASSERT_TRUE(exists_seq(seqs[1])); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + { + auto tx = utils::create_transaction(*db_, opts); + execute_statement("DROP TABLE t", *tx); + ASSERT_EQ(status::ok, tx->abort()); + } + ASSERT_EQ(2, seq_count()); // left entry due to abort + ASSERT_TRUE(! exists_seq(seqs[0])); + ASSERT_TRUE(! exists_seq(seqs[1])); + execute_statement("CREATE TABLE t (c0 int, c1 int generated always as identity)"); + ASSERT_EQ(4, seq_count()); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + execute_statement("DROP TABLE t"); + ASSERT_EQ(2, seq_count()); +} + +TEST_F(transaction_fail_ddl_test, drop_table_with_identity_column_with_restart) { + // same as drop_pkless_table_with_restart, but with identity column + if (jogasaki::kvs::implementation_id() == "memory") { + GTEST_SKIP() << "jogasaki-memory cannot rollback by abort"; + } + api::transaction_option opts{}; + execute_statement("CREATE TABLE t (c0 int, c1 int generated always as identity)"); + ASSERT_EQ(2, seq_count()); + auto seqs = seq_list(); + ASSERT_TRUE(exists_seq(seqs[0])); + ASSERT_TRUE(exists_seq(seqs[1])); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + { + auto tx = utils::create_transaction(*db_, opts); + execute_statement("DROP TABLE t", *tx); + ASSERT_EQ(status::ok, tx->abort()); + } + ASSERT_EQ(2, seq_count()); // left entry due to abort + ASSERT_TRUE(! exists_seq(seqs[0])); + ASSERT_TRUE(! exists_seq(seqs[1])); + + ASSERT_EQ(status::ok, db_->stop()); + ASSERT_EQ(status::ok, db_->start()); // warning message should be shown + + execute_statement("CREATE TABLE t (c0 int, c1 int generated always as identity)"); + ASSERT_EQ(4, seq_count()); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + execute_statement("DROP TABLE t"); + ASSERT_EQ(2, seq_count()); +} + +TEST_F(transaction_fail_ddl_test, drop_table_missing_sequence_entry) { + // simulate the situation that sequences system table entry is some how missing + // without db restrt, this works as in-memory object is used + if (jogasaki::kvs::implementation_id() == "memory") { + GTEST_SKIP() << "jogasaki-memory cannot rollback by abort"; + } + api::transaction_option opts{}; + execute_statement("CREATE TABLE t (c0 int, c1 int generated always as identity)"); + ASSERT_EQ(2, seq_count()); + auto seqs = seq_list(); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + { + // simulate system table entry is missing + auto tx = utils::create_transaction(*db_); + auto tctx = get_transaction_context(*tx); + executor::sequence::metadata_store ms{*tctx->object()}; + ms.remove(0); + ms.remove(1); + ASSERT_EQ(status::ok, tx->commit()); + } + ASSERT_EQ(0, seq_count()); + EXPECT_TRUE(exists_seq(seqs[0])); + EXPECT_TRUE(exists_seq(seqs[1])); + execute_statement("DROP TABLE t"); // this happens to work using in-memory object (sequences_) + ASSERT_EQ(0, seq_count()); + EXPECT_TRUE(! exists_seq(seqs[0])); + EXPECT_TRUE(! exists_seq(seqs[1])); + execute_statement("CREATE TABLE t (c0 int, c1 int generated always as identity)"); + ASSERT_EQ(2, seq_count()); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + execute_statement("DROP TABLE t"); + ASSERT_EQ(0, seq_count()); +} + +TEST_F(transaction_fail_ddl_test, drop_table_missing_sequence_entry_with_restart) { + // same as drop_table_missing_sequence_entry, but with restart + // with restart, drop fails to clean up the sharksfin sequence because there is no seq id available and it leaks + if (jogasaki::kvs::implementation_id() == "memory") { + GTEST_SKIP() << "jogasaki-memory cannot rollback by abort"; + } + api::transaction_option opts{}; + execute_statement("CREATE TABLE t (c0 int, c1 int generated always as identity)"); + ASSERT_EQ(2, seq_count()); + auto seqs = seq_list(); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + { + // simulate system table entry is missing + auto tx = utils::create_transaction(*db_); + auto tctx = get_transaction_context(*tx); + executor::sequence::metadata_store ms{*tctx->object()}; + ms.remove(0); + ms.remove(1); + ASSERT_EQ(status::ok, tx->commit()); + } + + ASSERT_EQ(status::ok, db_->stop()); + ASSERT_EQ(status::ok, db_->start()); // warning message should be shown + + ASSERT_EQ(0, seq_count()); + EXPECT_TRUE(exists_seq(seqs[0])); + EXPECT_TRUE(exists_seq(seqs[1])); + execute_statement("DROP TABLE t"); + ASSERT_EQ(0, seq_count()); + EXPECT_TRUE(exists_seq(seqs[0])); // fail to clean up + EXPECT_TRUE(exists_seq(seqs[1])); // fail to clean up + execute_statement("CREATE TABLE t (c0 int, c1 int generated always as identity)"); + ASSERT_EQ(2, seq_count()); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + execute_statement("DROP TABLE t"); + ASSERT_EQ(0, seq_count()); +} + +TEST_F(transaction_fail_ddl_test, drop_table_missing_sequence_with_restart) { + // verify drop table does clean up even if some of the sharksfin sequence is missing + if (jogasaki::kvs::implementation_id() == "memory") { + GTEST_SKIP() << "jogasaki-memory cannot rollback by abort"; + } + api::transaction_option opts{}; + execute_statement("CREATE TABLE t (c0 int, c1 int generated always as identity, c2 int generated always as identity, c3 int generated always as identity)"); + ASSERT_EQ(4, seq_count()); + auto seqs = seq_list(); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + { + // simulate part of the sequence is missing somehow + EXPECT_TRUE(exists_seq(seqs[2])); + EXPECT_TRUE(remove_seq(seqs[2])); + } + + ASSERT_EQ(status::ok, db_->stop()); + ASSERT_EQ(status::ok, db_->start()); + + execute_statement("DROP TABLE t"); + ASSERT_EQ(0, seq_count()); + EXPECT_TRUE(! exists_seq(seqs[0])); + EXPECT_TRUE(! exists_seq(seqs[1])); + EXPECT_TRUE(! exists_seq(seqs[2])); + EXPECT_TRUE(! exists_seq(seqs[3])); + + execute_statement("CREATE TABLE t (c0 int, c1 int generated always as identity, c2 int generated always as identity, c3 int generated always as identity)"); + ASSERT_EQ(4, seq_count()); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + execute_statement("INSERT INTO t(c0) VALUES (1)"); + execute_statement("DROP TABLE t"); + ASSERT_EQ(0, seq_count()); +} + +} diff --git a/test/jogasaki/executor/sequence/sequence_manager_test.cpp b/test/jogasaki/executor/sequence/sequence_manager_test.cpp index cdbb2339..7ab2e24d 100644 --- a/test/jogasaki/executor/sequence/sequence_manager_test.cpp +++ b/test/jogasaki/executor/sequence/sequence_manager_test.cpp @@ -67,7 +67,7 @@ TEST_F(sequence_manager_test, simple) { provider.add_sequence(storage::sequence{0, "SEQ"}); manager mgr{*db_}; EXPECT_EQ(0, mgr.load_id_map()); - mgr.register_sequences(nullptr, maybe_shared_ptr{&provider}); + mgr.register_sequences(nullptr, maybe_shared_ptr{&provider}, true); auto* seq = mgr.find_sequence(0); ASSERT_TRUE(seq); auto cur = seq->get(); @@ -87,7 +87,7 @@ TEST_F(sequence_manager_test, initialize) { provider.add_sequence(storage::sequence{1, "SEQ1"}); manager mgr{ *db_}; EXPECT_EQ(0, mgr.load_id_map()); - mgr.register_sequences(nullptr, maybe_shared_ptr{&provider}); + mgr.register_sequences(nullptr, maybe_shared_ptr{&provider}, true); ASSERT_EQ(1, mgr.sequences().size()); auto& info0 = *mgr.sequences().at(1).info(); @@ -116,7 +116,7 @@ TEST_F(sequence_manager_test, sequence_spec) { manager mgr{*db_}; // load mapping from kvs if exists EXPECT_EQ(0, mgr.load_id_map()); - mgr.register_sequences(nullptr, maybe_shared_ptr{&provider}); + mgr.register_sequences(nullptr, maybe_shared_ptr{&provider}, true); ASSERT_EQ(1, mgr.sequences().size()); auto& info0 = *mgr.sequences().at(111).info(); @@ -134,12 +134,12 @@ TEST_F(sequence_manager_test, initialize_with_existing_table_entries) { provider.add_sequence(storage::sequence{1, "SEQ1"}); manager mgr{*db_}; EXPECT_EQ(0, mgr.load_id_map()); - mgr.register_sequences(nullptr, maybe_shared_ptr{&provider}); + mgr.register_sequences(nullptr, maybe_shared_ptr{&provider}, true); wait_epochs(10); provider.add_sequence(storage::sequence{2, "SEQ2"}); manager mgr2{*db_}; EXPECT_EQ(1, mgr2.load_id_map()); - mgr2.register_sequences(nullptr, maybe_shared_ptr{&provider}); + mgr2.register_sequences(nullptr, maybe_shared_ptr{&provider}, true); wait_epochs(10); ASSERT_EQ(2, mgr2.sequences().size()); @@ -150,7 +150,7 @@ TEST_F(sequence_manager_test, initialize_with_existing_table_entries) { manager mgr3{*db_}; EXPECT_EQ(2, mgr3.load_id_map()); - mgr3.register_sequences(nullptr, maybe_shared_ptr{&provider}); + mgr3.register_sequences(nullptr, maybe_shared_ptr{&provider}, true); wait_epochs(10); ASSERT_EQ(2, mgr3.sequences().size());