Skip to content

Commit 0229709

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 19254ef commit 0229709

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
@@ -494,6 +494,7 @@ static void check_computed_dependencies(thread_db* tdbb, jrd_tra* transaction,
494494
const Firebird::MetaName& fieldName);
495495
static void check_dependencies(thread_db*, const TEXT*, const TEXT*, const TEXT*, int, jrd_tra*);
496496
static void check_filename(const Firebird::string&, bool);
497+
static void cleanup_index_creation(thread_db*, DeferredWork*, jrd_tra*);
497498
static bool formatsAreEqual(const Format*, const Format*);
498499
static bool find_depend_in_dfw(thread_db*, TEXT*, USHORT, USHORT, jrd_tra*);
499500
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)
14331434
{
14341435
more = false;
14351436
try {
1436-
tdbb->tdbb_flags |= (TDBB_dont_post_dfw | TDBB_use_db_page_space);
1437+
tdbb->tdbb_flags |= (TDBB_dont_post_dfw | TDBB_use_db_page_space |
1438+
(phase == 0 ? TDBB_dfw_cleanup : 0));
1439+
14371440
for (const deferred_task* task = task_table; task->task_type != dfw_null; ++task)
14381441
{
1439-
for (DeferredWork* work = transaction->tra_deferred_job->work; work; work = work->getNext())
1442+
for (DeferredWork* work = transaction->tra_deferred_job->work;
1443+
work; work = work->getNext())
14401444
{
14411445
if (work->dfw_type == task->task_type)
14421446
{
@@ -1451,17 +1455,20 @@ void DFW_perform_work(thread_db* tdbb, jrd_tra* transaction)
14511455
}
14521456
}
14531457
}
1458+
1459+
tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space | TDBB_dfw_cleanup);
1460+
14541461
if (!phase)
14551462
{
14561463
fb_utils::copyStatus(tdbb->tdbb_status_vector, &err_status);
14571464
ERR_punt();
14581465
}
1466+
14591467
++phase;
1460-
tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space);
14611468
}
14621469
catch (const Firebird::Exception& ex)
14631470
{
1464-
tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space);
1471+
tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space | TDBB_dfw_cleanup);
14651472

14661473
// Do any necessary cleanup
14671474
if (!phase)
@@ -2603,6 +2610,7 @@ static bool create_expression_index(thread_db* tdbb, SSHORT phase, DeferredWork*
26032610
switch (phase)
26042611
{
26052612
case 0:
2613+
cleanup_index_creation(tdbb, work, transaction);
26062614
MET_delete_dependencies(tdbb, work->dfw_name, obj_expression_index, transaction);
26072615
return false;
26082616

@@ -2946,6 +2954,64 @@ static void check_filename(const Firebird::string& name, bool shareExpand)
29462954
}
29472955

29482956

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

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

32203227
case 1:

‎src/jrd/jrd.cpp‎

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

7429-
if (tdbb_flags & (TDBB_verb_cleanup | TDBB_detaching | TDBB_wait_cancel_disable))
7429+
if (tdbb_flags & (TDBB_verb_cleanup | TDBB_dfw_cleanup | TDBB_detaching | TDBB_wait_cancel_disable))
74307430
return FB_SUCCESS;
74317431

74327432
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)