Merge pull request ClickHouse#6513 from yandex/parts-race-condition

alexey-milovidov · web-flow · commit e2687fc2d569 · 2019-08-17T22:30:16.000+03:00
Fix race condition in system.parts vs. ALTER
diff --git a/dbms/src/Storages/MergeTree/MergeTreeDataPart.cpp b/dbms/src/Storages/MergeTree/MergeTreeDataPart.cpp
@@ -409,6 +409,8 @@ void MergeTreeDataPart::remove() const
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wunused-variable"
 #endif
+        std::shared_lock<std::shared_mutex> lock(columns_lock);
+
         for (const auto & [file, _] : checksums.files)
         {
             String path_to_remove = to + "/" + file;
diff --git a/dbms/src/Storages/System/StorageSystemParts.cpp b/dbms/src/Storages/System/StorageSystemParts.cpp
@@ -114,7 +114,11 @@ void StorageSystemParts::processNextStorage(MutableColumns & columns_, const Sto
             columns_[i++]->insert(part->stateString());
 
         MinimalisticDataPartChecksums helper;
-        helper.computeTotalChecksums(part->checksums);
+        {
+            /// TODO: MergeTreeDataPart structure is too error-prone.
+            std::shared_lock<std::shared_mutex> lock(part->columns_lock);
+            helper.computeTotalChecksums(part->checksums);
+        }
 
         auto checksum = helper.hash_of_all_files;
         columns_[i++]->insert(getHexUIntLowercase(checksum.first) + getHexUIntLowercase(checksum.second));

Original file line number	Diff line number	Diff line change
`@@ -409,6 +409,8 @@ void MergeTreeDataPart::remove() const`
`409`	`409`	`#pragma GCC diagnostic push`
`410`	`410`	`#pragma GCC diagnostic ignored "-Wunused-variable"`
`411`	`411`	`#endif`
	`412`	`+ std::shared_lock<std::shared_mutex> lock(columns_lock);`
	`413`	`+`
`412`	`414`	`for (const auto & [file, _] : checksums.files)`
`413`	`415`	`{`
`414`	`416`	`String path_to_remove = to + "/" + file;`