From 0229709101b19012b8f3a9e5805c653178df0d1e Mon Sep 17 00:00:00 2001 From: Dmitry Yemanov Date: Sat, 18 Jun 2016 15:57:25 +0300 Subject: [PATCH] Bugfix for CORE-5275: Expression index may become inconsistent if CREATE INDEX was interrupted after b-tree creation but before commiting. --- src/jrd/dfw.epp | 135 +++++++++++++++++++++++++----------------------- src/jrd/jrd.cpp | 2 +- src/jrd/jrd.h | 1 + 3 files changed, 73 insertions(+), 65 deletions(-) diff --git a/src/jrd/dfw.epp b/src/jrd/dfw.epp index f5563fa14d..3a8d1cc1a7 100644 --- a/src/jrd/dfw.epp +++ b/src/jrd/dfw.epp @@ -494,6 +494,7 @@ static void check_computed_dependencies(thread_db* tdbb, jrd_tra* transaction, const Firebird::MetaName& fieldName); static void check_dependencies(thread_db*, const TEXT*, const TEXT*, const TEXT*, int, jrd_tra*); static void check_filename(const Firebird::string&, bool); +static void cleanup_index_creation(thread_db*, DeferredWork*, jrd_tra*); static bool formatsAreEqual(const Format*, const Format*); static bool find_depend_in_dfw(thread_db*, TEXT*, USHORT, USHORT, jrd_tra*); static void get_array_desc(thread_db*, const TEXT*, Ods::InternalArrayDesc*); @@ -1433,10 +1434,13 @@ void DFW_perform_work(thread_db* tdbb, jrd_tra* transaction) { more = false; try { - tdbb->tdbb_flags |= (TDBB_dont_post_dfw | TDBB_use_db_page_space); + tdbb->tdbb_flags |= (TDBB_dont_post_dfw | TDBB_use_db_page_space | + (phase == 0 ? TDBB_dfw_cleanup : 0)); + for (const deferred_task* task = task_table; task->task_type != dfw_null; ++task) { - for (DeferredWork* work = transaction->tra_deferred_job->work; work; work = work->getNext()) + for (DeferredWork* work = transaction->tra_deferred_job->work; + work; work = work->getNext()) { if (work->dfw_type == task->task_type) { @@ -1451,17 +1455,20 @@ void DFW_perform_work(thread_db* tdbb, jrd_tra* transaction) } } } + + tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space | TDBB_dfw_cleanup); + if (!phase) { fb_utils::copyStatus(tdbb->tdbb_status_vector, &err_status); ERR_punt(); } + ++phase; - tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space); } catch (const Firebird::Exception& ex) { - tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space); + tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space | TDBB_dfw_cleanup); // Do any necessary cleanup if (!phase) @@ -2603,6 +2610,7 @@ static bool create_expression_index(thread_db* tdbb, SSHORT phase, DeferredWork* switch (phase) { case 0: + cleanup_index_creation(tdbb, work, transaction); MET_delete_dependencies(tdbb, work->dfw_name, obj_expression_index, transaction); return false; @@ -2946,6 +2954,64 @@ static void check_filename(const Firebird::string& name, bool shareExpand) } +static void cleanup_index_creation(thread_db* tdbb, DeferredWork* work, jrd_tra* transaction) +{ + Database* const dbb = tdbb->getDatabase(); + + AutoRequest request; + + FOR(REQUEST_HANDLE request TRANSACTION_HANDLE transaction) IDXN IN RDB$INDICES CROSS + IREL IN RDB$RELATIONS OVER RDB$RELATION_NAME + WITH IDXN.RDB$INDEX_NAME EQ work->dfw_name.c_str() + // dimitr: I have no idea why the condition below is required here + AND IREL.RDB$VIEW_BLR MISSING // views do not have indices + { + jrd_rel* const relation = MET_lookup_relation(tdbb, IDXN.RDB$RELATION_NAME); + RelationPages* const relPages = relation->getPages(tdbb, MAX_TRA_NUMBER, false); + + if (relPages && relPages->rel_index_root) + { + // We need to special handle temp tables with ON PRESERVE ROWS only + const bool isTempIndex = (relation->rel_flags & REL_temp_conn) && + (relPages->rel_instance_id != 0); + + // Fetch the root index page and mark MUST_WRITE, and then + // delete the index. It will also clean the index slot. + + if (work->dfw_id != dbb->dbb_max_idx) + { + WIN window(relPages->rel_pg_space_id, relPages->rel_index_root); + CCH_FETCH(tdbb, &window, LCK_write, pag_root); + CCH_MARK_MUST_WRITE(tdbb, &window); + const bool tree_exists = BTR_delete_index(tdbb, &window, work->dfw_id); + + if (!isTempIndex) { + work->dfw_id = dbb->dbb_max_idx; + } + else if (tree_exists) + { + IndexLock* const idx_lock = CMP_get_index_lock(tdbb, relation, work->dfw_id); + + if (idx_lock) + { + if (!--idx_lock->idl_count) + LCK_release(tdbb, idx_lock->idl_lock); + } + } + } + + if (!IDXN.RDB$INDEX_ID.NULL) + { + MODIFY IDXN USING + IDXN.RDB$INDEX_ID.NULL = TRUE; + END_MODIFY + } + } + } + END_FOR +} + + static bool formatsAreEqual(const Format* old_format, const Format* new_format) { /************************************** @@ -3147,7 +3213,6 @@ static bool create_index(thread_db* tdbb, SSHORT phase, DeferredWork* work, jrd_ jrd_rel* partner_relation; index_desc idx; int key_count; - AutoRequest handle; SET_TDBB(tdbb); Jrd::Attachment* attachment = tdbb->getAttachment(); @@ -3156,65 +3221,7 @@ static bool create_index(thread_db* tdbb, SSHORT phase, DeferredWork* work, jrd_ switch (phase) { case 0: - handle.reset(); - - // Drop those indices at clean up time. - FOR(REQUEST_HANDLE handle TRANSACTION_HANDLE transaction) IDXN IN RDB$INDICES CROSS - IREL IN RDB$RELATIONS OVER RDB$RELATION_NAME - WITH IDXN.RDB$INDEX_NAME EQ work->dfw_name.c_str() - { - // Views do not have indices - if (IREL.RDB$VIEW_BLR.NULL) - { - relation = MET_lookup_relation(tdbb, IDXN.RDB$RELATION_NAME); - - RelationPages* relPages = relation->getPages(tdbb, MAX_TRA_NUMBER, false); - if (!relPages) { - return false; - } - - // we need to special handle temp tables with ON PRESERVE ROWS only - const bool isTempIndex = (relation->rel_flags & REL_temp_conn) && - (relPages->rel_instance_id != 0); - - // Fetch the root index page and mark MUST_WRITE, and then - // delete the index. It will also clean the index slot. Note - // that the previous fixed definition of MAX_IDX (64) has been - // dropped in favor of a computed value saved in the Database - if (relPages->rel_index_root) - { - if (work->dfw_id != dbb->dbb_max_idx) - { - WIN window(relPages->rel_pg_space_id, relPages->rel_index_root); - CCH_FETCH(tdbb, &window, LCK_write, pag_root); - CCH_MARK_MUST_WRITE(tdbb, &window); - const bool tree_exists = BTR_delete_index(tdbb, &window, work->dfw_id); - - if (!isTempIndex) { - work->dfw_id = dbb->dbb_max_idx; - } - else if (tree_exists) - { - IndexLock* idx_lock = CMP_get_index_lock(tdbb, relation, work->dfw_id); - if (idx_lock) - { - if (!--idx_lock->idl_count) { - LCK_release(tdbb, idx_lock->idl_lock); - } - } - } - } - if (!IDXN.RDB$INDEX_ID.NULL) - { - MODIFY IDXN USING - IDXN.RDB$INDEX_ID.NULL = TRUE; - END_MODIFY - } - } - } - } - END_FOR - + cleanup_index_creation(tdbb, work, transaction); return false; case 1: diff --git a/src/jrd/jrd.cpp b/src/jrd/jrd.cpp index 9d41e50f64..2368eb4e0b 100644 --- a/src/jrd/jrd.cpp +++ b/src/jrd/jrd.cpp @@ -7426,7 +7426,7 @@ ISC_STATUS thread_db::checkCancelState() // nor currently detaching, as these actions should never be interrupted. // Also don't break wait in LM if it is not safe. - if (tdbb_flags & (TDBB_verb_cleanup | TDBB_detaching | TDBB_wait_cancel_disable)) + if (tdbb_flags & (TDBB_verb_cleanup | TDBB_dfw_cleanup | TDBB_detaching | TDBB_wait_cancel_disable)) return FB_SUCCESS; if (attachment) diff --git a/src/jrd/jrd.h b/src/jrd/jrd.h index fdf7100a72..996d6d7979 100644 --- a/src/jrd/jrd.h +++ b/src/jrd/jrd.h @@ -334,6 +334,7 @@ const USHORT TDBB_wait_cancel_disable = 1024; // don't cancel current waiting o const USHORT TDBB_cache_unwound = 2048; // page cache was unwound const USHORT TDBB_trusted_ddl = 4096; // skip DDL permission checks. Set after DDL permission check and clear after DDL execution const USHORT TDBB_reset_stack = 8192; // stack should be reset after stack overflow exception +const USHORT TDBB_dfw_cleanup = 16384; // DFW cleanup phase is active class thread_db : public Firebird::ThreadData {