Merge

dbms/include/DB/Storages/AlterCommands.h

@@ -6,7 +6,7 @@
 namespace DB
 {
 
-/// Операция из запроса ALTER (кроме DROP PARTITION). Добавление столбцов типа Nested не развернуто в добавление отдельных столбцов.
+/// Операция из запроса ALTER (кроме манипуляции с PART/PARTITION). Добавление столбцов типа Nested не развернуто в добавление отдельных столбцов.
 struct AlterCommand
 {
 	enum Type

dbms/include/DB/Storages/MergeTree/AbandonableLockInZooKeeper.h

@@ -2,6 +2,7 @@
 
 #include <zkutil/ZooKeeper.h>
 #include <DB/Core/Exception.h>
+#include <DB/IO/ReadHelpers.h>
 
 
 namespace DB
@@ -11,7 +12,9 @@ namespace DB
   * При создании создает неэфемерную инкрементную ноду и помечает ее как заблокированную (LOCKED).
   * unlock() разблокирует ее (UNLOCKED).
   * При вызове деструктора или завершении сессии в ZooKeeper, переходит в состояние ABANDONED.
-  *  (В том числе при падении программы)
+  *  (В том числе при падении программы).
+  *
+  * Из состояния ABANDONED не может перейти ни в какое другое состояние.
   */
 class AbandonableLockInZooKeeper : private boost::noncopyable
 {
@@ -32,6 +35,9 @@ public:
 
 		/// Запишем в основную ноду путь к вспомогательной.
 		path = zookeeper.create(path_prefix, holder_path, zkutil::CreateMode::PersistentSequential);
+
+		if (path.size() <= path_prefix.size())
+			throw Exception("Logical error: name of sequential node is shorter than prefix.", ErrorCodes::LOGICAL_ERROR);
 	}
 
 	AbandonableLockInZooKeeper(AbandonableLockInZooKeeper && rhs)
@@ -42,15 +48,15 @@ public:
 		std::swap(holder_path, rhs.holder_path);
 	}
 
-	String getPath()
+	String getPath() const
 	{
 		return path;
 	}
 
 	/// Распарсить число в конце пути.
-	UInt64 getNumber()
+	UInt64 getNumber() const
 	{
-		return static_cast<UInt64>(atol(path.substr(path_prefix.size()).c_str()));
+		return parse<UInt64>(path.c_str() + path_prefix.size(), path.size() - path_prefix.size());
 	}
 
 	void unlock()

dbms/src/Storages/StorageReplicatedMergeTree.cpp

@@ -1299,6 +1299,34 @@ void StorageReplicatedMergeTree::mergeSelectingThread()
 {
 	bool need_pull = true;
 
+	/** Может много времени тратиться на определение, можно ли мерджить два рядом стоящих куска.
+	  * Два рядом стоящих куска можно мерджить, если все номера блоков между их номерами не используются ("заброшены", abandoned).
+	  * Это значит, что между этими кусками не может быть вставлен другой кусок.
+	  *
+	  * Но если номера соседних блоков отличаются достаточно сильно (обычно, если между ними много "заброшенных" блоков),
+	  *  то делается слишком много чтений из ZooKeeper, чтобы узнать, можно ли их мерджить.
+	  *
+	  * Воспользуемся утверждением, что если пару кусков можно мерджить, то всегда будет можно мерджить,
+	  *  и будем запоминать это состояние, чтобы не делать много раз одинаковые запросы в ZooKeeper.
+	  *
+	  * TODO Интересно, как это сочетается с DROP PARTITION и затем ATTACH PARTITION.
+	  */
+	std::set<std::pair<std::string, std::string>> memoized_parts_that_could_be_merged;
+
+	auto can_merge = [&memoized_parts_that_could_be_merged, this]
+		(const MergeTreeData::DataPartPtr & left, const MergeTreeData::DataPartPtr & right) -> bool
+	{
+		auto key = std::make_pair(left->name, right->name);
+		if (memoized_parts_that_could_be_merged.count(key))
+			return true;
+
+		bool res = canMergeParts(left, right);
+		if (res)
+			memoized_parts_that_could_be_merged.insert(key);
+
+		return res;
+	};
+
 	while (!shutdown_called && is_leader_node)
 	{
 		bool success = false;
@@ -1355,8 +1383,6 @@ void StorageReplicatedMergeTree::mergeSelectingThread()
 				MergeTreeData::DataPartsVector parts;
 
 				String merged_name;
-				auto can_merge = std::bind(
-					&StorageReplicatedMergeTree::canMergeParts, this, std::placeholders::_1, std::placeholders::_2);
 
 				if (!merger.selectPartsToMerge(parts, merged_name, MergeTreeDataMerger::NO_LIMIT,
 												false, false, has_big_merge, can_merge) &&
@@ -1834,7 +1860,7 @@ bool StorageReplicatedMergeTree::canMergeParts(const MergeTreeData::DataPartPtr
 	String month_name = left->name.substr(0, 6);
 
 	/// Можно слить куски, если все номера между ними заброшены - не соответствуют никаким блокам.
-	for (UInt64 number = left->right + 1; number <= right->left - 1; ++number)
+	for (UInt64 number = left->right + 1; number <= right->left - 1; ++number)	/// Номера блоков больше нуля.
 	{
 		String path1 = zookeeper_path +              "/block_numbers/" + month_name + "/block-" + padIndex(number);
 		String path2 = zookeeper_path + "/nonincrement_block_numbers/" + month_name + "/block-" + padIndex(number);

libs/libzkutil/include/zkutil/ZooKeeper.h

@@ -162,6 +162,7 @@ public:
 	static const size_t SEQUENTIAL_SUFFIX_SIZE = 64;
 private:
 	friend struct WatchWithEvent;
+	friend class EphemeralNodeHolder;
 
 	void init(const std::string & hosts, int32_t sessionTimeoutMs);
 	void removeChildrenRecursive(const std::string & path);
@@ -255,7 +256,10 @@ public:
 		{
 			zookeeper.tryRemove(path);
 		}
-		catch (KeeperException) {}
+		catch (const KeeperException & e)
+		{
+			LOG_ERROR(zookeeper.log, "~EphemeralNodeHolder(): " << e.displayText());
+		}
 	}
 
 private: