Add missed lock

alesapin · alesapin · commit 292016e04515 · 2020-06-23T11:04:43.000+03:00
diff --git a/src/Storages/MergeTree/MergeTreeData.h b/src/Storages/MergeTree/MergeTreeData.h
@@ -477,7 +477,7 @@ class MergeTreeData : public IStorage
 
     /// Delete all directories which names begin with "tmp"
     /// Set non-negative parameter value to override MergeTreeSettings temporary_directories_lifetime
-    /// Must be called with locked lockForShare().
+    /// Must be called with locked lockForShare() because use relative_data_path.
     void clearOldTemporaryDirectories(ssize_t custom_directories_lifetime_seconds = -1);
 
     /// After the call to dropAllData() no method can be called.
@@ -487,9 +487,9 @@ class MergeTreeData : public IStorage
     /// Drop data directories if they are empty. It is safe to call this method if table creation was unsuccessful.
     void dropIfEmpty();
 
-    /// Moves the entire data directory.
-    /// Flushes the uncompressed blocks cache and the marks cache.
-    /// Must be called with locked lockForShare().
+    /// Moves the entire data directory. Flushes the uncompressed blocks cache
+    /// and the marks cache. Must be called with locked lockExclusively()
+    /// because changes relative_data_path.
     void rename(const String & new_table_path, const StorageID & new_table_id) override;
 
     /// Check if the ALTER can be performed:
diff --git a/src/Storages/StorageMergeTree.cpp b/src/Storages/StorageMergeTree.cpp
@@ -865,8 +865,11 @@ BackgroundProcessingPoolTaskResult StorageMergeTree::mergeMutateTask()
         /// Clear old parts. It is unnecessary to do it more than once a second.
         if (auto lock = time_after_previous_cleanup.compareAndRestartDeferred(1))
         {
-            clearOldPartsFromFilesystem();
-            clearOldTemporaryDirectories();
+            {
+                auto share_lock = lockForShare(RWLockImpl::NO_QUERY, getSettings()->lock_acquire_timeout_for_background_operations);
+                clearOldPartsFromFilesystem();
+                clearOldTemporaryDirectories();
+            }
             clearOldMutations();
         }
 

Original file line number	Diff line number	Diff line change
`@@ -865,8 +865,11 @@ BackgroundProcessingPoolTaskResult StorageMergeTree::mergeMutateTask()`
`865`	`865`	`/// Clear old parts. It is unnecessary to do it more than once a second.`
`866`	`866`	`if (auto lock = time_after_previous_cleanup.compareAndRestartDeferred(1))`
`867`	`867`	`{`
`868`		`- clearOldPartsFromFilesystem();`
`869`		`- clearOldTemporaryDirectories();`
	`868`	`+ {`
	`869`	`+ auto share_lock = lockForShare(RWLockImpl::NO_QUERY, getSettings()->lock_acquire_timeout_for_background_operations);`
	`870`	`+ clearOldPartsFromFilesystem();`
	`871`	`+ clearOldTemporaryDirectories();`
	`872`	`+ }`
`870`	`873`	`clearOldMutations();`
`871`	`874`	`}`
`872`	`875`