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

Upgrade to Cassandra driver 4 #18621

Closed
mp911de opened this issue Oct 16, 2019 · 20 comments
Closed

Upgrade to Cassandra driver 4 #18621

mp911de opened this issue Oct 16, 2019 · 20 comments
Labels
type: enhancement A general enhancement
Milestone

Comments

@mp911de
Copy link
Member

mp911de commented Oct 16, 2019

The Cassandra 4 driver comes with several changes (in comparison to the 3.x driver). An upgrade requires some coordination as the 4.0 driver comes with:

  • An entirely new package structure (Old: com.datastax.driver, new: com.datastax.oss.driver.api) and new maven coordinates (com.datastax.cassandra:cassandra-driver-core vs. new com.datastax.oss:java-driver-core)
  • Changed concepts (new driver drops the intermediate Cluster concept in favor of a single CqlSession entry point)
  • Several changes to configuration properties

Special attention deserves DriverConfigLoader that is a configuration utility to load driver config from a .conf/.json/.properties file and the fact that although Guava is no longer required as a dependency, the driver still uses a shaded variant of Guava.

It looks as if both drivers could be used side by side until Spring Data for Apache Cassandra and Zipkin are able to catch up.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 16, 2019
@mp911de
Copy link
Member Author

mp911de commented Oct 16, 2019

Spring Data Cassandra issue to upgrade the driver: DATACASS-656.

@lankydan

This comment has been minimized.

@mp911de

This comment has been minimized.

@lankydan

This comment has been minimized.

@lankydan

This comment has been minimized.

@wilkinsona wilkinsona added type: enhancement A general enhancement status: on-hold We can't start working on this issue yet and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 20, 2019
@wilkinsona
Copy link
Member

I'm not sure that it's worth us offering auto-configuration for the new driver before Spring Data for Apache Cassandra has caught up, particularly with all the other things going on at the moment. I've placed this on hold until the update in Spring Data has been scheduled.

@mp911de
Copy link
Member Author

mp911de commented Dec 5, 2019

I'm in the middle of migrating Spring Data to the 4.x driver. I noticed that running both drivers, 3.x and 4.x side-by-side does not work because of netty incompatibilities. Timing-wise it would make sense to synchronize Driver and Spring Data upgrades.

@mp911de
Copy link
Member Author

mp911de commented Dec 12, 2019

I summarized the changes for the Cassandra migration (from a Spring Data perspective) at spring-projects/spring-data-cassandra#167. Snapshot builds are available for the 3.0.0.BUILD-SNAPSHOT version.

@adutra
Copy link
Contributor

adutra commented Dec 12, 2019

Hi, DataStax has been working lately on a Spring Boot module for Apache Cassandra and DSE.

It uses the DataStax driver 4.x and offers out-of-the-box driver configuration and injection of session beans. Driver configuration is created by merging info from other Spring beans, Spring configuration sources and driver configuration files. Spring profiles are also honored. It also offers health indicators and metrics (driver metrics are translated into Micrometer metrics).

We would like to donate this code to you. Could you please tell us what is the best way to proceed? Feel free to ping me privately if necessary: alexandre [dot] dutra [at] datastax [dot] com.

@philwebb
Copy link
Member

@adutra Is the module currently open source? Can we take a quick look at the code to see what's involved?

@philwebb philwebb added this to the 2.3.x milestone Dec 13, 2019
@adutra
Copy link
Contributor

adutra commented Dec 17, 2019

@philwebb it's not, it's still in beta preview. I will grant you access to the repo. Anyone else that you would like to be able to browse that repo as well?

@philwebb
Copy link
Member

Thanks @adutra. Could you please give access to @wilkinsona and @snicoll as well.

@adutra
Copy link
Contributor

adutra commented Dec 18, 2019

Absolutely! Please feel free to leave comments and/or open issues.

@mp911de
Copy link
Member Author

mp911de commented Jan 8, 2020

Spring Data for Apache Cassandra is now available in the Neumann release train. The Spring Data BOM points to version 3.0.0.BUILD-SNAPSHOT.

@philwebb philwebb removed the status: on-hold We can't start working on this issue yet label Jan 9, 2020
@snicoll snicoll self-assigned this Jan 11, 2020
@snicoll snicoll modified the milestones: 2.3.x, 2.3.0.M1 Jan 11, 2020
@snicoll
Copy link
Member

snicoll commented Jan 11, 2020

The upgrade is taken care of as part of #19588. There are a number of things we can do to ease the upgrade from a user's perspective.

  • The port is gone in favour of host:port contact points (vs. host before). If the contact points are still defined without the port, we could build the new format using the port. The property should be deprecated but I am not sure yet if we want to keep that in 2.3 proper
  • cluster-name should be renamed to session-name
  • The jmx flag is now useless as that support has moved outside of the driver (we introduced this flag to only enable JMX when appropriate). We should deprecate the property with ERROR level
  • fetch-size renamed to page-size. There is also a case for a request sub-namespace
  • I am not sure what connectTimeout is now. We found CONNECTION_INIT_QUERY_TIMEOUT but I don't think this is equivalent
  • readTimeout was mapped to request-timeout. Again, not sure this is accurate
  • I didn't find a replacement for poolTimeout
  • The maxQueueSize could be mapped to REQUEST_THROTTLER_MAX_QUEUE_SIZE. There is a case there to improve the support in future milestones with additional properties for a request.throttler sub-namespace

@adutra the project you've shared with us was helpful to better understand some of the changes in v4, thank you! Can you please look at the above? Our plan for M1 is to map the existing keys that we current have with v3, deprecating those who are not reflected in the new driver anymore. Future milestone will improve the configuration by offering additional mappings (I've added some suggestions but I am sure we'll have more).

@mp911de thank you so much for the assistance. Upgrading Spring Data was relatively straightforward. I haven't managed to fix the integration tests yet as I don't understand how I should/can create the keyspace. I'll resume work on this next week to make sure the integration tests are green too.

@mp911de
Copy link
Member Author

mp911de commented Jan 11, 2020

This snipped translates to what was previously done with Cluster.builder():

private void createTestKeyspaceIfNotExists() {
	try (CqlSession session = CqlSession.builder()
			.addContactPoint(new InetSocketAddress(cassandra.getContainerIpAddress(), cassandra.getFirstMappedPort()))
			.withLocalDatacenter("datacenter1")
			.build()) {

		session.execute("CREATE KEYSPACE IF NOT EXISTS boot_test"
				+ "  WITH REPLICATION = { 'class' : 'SimpleStrategy', 'replication_factor' : 1 };");
	}
}

I'm not sure why withLocalDatacenter is required and cannot be inferred, especially in single-node arrangements. I also noticed that contact point configuration via DefaultDriverOption.CONTACT_POINTS tried to use localhost:9042 (in CassandraAutoConfiguration).

@snicoll
Copy link
Member

snicoll commented Jan 13, 2020

This is now superseded by #19588 and a number of issues that I've just created (see above). We'll continue improving Cassandra v4 support in future 2.3 milestones and welcome feedback.

@snicoll snicoll closed this as completed Jan 13, 2020
@snicoll snicoll added the status: superseded An issue that has been superseded by another label Jan 13, 2020
@snicoll snicoll removed their assignment Jan 13, 2020
@snicoll snicoll removed this from the 2.3.0.M1 milestone Jan 13, 2020
@wilkinsona
Copy link
Member

I think we should keep this in 2.3.0.M1 so that it appears in the release notes.

@wilkinsona wilkinsona added this to the 2.3.0.M1 milestone Jan 13, 2020
@wilkinsona wilkinsona removed the status: superseded An issue that has been superseded by another label Jan 13, 2020
@adutra
Copy link
Contributor

adutra commented Jan 17, 2020

@snicoll I replied to most of your bullet points above under the respective issues that you created.

If the contact points are still defined without the port, we could build the new format using the port

Beware that the driver uses a colon to separate the address from the port, as in adress:port; if the port is made optional, this could lead to ambiguities when using IPv6 addresses in compact format, that is, a contact point ending with :1234 might be interpreted as address+port or just an address without port.

@snicoll
Copy link
Member

snicoll commented Jan 20, 2020

@adutra thanks, I thought of that one and was considering doing the replacement only for basic cases (i.e. no presence of : at all). I know this wouldn't cover all advanced use cases but this could be good enough for a start.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

7 participants