ISPN-2621 + ISPN-2606 Rolling Upgrades via HotRod #1523
Conversation
ghost
assigned 
| @@ -56,7 +56,7 @@ | |||
| <dependency> | |||
| <groupId>${project.groupId}</groupId> | |||
| <artifactId>infinispan-server-hotrod</artifactId> | |||
| <scope>test</scope> | |||
| <optional>true</optional> | |||
There was a problem hiding this comment.
Remote cache store depends on a Hot Rod server? :) Even if optional, surely this is just test-scope?
There was a problem hiding this comment.
Two choices: move CacheValue out of server (into core maybe ?) or move the entrywrapper to server (nonsense imho).
This is actually not really that much of an issue since client and server will be colocated in the rolling upgrade scenario
There was a problem hiding this comment.
I think this would be a temporary measure, Tristan had some nice ideas to avoid the need of wrapper classes around keys or values...
There was a problem hiding this comment.
Another option would be to have CacheValue migrated to o.i.util. ByteArrayKey is already there...
There was a problem hiding this comment.
+1 to move CacheValue to core
There was a problem hiding this comment.
Wouldn't moving it in 5.2 cause problems ? I'm thinking about a JDBC cachestore containing serialized InternalCacheEntries containing serialized CacheValues that need deserializing.
There was a problem hiding this comment.
@tristantarrant +1. I guess a better time for such changes is 6.0
There was a problem hiding this comment.
If the CacheValue has an externalizer, I don't think changing the class name would break deserialization. Might be a problem in other areas though.
There was a problem hiding this comment.
Let's not be too adventurous here, the optional dependency is not ideal, but the consequences of moving classes around at this stage might be a bit too risky.
|
|
||
| } | ||
| es.shutdown(); | ||
| while (!es.isTerminated()) LockSupport.parkNanos(TimeUnit.NANOSECONDS.convert(100, TimeUnit.MILLISECONDS)); |
There was a problem hiding this comment.
ExecutorService.awaitTermination?
There was a problem hiding this comment.
while (!es.awaitTermination(100, TimeUnit.MILLISECONDS)); ?
(nanos seems a bit excessive, and I'm not really fussed about waiting for too long)
There was a problem hiding this comment.
Actually, while awaitTermination() might be clearer to read, it has to be wrapped in a try/catch
There was a problem hiding this comment.
Don't you want to stop waiting at some point?
I actually consider allowing interruptions a plus, so that it works with Future.cancel(), but that's your call.
ISPN-2621 + ISPN-2606 Rolling Upgrades via HotRod
https://issues.jboss.org/browse/ISPN-2621
Introduce an entry wrapping mechanism for RemoteCacheStore
https://issues.jboss.org/browse/ISPN-2606
Implement all phases of rolling upgrades using an MBean
Separate the concept of source and target migrators
Modify and document the upgrade CLI command to reflect all of the above