From d5dcae52cb99c4cc4cfb62a4d05361b1ec6082ff Mon Sep 17 00:00:00 2001 From: drrtuy Date: Thu, 20 Feb 2025 16:33:44 +0000 Subject: [PATCH] fix(BRM): MCOL-5879 DBRM::clearShm runs crit sections w/o sync mechanism (#3391) --- dbcon/mysql/ha_mcs_partition.cpp | 6 +++--- dbcon/mysql/is_columnstore_extents.cpp | 18 ++---------------- dbcon/mysql/is_columnstore_files.cpp | 2 +- versioning/BRM/dbrm.h | 8 ++++---- versioning/BRM/extentmap.h | 13 +++++++++++++ versioning/BRM/mastersegmenttable.h | 6 ++++++ 6 files changed, 29 insertions(+), 24 deletions(-) diff --git a/dbcon/mysql/ha_mcs_partition.cpp b/dbcon/mysql/ha_mcs_partition.cpp index bfb677718..0a4538e74 100644 --- a/dbcon/mysql/ha_mcs_partition.cpp +++ b/dbcon/mysql/ha_mcs_partition.cpp @@ -330,7 +330,7 @@ void partitionByValue_common(UDF_ARGS* args, // inp string functionName) // input { // identify partitions by the range - BRM::DBRM::refreshShm(); + BRM::DBRM::refreshShmWithLock(); DBRM em; vector entries; vector::iterator iter; @@ -575,7 +575,7 @@ extern "C" const char* calshowpartitions(UDF_INIT* initid, UDF_ARGS* args, char* result, unsigned long* length, char* is_null, char* error) { - BRM::DBRM::refreshShm(); + BRM::DBRM::refreshShmWithLock(); DBRM em; vector entries; vector::iterator iter; @@ -1170,7 +1170,7 @@ extern "C" const char* calshowpartitionsbyvalue(UDF_INIT* initid, UDF_ARGS* args, char* result, unsigned long* length, char* is_null, char* error) { - BRM::DBRM::refreshShm(); + BRM::DBRM::refreshShmWithLock(); DBRM em; vector entries; vector::iterator iter; diff --git a/dbcon/mysql/is_columnstore_extents.cpp b/dbcon/mysql/is_columnstore_extents.cpp index c12793cbf..b8f0cacf0 100644 --- a/dbcon/mysql/is_columnstore_extents.cpp +++ b/dbcon/mysql/is_columnstore_extents.cpp @@ -196,27 +196,13 @@ static int generate_result(BRM::OID_t oid, BRM::DBRM* emp, TABLE* table, THD* th return 0; } -struct refresher -{ - BRM::DBRM* guarded; - refresher() - { - guarded = new BRM::DBRM(); - } - ~refresher() - { - delete guarded; - BRM::DBRM::refreshShm(); - } -}; static int is_columnstore_extents_fill(THD* thd, TABLE_LIST* tables, COND* cond) { BRM::OID_t cond_oid = 0; TABLE* table = tables->table; - BRM::DBRM* emp; - refresher shmRefresher; - emp = shmRefresher.guarded; + BRM::DBRM::refreshShmWithLock(); + BRM::DBRM* emp = new BRM::DBRM(); if (!emp || !emp->isDBRMReady()) { diff --git a/dbcon/mysql/is_columnstore_files.cpp b/dbcon/mysql/is_columnstore_files.cpp index 5c3ea2d12..11f64e1d2 100644 --- a/dbcon/mysql/is_columnstore_files.cpp +++ b/dbcon/mysql/is_columnstore_files.cpp @@ -215,7 +215,7 @@ static int generate_result(BRM::OID_t oid, BRM::DBRM* emp, TABLE* table, THD* th static int is_columnstore_files_fill(THD* thd, TABLE_LIST* tables, COND* cond) { - BRM::DBRM::refreshShm(); + BRM::DBRM::refreshShmWithLock(); BRM::DBRM* emp = new BRM::DBRM(); BRM::OID_t cond_oid = 0; TABLE* table = tables->table; diff --git a/versioning/BRM/dbrm.h b/versioning/BRM/dbrm.h index eb5dc557e..ed2298ca6 100644 --- a/versioning/BRM/dbrm.h +++ b/versioning/BRM/dbrm.h @@ -101,11 +101,11 @@ class DBRM EXPORT DBRM(bool noBRMFcns = false); EXPORT ~DBRM(); - EXPORT static void refreshShm() + static void refreshShmWithLock() { - MasterSegmentTableImpl::refreshShm(); - ExtentMapRBTreeImpl::refreshShm(); - FreeListImpl::refreshShm(); + MasterSegmentTableImpl::refreshShmWithLock(); + ExtentMapRBTreeImpl::refreshShmWithLock(); + FreeListImpl::refreshShmWithLock(); } // @bug 1055+ - Added functions below for multiple files per OID enhancement. diff --git a/versioning/BRM/extentmap.h b/versioning/BRM/extentmap.h index cdfa738a4..96e5f4b40 100644 --- a/versioning/BRM/extentmap.h +++ b/versioning/BRM/extentmap.h @@ -265,6 +265,12 @@ class ExtentMapRBTreeImpl static ExtentMapRBTreeImpl* makeExtentMapRBTreeImpl(unsigned key, off_t size, bool readOnly = false); + static void refreshShmWithLock() + { + boost::mutex::scoped_lock lk(fInstanceMutex); + return refreshShm(); + } + static void refreshShm() { if (fInstance) @@ -317,6 +323,13 @@ class FreeListImpl ~FreeListImpl(){}; static FreeListImpl* makeFreeListImpl(unsigned key, off_t size, bool readOnly = false); + + static void refreshShmWithLock() + { + boost::mutex::scoped_lock lk(fInstanceMutex); + return refreshShm(); + } + static void refreshShm() { if (fInstance) diff --git a/versioning/BRM/mastersegmenttable.h b/versioning/BRM/mastersegmenttable.h index 2644a81b8..06bc1d9ae 100644 --- a/versioning/BRM/mastersegmenttable.h +++ b/versioning/BRM/mastersegmenttable.h @@ -61,6 +61,12 @@ class MasterSegmentTableImpl ~MasterSegmentTableImpl(){}; static MasterSegmentTableImpl* makeMasterSegmentTableImpl(int key, int size); + static void refreshShmWithLock() + { + boost::mutex::scoped_lock lk(fInstanceMutex); + return refreshShm(); + } + static void refreshShm() { if (fInstance)