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.

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

Renamed cassandra.yaml, read property list more robust #24

Closed
wants to merge 1 commit into from
Closed

Renamed cassandra.yaml, read property list more robust #24

wants to merge 1 commit into from

Conversation

sorenvind
Copy link

Read property strings more robust (strip spaces), rename cassandra.yaml to remove clash with default configuration file (cherry picked from commit dba83d9)

…ml to remove clash with default configuration file (cherry picked from commit dba83d9)
@sorenvind
Copy link
Author

I don't see why SSTableLoaderWrapper is looking for the default included cassandra.yaml instead of the file actually configured to be the configuration file. You seem to have the IConfiguration available there, so why not use it (as in TuneCassandra.java)? Replace

URL url = this.getClass().getClassLoader().getResource("cassandra.yaml");
logger.info("Trying to load the yaml file from: " + url);
TuneCassandra.updateYaml(config, url.getPath(), "localhost", "org.apache.cassandra.locator.SimpleSeedProvider");

with (using the ConfigDir property from the other pull request)

TuneCassandra.updateYaml(config, config.getCassConfigDir() + "/cassandra.yaml", "localhost", "org.apache.cassandra.locator.SimpleSeedProvider");

In this way the configuration is at least consistent. Do let me know if there's something I'm missing.

@Vijay2win
Copy link
Contributor

It is SSTableLoaderWrapper.java i am talking about Cassandra source code SSTableLoaderWrapper will start Cassandra by starting the MessagingService.

Cassandra.yaml in this directory is completely diffrent than the real cassandra.yaml file which will be in the config directory. We should not be messing with it for incremental restore.

@sorenvind
Copy link
Author

I don't really follow how SSTableLoaderWrapper starts the MessagingService (it's not clear for me how this happens), but fair enough. I understand the need for having a different cassandra.yaml for the incremental restore.

Another possible approach that gets this problem out of the way: Use a config file with a different name (e.g. cassandra_priam_incremental_restore.yaml) for incremental restore, which could be included in Priam safely. EmbeddedServerHelper from Cassandra-Unit shows how this could be done. Since DatabaseDescriptor looks for the cassandra.config property this approach would also work in this case.

@sorenvind
Copy link
Author

Looking at the cassandra properties being edited in EmbeddedServerHelper, I just noticed that they are not reversed to their original value after starting Cassandra. This should obviously be done after the incremental restore completes.

@sorenvind
Copy link
Author

FYI: It seems that aryanet ran into exactly the same issue with the clashing cassandra.yaml files on the CP.

@sagarl
Copy link
Contributor

sagarl commented Nov 13, 2012

Integrated code changes (commit 2048eb7) related to reading property string by stripping spaces.

Not sure if cassandra.yaml clashing issue still exist, if yes, please let us know.

@sagarl sagarl closed this Nov 13, 2012
schakrovorthy added a commit that referenced this pull request May 18, 2020
…rthy-3.11

Feature/cass 1799 schakrovorthy 3.11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants