-
Notifications
You must be signed in to change notification settings - Fork 2
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
update to cassandra 4 #325
Conversation
this currently works on java 8, but on java 11, i am getting
|
also:
but i dont really understand how to solve this |
i think we will need to set all of thse jvm args:
|
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 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 |
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)
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 |
@wakingrufus Thank you for the PR. What's not clear to me is the purpose of the |
@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. |
It seems Cassandra server should automatically use this file. Is that not what you’re seeing?
Also note that |
I will look into both of those options. Thanks! |
I looked into these:
|
@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 |
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. |
Thanks for your input on the issue. I appreciate you looking at it, despite not being a maintainer. |
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 |
Is there something online that we could evaluate, too? |
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. |
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? |
Closing this PR as I don't think it will ever be merged, and I, personally, have moved on to testcontainers |
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)