Skip to content

Commit 062f5d7

Browse files
dyemanovdyemanov
committed
Bugfix for CORE-5275: Expression index may become inconsistent if CREATE
INDEX was interrupted after b-tree creation but before commiting.
1 parent f6c9b11 commit 062f5d7

3 files changed

Lines changed: 73 additions & 65 deletions

File tree

‎src/jrd/dfw.epp‎

Lines changed: 71 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,7 @@ static void check_computed_dependencies(thread_db* tdbb, jrd_tra* transaction,
493493
const Firebird::MetaName& fieldName);
494494
static void check_dependencies(thread_db*, const TEXT*, const TEXT*, const TEXT*, int, jrd_tra*);
495495
static void check_filename(const Firebird::string&, bool);
496+
static void cleanup_index_creation(thread_db*, DeferredWork*, jrd_tra*);
496497
static bool formatsAreEqual(const Format*, const Format*);
497498
static bool find_depend_in_dfw(thread_db*, TEXT*, USHORT, USHORT, jrd_tra*);
498499
static void get_array_desc(thread_db*, const TEXT*, Ods::InternalArrayDesc*);
@@ -1436,10 +1437,13 @@ void DFW_perform_work(thread_db* tdbb, jrd_tra* transaction)
14361437
{
14371438
more = false;
14381439
try {
1439-
tdbb->tdbb_flags |= (TDBB_dont_post_dfw | TDBB_use_db_page_space);
1440+
tdbb->tdbb_flags |= (TDBB_dont_post_dfw | TDBB_use_db_page_space |
1441+
(phase == 0 ? TDBB_dfw_cleanup : 0));
1442+
14401443
for (const deferred_task* task = task_table; task->task_type != dfw_null; ++task)
14411444
{
1442-
for (DeferredWork* work = transaction->tra_deferred_job->work; work; work = work->getNext())
1445+
for (DeferredWork* work = transaction->tra_deferred_job->work;
1446+
work; work = work->getNext())
14431447
{
14441448
if (work->dfw_type == task->task_type)
14451449
{
@@ -1454,17 +1458,20 @@ void DFW_perform_work(thread_db* tdbb, jrd_tra* transaction)
14541458
}
14551459
}
14561460
}
1461+
1462+
tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space | TDBB_dfw_cleanup);
1463+
14571464
if (!phase)
14581465
{
14591466
fb_utils::copyStatus(tdbb->tdbb_status_vector, &err_status);
14601467
ERR_punt();
14611468
}
1469+
14621470
++phase;
1463-
tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space);
14641471
}
14651472
catch (const Firebird::Exception& ex)
14661473
{
1467-
tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space);
1474+
tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space | TDBB_dfw_cleanup);
14681475

14691476
// Do any necessary cleanup
14701477
if (!phase)
@@ -2606,6 +2613,7 @@ static bool create_expression_index(thread_db* tdbb, SSHORT phase, DeferredWork*
26062613
switch (phase)
26072614
{
26082615
case 0:
2616+
cleanup_index_creation(tdbb, work, transaction);
26092617
MET_delete_dependencies(tdbb, work->dfw_name, obj_expression_index, transaction);
26102618
return false;
26112619

@@ -2949,6 +2957,64 @@ static void check_filename(const Firebird::string& name, bool shareExpand)
29492957
}
29502958

29512959

2960+
static void cleanup_index_creation(thread_db* tdbb, DeferredWork* work, jrd_tra* transaction)
2961+
{
2962+
Database* const dbb = tdbb->getDatabase();
2963+
2964+
AutoRequest request;
2965+
2966+
FOR(REQUEST_HANDLE request TRANSACTION_HANDLE transaction) IDXN IN RDB$INDICES CROSS
2967+
IREL IN RDB$RELATIONS OVER RDB$RELATION_NAME
2968+
WITH IDXN.RDB$INDEX_NAME EQ work->dfw_name.c_str()
2969+
// dimitr: I have no idea why the condition below is required here
2970+
AND IREL.RDB$VIEW_BLR MISSING // views do not have indices
2971+
{
2972+
jrd_rel* const relation = MET_lookup_relation(tdbb, IDXN.RDB$RELATION_NAME);
2973+
RelationPages* const relPages = relation->getPages(tdbb, MAX_TRA_NUMBER, false);
2974+
2975+
if (relPages && relPages->rel_index_root)
2976+
{
2977+
// We need to special handle temp tables with ON PRESERVE ROWS only
2978+
const bool isTempIndex = (relation->rel_flags & REL_temp_conn) &&
2979+
(relPages->rel_instance_id != 0);
2980+
2981+
// Fetch the root index page and mark MUST_WRITE, and then
2982+
// delete the index. It will also clean the index slot.
2983+
2984+
if (work->dfw_id != dbb->dbb_max_idx)
2985+
{
2986+
WIN window(relPages->rel_pg_space_id, relPages->rel_index_root);
2987+
CCH_FETCH(tdbb, &window, LCK_write, pag_root);
2988+
CCH_MARK_MUST_WRITE(tdbb, &window);
2989+
const bool tree_exists = BTR_delete_index(tdbb, &window, work->dfw_id);
2990+
2991+
if (!isTempIndex) {
2992+
work->dfw_id = dbb->dbb_max_idx;
2993+
}
2994+
else if (tree_exists)
2995+
{
2996+
IndexLock* const idx_lock = CMP_get_index_lock(tdbb, relation, work->dfw_id);
2997+
2998+
if (idx_lock)
2999+
{
3000+
if (!--idx_lock->idl_count)
3001+
LCK_release(tdbb, idx_lock->idl_lock);
3002+
}
3003+
}
3004+
}
3005+
3006+
if (!IDXN.RDB$INDEX_ID.NULL)
3007+
{
3008+
MODIFY IDXN USING
3009+
IDXN.RDB$INDEX_ID.NULL = TRUE;
3010+
END_MODIFY
3011+
}
3012+
}
3013+
}
3014+
END_FOR
3015+
}
3016+
3017+
29523018
static bool formatsAreEqual(const Format* old_format, const Format* new_format)
29533019
{
29543020
/**************************************
@@ -3150,7 +3216,6 @@ static bool create_index(thread_db* tdbb, SSHORT phase, DeferredWork* work, jrd_
31503216
jrd_rel* partner_relation;
31513217
index_desc idx;
31523218
int key_count;
3153-
AutoRequest handle;
31543219

31553220
SET_TDBB(tdbb);
31563221
Jrd::Attachment* attachment = tdbb->getAttachment();
@@ -3159,65 +3224,7 @@ static bool create_index(thread_db* tdbb, SSHORT phase, DeferredWork* work, jrd_
31593224
switch (phase)
31603225
{
31613226
case 0:
3162-
handle.reset();
3163-
3164-
// Drop those indices at clean up time.
3165-
FOR(REQUEST_HANDLE handle TRANSACTION_HANDLE transaction) IDXN IN RDB$INDICES CROSS
3166-
IREL IN RDB$RELATIONS OVER RDB$RELATION_NAME
3167-
WITH IDXN.RDB$INDEX_NAME EQ work->dfw_name.c_str()
3168-
{
3169-
// Views do not have indices
3170-
if (IREL.RDB$VIEW_BLR.NULL)
3171-
{
3172-
relation = MET_lookup_relation(tdbb, IDXN.RDB$RELATION_NAME);
3173-
3174-
RelationPages* relPages = relation->getPages(tdbb, MAX_TRA_NUMBER, false);
3175-
if (!relPages) {
3176-
return false;
3177-
}
3178-
3179-
// we need to special handle temp tables with ON PRESERVE ROWS only
3180-
const bool isTempIndex = (relation->rel_flags & REL_temp_conn) &&
3181-
(relPages->rel_instance_id != 0);
3182-
3183-
// Fetch the root index page and mark MUST_WRITE, and then
3184-
// delete the index. It will also clean the index slot. Note
3185-
// that the previous fixed definition of MAX_IDX (64) has been
3186-
// dropped in favor of a computed value saved in the Database
3187-
if (relPages->rel_index_root)
3188-
{
3189-
if (work->dfw_id != dbb->dbb_max_idx)
3190-
{
3191-
WIN window(relPages->rel_pg_space_id, relPages->rel_index_root);
3192-
CCH_FETCH(tdbb, &window, LCK_write, pag_root);
3193-
CCH_MARK_MUST_WRITE(tdbb, &window);
3194-
const bool tree_exists = BTR_delete_index(tdbb, &window, work->dfw_id);
3195-
3196-
if (!isTempIndex) {
3197-
work->dfw_id = dbb->dbb_max_idx;
3198-
}
3199-
else if (tree_exists)
3200-
{
3201-
IndexLock* idx_lock = CMP_get_index_lock(tdbb, relation, work->dfw_id);
3202-
if (idx_lock)
3203-
{
3204-
if (!--idx_lock->idl_count) {
3205-
LCK_release(tdbb, idx_lock->idl_lock);
3206-
}
3207-
}
3208-
}
3209-
}
3210-
if (!IDXN.RDB$INDEX_ID.NULL)
3211-
{
3212-
MODIFY IDXN USING
3213-
IDXN.RDB$INDEX_ID.NULL = TRUE;
3214-
END_MODIFY
3215-
}
3216-
}
3217-
}
3218-
}
3219-
END_FOR
3220-
3227+
cleanup_index_creation(tdbb, work, transaction);
32213228
return false;
32223229

32233230
case 1:

‎src/jrd/jrd.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7444,7 +7444,7 @@ ISC_STATUS thread_db::checkCancelState()
74447444
// nor currently detaching, as these actions should never be interrupted.
74457445
// Also don't break wait in LM if it is not safe.
74467446

7447-
if (tdbb_flags & (TDBB_verb_cleanup | TDBB_detaching | TDBB_wait_cancel_disable))
7447+
if (tdbb_flags & (TDBB_verb_cleanup | TDBB_dfw_cleanup | TDBB_detaching | TDBB_wait_cancel_disable))
74487448
return FB_SUCCESS;
74497449

74507450
if (attachment)

‎src/jrd/jrd.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,7 @@ const USHORT TDBB_wait_cancel_disable = 1024; // don't cancel current waiting o
334334
const USHORT TDBB_cache_unwound = 2048; // page cache was unwound
335335
const USHORT TDBB_trusted_ddl = 4096; // skip DDL permission checks. Set after DDL permission check and clear after DDL execution
336336
const USHORT TDBB_reset_stack = 8192; // stack should be reset after stack overflow exception
337+
const USHORT TDBB_dfw_cleanup = 16384; // DFW cleanup phase is active
337338

338339
class thread_db : public Firebird::ThreadData
339340
{

0 commit comments

Comments
 (0)