ISPN-2089 Commit not working for distributed shared cache stores #1159
Conversation
|
Infinispan » infinispan #80 SUCCESS |
ghost
assigned 
| @@ -148,6 +149,14 @@ public Object visitPrepareCommand(TxInvocationContext ctx, PrepareCommand comman | |||
| return invokeNextInterceptor(ctx, command); | |||
| } | |||
|
|
|||
| @Override | |||
| public Object visitCommitCommand(TxInvocationContext ctx, CommitCommand command) throws Throwable { | |||
There was a problem hiding this comment.
visitCommitCommand looks exactly the same as the overridden method in CacheStoreInterceptor. Do you really need the override?
There was a problem hiding this comment.
Hmm, I see now, the skip method called here does not override CSI.skip(InvocationContext, VisitableCommand).
I think it would be clearer if the skip method really did override the one in the base class and visitCommitCommand wouldn't be overridden.
There was a problem hiding this comment.
Yeah, that could work too. I'll have a look...
|
@danberindei I've updated the pull req, have a look :) |
|
Galder, DistSyncCacheStoreSharedTest.testClear() is now failing: Looks like clear always needs to run on only one node, and using the overridden implementation of skip in DistCacheStoreInterceptor breaks that. I guess I should have just pushed your original commit :) |
|
BTW Galder, buildhive seems to abort the build after 15 minutes, even though it says it waited 120 minutes... |
|
@danberindei, I'll check clear. WRT build, I changed it to do the full build cos they've changed the default to 120mins (I think I emailed the team about it), but 120 mins is not enough to run our full build in the env buildhive uses, so reverting to previous, compile only set up. |
|
@danberindei Tbh, what's different here is clear(), that's different to all the rest of operations in dist cache store, so it makes sense to override it and make it behave as it should. commit() should have been acting like the rest of operations for a dist cache store, ignoring isOriginLocal(). I'll update asap... |
|
@danberindei Pull req updated. |
|
Infinispan » infinispan #83 SUCCESS |
|
Galder, CacheLoaderManagerConfig.shared() doesn't exist in 5.1.x, so your 5.1.x branch doesn't compile. |
|
Infinispan » infinispan #92 SUCCESS |
|
@danberindei Fixed... once buildhive or similar works, need to figure out a way for the hook to find other branches in the comments and fire builds for those :) |
Pretty much all DIST cache store operations should check the locality of keys and take that into account when making decisions to operate on the cache store, except one, clear. Treat this special case differently in a explicit way to avoid breaking functionality in the future.
https://issues.jboss.org/browse/ISPN-2089
5.1.xbranch:t_dist_shared_5