[infinispan-issues] [JBoss JIRA] (ISPN-2300) Versioned Transactional Cache issues
Galder Zamarreño (JIRA)
jira-events at lists.jboss.org
Wed Jan 9 10:52:08 EST 2013
[ https://issues.jboss.org/browse/ISPN-2300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12744450#comment-12744450 ]
Galder Zamarreño commented on ISPN-2300:
----------------------------------------
Fix for 1 is not as simple as it looks:
- In the current code base, when a versioned entry fails to update due to conditional operation not fulfilling the condition, the previous value entry is added to the context with isChanged=true (if present in the cache, as a result of marking RepeatableReadEntry.copyForUpdate), and then EntryWrappingInterceptor loops through all the entries in the context, and if "changed", it will try commit them. What happens here is that the entry is not the result of an actual update in the transaction, so its version is null and it breaks later on when ClusteredRepeatableReadEntry checks for the version.
- Pedro's fix works for this particular case, but breaks other use cases, such as CacheLoaderFunctionalTest.testLoading and PassivationFunctionalTest.testRemoveAndReplace. What happens there is that if an entry is in the cache but not in the cache, and the conditional operation to try to putIfAbsent a new value fails, it should load the previous value into the cache. With Pedro's fix, the cache is no longer updated since it's marked that entry to not be "changed", and it's "changed" since you're storing it in the cache, even if it's the result of activating/loading the entry.
A better fix IMO would be to avoid committing the entry if the version is null in the 1st place. This is a more controlled or isolated fix, and avoids messing up with the business of unsetting the "changed" flag which is a bit fragile. Having just tested this quickly, it breaks other things... I'll keep trying things...
> 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.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
More information about the infinispan-issues
mailing list