[infinispan-issues] [JBoss JIRA] (ISPN-2300) Versioned Transactional Cache issues

[Galder Zamarreño (JIRA)]

    [ https://issues.jboss.org/browse/ISPN-2300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12745572#comment-12745572 ] 

Galder Zamarreño commented on ISPN-2300:
----------------------------------------

@Mircea, that was precisely Pedro's fix but as I said, that causes other issues which are not as easy to fix because of the overloaded use of isChanged. This flag really means: "should the new value be applied to the container?", which based on the code must return true when:

1. A conditional operation succeeds.
2. When an entry in the cache store, should be stored in the cache as a result of an activation, or as result of loading it from the cache to check the result of the conditional operation, and the conditional operation fails to succeeded.

If 1 is true, the new value is updated in the cache. If 1. is false, the old value is loaded into the cache.

The problem that Pedro found is that the conditional operation did not succeed (point 1.), so according to him isChanged=false, but if you do that, the entry is not stored in the cache (point 2.) and other tests fail.

My gut feeling is that to fix this properly there really should be 2 flags, one to indicate if the conditional operation succeeded or not, the 2nd to indicate whether the command should make a change in the container. The latter flag, based on the 1st flag, would decide which value to put in the container, the previous value (if conditional operation failed), or the new value (if conditional flag suceeded).

The fix I submitted is really a stop gap, since separating isChanged into two flags would require further refactoring.
                
> Versioned Transactional Cache issues
> ------------------------------------
>
>                 Key: ISPN-2300
>                 URL: https://issues.jboss.org/browse/ISPN-2300
>             Project: Infinispan
>          Issue Type: Bug
>          Components: Transactions
>    Affects Versions: 5.2.0.Alpha3
>            Reporter: Pedro Ruivo
>            Assignee: Galder Zamarreño
>            Priority: Critical
>              Labels: Commands, Conditional, Transactions, Version
>             Fix For: 5.2.0.CR2, 5.2.0.Final
>
>
> Description from http://lists.jboss.org/pipermail/infinispan-dev/2012-September/011205.html
> 1) testPutIfAbsent, testRemoveIfPresent, testReplaceWithOldVal (methods 
> the test cases)
> In this tests, it was updating the cache entry with a null version. This 
> originates later a IllegalStateException when it tries to perform the 
> write skew check and the version is null.
> I have fixed this problem in this way: if the command fails (PutCommand, 
> RemoveCommand, ReplaceCommand), I unset the flag CHANGED in the 
> MvccEntry to avoid to update the entry in the DataContainer.
> 2) testClear in distributed mode
> I have no clear idea how to solve this problem but it looks like the 
> PrepareCommand (with the ClearCommand) is not sent to all the nodes in 
> the cluster. Then, when I do a get, a remote get is performed and the 
> key is still there. I think that it is not the desired behavior.
> 3) testRemoveUnexistingEntry
> In this test, it tries to remove a key that does not exists but it does 
> not success due to a NullPointerException. I have looked deeper and I 
> check that the transaction's lookup entries map has an entry with [key 
> => null] and when it tries to perform the write skew check in that key, 
> a NullPointerException is thrown in here [2] (line 80, WriteSkewHelper)
> [1] https://github.com/pruivo/infinispan/tree/t_replace_fix
> [2] https://github.com/pruivo/infinispan/blob/t_replace_fix/core/src/main/java/org/infinispan/transaction/WriteSkewHelper.java#L80

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira