Skip to content
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.

Sign up for GitHub

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

Jump to bottom

clustergen: add all-clusters cluster #1409

Merged
merged 1 commit into from
May 7, 2024
Merged

clustergen: add all-clusters cluster #1409

merged 1 commit into from
May 7, 2024

Conversation

tanner-bruce
Copy link
Contributor

Adds an all-shards cluster, which contains the shard definitions for each cluster specified in the ClickhouseInstallation. This is used by our query nodes to allow scattering reads across all clusters in the CHI while respecting the shard/replica.

Please check items PR complies to:

  • All commits in the PR are squashed. More info
  • The PR is made into dedicated next-release branch, not into master branch1. More info
  • The PR is signed. More info

fixes #1291

@Slach Slach changed the base branch from 0.23.5 to 0.24.0 April 30, 2024 16:15
@Slach Slach requested review from sunsingerus and Slach April 30, 2024 16:15
Slach
Slach requested changes May 1, 2024
View reviewed changes
Copy link
Collaborator

@Slach Slach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are ignored our recommendations about non-mental name all-shards
#1291 (comment)
could you please use another name like all-clusters?

@Slach
Copy link
Collaborator

Slach commented May 1, 2024

Or even all-clusters-shards

@tanner-bruce
Copy link
Contributor Author

You are ignored our recommendations about non-mental name all-shards #1291 (comment) could you please use another name like all-clusters?

Let me check what happened, I pushed the wrong code. We have it as chi-<cluster>

@tanner-bruce tanner-bruce force-pushed the add-all-shards-cluster branch from 6839427 to 8694ccc Compare May 2, 2024 13:36
@tanner-bruce
Copy link
Contributor Author

You are ignored our recommendations about non-mental name all-shards #1291 (comment) could you please use another name like all-clusters?

Fixed now. I've pushed the proper code, which changed the name to what you suggested in #1291 - chi-<cluster name>. Please let me know if you'd prefer something else. Or maybe we could make it configurable?

@tanner-bruce tanner-bruce requested a review from Slach May 2, 2024 13:38
@Slach Slach changed the title clustergen: add all-shards cluster clustergen: add all-clusters cluster May 6, 2024
Slach
Slach requested changes May 6, 2024
View reviewed changes
pkg/model/chi/ch_config_generator.go Outdated Show resolved Hide resolved
@tanner-bruce tanner-bruce force-pushed the add-all-shards-cluster branch from 8694ccc to b069085 Compare May 6, 2024 14:57
@Slach Slach merged commit eba971b into Altinity:0.24.0 May 7, 2024
Slach added a commit that referenced this pull request May 7, 2024
@Slach
add ability running tests on every commit into every pull request or …
8b991fe 
…run manually, after 34cc6c2 and #1409

Signed-off-by: Slach <bloodjazman@gmail.com>
@Slach Slach mentioned this pull request May 7, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Slach Slach Slach approved these changes

@sunsingerus sunsingerus Awaiting requested review from sunsingerus

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tanner-bruce @Slach