Skip to content

Fix multi dc provisioning #653

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
Open
dkropachev wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Fix multi dc provisioning #653

dkropachev wants to merge 7 commits into from

Conversation

@dkropachev
Copy link
Contributor

@dkropachev dkropachev commented Apr 21, 2025 •
edited
Loading

Multi DC/rack provisioning is broken in multiple cases:

  1. Rack for node is not stored
  2. Manual provisioning is broken (create, add, add, ..., start)
  3. Start, add, node start is broken
    Fixes: Adding node without dc leads to broken cassandra-rackdc.properties #650

Changes:

  1. is_single_dc_cluster flag is introduced to track cluster snitch
  2. Rack now is stored in node.conf
  3. DEFAULT_DC and DEFAULT_RACK is introduced
  4. Naming for datacenters is changed to datacenter{id} to match default dc naming of scylla and cassandra.

@dkropachev dkropachev force-pushed the dk/fix-multidc-provisioning branch from f41aca7 to 9d937dc Compare April 21, 2025 03:24
@dkropachev dkropachev self-assigned this Apr 21, 2025
@dkropachev dkropachev requested a review from fruch April 21, 2025 03:25
@dkropachev
Copy link
Contributor Author

dkropachev commented Apr 21, 2025

@fruch , there are two debatable changes:

  1. Changing default dc pattern to datacenter{id}. It can lead to problems in test because dc is going to change, but makes dc naming consistent.
  2. Adding is_single_dc_cluster flag to cluster. It is needed for cluster to know snitch for nodes they were provisioned with, in multi dc cluster it is required to use GossipingPropertyFileSnitch, while in single dc SimpleSnitch is used. All nodes in cluster should have same snitch, otherwise node joining to the cluster will fail. Another way to solve it by defaulting to GossipingPropertyFileSnitch.

@dkropachev dkropachev force-pushed the dk/fix-multidc-provisioning branch 2 times, most recently from 8cfdb90 to 9dbf6d7 Compare April 21, 2025 04:53
fruch
fruch reviewed Apr 21, 2025
View reviewed changes
ccmlib/common.py Outdated
DOWNLOAD_IN_PROGRESS_FILE = "download_in_progress"

DEFAULT_DATACENTER = 'datacenter1'
DEFAULT_RACK = 'rack1'
Copy link
Contributor

@fruch fruch Apr 21, 2025

Choose a reason for hiding this comment

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

this is breaking checks for dtest....

you would need to run all test, and prove there's no dependency

@fruch
Copy link
Contributor

fruch commented Apr 21, 2025

@fruch , there are two debatable changes:

  1. Changing default dc pattern to datacenter{id}. It can lead to problems in test because dc is going to change, but makes dc naming consistent.

this is a breaking change... to do it, one would need to prove it's not break any user of CCM (i.e. dtest, all drivers)

  1. Adding is_single_dc_cluster flag to cluster. It is needed for cluster to know snitch for nodes they were provisioned with, in multi dc cluster it is required to use GossipingPropertyFileSnitch, while in single dc SimpleSnitch is used. All nodes in cluster should have same snitch, otherwise node joining to the cluster will fail. Another way to solve it by defaulting to GossipingPropertyFileSnitch.

I don't mind changing the default to GossipingPropertyFileSnitch, but it should be synced with Core, i.e. they might want to make it default as well ?

@dkropachev dkropachev force-pushed the dk/fix-multidc-provisioning branch 2 times, most recently from fc480c1 to 64bca9f Compare April 21, 2025 19:07
@dkropachev
Ignore test temp directory
f41e04a
@dkropachev
Store node rack in node.conf
c3f2979
@dkropachev
Do not read host-id on watch_log_for_alive
b6dbe75 
host-id requires API to be up and running, which is not always the case
right away.
Ignoring it there will help to reduce chance of having host-id reading
error while node is starting
@dkropachev
Enhance cluster test API
15dd079 
Add API to get `CCMCluster`
Add API for populate command
Add options to add API
Add API to read cluster.conf
Add API to read node `cassandra-rackdc.properties`
@dkropachev
Fix dc provisioning
c725db7 
Clearly define DEFAULT_DC and DEFAULT_RACK and fix them to match default
scylla values.
Fix dc and rack naming accordingly.
Store single dc flag in cluster.conf and use it to prevent provisioning
a node with different snitch.
@dkropachev
Start old cassandra and scylla nodes one by one
c0a9457 
Cassandra 3.0 and Scylla older than 6.x does not support parallel node initialization.
Tune CCM to start nodes one by one for it.
@dkropachev dkropachev force-pushed the dk/fix-multidc-provisioning branch from 64bca9f to 3bf76a7 Compare April 21, 2025 21:52
@dkropachev dkropachev force-pushed the dk/fix-multidc-provisioning branch from 3bf76a7 to 7ba8088 Compare April 21, 2025 22:16
@fruch
Copy link
Contributor

fruch commented Oct 19, 2025

@dkropachev is this one still relevant or needed ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fruch fruch fruch left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@dkropachev dkropachev

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Adding node without dc leads to broken cassandra-rackdc.properties

2 participants

@dkropachev @fruch