vert-x3/vertx-cassandra-client

Update to use version 4.x of the datastax oss driver

Closed this issue · 14 comments

mahinshaw commented

Re the comments in the dev mailing list:

  • "the latest driver is compatible is still compatible with a good range of Cassandra versions"

    The runtime requirements note Apache Cassandra®: 2.1 and above;

  • the changes don't impact change the Vert.x API too much:

    In a cursory update of dependencies, I've found a couple areas that will lead to breaking changes.

    1. ResultSet - 4.x splits this interface into a ResultSet and a AsyncResultSet. Both are backed by Paging interfaces PagingIterable and AsyncPagingIterable respectively. Looking at the usages within the vertx ResultSet, there are two methods that no longer exist within the new ResultSet - isExhausted and fetchMoreResults. The former seems to be communicable via a call to one where the result is null. The latter does not appear to have an analogue. Also, as the vertx client uses executeAsync (and other async based calls), an AsyncResultSet will be returned. This api deals in pages more directly. So the vertx ResultSet would need to bridge the concepts (which appears possible from the outside) or conform. This is likely the most significant of changes with respect to the current api. One win here however is the use of a CompletionState<> for responses to a query execution. The benefit here is no longer being tied to ListenableFuture in guava.

    2. There is no longer a Cluster class. Construction of a connection happens with the new CqlSession. This shouldn't affect the user.

    3. CassandraClientOptions#getContactPoints() no longer has a way to hook into the CqlSessionBuilder (follow through to the SessionBuilder abstract class). This appears to be most useful with the copy constructor. In that case the CqlSession has a wrap(CqlSession defaultSession) call which could be used as a replacement.

    4. The CqlSessionBuilder uses java.net.InetSocketAddress for contact points. So no more host strings with a separate port. This should be minimally invasive. As a note as well, it seems there is a preference on the driver end to use a static file for configuration rather than the builder (Typsafe config). This is an option though.

For reference, see the upgrade guide. I have worked towards a simple update locally, and will create a branch on a fork of this repo. When I get that done, I will post here. I'm going to do the minimal changes to conform to the existing api. The intent is to use it as a talking point rather than a "best" implementation.

tsegismont commented

Thanks @mahinshaw ! Yes please go ahead and share your findings.

vietj commented

please study how the transitive dependency changes works with the whole stack

mahinshaw commented

One thing to note - The mapper has changed quite a bit. See here. I don't use the mapper, so I am no expert on this. The major points I'm seeing is that the mapper is now code generation driven, thus providing mapped object builders at compile time. So operations live on DAO objects (Save, Delete, etc) which are also generated. This means that the existing io.vertx.cassandra.Mapper interface no longer serves the intended purpose in the context of the 4.1+ driver. (the new mapper was added in 4.1).

Due to the mapper issues, I'm not able to get tests to run but I have a branch on my fork. The changes on the main apis were simple enough (mostly types). It's also worth reiterating this is intended as a POC.

@vietj I haven't had a chance yet to dig into the transitive dependency changes. I have some pending work deadlines, so my input here might be slow. But I will dig in.

I was able to change the handleOnContext to use CompletionStage, which, in my mind, was a big win with respect to dependencies, as there is no longer a direct need for guava. But the api changes, especially for the mapper may be a difficult pill to swallow for users. There is a potential to extend the ResultSet to have conforming methods to the AsyncResultSet interface in 4.x and in the future Deprecate the 3.x specific methods. Is there a space where there are two versions, like RxJava? I don't particularly find that appealing, but I thought it would be worth asking.

tsegismont commented

@mahinshaw I would prefer to avoid two concurrent versions of the Cassandra client. As for the timeframe, if we need to break a few things like the mapper support, I believe having this in Vert.x 4.0 is fine.

Regarding mapper support in particular, I believe we could drop specific code from the Cassandra client. Users would create their annotated code and let the Datastax processor generate the implementations. As long as they define Daos that return CompletionStage, the invocation will be non blocking. Then when the dao is created, they have to provide a Session. But since Session is now an interface, the Vert.x Cassandra client could provide one that makes sure any CompletionStage returned operates on the Vert.x context of the caller.
Could you explore this?

tsegismont commented

@Sammers21 What's your opinion about this thread?

mahinshaw commented

@tsegismont That all sounds good. I will explore integrating the new data mapper.

Sammers21 commented

@tsegismont , i would rather prefer this:

I would prefer to avoid two concurrent versions of the Cassandra client. As for the timeframe, if we need to break a few things like the mapper support, I believe having this in Vert.x 4.0 is fine.

mahinshaw commented

The DAO annotations in the new Data Mapper seem to do the heavy lifting (ie, execute statements). Each DAO has the option for providing return types (see the @Select here). This seems to be where the work will need to happen. Extending the annotations to provide types and generated code that supports a Vert.x workflow would likely be the most successful way to provide a proper data mapper.

tsegismont commented
mahinshaw commented

@tsegismont I don't think that it is pluggable. I prefer not to have to write or maintain that code either. However, I'm not sure how one would map calls from a DAO to execute on the context, without manual intervention.

tsegismont commented

@mahinshaw the Session in driver v4 is now an interface. We could provide an implementation that makes sure that, if the returned object is a CompletionStage, the callback is executed on the caller's Vert.x context.

mahinshaw commented

@tsegismont That makes sense. Awesome! I’ll start digging in to that. I’m fairly busy this week, but as time allows I’ll try and get some work done on my branch.

Sammers21 commented

@tsegismont, looks like an extension of the mapper code-generator is not a trivial task. Can we migrate to datastax driver 4.2 without support for mappers?

tsegismont commented

@Sammers21 see #42 (comment) and https://docs.datastax.com/en/developer/java-driver/4.3/manual/mapper/

I believe we should only document how to create a session that invokes callbacks on a specific Vert.x context.