From e5f6ec1e4ee4af7df81c90072c0e69133f6c2ada Mon Sep 17 00:00:00 2001 From: Dmitry Yemanov Date: Sun, 21 Sep 2025 10:11:15 +0300 Subject: [PATCH 1/3] Allow isc_tpb_read_consistency used alone to imply Read Committed Read Consistency --- src/jrd/tra.cpp | 80 +++++++++++++++++++++++++++++++++++-------------- 1 file changed, 58 insertions(+), 22 deletions(-) diff --git a/src/jrd/tra.cpp b/src/jrd/tra.cpp index 518dd9648ae..cf448f2ef94 100644 --- a/src/jrd/tra.cpp +++ b/src/jrd/tra.cpp @@ -2929,39 +2929,85 @@ static void transaction_options(thread_db* tdbb, { case isc_tpb_consistency: if (!isolation.assignOnce(true)) + { ERR_post(Arg::Gds(isc_bad_tpb_content) << Arg::Gds(isc_tpb_multiple_txn_isolation)); + } + + if (read_consistency.isAssigned()) + { + ERR_post(Arg::Gds(isc_bad_tpb_content) << + // 'Option @1 is not valid if @2 was used previously in TPB' + Arg::Gds(isc_tpb_conflicting_options) << + Arg::Str("isc_tpb_consistency") << Arg::Str("isc_tpb_read_consistency")); + } + + if (rec_version.isAssigned()) + { + const auto tpbStr = rec_version.asBool() ? + "isc_tpb_rec_version" : "isc_tpb_no_rec_version"; + + ERR_post(Arg::Gds(isc_bad_tpb_content) << + // 'Option @1 is not valid if @2 was used previously in TPB' + Arg::Gds(isc_tpb_conflicting_options) << + Arg::Str("isc_tpb_consistency") << Arg::Str(tpbStr) ); + } if (shared_snapshot) { ERR_post( Arg::Gds(isc_bad_tpb_content) << + // 'Option @1 is not valid if @2 was used previously in TPB' Arg::Gds(isc_tpb_conflicting_options) << Arg::Str("isc_tpb_consistency") << Arg::Str("isc_tpb_at_snapshot_number")); } transaction->tra_flags |= TRA_degree3; - transaction->tra_flags &= ~TRA_read_committed; + transaction->tra_flags &= ~(TRA_read_committed | TRA_read_consistency | TRA_rec_version); break; case isc_tpb_concurrency: if (!isolation.assignOnce(true)) + { ERR_post(Arg::Gds(isc_bad_tpb_content) << Arg::Gds(isc_tpb_multiple_txn_isolation)); + } + + if (read_consistency.isAssigned()) + { + ERR_post(Arg::Gds(isc_bad_tpb_content) << + // 'Option @1 is not valid if @2 was used previously in TPB' + Arg::Gds(isc_tpb_conflicting_options) << + Arg::Str("isc_tpb_concurrency") << Arg::Str("isc_tpb_read_consistency")); + } + + if (rec_version.isAssigned()) + { + const auto tpbStr = rec_version.asBool() ? + "isc_tpb_rec_version" : "isc_tpb_no_rec_version"; + + ERR_post(Arg::Gds(isc_bad_tpb_content) << + // 'Option @1 is not valid if @2 was used previously in TPB' + Arg::Gds(isc_tpb_conflicting_options) << + Arg::Str("isc_tpb_concurrency") << Arg::Str(tpbStr) ); + } transaction->tra_flags &= ~TRA_degree3; - transaction->tra_flags &= ~TRA_read_committed; + transaction->tra_flags &= ~(TRA_read_committed | TRA_read_consistency | TRA_rec_version); break; case isc_tpb_read_committed: if (!isolation.assignOnce(true)) + { ERR_post(Arg::Gds(isc_bad_tpb_content) << Arg::Gds(isc_tpb_multiple_txn_isolation)); + } if (shared_snapshot) { ERR_post( Arg::Gds(isc_bad_tpb_content) << + // 'Option @1 is not valid if @2 was used previously in TPB' Arg::Gds(isc_tpb_conflicting_options) << Arg::Str("isc_tpb_read_committed") << Arg::Str("isc_tpb_at_snapshot_number")); } @@ -3066,14 +3112,16 @@ static void transaction_options(thread_db* tdbb, if (rec_version.isAssigned()) { + const auto tpbStr = rec_version.asBool() ? + "isc_tpb_rec_version" : "isc_tpb_no_rec_version"; + ERR_post(Arg::Gds(isc_bad_tpb_content) << // 'Option @1 is not valid if @2 was used previously in TPB' Arg::Gds(isc_tpb_conflicting_options) << - Arg::Str("isc_tpb_read_consistency") << (rec_version.asBool() ? - Arg::Str("isc_tpb_rec_version") : Arg::Str("isc_tpb_no_rec_version")) ); + Arg::Str("isc_tpb_read_consistency") << Arg::Str(tpbStr) ); } - transaction->tra_flags |= TRA_read_consistency | TRA_rec_version; + transaction->tra_flags |= TRA_read_committed | TRA_read_consistency | TRA_rec_version; break; case isc_tpb_nowait: @@ -3479,24 +3527,12 @@ static void transaction_options(thread_db* tdbb, } } - if (rec_version.isAssigned() && !(transaction->tra_flags & TRA_read_committed)) - { - if (rec_version.asBool()) - { - ERR_post(Arg::Gds(isc_bad_tpb_content) << - Arg::Gds(isc_tpb_option_without_rc) << Arg::Str("isc_tpb_rec_version")); - } - else - { - ERR_post(Arg::Gds(isc_bad_tpb_content) << - Arg::Gds(isc_tpb_option_without_rc) << Arg::Str("isc_tpb_no_rec_version")); - } - } - - if ((transaction->tra_flags & TRA_read_committed) && !(tdbb->tdbb_flags & TDBB_sweeper)) + if ((transaction->tra_flags & TRA_read_committed) && + !(transaction->tra_flags & TRA_read_consistency) && + !(tdbb->tdbb_flags & TDBB_sweeper) && + tdbb->getDatabase()->dbb_config->getReadConsistency()) { - if (tdbb->getDatabase()->dbb_config->getReadConsistency()) - transaction->tra_flags |= TRA_read_consistency | TRA_rec_version; + transaction->tra_flags |= TRA_read_consistency | TRA_rec_version; } if (transaction->tra_attachment->isGbak()) From 44e9748274d50000af100afb474ba7da9612a91d Mon Sep 17 00:00:00 2001 From: Dmitry Yemanov Date: Sun, 21 Sep 2025 10:14:00 +0300 Subject: [PATCH 2/3] Sync the trace logic with MON$ tables regarding txn isolation, make it more robust to improper set of flags --- src/jrd/trace/TraceObjects.cpp | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/jrd/trace/TraceObjects.cpp b/src/jrd/trace/TraceObjects.cpp index 35207f95634..9548ed141a4 100644 --- a/src/jrd/trace/TraceObjects.cpp +++ b/src/jrd/trace/TraceObjects.cpp @@ -210,27 +210,21 @@ int TraceTransactionImpl::getWait() unsigned TraceTransactionImpl::getIsolation() { - switch (m_tran->tra_flags & (TRA_read_committed | TRA_rec_version | TRA_degree3 | TRA_read_consistency)) - { - case TRA_degree3: + if (m_tran->tra_flags & TRA_degree3) return ISOLATION_CONSISTENCY; - case TRA_read_committed: - return ISOLATION_READ_COMMITTED_NORECVER; - - case TRA_read_committed | TRA_rec_version: - return ISOLATION_READ_COMMITTED_RECVER; - - case TRA_read_committed | TRA_rec_version | TRA_read_consistency: - return ISOLATION_READ_COMMITTED_READ_CONSISTENCY; + if (m_tran->tra_flags & TRA_read_committed) + { + if (m_tran->tra_flags & TRA_read_consistency) + return ISOLATION_READ_COMMITTED_READ_CONSISTENCY; - case 0: - return ISOLATION_CONCURRENCY; + if (m_tran->tra_flags & TRA_rec_version) + return ISOLATION_READ_COMMITTED_RECVER; - default: - fb_assert(false); - return ISOLATION_CONCURRENCY; + return ISOLATION_READ_COMMITTED_NORECVER; } + + return ISOLATION_CONCURRENCY; } ISC_INT64 TraceTransactionImpl::getInitialID() From 8b1507e21165bfb3682823ff593215c5a02d63cd Mon Sep 17 00:00:00 2001 From: Dmitry Yemanov Date: Sun, 21 Sep 2025 18:20:27 +0300 Subject: [PATCH 3/3] Restore the original error for isc_tpb_[no_]rec_version version specified without isc_tpb_read_committed --- src/jrd/tra.cpp | 31 +++++++++---------------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/src/jrd/tra.cpp b/src/jrd/tra.cpp index cf448f2ef94..08fe4df39e4 100644 --- a/src/jrd/tra.cpp +++ b/src/jrd/tra.cpp @@ -2942,17 +2942,6 @@ static void transaction_options(thread_db* tdbb, Arg::Str("isc_tpb_consistency") << Arg::Str("isc_tpb_read_consistency")); } - if (rec_version.isAssigned()) - { - const auto tpbStr = rec_version.asBool() ? - "isc_tpb_rec_version" : "isc_tpb_no_rec_version"; - - ERR_post(Arg::Gds(isc_bad_tpb_content) << - // 'Option @1 is not valid if @2 was used previously in TPB' - Arg::Gds(isc_tpb_conflicting_options) << - Arg::Str("isc_tpb_consistency") << Arg::Str(tpbStr) ); - } - if (shared_snapshot) { ERR_post( @@ -2981,17 +2970,6 @@ static void transaction_options(thread_db* tdbb, Arg::Str("isc_tpb_concurrency") << Arg::Str("isc_tpb_read_consistency")); } - if (rec_version.isAssigned()) - { - const auto tpbStr = rec_version.asBool() ? - "isc_tpb_rec_version" : "isc_tpb_no_rec_version"; - - ERR_post(Arg::Gds(isc_bad_tpb_content) << - // 'Option @1 is not valid if @2 was used previously in TPB' - Arg::Gds(isc_tpb_conflicting_options) << - Arg::Str("isc_tpb_concurrency") << Arg::Str(tpbStr) ); - } - transaction->tra_flags &= ~TRA_degree3; transaction->tra_flags &= ~(TRA_read_committed | TRA_read_consistency | TRA_rec_version); break; @@ -3527,6 +3505,15 @@ static void transaction_options(thread_db* tdbb, } } + if (rec_version.isAssigned() && !(transaction->tra_flags & TRA_read_committed)) + { + const auto tpbStr = rec_version.asBool() ? + "isc_tpb_rec_version" : "isc_tpb_no_rec_version"; + + ERR_post(Arg::Gds(isc_bad_tpb_content) << + Arg::Gds(isc_tpb_option_without_rc) << Arg::Str(tpbStr)); + } + if ((transaction->tra_flags & TRA_read_committed) && !(transaction->tra_flags & TRA_read_consistency) && !(tdbb->tdbb_flags & TDBB_sweeper) &&