Skip to content

Conversation

@jkni
Copy link

@jkni jkni commented Apr 7, 2025

What is the issue

#1563 upgraded our version of snakeyaml. This new version logs when duplicate keys are found. The test-cdc target concatenates a CDC-specific yaml to the base yaml. Both contain entries for the cdc_enabled key, causing tests that load this yaml to log the warning. This causes CDC CI failures.

What does this PR fix and why was it fixed

This PR removes the cdc_enabled entry in the base yaml, as it's the same as the default. This is consistent with how we handle other default keys that might be duplicated across multiple yaml fragments in the test commands.

@jkni jkni self-assigned this Apr 7, 2025
@jkni jkni added the core-team label Apr 7, 2025
@github-actions
Copy link

github-actions bot commented Apr 7, 2025

Checklist before you submit for review

  • Make sure there is a PR in the CNDB project updating the Converged Cassandra version
  • Use NoSpamLogger for log lines that may appear frequently in the logs
  • Verify test results on Butler
  • Test coverage for new/modified code is > 80%
  • Proper code formatting
  • Proper title for each commit staring with the project-issue number, like CNDB-1234
  • Each commit has a meaningful description
  • Each commit is not very long and contains related changes
  • Renames, moves and reformatting are in distinct commits
  • All new files should contain the DataStax copyright header instead of the Apache License one

@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 8, 2025

@cassci-bot
Copy link

✔️ Build ds-cassandra-pr-gate/PR-1680 approved by Butler


Approved by Butler
See build details here

@ekaterinadimitrova2 ekaterinadimitrova2 self-requested a review April 8, 2025 01:27
Copy link

@ekaterinadimitrova2 ekaterinadimitrova2 left a comment

Choose a reason for hiding this comment

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

+1
commitlog_compression also exists in the base yaml, but it is commented out so it does not produce any warnings.

This new version logs when duplicate keys are found.

It was high time, this was a reason for confusion and some bugs. Half people actively use it, half people have no clue overriding exists. That is how CASSANDRA-17379 was born. From Apache Cassandra 4.1+:
[CASSANDRA-17379](https://issues.apache.org/jira/browse/CASSANDRA-17379) was opened to improve the user experience and deprecate the overloading. By default, we refuse starting Cassandra with a config containing both old and new config keys for the same parameter. Start Cassandra with -Dcassandra.allow_new_old_config_keys=true to override. For historical reasons duplicate config keys in cassandra.yaml are allowed by default, start Cassandra with -Dcassandra.allow_duplicate_config_keys=false to disallow this. Please note that key_cache_save_period, row_cache_save_period, counter_cache_save_period will be affected only by -Dcassandra.allow_duplicate_config_keys.

@jkni
Copy link
Author

jkni commented Apr 8, 2025

Thanks for the review! I briefly considered disabling these warnings to revert to the old behavior, but I think they're a positive change.

@jkni jkni merged commit 18fe538 into main Apr 8, 2025
476 of 480 checks passed
@jkni jkni deleted the CNDB-13624 branch April 8, 2025 17:02
djatnieks pushed a commit that referenced this pull request Apr 14, 2025
…ate cdc_enabled key in base yaml (#1680)

### What is the issue
#1563 upgraded our version of snakeyaml. This new version logs when
duplicate keys are found. The test-cdc target concatenates a
CDC-specific yaml to the base yaml. Both contain entries for the
cdc_enabled key, causing tests that load this yaml to log the warning.
This causes CDC CI failures.

### What does this PR fix and why was it fixed
This PR removes the cdc_enabled entry in the base yaml, as it's the same
as the default. This is consistent with how we handle other default keys
that might be duplicated across multiple yaml fragments in the test
commands.
djatnieks pushed a commit that referenced this pull request May 18, 2025
…ate cdc_enabled key in base yaml (#1680)

### What is the issue
#1563 upgraded our version of snakeyaml. This new version logs when
duplicate keys are found. The test-cdc target concatenates a
CDC-specific yaml to the base yaml. Both contain entries for the
cdc_enabled key, causing tests that load this yaml to log the warning.
This causes CDC CI failures.

### What does this PR fix and why was it fixed
This PR removes the cdc_enabled entry in the base yaml, as it's the same
as the default. This is consistent with how we handle other default keys
that might be duplicated across multiple yaml fragments in the test
commands.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants