From 223f45685f93e50f04c5e306d67339d54e4ea497 Mon Sep 17 00:00:00 2001
From: alesapin <alesapin@gmail.com>
Date: Mon, 8 Jun 2020 21:08:55 +0300
Subject: [PATCH] Review fixes

---
 src/Storages/StorageMergeTree.cpp             | 27 ++++++++++++-------
 src/Storages/StorageReplicatedMergeTree.cpp   |  6 ++---
 .../01079_parallel_alter_modify_zookeeper.sh  |  2 +-
 3 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/src/Storages/StorageMergeTree.cpp b/src/Storages/StorageMergeTree.cpp
index 4650485847c..15e662b27b5 100644
--- a/src/Storages/StorageMergeTree.cpp
+++ b/src/Storages/StorageMergeTree.cpp
@@ -141,16 +141,6 @@ void StorageMergeTree::shutdown()
         mutation_wait_event.notify_all();
     }
 
-    try
-    {
-        clearOldPartsFromFilesystem(true);
-    }
-    catch (...)
-    {
-        /// Example: the case of readonly filesystem, we have failure removing old parts.
-        /// Should not prevent table shutdown.
-        tryLogCurrentException(log);
-    }
 
     merger_mutator.merges_blocker.cancelForever();
     parts_mover.moves_blocker.cancelForever();
@@ -160,6 +150,23 @@ void StorageMergeTree::shutdown()
 
     if (moving_task_handle)
         global_context.getBackgroundMovePool().removeTask(moving_task_handle);
+
+
+    try
+    {
+        /// We clear all old parts after stopping all background operations.
+        /// It's important, because background operations can produce temporary
+        /// parts which will remove themselves in their descrutors. If so, we
+        /// may have race condition between our remove call and background
+        /// process.
+        clearOldPartsFromFilesystem(true);
+    }
+    catch (...)
+    {
+        /// Example: the case of readonly filesystem, we have failure removing old parts.
+        /// Should not prevent table shutdown.
+        tryLogCurrentException(log);
+    }
 }
 
 
diff --git a/src/Storages/StorageReplicatedMergeTree.cpp b/src/Storages/StorageReplicatedMergeTree.cpp
index f2ac6678764..d109fa464b0 100644
--- a/src/Storages/StorageReplicatedMergeTree.cpp
+++ b/src/Storages/StorageReplicatedMergeTree.cpp
@@ -2997,10 +2997,10 @@ void StorageReplicatedMergeTree::shutdown()
     }
     data_parts_exchange_endpoint.reset();
 
-    /// We clear all parts after stopping all background operations. It's
+    /// We clear all old parts after stopping all background operations. It's
     /// important, because background operations can produce temporary parts
-    /// which will remove themselfs in their descrutors. If so, we may have race
-    /// condition between our remove call and background process.
+    /// which will remove themselves in their descrutors. If so, we may have
+    /// race condition between our remove call and background process.
     clearOldPartsFromFilesystem(true);
 }
 
diff --git a/tests/queries/0_stateless/01079_parallel_alter_modify_zookeeper.sh b/tests/queries/0_stateless/01079_parallel_alter_modify_zookeeper.sh
index effc9f540a1..05ef4a1a675 100755
--- a/tests/queries/0_stateless/01079_parallel_alter_modify_zookeeper.sh
+++ b/tests/queries/0_stateless/01079_parallel_alter_modify_zookeeper.sh
@@ -102,7 +102,7 @@ echo "Finishing alters"
 
 # This alter will finish all previous, but replica 1 maybe still not up-to-date.
 # If query will throw something, than we will sleep 1 and retry. If timeout
-# happened we will silentrly go out of loop and probably fail tests in the
+# happened we will silently go out of loop and probably fail tests in the
 # following for loop.
 #
 # 120 seconds is more than enough, but in rare cases for slow builds (debug,