patka / cassandra-migration

Schema migration library for Cassandra
MIT License
152 stars 47 forks source link

Add support for AWS MCS (Amazon Keyspaces) #56

Open mihai-vasilache opened 3 years ago

mihai-vasilache commented 3 years ago

AWS MCS executes all DDLs asynchronously. As discussed in issue 51 after executing a ddl, the tables are not ready only after some time and the subsequent migration scripts will fail. I've added support for an "Executor" of cql statements. The default one (SimpleExecutor) only forwards to the CqlSession. There is support in org.cognitor.cassandra.migration.Database for autodetecting if it running against aws and choose AwsMcsExecutor instead of SimpleExecutor.

AwsMcsExecutor detects if there is a ddl to be executed, extract the name of the table or keyspace, then pause the execution until the table or keyspace is ready.

There is not yet support for DROP KEYSPACE and ALTER KEYSPACE. It is tested with my use case with few create table and insert statements.

mihai-vasilache commented 3 years ago

And on my Windows machine there is something wrong with two tests:

JarLocationScannerTest.shouldReturnTwoResourcesWhenJarFileWithOneScriptGiven 
JarLocationScannerTest.shouldThrowExceptionWhenNonExistingPathGiven

they are trying to open jar:file:C:/Users/MVASIL~1/AppData/Local/Temp/Test14094071718996507780.jar on JarLocationScanner.getFileSystem() throwing:

java.lang.IllegalArgumentException: URI is not hierarchical

    at java.base/sun.nio.fs.WindowsUriSupport.fromUri(WindowsUriSupport.java:122)
    at java.base/sun.nio.fs.WindowsFileSystemProvider.getPath(WindowsFileSystemProvider.java:97)
    at java.base/java.nio.file.Path.of(Path.java:203)
    at java.base/java.nio.file.Paths.get(Paths.java:98)
    at jdk.zipfs/jdk.nio.zipfs.ZipFileSystemProvider.uriToPath(ZipFileSystemProvider.java:76)
    at jdk.zipfs/jdk.nio.zipfs.ZipFileSystemProvider.getFileSystem(ZipFileSystemProvider.java:151)
    at java.base/java.nio.file.FileSystems.getFileSystem(FileSystems.java:231)
    at org.cognitor.cassandra.migration.scanner.JarLocationScanner.getFileSystem(JarLocationScanner.java:44)
    at org.cognitor.cassandra.migration.scanner.JarLocationScanner.findResourceNames(JarLocationScanner.java:32)
    at org.cognitor.cassandra.migration.scanner.JarLocationScannerTest.shouldReturnTwoResourcesWhenJarFileWithOneScriptGiven(JarLocationScannerTest.java:47)
    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:564)
    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.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
    at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
    at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
    at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
    at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:230)
    at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:58)

The code is: return FileSystems.getFileSystem(location); where location is: jar:file:C:/Users/MVASIL~1/AppData/Local/Temp/Test17443126786644398458.jar Note the jar:file:C

However it works with: FileSystems.newFileSystem(URI.create("jar:file:///C:/Users/MVASIL\~1/AppData/Local/Temp/Test17443126786644398458.jar"), new java.util.HashMap<>()) and getFileSystem() works after it was created with the above newFileSystem() FileSystems.getFileSystem(URI.create("jar:file:///C:/Users/MVASIL\~1/AppData/Local/Temp/Test17443126786644398458.jar"))

EDIT better I will open an issue

patka commented 3 years ago

First of all thanks for this pull request. I will have a look at it in the next couple of days. Has been some crazy times lately and I did not yet have the time. Sorry for that!

patka commented 3 years ago

Hi,

sorry, it took me obviously longer than a couple of days. Home Office, Covid and Home Schooling did its thing :( I finally had a look at this and I would like to propose a slightly different approach here.

Do you see any issue if instead of creating this Executor abstraction we would introduce something like an ExecutionHandler that would have beforeExecution() and afterExecution() methods? That is something I am anyway planning to do to keep the whole thing more open for modifications while not having to modify central code places for every new feature. As far as I can see the AWS scenario is behaving like the normal scenario except that some things need to be done after execution of the statement, right?

One of the main problems I see here is that we now throw away the PreparedStatements after they have been executed. At least for the logMigration that will most likely end up in quite some warnings in the log file coming from the datastax driver.

Cheers Patrick

mihai-vasilache commented 2 years ago

Hi,

If you responded in one month, I am responding in one year :) Instead of ExecutionHandler maybe it is a better name ExecutionAdvisor? But it is a name used in AOP and can cause confusion.... And if we have a list of "advisors" where can we configure the ones (a list of them) that will be executed? We need to create a configuration file? Or we can scan for all of them in the classpath? But maybe some users don't want to apply all advisors.

Mihai

mihai-vasilache commented 2 years ago

Never mind. I've added: getAdvisors() setAdvisors() addAdvisor() in Database class

mihai-vasilache commented 2 years ago

I've made a refactoring based on your comment. Can you take a look?

vaibhavflip commented 2 years ago

@patka we are also planning to leverage this. would be great of you could take a look

patka commented 2 years ago

@vaibhavflip Thanks for letting me know that you will use this. Trust me, I want to ship it as much as you but I am currently in the situation that many people are in, meaning that although officially kindergarten and schools are open, in reality they are closed as either their personel is sick or there are reported casis of Covid and they are closed because of this. At the moment I am already happy when I manage my day job and take care of my kids. Therefore I am very sorry but I cannot give you an ETA for it as it is also a bigger feature but I definitely want to look at it.

bulanovk commented 1 year ago

Hello, ant plan to release this ?

patka commented 1 year ago

Yes, I do have a plan to release it but it requires some work and I do not know when I will find the time for it. That is currently my main blocker for this.