-
Notifications
You must be signed in to change notification settings - Fork 7k
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
Add clusters with replicas from all replica groups #64312
Conversation
This is an automated comment for commit 553fcb5 with description of existing statuses. It's updated for the latest CI running ❌ Click here to open a full report in a separate page
Successful checks
|
ed8434c
to
5944081
Compare
5944081
to
73f42b0
Compare
{ | ||
std::lock_guard lock{mutex}; | ||
cluster = std::move(new_cluster); | ||
if (replica_group_name.empty()) | ||
return nullptr; |
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.
should it just return regular getClusterImpl when we accidentally call tryGetAllGroups on a clickhouse instance without replicaGroup?
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 wanted to avoid creating all_groups clusters (and polluting system.clusters) on instances with no replica group. It's hard to call it accidentally, although it's possible to specify smth like all_groups.default
as a cluster name in some query on an instance without replica groups. The current implementation throws "unknow cluster" in this case, but I can change it and return regular cluster
fb1d332
to
6725168
Compare
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.
Maybe add a small test?
ed96236
to
22b8108
Compare
22b8108
to
ef3b802
Compare
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
If "replica group" is configured for a
Replicated
database, automatically create a cluster that includes replicas from all groups.Closes #62725
Documentation entry for user-facing changes
CI Settings
NOTE: If your merge the PR with modified CI you MUST KNOW what you are doing
NOTE: Checked options will be applied if set before CI RunConfig/PrepareRunConfig step
Run these jobs only (required builds will be added automatically):
Deny these jobs:
Extra options:
Only specified batches in multi-batch jobs: