Skip to content

Commit 38d2631

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 471fa9f commit 38d2631

3 files changed

Lines changed: 74 additions & 65 deletions

File tree

‎src/jrd/dfw.epp‎

Lines changed: 72 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,7 @@ static void check_computed_dependencies(thread_db* tdbb, jrd_tra* transaction,
383383
static void check_dependencies(thread_db*, const TEXT*, const TEXT*, int, jrd_tra*);
384384
static void check_filename(const Firebird::string&, bool);
385385
static void check_system_generator(const TEXT*, const dfw_t);
386+
static void cleanup_index_creation(thread_db*, DeferredWork*, jrd_tra*);
386387
static bool formatsAreEqual(const Format*, const Format*);
387388
static bool find_depend_in_dfw(thread_db*, TEXT*, USHORT, USHORT, jrd_tra*);
388389
static void get_array_desc(thread_db*, const TEXT*, Ods::InternalArrayDesc*);
@@ -720,7 +721,9 @@ void DFW_perform_work(thread_db* tdbb, jrd_tra* transaction)
720721
{
721722
more = false;
722723
try {
723-
tdbb->tdbb_flags |= (TDBB_dont_post_dfw | TDBB_use_db_page_space);
724+
tdbb->tdbb_flags |= (TDBB_dont_post_dfw | TDBB_use_db_page_space |
725+
(phase == 0 ? TDBB_dfw_cleanup : 0));
726+
724727
for (const deferred_task* task = task_table; task->task_type != dfw_null; ++task)
725728
{
726729
for (DeferredWork* work = transaction->tra_deferred_job->work; work; work = work->getNext())
@@ -738,15 +741,18 @@ void DFW_perform_work(thread_db* tdbb, jrd_tra* transaction)
738741
}
739742
}
740743
}
744+
745+
tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space | TDBB_dfw_cleanup);
746+
741747
if (!phase) {
742748
Firebird::makePermanentVector(tdbb->tdbb_status_vector, err_status);
743749
ERR_punt();
744750
}
751+
745752
++phase;
746-
tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space);
747753
}
748754
catch (const Firebird::Exception& ex) {
749-
tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space);
755+
tdbb->tdbb_flags &= ~(TDBB_dont_post_dfw | TDBB_use_db_page_space | TDBB_dfw_cleanup);
750756

751757
/* Do any necessary cleanup */
752758
if (!phase) {
@@ -1570,6 +1576,7 @@ static bool create_expression_index(thread_db* tdbb, SSHORT phase, DeferredWork*
15701576
switch (phase)
15711577
{
15721578
case 0:
1579+
cleanup_index_creation(tdbb, work, transaction);
15731580
MET_delete_dependencies(tdbb, work->dfw_name, obj_expression_index, transaction);
15741581
return false;
15751582

@@ -1840,7 +1847,67 @@ static void check_system_generator(const TEXT* gen_name, const dfw_t action)
18401847
}
18411848

18421849

1843-
static bool formatsAreEqual(const Format* old_format, const Format* new_format)
1850+
static void cleanup_index_creation(thread_db* tdbb, DeferredWork* work, jrd_tra* transaction)
1851+
{
1852+
Database* const dbb = tdbb->getDatabase();
1853+
1854+
jrd_req* request = NULL;
1855+
1856+
FOR(REQUEST_HANDLE request TRANSACTION_HANDLE transaction) IDXN IN RDB$INDICES CROSS
1857+
IREL IN RDB$RELATIONS OVER RDB$RELATION_NAME
1858+
WITH IDXN.RDB$INDEX_NAME EQ work->dfw_name.c_str()
1859+
// dimitr: I have no idea why the condition below is required here
1860+
AND IREL.RDB$VIEW_BLR MISSING // views do not have indices
1861+
{
1862+
jrd_rel* const relation = MET_lookup_relation(tdbb, IDXN.RDB$RELATION_NAME);
1863+
RelationPages* const relPages = relation->getPages(tdbb, -1, false);
1864+
1865+
if (relPages && relPages->rel_index_root)
1866+
{
1867+
// We need to special handle temp tables with ON PRESERVE ROWS only
1868+
const bool isTempIndex = (relation->rel_flags & REL_temp_conn) &&
1869+
(relPages->rel_instance_id != 0);
1870+
1871+
// Fetch the root index page and mark MUST_WRITE, and then
1872+
// delete the index. It will also clean the index slot.
1873+
1874+
if (work->dfw_id != dbb->dbb_max_idx)
1875+
{
1876+
WIN window(relPages->rel_pg_space_id, relPages->rel_index_root);
1877+
CCH_FETCH(tdbb, &window, LCK_write, pag_root);
1878+
CCH_MARK_MUST_WRITE(tdbb, &window);
1879+
const bool tree_exists = BTR_delete_index(tdbb, &window, work->dfw_id);
1880+
1881+
if (!isTempIndex) {
1882+
work->dfw_id = dbb->dbb_max_idx;
1883+
}
1884+
else if (tree_exists)
1885+
{
1886+
IndexLock* const idx_lock = CMP_get_index_lock(tdbb, relation, work->dfw_id);
1887+
1888+
if (idx_lock)
1889+
{
1890+
if (!--idx_lock->idl_count)
1891+
LCK_release(tdbb, idx_lock->idl_lock);
1892+
}
1893+
}
1894+
}
1895+
1896+
if (!IDXN.RDB$INDEX_ID.NULL)
1897+
{
1898+
MODIFY IDXN USING
1899+
IDXN.RDB$INDEX_ID.NULL = TRUE;
1900+
END_MODIFY
1901+
}
1902+
}
1903+
}
1904+
END_FOR
1905+
1906+
CMP_release(tdbb, request);
1907+
}
1908+
1909+
1910+
static bool formatsAreEqual(const Format* old_format, const Format* new_format)
18441911
{
18451912
/**************************************
18461913
*
@@ -2049,72 +2116,14 @@ static bool create_index(thread_db* tdbb,
20492116
jrd_rel* partner_relation;
20502117
index_desc idx;
20512118
int key_count;
2052-
jrd_req* handle;
20532119

20542120
SET_TDBB(tdbb);
20552121
Database* dbb = tdbb->getDatabase();
20562122

20572123
switch (phase)
20582124
{
20592125
case 0:
2060-
handle = NULL;
2061-
2062-
/* Drop those indices at clean up time. */
2063-
FOR(REQUEST_HANDLE handle TRANSACTION_HANDLE transaction) IDXN IN RDB$INDICES CROSS
2064-
IREL IN RDB$RELATIONS OVER RDB$RELATION_NAME
2065-
WITH IDXN.RDB$INDEX_NAME EQ work->dfw_name.c_str()
2066-
/* Views do not have indices */
2067-
if (IREL.RDB$VIEW_BLR.NULL)
2068-
{
2069-
relation = MET_lookup_relation(tdbb, IDXN.RDB$RELATION_NAME);
2070-
2071-
RelationPages* relPages = relation->getPages(tdbb, -1, false);
2072-
if (!relPages) {
2073-
return false;
2074-
}
2075-
2076-
// we need to special handle temp tables with ON PRESERVE ROWS only
2077-
const bool isTempIndex = (relation->rel_flags & REL_temp_conn) &&
2078-
(relPages->rel_instance_id != 0);
2079-
2080-
/* Fetch the root index page and mark MUST_WRITE, and then
2081-
delete the index. It will also clean the index slot. Note
2082-
that the previous fixed definition of MAX_IDX (64) has been
2083-
dropped in favor of a computed value saved in the Database */
2084-
if (relPages->rel_index_root)
2085-
{
2086-
if (work->dfw_id != dbb->dbb_max_idx)
2087-
{
2088-
WIN window(relPages->rel_pg_space_id, relPages->rel_index_root);
2089-
CCH_FETCH(tdbb, &window, LCK_write, pag_root);
2090-
CCH_MARK_MUST_WRITE(tdbb, &window);
2091-
const bool tree_exists = BTR_delete_index(tdbb, &window, work->dfw_id);
2092-
2093-
if (!isTempIndex) {
2094-
work->dfw_id = dbb->dbb_max_idx;
2095-
}
2096-
else if (tree_exists)
2097-
{
2098-
IndexLock* idx_lock = CMP_get_index_lock(tdbb, relation, work->dfw_id);
2099-
if (idx_lock)
2100-
{
2101-
if (!--idx_lock->idl_count) {
2102-
LCK_release(tdbb, idx_lock->idl_lock);
2103-
}
2104-
}
2105-
}
2106-
}
2107-
if (!IDXN.RDB$INDEX_ID.NULL)
2108-
{
2109-
MODIFY IDXN USING
2110-
IDXN.RDB$INDEX_ID.NULL = TRUE;
2111-
END_MODIFY;
2112-
}
2113-
}
2114-
}
2115-
END_FOR;
2116-
2117-
CMP_release(tdbb, handle);
2126+
cleanup_index_creation(tdbb, work, transaction);
21182127
return false;
21192128

21202129
case 1:

‎src/jrd/jrd.cpp‎

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

6785-
if (tdbb_flags & (TDBB_verb_cleanup | TDBB_detaching | TDBB_wait_cancel_disable))
6785+
if (tdbb_flags & (TDBB_verb_cleanup | TDBB_dfw_cleanup | TDBB_detaching | TDBB_wait_cancel_disable))
67866786
return false;
67876787

67886788
try

‎src/jrd/jrd.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -789,7 +789,7 @@ const USHORT TDBB_use_db_page_space = 512; // use database (not temporary) page
789789
const USHORT TDBB_detaching = 1024; // detach is in progress
790790
const USHORT TDBB_wait_cancel_disable = 2048; // don't cancel current waiting operation
791791
const USHORT TDBB_cache_unwound = 4096; // page cache was unwound
792-
792+
const USHORT TDBB_dfw_cleanup = 8192; // DFW cleanup phase is active
793793

794794
class ThreadContextHolder
795795
{

0 commit comments

Comments
 (0)