From 5b14baa37b6ee214cd8ccc21f2e99dce119fe60e Mon Sep 17 00:00:00 2001 From: Adriano dos Santos Fernandes <529415+asfernandes@users.noreply.github.com> Date: Tue, 12 Sep 2023 21:50:06 -0300 Subject: [PATCH] Fix #7700 - SKIP LOCKED returns conflict error or wait when config ReadConsistency = 0 (#7701) * Fix #7700 - SKIP LOCKED returns conflict error or wait when config ReadConsistency = 0. * Rename RecordLock::DONT to NONE. --- src/dsql/StmtNodes.cpp | 8 ++++---- src/jrd/recsrc/RecordSource.cpp | 2 +- src/jrd/recsrc/SortedStream.cpp | 2 +- src/jrd/vio.cpp | 25 +++++++++++++------------ src/jrd/vio_proto.h | 11 +++++++++-- 5 files changed, 28 insertions(+), 20 deletions(-) diff --git a/src/dsql/StmtNodes.cpp b/src/dsql/StmtNodes.cpp index c2dce47197..cb7f0a6a33 100644 --- a/src/dsql/StmtNodes.cpp +++ b/src/dsql/StmtNodes.cpp @@ -2658,7 +2658,7 @@ const StmtNode* EraseNode::erase(thread_db* tdbb, Request* request, WhichTrigger if (rpb->rpb_runtime_flags & RPB_refetch) { - VIO_refetch_record(tdbb, rpb, transaction, false, false); + VIO_refetch_record(tdbb, rpb, transaction, RecordLock::NONE, false); rpb->rpb_runtime_flags &= ~RPB_refetch; } @@ -7112,7 +7112,7 @@ const StmtNode* ModifyNode::modify(thread_db* tdbb, Request* request, WhichTrigg if (orgRpb->rpb_runtime_flags & RPB_refetch) { - VIO_refetch_record(tdbb, orgRpb, transaction, false, false); + VIO_refetch_record(tdbb, orgRpb, transaction, RecordLock::NONE, false); orgRpb->rpb_runtime_flags &= ~RPB_refetch; } @@ -10547,9 +10547,9 @@ static void cleanupRpb(thread_db* tdbb, record_param* rpb) } // Try to set write lock on record until success or record exists -static void forceWriteLock(thread_db * tdbb, record_param * rpb, jrd_tra * transaction) +static void forceWriteLock(thread_db* tdbb, record_param* rpb, jrd_tra* transaction) { - while (VIO_refetch_record(tdbb, rpb, transaction, true, true)) + while (VIO_refetch_record(tdbb, rpb, transaction, RecordLock::LOCK, true)) { rpb->rpb_runtime_flags &= ~RPB_refetch; diff --git a/src/jrd/recsrc/RecordSource.cpp b/src/jrd/recsrc/RecordSource.cpp index a133dfe29e..cdc96663de 100644 --- a/src/jrd/recsrc/RecordSource.cpp +++ b/src/jrd/recsrc/RecordSource.cpp @@ -240,7 +240,7 @@ bool RecordStream::refetchRecord(thread_db* tdbb) const if (rpb->rpb_runtime_flags & RPB_refetch) { - if (VIO_refetch_record(tdbb, rpb, transaction, true, false)) + if (VIO_refetch_record(tdbb, rpb, transaction, RecordLock::LOCK, false)) { rpb->rpb_runtime_flags &= ~RPB_refetch; return true; diff --git a/src/jrd/recsrc/SortedStream.cpp b/src/jrd/recsrc/SortedStream.cpp index fc5df6e998..eacd72954b 100644 --- a/src/jrd/recsrc/SortedStream.cpp +++ b/src/jrd/recsrc/SortedStream.cpp @@ -478,7 +478,7 @@ void SortedStream::mapData(thread_db* tdbb, Request* request, UCHAR* data) const tdbb->bumpRelStats(RuntimeStatistics::RECORD_RPT_READS, relation->rel_id); - if (VIO_chase_record_version(tdbb, &temp, transaction, tdbb->getDefaultPool(), false, false)) + if (VIO_chase_record_version(tdbb, &temp, transaction, tdbb->getDefaultPool(), RecordLock::NONE, false)) { if (!(temp.rpb_runtime_flags & RPB_undo_data)) VIO_data(tdbb, &temp, tdbb->getDefaultPool()); diff --git a/src/jrd/vio.cpp b/src/jrd/vio.cpp index fbb719e2df..41cd6253f1 100644 --- a/src/jrd/vio.cpp +++ b/src/jrd/vio.cpp @@ -1109,7 +1109,7 @@ void VIO_backout(thread_db* tdbb, record_param* rpb, const jrd_tra* transaction) bool VIO_chase_record_version(thread_db* tdbb, record_param* rpb, jrd_tra* transaction, MemoryPool* pool, - bool writelock, bool noundo) + RecordLock recordLock, bool noundo) { /************************************** * @@ -1258,9 +1258,10 @@ bool VIO_chase_record_version(thread_db* tdbb, record_param* rpb, // If the transaction is a read committed and chooses the no version // option, wait for reads also! - if ((transaction->tra_flags & TRA_read_committed) && + if (recordLock != RecordLock::SKIP && + (transaction->tra_flags & TRA_read_committed) && !(transaction->tra_flags & TRA_read_consistency) && - (!(transaction->tra_flags & TRA_rec_version) || writelock)) + (!(transaction->tra_flags & TRA_rec_version) || recordLock == RecordLock::LOCK)) { if (state == tra_limbo) { @@ -1975,7 +1976,7 @@ bool VIO_erase(thread_db* tdbb, record_param* rpb, jrd_tra* transaction) if (rpb->rpb_runtime_flags & (RPB_refetch | RPB_undo_read)) { - VIO_refetch_record(tdbb, rpb, transaction, false, true); + VIO_refetch_record(tdbb, rpb, transaction, RecordLock::NONE, true); rpb->rpb_runtime_flags &= ~RPB_refetch; fb_assert(!(rpb->rpb_runtime_flags & RPB_undo_read)); } @@ -2905,7 +2906,7 @@ bool VIO_get(thread_db* tdbb, record_param* rpb, jrd_tra* transaction, MemoryPoo const USHORT lock_type = (rpb->rpb_stream_flags & RPB_s_update) ? LCK_write : LCK_read; if (!DPM_get(tdbb, rpb, lock_type) || - !VIO_chase_record_version(tdbb, rpb, transaction, pool, false, false)) + !VIO_chase_record_version(tdbb, rpb, transaction, pool, RecordLock::NONE, false)) { return false; } @@ -3280,7 +3281,7 @@ bool VIO_modify(thread_db* tdbb, record_param* org_rpb, record_param* new_rpb, j old_record->copyFrom(org_rpb->rpb_record); } - VIO_refetch_record(tdbb, org_rpb, transaction, false, true); + VIO_refetch_record(tdbb, org_rpb, transaction, RecordLock::NONE, true); org_rpb->rpb_runtime_flags &= ~RPB_refetch; fb_assert(!(org_rpb->rpb_runtime_flags & RPB_undo_read)); @@ -3763,7 +3764,7 @@ bool VIO_next_record(thread_db* tdbb, { return false; } - } while (!VIO_chase_record_version(tdbb, rpb, transaction, pool, false, false)); + } while (!VIO_chase_record_version(tdbb, rpb, transaction, pool, RecordLock::NONE, false)); if (rpb->rpb_runtime_flags & RPB_undo_data) fb_assert(rpb->getWindow(tdbb).win_bdb == NULL); @@ -3841,7 +3842,7 @@ Record* VIO_record(thread_db* tdbb, record_param* rpb, const Format* format, Mem bool VIO_refetch_record(thread_db* tdbb, record_param* rpb, jrd_tra* transaction, - bool writelock, bool noundo) + RecordLock recordLock, bool noundo) { /************************************** * @@ -3864,9 +3865,9 @@ bool VIO_refetch_record(thread_db* tdbb, record_param* rpb, jrd_tra* transaction const TraNumber tid_fetch = rpb->rpb_transaction_nr; if (!DPM_get(tdbb, rpb, LCK_read) || - !VIO_chase_record_version(tdbb, rpb, transaction, tdbb->getDefaultPool(), writelock, noundo)) + !VIO_chase_record_version(tdbb, rpb, transaction, tdbb->getDefaultPool(), recordLock, noundo)) { - if (writelock) + if (recordLock == RecordLock::LOCK) return false; ERR_post(Arg::Gds(isc_no_cur_rec)); @@ -3890,7 +3891,7 @@ bool VIO_refetch_record(thread_db* tdbb, record_param* rpb, jrd_tra* transaction // make sure the record has not been updated. Also, punt after // VIO_data() call which will release the page. - if (!writelock && + if (recordLock != RecordLock::LOCK && (transaction->tra_flags & TRA_read_committed) && (tid_fetch != rpb->rpb_transaction_nr) && // added to check that it was not current transaction, @@ -4486,7 +4487,7 @@ WriteLockResult VIO_writelock(thread_db* tdbb, record_param* org_rpb, jrd_tra* t if (org_rpb->rpb_runtime_flags & (RPB_refetch | RPB_undo_read)) { - if (!VIO_refetch_record(tdbb, org_rpb, transaction, true, true)) + if (!VIO_refetch_record(tdbb, org_rpb, transaction, (skipLocked ? RecordLock::SKIP : RecordLock::LOCK), true)) return WriteLockResult::CONFLICTED; org_rpb->rpb_runtime_flags &= ~RPB_refetch; diff --git a/src/jrd/vio_proto.h b/src/jrd/vio_proto.h index 1b4ce3da9b..a9a07a941e 100644 --- a/src/jrd/vio_proto.h +++ b/src/jrd/vio_proto.h @@ -44,6 +44,13 @@ namespace Jrd DPM_next_pointer_page // data pages from one pointer page }; + enum class RecordLock + { + NONE, + LOCK, + SKIP + }; + enum class WriteLockResult { LOCKED, @@ -54,7 +61,7 @@ namespace Jrd void VIO_backout(Jrd::thread_db*, Jrd::record_param*, const Jrd::jrd_tra*); bool VIO_chase_record_version(Jrd::thread_db*, Jrd::record_param*, - Jrd::jrd_tra*, MemoryPool*, bool, bool); + Jrd::jrd_tra*, MemoryPool*, Jrd::RecordLock, bool); void VIO_copy_record(Jrd::thread_db*, Jrd::jrd_rel*, Jrd::Record*, Jrd::Record*); void VIO_data(Jrd::thread_db*, Jrd::record_param*, MemoryPool*); bool VIO_erase(Jrd::thread_db*, Jrd::record_param*, Jrd::jrd_tra*); @@ -69,7 +76,7 @@ Jrd::WriteLockResult VIO_writelock(Jrd::thread_db*, Jrd::record_param*, Jrd::jrd bool VIO_modify(Jrd::thread_db*, Jrd::record_param*, Jrd::record_param*, Jrd::jrd_tra*); bool VIO_next_record(Jrd::thread_db*, Jrd::record_param*, Jrd::jrd_tra*, MemoryPool*, Jrd::FindNextRecordScope); Jrd::Record* VIO_record(Jrd::thread_db*, Jrd::record_param*, const Jrd::Format*, MemoryPool*); -bool VIO_refetch_record(Jrd::thread_db*, Jrd::record_param*, Jrd::jrd_tra*, bool, bool); +bool VIO_refetch_record(Jrd::thread_db*, Jrd::record_param*, Jrd::jrd_tra*, Jrd::RecordLock, bool); void VIO_store(Jrd::thread_db*, Jrd::record_param*, Jrd::jrd_tra*); bool VIO_sweep(Jrd::thread_db*, Jrd::jrd_tra*, Jrd::TraceSweepEvent*); void VIO_intermediate_gc(Jrd::thread_db* tdbb, Jrd::record_param* rpb, Jrd::jrd_tra* transaction);