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

Restore compatibility with Cassandra 1.2 & 2.0 #631

Conversation

rborer
Copy link
Contributor

@rborer rborer commented Mar 19, 2019

Fixes #629.

As ColumnFamilyStoreMBean.getCompactionParameters() is only available since Cassandra 2.1, this PR restore compatibility with older versions which doesn't expose this method.

On such versions, the default compaction strategy is used (TimeWindowCompactionStrategy & DateTieredCompactionStrategy strategies are not available for those versions).

This PR also configures Travis CI to run integration tests with Cassandra versions 1.2.19 & 2.0.17 (hopefully since I have no clue how to test this).

I've manually deployed this version on my cluster running Cassandra 1.2.19 and so far it's running repairs properly.

Reynald Borer added 2 commits March 19, 2019 11:32
…nce Cassandra 2.1

Restore compatibility with versions prior to 2.1 which doesn't expose this method. On such versions, the default compaction strategy is used (TimeWindowCompactionStrategy & DateTieredCompactionStrategy strategies are not available for those versions).
@rborer rborer force-pushed the cassandra-1.2-no-compaction-parameters branch from 63a5b15 to 298cbd0 Compare March 19, 2019 10:32
* Default compaction strategy, used when the strategy cannot be retrieved from JMX. The
* information is only available since Cassandra 2.1
*/
private static final String DEFAULT_COMPACTION_STRATEGY = "SizeTieredCompactionStrategy";
Copy link
Member

Choose a reason for hiding this comment

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

@rborer would/does the code break if we make the default value "unknown"? i'm hesitant about presuming a compaction strategy, as we may take an action based on that and it would not be accurate against Cassandra <2.1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I've been wondering about putting a default value that doesn't exists here. As far as I can tell the compaction strategy information is only used to skip tables using Time Window / Date Tiered data. So it should be safe to put "unknown" in there instead.

@michaelsembwever
Copy link
Member

michaelsembwever commented Mar 19, 2019

This looks good to me. Only the one question about what the filler value for Cassandra 1.2 and 2.0 should be.

The integration tests appear to be failing. See https://api.travis-ci.org/v3/job/508315673/log.txt The POST /repair_schedule? returns an unexpected 400
If this is not an actual breakage for this PR we can leave 298cbd0 out initially, and the breakage dealt with separately.

@rborer
Copy link
Contributor Author

rborer commented Mar 19, 2019

@michaelsembwever as far as I understand Travis CI output, the integration tests are failing with versions 1.2 & 2.0. The 400 you are seeing is when it attempts to start an incremental repair which is something introduced in version 2.1 (based on https://www.datastax.com/dev/blog/more-efficient-repairs) as far as I understand it?

…etrieved since Cassandra 2.1) more explicit.
@rborer rborer force-pushed the cassandra-1.2-no-compaction-parameters branch from 298cbd0 to 711f941 Compare March 19, 2019 13:07
michaelsembwever pushed a commit that referenced this pull request Mar 20, 2019
…nce Cassandra 2.1

 - Restore compatibility with versions prior to 2.1 which doesn't expose this method.
 - Make unknown compaction strategy (an info that can only be remotely retrieved since Cassandra 2.1) more explicit.

ref: #631
michaelsembwever pushed a commit that referenced this pull request Mar 21, 2019
…nce Cassandra 2.1

 - Restore compatibility with versions prior to 2.1 which doesn't expose this method.
 - Make unknown compaction strategy (an info that can only be remotely retrieved since Cassandra 2.1) more explicit.

ref: #631
@michaelsembwever
Copy link
Member

Merged to both 1.4 and master with commit b075113

@michaelsembwever
Copy link
Member

Thanks again @rborer :-)

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.

2 participants