Conversation
This reverts commit a07a252.
There was a problem hiding this comment.
Yes .... my skills with the repo and submodules are lacking 😄
| - String. | ||
| To target an auxiliary ZooKeeper cluster, prefix the value with the configured name, for example `analytics_keeper:/clickhouse/queue/orders`. The name must exist in `<auxiliary_zookeepers>`; otherwise the engine reports `Unknown auxiliary ZooKeeper name ...`. The full string (including the prefix) is preserved in `SHOW CREATE TABLE` so the statement can be replicated verbatim. | ||
|
|
||
| Possible values: string. |
There was a problem hiding this comment.
Keep format with values on separate string, as is all other places.
| bool is_attach, | ||
| LoggerPtr log); | ||
| LoggerPtr log, | ||
| const String & zookeeper_name); |
There was a problem hiding this comment.
sookeeper_name goes in par with zookeeper_path, better to place both in parameters one by one in same order, as in ObjectStorageQueueMetadata constructor. Now sookeeper_name in different methods is added in random places throughout the code.
| { | ||
| std::string makeMetadataKey(const std::string & zookeeper_name, const std::string & zookeeper_path) | ||
| { | ||
| if (zookeeper_name.empty() || zookeeper_name == zkutil::DEFAULT_ZOOKEEPER_NAME) |
There was a problem hiding this comment.
Is empty zookeeper_name possible? As I see it is initialized with DEFAULT_ZOOKEEPER_NAME if is missing in path, so empty name may be a sign of logical error somewhere.
There was a problem hiding this comment.
It is indeed initialized with const so removing empty()
| { | ||
| const auto & name = metadata.metadata->getZooKeeperName(); | ||
| const auto & path = metadata.metadata->getPath(); | ||
| result.emplace(makeMetadataKey(name, path), metadata.metadata); |
There was a problem hiding this comment.
key created exactly as makeMetadataKey(zookeeper_name, zookeeper_path), why need to create the same string again for result?
There was a problem hiding this comment.
OK more simple, result.emplace(key, metadata.metadata). Those fields are not mutable correct.
| const std::string & processing_id) | ||
| { | ||
| auto zk_client = ObjectStorageQueueMetadata::getZooKeeper(log); | ||
| auto zk_client = ObjectStorageQueueMetadata::getZooKeeper(log, zookeeper_name); |
| setVirtuals(VirtualColumnUtils::getVirtualsForFileLikeStorage(storage_metadata.columns, context_)); | ||
| setInMemoryMetadata(storage_metadata); | ||
|
|
||
| String resolved_zookeeper_name; |
There was a problem hiding this comment.
Why need temporary resolved_zookeeper_name? You can just send ref on zookeeper_name in chooseZooKeeperPath.
| if (has_keeper_prefix || starts_with_slash) | ||
| result_zk_path = configured_path; | ||
| else | ||
| result_zk_path = appendWithPrefix(configured_path); |
There was a problem hiding this comment.
You use single line lambda appendWithPrefix only once here. Better to just concatenate strings right here.
| database_uuid = DatabaseCatalog::instance().getDatabase(table_id.database_name)->getUUID(); | ||
|
|
||
| result_zk_path = fs::path(zk_path_prefix) / toString(database_uuid) / toString(table_id.uuid); | ||
| result_zk_path = (fs::path(zk_path_prefix) / toString(database_uuid) / toString(table_id.uuid)).string(); |
There was a problem hiding this comment.
fs::path must be converted to sting automatically, calling string() is unnecessary
| }; | ||
|
|
||
| std::string result_zk_path; | ||
| String resolved_zookeeper_name(zkutil::DEFAULT_ZOOKEEPER_NAME.data(), zkutil::DEFAULT_ZOOKEEPER_NAME.size()); |
There was a problem hiding this comment.
This value is not used, just overwrited later with result of zkutil::extractZooKeeperName.
|
|
||
| resolved_zookeeper_name = zkutil::extractZooKeeperName(result_zk_path); | ||
| if (zookeeper_name_out) | ||
| *zookeeper_name_out = resolved_zookeeper_name; |
There was a problem hiding this comment.
Just
if (zookeeper_name_out)
*zookeeper_name_out = zkutil::extractZooKeeperName(result_zk_path);
without resolved_zookeeper_name
Docs: move aggregate functions docs to source (#2)
…and more predictable timings, #2
S3Queue auxiliary Zookeeper support using keeper_path setting from s3Queue
This will allow to do workload separation for heavy zk usage modes like unordered, by creating a zk/keeper specifically for some S3Queue tables and creating an S3queue specifying
keeper_pathwith an auxiliary zookeeper like in a ReplicatedMergeTree table:'auxiliary_zookeeper:/clickhouse/s3queue/my_s3queue_table'