Fix the bug that some decoding is not protected by alter_lock (pingca…

…p#1945) Signed-off-by: JaySon-Huang <jayson.hjs@gmail.com>
JaySon-Huang · Jun 21, 2021 · 25a038b · 25a038b
1 parent c1185a2
commit 25a038b
Show file tree

Hide file tree

Showing 3 changed files with 6 additions and 7 deletions.
diff --git a/dbms/src/Storages/Transaction/KVStore.cpp b/dbms/src/Storages/Transaction/KVStore.cpp
@@ -106,7 +106,7 @@ void KVStore::tryFlushRegionCacheInStorage(TMTContext & tmt, const Region & regi
 
         try
         {
-            // Try to get a read lock on `storage` so it won't be dropped during `flushCache`
+            // Acquire `drop_lock` so that no other threads can drop the storage during `flushCache`. `alter_lock` is not required.
             auto storage_lock = storage->lockForShare(getThreadName());
             auto rowkey_range = DM::RowKeyRange::fromRegionRange(
                 region.getRange(), region.getRange()->getMappedTableID(), storage->isCommonHandle(), storage->getRowKeyColumnSize());

diff --git a/dbms/src/Storages/Transaction/PartitionStreams.cpp b/dbms/src/Storages/Transaction/PartitionStreams.cpp
@@ -54,15 +54,15 @@ static void writeRegionDataToStorage(
                 return true;
         }
 
-        /// Lock throughout decode and write, during which schema must not change.
+        /// Get a structure read lock throughout decode, during which schema must not change.
         TableStructureLockHolder lock;
         try
         {
             lock = storage->lockStructureForShare(getThreadName());
         }
         catch (DB::Exception & e)
         {
-            // If the storage is physical dropped (but not removed from `ManagedStorages`) when we want to flsuh raft data into it, consider the write done.
+            // If the storage is physical dropped (but not removed from `ManagedStorages`) when we want to write raft data into it, consider the write done.
             if (e.code() == ErrorCodes::TABLE_IS_DROPPED)
                 return true;
             else
@@ -437,7 +437,7 @@ RegionPtrWithBlock::CachePtr GenRegionPreDecodeBlockData(const RegionPtr & regio
                 return true;
         }
 
-        /// Lock throughout decode and write, during which schema must not change.
+        /// Get a structure read lock throughout decode, during which schema must not change.
         TableStructureLockHolder lock;
         try
         {

diff --git a/dbms/src/Storages/Transaction/RegionTable.cpp b/dbms/src/Storages/Transaction/RegionTable.cpp
@@ -238,8 +238,7 @@ void removeObsoleteDataInStorage(
 
     try
     {
-        // acquire a read lock so that no other threads can drop the `storage`
-        // if storage is already dropped, this will throw exception
+        // Acquire a `drop_lock` so that no other threads can drop the `storage`
         auto storage_lock = storage->lockForShare(getThreadName());
 
         auto dm_storage = std::dynamic_pointer_cast<StorageDeltaMerge>(storage);
@@ -254,7 +253,7 @@ void removeObsoleteDataInStorage(
     }
     catch (DB::Exception & e)
     {
-        // We can ignore if storage is already dropped.
+        // We can ignore if the storage is already dropped.
         if (e.code() != ErrorCodes::TABLE_IS_DROPPED)
             throw;
     }