Skip to content

Commit

Permalink
Fix #7783: Deadlock during index creation when parallel workers are used
Browse files Browse the repository at this point in the history
Deadlock is possible when index deletion and creation are performed in the same transaction and a deleted index is used by a computed field expression. Each worker attachment loads table's metadata, parses the expression and tries to get SR lock on the deleted index. They will never get it because the main thread holds EX lock and waits for worker attachments to complete. The solution is to prevent the optimizer from using deleted indexes by making an attempt to get SR lock with NO_WAIT.
  • Loading branch information
ilya071294 committed Apr 11, 2024
1 parent 3496c5d commit ad3e7eb
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 10 deletions.
11 changes: 2 additions & 9 deletions src/jrd/Statement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,8 @@ Statement::Statement(thread_db* tdbb, MemoryPool* p, CompilerScratch* csb)

case Resource::rsc_index:
{
jrd_rel* relation = resource->rsc_rel;
IndexLock* index = CMP_get_index_lock(tdbb, relation, resource->rsc_id);
if (index)
{
++index->idl_count;
if (index->idl_count == 1) {
LCK_lock(tdbb, index->idl_lock, LCK_SR, LCK_WAIT);
}
}
// No need to lock the index here because it is already
// locked by Optimizer::compileRelation.
break;
}

Expand Down
54 changes: 53 additions & 1 deletion src/jrd/optimizer/Optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1149,6 +1149,38 @@ void Optimizer::compileRelation(StreamType stream)
}
}

{ // scope
ThreadStatusGuard tempStatus(tdbb);

// Check if some indices are being deleted and don't use them in this case
for (auto idx = idxList.begin(); idx < idxList.end();)
{
IndexLock* idx_lock = CMP_get_index_lock(tdbb, relation, idx->idx_id);

if (idx_lock)
{
if (idx_lock->idl_count == 1 && idx_lock->idl_lock->lck_logical == LCK_EX)
{
idxList.remove(idx);
continue;
}
else
{
if (idx_lock->idl_count == 0 && !LCK_lock(tdbb, idx_lock->idl_lock, LCK_SR, LCK_NO_WAIT))
{
idxList.remove(idx);
continue;
}

fb_assert(idx_lock->idl_lock->lck_logical == LCK_SR);
idx_lock->idl_count++;
}
}

idx++;
}
}

if (idxList.hasData())
tail->csb_idx = FB_NEW_POOL(getPool()) IndexDescList(getPool(), idxList);

Expand Down Expand Up @@ -1656,6 +1688,27 @@ void Optimizer::checkIndices()
for (const auto compileStream : compileStreams)
{
const auto tail = &csb->csb_rpt[compileStream];
const auto relation = tail->csb_relation;

if (tail->csb_idx && relation)
{
// Release locks of unused indices
for (const auto& idx : *tail->csb_idx)
{
if (!(idx.idx_runtime_flags & idx_used))
{
IndexLock* index = CMP_get_index_lock(tdbb, relation, idx.idx_id);

if (index && index->idl_count)
{
--index->idl_count;

if (!index->idl_count)
LCK_release(tdbb, index->idl_lock);
}
}
}
}

const auto plan = tail->csb_plan;
if (!plan)
Expand All @@ -1664,7 +1717,6 @@ void Optimizer::checkIndices()
if (plan->type != PlanNode::TYPE_RETRIEVE)
continue;

const auto relation = tail->csb_relation;
if (!relation)
return;

Expand Down

0 comments on commit ad3e7eb

Please sign in to comment.