Add support for AWS MCS (Amazon Keyspaces)

[mihai-vasilache]

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]

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

[Nihilum]

logMigration query should include keyspace name. When using AWS Keyspaces, having multiple tables with the same name (even if they're in different keyspaces), sometimes makes the tool insert log entries into improper tables (or executing the query ends with an exception about insufficient access rights, depending on IAM policy configuration). Consider the following scenario:

Keyspaces: A, B, C
Tables: A.schema_migration, B.schema_migration, C.schema_migration
Query in the BoundStatement: "insert into schema_migration (applied_successful, version, script_name, script, executed_at) values(?, ?, ?, ?, ?)"

The above statement executed in the context of keyspace A will sometimes end up modifying the contents of the table B.schema_migration or C.schema_migration.

I was able to avoid this problem by modifying the query and explicitly stating the keyspace name:
"insert into %s.%s (applied_successful, version, script_name, script, executed_at) values(?, ?, ?, ?, ?)"

Not sure what is the root cause - perhaps it's because I'm using datastax driver 3.7.2 and it's not properly taking into account the context in which the query is executed? (ignoring USE keyspace?). Then again, modifying the statement here would avoid any ambiguity (related to the driver etc.)

[mihai-vasilache]

Thanks @Nihilum. I will include this in the refactoring.

[mihai-vasilache]

It is included in my last commit.

[patka]

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]

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]

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]

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

[mihai-vasilache]

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

[vaibhavflip]

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

[patka]

@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]

Hello, ant plan to release this ?

[patka]

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.