jsevellec / cassandra-unit

Utility tool to load Data into Cassandra to help you writing good isolated JUnit Test into your application
GNU Lesser General Public License v3.0
425 stars 0 forks source link

update to cassandra 4 #325

Closed wakingrufus closed 1 year ago

wakingrufus commented 3 years ago

update to cassandra 4 (#315) (#294) (#321)

This commit depends on cassandra 4 rc1 which should be updated when the final release comes out These changes are verified against java 8 AND 11, but in order to run the tests, you must specify -Djdk.version=1.8 or -Djdk.version=11 depending on which version of java your maven is executing with Users of cassandra-unit will need to provide additional jvm args to their test runtime if they are running on java 11 as specified on pom.xml#165. this is due to a requirement of cassandra 4 (see https://github.com/apache/cassandra/blob/trunk/conf/jvm11-server.options#L59 for the default start script for cassandra4 on jvm11)

wakingrufus commented 3 years ago

this currently works on java 8, but on java 11, i am getting


java.lang.ExceptionInInitializerError
    at org.apache.cassandra.config.DatabaseDescriptor.guessFileStore(DatabaseDescriptor.java:1193)
    at org.apache.cassandra.config.DatabaseDescriptor.applySimpleConfig(DatabaseDescriptor.java:550)
    at org.apache.cassandra.config.DatabaseDescriptor.applyAll(DatabaseDescriptor.java:350)
    at org.apache.cassandra.config.DatabaseDescriptor.daemonInitialization(DatabaseDescriptor.java:178)
    at org.apache.cassandra.config.DatabaseDescriptor.daemonInitialization(DatabaseDescriptor.java:162)
    at org.cassandraunit.utils.EmbeddedCassandraServerHelper.startEmbeddedCassandra(EmbeddedCassandraServerHelper.java:148)
    at org.cassandraunit.utils.EmbeddedCassandraServerHelper.startEmbeddedCassandra(EmbeddedCassandraServerHelper.java:111)
    at org.cassandraunit.utils.EmbeddedCassandraServerHelper.startEmbeddedCassandra(EmbeddedCassandraServerHelper.java:91)
    at org.cassandraunit.utils.EmbeddedCassandraServerHelper.startEmbeddedCassandra(EmbeddedCassandraServerHelper.java:83)
    at org.cassandraunit.utils.EmbeddedCassandraServerHelper.startEmbeddedCassandra(EmbeddedCassandraServerHelper.java:79)
    at org.cassandraunit.cli.CassandraUnitCommandLineLoaderTest.shouldLoadCQLDataSet(CassandraUnitCommandLineLoaderTest.java:72)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.apache.maven.surefire.junit4.JUnit4TestSet.execute(JUnit4TestSet.java:35)
    at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:115)
    at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:97)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.apache.maven.surefire.booter.ProviderFactory$ClassLoaderProxy.invoke(ProviderFactory.java:103)
    at com.sun.proxy.$Proxy0.invoke(Unknown Source)
    at org.apache.maven.surefire.booter.SurefireStarter.invokeProvider(SurefireStarter.java:150)
    at org.apache.maven.surefire.booter.SurefireStarter.runSuitesInProcess(SurefireStarter.java:91)
    at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:69)
Caused by: java.lang.RuntimeException: java.lang.IllegalAccessException: access to public member failed: jdk.internal.ref.Cleaner.clean[Ljava.lang.Object;@aeab9a1/invokeVirtual, from org.apache.cassandra.io.util.FileUtils (unnamed module @50eac852)
    at org.apache.cassandra.io.util.FileUtils.<clinit>(FileUtils.java:107)
    ... 40 more
Caused by: java.lang.IllegalAccessException: access to public member failed: jdk.internal.ref.Cleaner.clean[Ljava.lang.Object;@aeab9a1/invokeVirtual, from org.apache.cassandra.io.util.FileUtils (unnamed module @50eac852)
    at java.base/java.lang.invoke.MemberName.makeAccessException(MemberName.java:942)
    at java.base/java.lang.invoke.MethodHandles$Lookup.checkAccess(MethodHandles.java:2201)
    at java.base/java.lang.invoke.MethodHandles$Lookup.checkMethod(MethodHandles.java:2141)
    at java.base/java.lang.invoke.MethodHandles$Lookup.getDirectMethodCommon(MethodHandles.java:2285)
    at java.base/java.lang.invoke.MethodHandles$Lookup.getDirectMethodNoSecurityManager(MethodHandles.java:2278)
    at java.base/java.lang.invoke.MethodHandles$Lookup.unreflect(MethodHandles.java:1747)
    at org.apache.cassandra.io.util.FileUtils.<clinit>(FileUtils.java:96)
    ... 40 more```
wakingrufus commented 3 years ago

also:

2021-04-30 13:48:31,373 [main] ERROR o.a.c.i.u.FileUtils - FATAL: Cassandra is unable to access required classes. This usually means it has been run without the aid of the standard startup scripts or the scripts have been edited. If this was intentional, and you are attempting to use Java 11+ you may need to add the --add-exports and --add-opens jvm options from either jvm11-server.options or jvm11-client.options

but i dont really understand how to solve this

wakingrufus commented 3 years ago

i think we will need to set all of thse jvm args:

### JPMS

-Djdk.attach.allowAttachSelf=true
--add-exports java.base/jdk.internal.misc=ALL-UNNAMED
--add-exports java.base/jdk.internal.ref=ALL-UNNAMED
--add-exports java.base/sun.nio.ch=ALL-UNNAMED
--add-exports java.management.rmi/com.sun.jmx.remote.internal.rmi=ALL-UNNAMED
--add-exports java.rmi/sun.rmi.registry=ALL-UNNAMED
--add-exports java.rmi/sun.rmi.server=ALL-UNNAMED
--add-exports java.sql/java.sql=ALL-UNNAMED

--add-opens java.base/java.lang.module=ALL-UNNAMED
--add-opens java.base/jdk.internal.loader=ALL-UNNAMED
--add-opens java.base/jdk.internal.ref=ALL-UNNAMED
--add-opens java.base/jdk.internal.reflect=ALL-UNNAMED
--add-opens java.base/jdk.internal.math=ALL-UNNAMED
--add-opens java.base/jdk.internal.module=ALL-UNNAMED
--add-opens java.base/jdk.internal.util.jar=ALL-UNNAMED
--add-opens jdk.management/com.sun.management.internal=ALL-UNNAMED
wakingrufus commented 3 years ago

following up on this, the args present a problem when shipping a unit testing library, as we don't have control over the jvm args of the jvm running the test suite. But i think there may be a safer, more robust way to accomplish what FileUtils needs to do, based on this other implementation I found in apache parquet: https://github.com/apache/parquet-mr/pull/654/files#diff-bcfe83ece28dea138fb9592ee822b459dbcf768cb315f1a137bea6243a8d6d1b

or: is there a way to provide the settings in the module-info of the library? i think the module-info for cassandra needs requires jdk.unsupported; in order to make the Unsafe call... but i dont see a module-info where I would expect in the project

wakingrufus commented 3 years ago

I am trying to wrap my head around the module system... from what i can tell, cassandra is just an unnamed module (uses classpath). in that case, it should have all modules open to it by default. so i dont understand why all the --add-opens and --add-exports args are needed. the main thing I am trying to figure out is how to have libraries that use cassandra as a library not have to set all those args itself, i want it to work that way by default

wakingrufus commented 3 years ago

I don't think there is currently a way to get around these extra jvm args on jvm11. once cassandra moves to the module system proper (instead of classpath legacy mode), this may change. i updated this PR with a build flag to enable the jdk11 args needed and documentation about what downstream will need to do to run on jvm11. id be happy to contribute docs to the wiki about this as well

asarkar commented 3 years ago

@wakingrufus Thank you for the PR. What's not clear to me is the purpose of the jvm11-server.options file. If every application using cassandra-unit has to go look in that file, and copy-paste the JPMS hacks, it's a non-starter. That list is subject to change, and is Cassandra internal logic, not something users should know. It would appear that there should be a way to tell Cassandra to use the jvm11-server.options file that someone has painstakingly put together without actually needing to know what's in it. Note that an argument file doesn't work with Maven (https://stackoverflow.com/q/43361227/839733); however, I'm not talking about the tests. I'm talking about cassandra-unit starting a Cassandra server.

wakingrufus commented 3 years ago

@asarkar I agree with you. I searched far and wide for a way to programmatically pass these args, but I couldn't find a way. I even looked at what could be done upstream in cassandra to help, and I couldn't figure out anything short of totally converting to JPMS instead of classpath. ASF slack was not very helpful. Since the cassandra 4 release is my organizations last blocker to building on jdk11, i opened this PR so we could have an option to get it working, but I would love a solution that doesn't involve copying these args in every downstream.

asarkar commented 3 years ago

It seems Cassandra server should automatically use this file. Is that not what you’re seeing?

Several files for JVM configuration are included in Cassandra. https://cassandra.apache.org/doc/latest/configuration/cass_jvm_options_file.html

Also note that CassandraDaemon has an init method that allows for JVM options to be set before startup.

wakingrufus commented 3 years ago

It seems Cassandra server should automatically use this file. Is that not what you’re seeing?

Several files for JVM configuration are included in Cassandra. https://cassandra.apache.org/doc/latest/configuration/cass_jvm_options_file.html

Also note that CassandraDaemon has an init method that allows for JVM options to be set before startup.

I will look into both of those options. Thanks!

wakingrufus commented 3 years ago

I looked into these: 1) putting the file on the classpath does not seem to have any effect 2) The EmbeddedCassandraService always passes null to the init method (this is cassandra code), but even if it didn't, the init method doesn't seem to actually do anything with the args that are passed to it (at least in the source for cassandra 4 rc1).

    public void init(String[] arguments) throws IOException
    {
        setup();
    }
wakingrufus commented 3 years ago

@asarkar can you please take a look at my response above, and let me k now if there are any other approaches we can try? I think the good news here is that if you are on jvm8, it will continue to work out of the box, it is only jdk11+ that will have the extra step

asarkar commented 3 years ago

Honestly, if I were a reviewer with the authority to make a go/no-go call, I’d not approve this PR because it imposes a bunch of configuration on the client that are supposed to be abstracted away by Cassandra. The fact that it works on JDK 8 means nothing to me, since we can’t be stuck with something that’s end of life and released almost a decade ago.

I don’t have maintainer authority on this repo, so I’ll leave it to the maintainers. Given the fact no one else looked at this PR yet, I am afraid no one cares about maintenance much.

wakingrufus commented 3 years ago

Thanks for your input on the issue. I appreciate you looking at it, despite not being a maintainer. I agree about jdk8, but I only mention it because jdk 8 is the only jdk supported currently by this library (because cassandra did not support 11 until cassandra4) so the point is that this new requirement would not be imposed on any current consumers, unless they ALSO do the JDK 11 upgrade, which is the type of thing you might expect some breaking changes on anyway. Either way, as far as I can tell, at this point, the choices are don't support cassandra 4 at all, or support it on JDK8 out of the box and JDK11 with the additional jvm args. I have asked for help directly from the Apache Cassandra Slack, and have not received any good advice on how the args can be applied automatically. If we can't get this merged here, I will end up having to fork this repo so my organization can move forward with our JDK11 migration, which I would really rather not do. I suppose it is up to @jsevellec

nikeee commented 2 years ago

Cassandra 4.0.1 is available. Could we update this PR and get it merged somehow?

wakingrufus commented 2 years ago

Cassandra 4.0.1 is available. Could we update this PR and get it merged somehow?

If i got an indication that the maintainers are willing to merge this, i would be more than happy to rebase and target the latest cassandra. As it stands, my org is now using my fork, and applying the extra args in jdk11 projects with a gradle plugin

nikeee commented 2 years ago

As it stands, my org is now using my fork, and applying the extra args in jdk11 projects with a gradle plugin

Is there something online that we could evaluate, too?

nikeee commented 2 years ago

I discovered https://github.com/btjfsu/cassandra-unit by @btjfsu which works perfectly for me in case anyone needs a drop-in replacement for cassandra-unit which supports Cassandra 4.

asarkar commented 2 years ago

What sort of maintenance commitment do we have on https://github.com/btjfsu/cassandra-unit? Obviously, there’d not be a need for a fork if the maintainers were active on this repo, and there are more than one. It seems the repo above has a single author; who’s to say it won’t be abandoned in 6 months?

wakingrufus commented 1 year ago

Closing this PR as I don't think it will ever be merged, and I, personally, have moved on to testcontainers