fix(brm): This refactors shmem RWLocks operations for EM, EMIndex and FreeList so that re-map operation always takes shmem lock in Exclusive mode

---------

Co-authored-by: Roman Nozdrin <rnozdrin@mariadb.com>
(cherry picked from commit 5f4d2705fd)

versioning/BRM/extentmap.cpp

@@ -1929,44 +1929,55 @@ void ExtentMap::save(const string& filename)
   releaseEMEntryTable(READ);
 }
 
+// This routine takes shmem RWLock in appropriate mode.
+MSTEntry* ExtentMap::_getTableLock(const OPS op, std::atomic<bool>& lockedState, const int table)
+{
+  if (op == READ)
+  {
+    return fMST.getTable_read(table);
+  }
+  // WRITE/NONE op
+  auto result = fMST.getTable_write(table);
+  lockedState = true;
+  return result;
+}
+
+// This routine upgrades shmem RWlock mode if needed.
+void ExtentMap::_getTableLockUpgradeIfNeeded(const OPS op, std::atomic<bool>& lockedState, const int table)
+{
+  if (op == READ)
+  {
+    fMST.getTable_upgrade(table);
+    lockedState = true;
+  }
+}
+
+// This routine downgrades shmem RWlock if it was previously upgraded.
+void ExtentMap::_getTableLockDowngradeIfNeeded(const OPS op, std::atomic<bool>& lockedState, const int table)
+{
+  if (op == READ)
+  {
+    // Look releaseEMEntryTable() for the explanation why lockedState is set before the lock is downgraded.
+    lockedState = false;
+    fMST.getTable_downgrade(table);
+  }
+}
+
 /* always returns holding the EM lock, and with the EM seg mapped */
 void ExtentMap::grabEMEntryTable(OPS op)
 {
   boost::mutex::scoped_lock lk(mutex);
 
-  if (op == READ)
+  fEMRBTreeShminfo = _getTableLock(op, emLocked, MasterSegmentTable::EMTable);
-  {
-    fEMRBTreeShminfo = fMST.getTable_read(MasterSegmentTable::EMTable);
-  }
-  else
-  {
-    fEMRBTreeShminfo = fMST.getTable_write(MasterSegmentTable::EMTable);
-    emLocked = true;
-  }
 
   if (!fPExtMapRBTreeImpl || fPExtMapRBTreeImpl->key() != (uint32_t)fEMRBTreeShminfo->tableShmkey)
   {
-    if (fEMRBTreeShminfo->allocdSize == 0)
+    _getTableLockUpgradeIfNeeded(op, emLocked, MasterSegmentTable::EMTable);
-    {
-      if (op == READ)
-      {
-        fMST.getTable_upgrade(MasterSegmentTable::EMTable);
-        emLocked = true;
 
     if (fEMRBTreeShminfo->allocdSize == 0)
     {
       growEMShmseg();
     }
-
-        // Has to be done holding the write lock.
-        emLocked = false;
-        fMST.getTable_downgrade(MasterSegmentTable::EMTable);
-      }
-      else
-      {
-        growEMShmseg();
-      }
-    }
     else
     {
       const bool isReadOnly = false;
@@ -1983,6 +1994,8 @@ void ExtentMap::grabEMEntryTable(OPS op)
         throw runtime_error("ExtentMap cannot create RBTree in shared memory segment");
       }
     }
+
+    _getTableLockDowngradeIfNeeded(op, emLocked, MasterSegmentTable::EMTable);
   }
   else
   {
@@ -1993,21 +2006,14 @@ void ExtentMap::grabEMEntryTable(OPS op)
 /* always returns holding the FL lock */
 void ExtentMap::grabFreeList(OPS op)
 {
-  boost::mutex::scoped_lock lk(mutex, boost::defer_lock);
+  boost::mutex::scoped_lock lk(mutex);
 
-  if (op == READ)
+  fFLShminfo = _getTableLock(op, flLocked, MasterSegmentTable::EMFreeList);
-  {
-    fFLShminfo = fMST.getTable_read(MasterSegmentTable::EMFreeList);
-    lk.lock();
-  }
-  else
-  {
-    fFLShminfo = fMST.getTable_write(MasterSegmentTable::EMFreeList);
-    flLocked = true;
-  }
 
   if (!fPFreeListImpl || fPFreeListImpl->key() != (unsigned)fFLShminfo->tableShmkey)
   {
+    _getTableLockUpgradeIfNeeded(op, flLocked, MasterSegmentTable::EMFreeList);
+
     if (fFreeList != nullptr)
     {
       fFreeList = nullptr;
@@ -2015,19 +2021,6 @@ void ExtentMap::grabFreeList(OPS op)
 
     if (fFLShminfo->allocdSize == 0)
     {
-      if (op == READ)
-      {
-        lk.unlock();
-        fMST.getTable_upgrade(MasterSegmentTable::EMFreeList);
-        flLocked = true;
-
-        if (fFLShminfo->allocdSize == 0)
-          growFLShmseg();
-
-        flLocked = false;  // has to be done holding the write lock
-        fMST.getTable_downgrade(MasterSegmentTable::EMFreeList);
-      }
-      else
       growFLShmseg();
     }
     else
@@ -2045,17 +2038,12 @@ void ExtentMap::grabFreeList(OPS op)
         log_errno("ExtentMap::grabFreeList(): shmat");
         throw runtime_error("ExtentMap::grabFreeList(): shmat failed.  Check the error log.");
       }
-
-      if (op == READ)
-        lk.unlock();
     }
+    _getTableLockDowngradeIfNeeded(op, flLocked, MasterSegmentTable::EMFreeList);
   }
   else
   {
     fFreeList = fPFreeListImpl->get();
-
-    if (op == READ)
-      lk.unlock();
   }
 }
 
@@ -2063,36 +2051,20 @@ void ExtentMap::grabEMIndex(OPS op)
 {
   boost::mutex::scoped_lock lk(emIndexMutex);
 
-  if (op == READ)
+  fEMIndexShminfo = _getTableLock(op, emIndexLocked, MasterSegmentTable::EMIndex);
+
+  if (fPExtMapIndexImpl_ && (fPExtMapIndexImpl_->getShmemImplSize() == (unsigned)fEMIndexShminfo->allocdSize))
   {
-    fEMIndexShminfo = fMST.getTable_read(MasterSegmentTable::EMIndex);
+    return;
-  }
-  else
-  {
-    fEMIndexShminfo = fMST.getTable_write(MasterSegmentTable::EMIndex);
-    emIndexLocked = true;
   }
 
+  _getTableLockUpgradeIfNeeded(op, emIndexLocked, MasterSegmentTable::EMIndex);
+
   if (!fPExtMapIndexImpl_)
   {
     if (fEMIndexShminfo->allocdSize == 0)
     {
-      if (op == READ)
-      {
-        fMST.getTable_upgrade(MasterSegmentTable::EMIndex);
-        emIndexLocked = true;
-
-        // Checking race conditions
-        if (fEMIndexShminfo->allocdSize == 0)
       growEMIndexShmseg();
-
-        emIndexLocked = false;
-        fMST.getTable_downgrade(MasterSegmentTable::EMIndex);
-      }
-      else
-      {
-        growEMIndexShmseg();
-      }
     }
     else
     {
@@ -2145,13 +2117,14 @@ void ExtentMap::grabEMIndex(OPS op)
           ExtentMapIndexImpl::makeExtentMapIndexImpl(getInitialEMIndexShmkey(), fEMIndexShminfo->allocdSize);
     }
   }
+  _getTableLockDowngradeIfNeeded(op, emIndexLocked, MasterSegmentTable::EMIndex);
 }
 
-void ExtentMap::releaseEMEntryTable(OPS op)
+void ExtentMap::_releaseTable(const OPS op, std::atomic<bool>& lockedState, const int table)
 {
   if (op == READ)
   {
-    fMST.releaseTable_read(MasterSegmentTable::EMTable);
+    fMST.releaseTable_read(table);
   }
   else
   {
@@ -2160,33 +2133,24 @@ void ExtentMap::releaseEMEntryTable(OPS op)
     // here will fail is if the underlying semaphore doesn't exist anymore
     // or there is a locking logic error somewhere else.  Either way,
     // declaring the EM unlocked here is OK. Same with all similar assignments.
-    emLocked = false;
+    lockedState = false;
-    fMST.releaseTable_write(MasterSegmentTable::EMTable);
+    fMST.releaseTable_write(table);
   }
 }
 
+void ExtentMap::releaseEMEntryTable(OPS op)
+{
+  _releaseTable(op, emLocked, MasterSegmentTable::EMTable);
+}
+
 void ExtentMap::releaseFreeList(OPS op)
 {
-  if (op == READ)
+  _releaseTable(op, flLocked, MasterSegmentTable::EMFreeList);
-    fMST.releaseTable_read(MasterSegmentTable::EMFreeList);
-  else
-  {
-    flLocked = false;
-    fMST.releaseTable_write(MasterSegmentTable::EMFreeList);
-  }
 }
 
 void ExtentMap::releaseEMIndex(OPS op)
 {
-  if (op == READ)
+  _releaseTable(op, emIndexLocked, MasterSegmentTable::EMIndex);
-  {
-    fMST.releaseTable_read(MasterSegmentTable::EMIndex);
-  }
-  else
-  {
-    emIndexLocked = false;
-    fMST.releaseTable_write(MasterSegmentTable::EMIndex);
-  }
 }
 
 key_t ExtentMap::chooseEMShmkey()

versioning/BRM/extentmap.h

@@ -1200,6 +1200,11 @@ class ExtentMap : public Undoable
 
   int _markInvalid(const LBID_t lbid, const execplan::CalpontSystemCatalog::ColDataType colDataType);
 
+  MSTEntry* _getTableLock(const OPS op, std::atomic<bool>& lockedState, const int table);
+  void _getTableLockUpgradeIfNeeded(const OPS op, std::atomic<bool>& lockedState, const int table);
+  void _getTableLockDowngradeIfNeeded(const OPS op, std::atomic<bool>& lockedState, const int table);
+  void _releaseTable(const OPS op, std::atomic<bool>& lockedState, const int table);
+
   template <typename T>
   void load(T* in);