New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support multiple ZooKeeper clusters #17070
Conversation
@@ -212,6 +216,22 @@ StorageReplicatedMergeTree::StorageReplicatedMergeTree( | |||
, allow_renaming(allow_renaming_) | |||
, replicated_fetches_pool_size(global_context.getSettingsRef().background_fetches_pool_size) | |||
{ | |||
if (zookeeper_path_.empty()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like copy-paste. We can have a common function instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. I fix it soon. Thanks.
Looking good to me. Let's wait for you to make CI checks happy and merge |
@fastio FYI, failed "Stress test (memory)" is irrelevant to this PR and is a known issue with RocksDB lib in our contrib. Received signal Aborted (6)
|
Thanks for the reminder. |
Ping @Akazz :) |
@Akazz Please merge this pr to master. I am worried that it will be conflicted with master branch again. Thanks :) |
I will take this PR for review. |
But Akazz already approved :) |
ReplicatedMergeTree engine as follow: | ||
|
||
``` | ||
CREATE TABLE table_name ( ... ) ENGINE = ReplicatedMergeTree('zookeeper_name_configurated_in_auxiliary_zookeepers:path', 'replica_name') ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
configurated -> configured
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in master.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also reviewed and it LGTM.
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Now it's possible to make different ReplicatedMergeTree engine with different ZooKeeper clusters.
The auxiliary zookeepers configuration (#14155 ) is reused to support this feature.
We can create table like follow:
CREATE TABLE t1 (a String) ENGINE= ReplicatedMergeTree('test:/tables/t1','{replica}') ORDER BY a;
It means that the metadata of the table named t1 is saved in zookeeper cluster named test, which configurated in
auxiliary_zookeepers.
CREATE TABLE t2 (a String) ENGINE= ReplicatedMergeTree('/tables/t2','{replica}') ORDER BY a;
It means that the metadata of the table named t2 is saved in the default zookeeper cluster, which configurated in
zookeeper.
<auxiliary_zookeepers>
172.19.0.11
2181
</auxiliary_zookeepers>
In fact, the implementation compatibles with the old configuration.
Detailed description / Documentation draft: