[JBoss JIRA] (ISPN-2103) Concurrent access after removal of an AtomicMap should NOT result in an IllegalStateException when accessed by other threads
by Pedro Ruivo (JIRA)
[ https://issues.jboss.org/browse/ISPN-2103?page=com.atlassian.jira.plugin.... ]
Pedro Ruivo commented on ISPN-2103:
-----------------------------------
Hey guys,
I need an opinion. I've changed the code to make the locking mode to handle the conflicts. However, when a transaction starts and the map does not exists, what behavior do you prefer?
1) throw ISE.
2) create a new map.
> Concurrent access after removal of an AtomicMap should NOT result in an IllegalStateException when accessed by other threads
> ----------------------------------------------------------------------------------------------------------------------------
>
> Key: ISPN-2103
> URL: https://issues.jboss.org/browse/ISPN-2103
> Project: Infinispan
> Issue Type: Bug
> Components: Core
> Affects Versions: 5.1.5.FINAL
> Reporter: Adrian Nistor
> Assignee: Pedro Ruivo
>
> ISPN-1121 introduces an IllegalStateException that is being thrown from AtomicMap methods once the map handle has become stale (ie. removed from cache). In case of concurrent access, the exception is thrown to all threads not just to the thread that performed the removal. This was fine-ish in older versions of Infinispan before introduction of optimistic and pessimistic locking but it should be reconsidered now because:
> 1. It interferes/overlaps with transaction isolation. We should rely on the selected locking scheme (OL/PL) to detect conflicts between transactions and report the problem accordingly. Especially if the stale map is used just for reading - this should be allowed to work rather than stop it.
> 2. This exception is pretty disruptive and awkward to handle. All methods of an AtomicMap can result in this exception and the current thread has to be prepared for handling it if other threads remove the map. Not much transaction isolation.
> 3. Since the TreeCache is backed by AtomicMap nearly all Tree API can throw this.
> The proposed fix consists of:
> 1. removing AtomicHashMap.removed flag and AtomicHashMap.markRemoved() method.
> 2. revising AtomicHashMapProxy.assertValid() method to check only if the map is null (ie. removed) but no longer use the removed flag.
> 3. revising ReadCommittedEntry.commit() method to no longer call markRemoved() method.
> The consequences of these changes are:
> 1. Any further access to a stale map results in IllegalStateException ONLY in the thread that performed the removal. This thread 'knows' the map is stale so it is fine to punish it. Other threads remain unaffected until lock acquisition or commit is performed (depending on locking model).
> 2. Other threads can continue to use the previously obtained map handle for reads without danger of getting an exception.
> 3. If a write operation is done on the map, the results depend on the locking model:
> 3.1 optimistic locking + write skew check: a WriteSkewException will stop the commit during prepare
> 3.2 optimistic locking, no write skew check: the write is committed and the work of the transaction that removed the map is overwritten. The map is effectively revived.
> 3.3 pessimistic locking: same as 3.2
> Please note 3.2 and 3.3 work the same as for normal values (not atomic maps).
--
This message was sent by Atlassian JIRA
(v6.4.11#64026)
8 years, 3 months
[JBoss JIRA] (ISPN-4016) Write operations in invalidation mode can fail with OutdatedTopologyException
by Tristan Tarrant (JIRA)
[ https://issues.jboss.org/browse/ISPN-4016?page=com.atlassian.jira.plugin.... ]
Tristan Tarrant updated ISPN-4016:
----------------------------------
Status: Resolved (was: Pull Request Sent)
Fix Version/s: 9.0.0.Beta1
9.0.0.Final
8.2.5.Final
Resolution: Done
> Write operations in invalidation mode can fail with OutdatedTopologyException
> -----------------------------------------------------------------------------
>
> Key: ISPN-4016
> URL: https://issues.jboss.org/browse/ISPN-4016
> Project: Infinispan
> Issue Type: Bug
> Components: State Transfer
> Affects Versions: 6.0.1.Final, 7.0.0.Alpha1, 7.0.0.Alpha2, 7.0.0.Alpha3
> Reporter: Dan Berindei
> Assignee: Dan Berindei
> Priority: Critical
> Labels: 630
> Fix For: 9.0.0.Beta1, 9.0.0.Final, 8.2.5.Final
>
> Attachments: NonTxStateTransferInvalidationTest.log.zip
>
>
> I introduced this problem with the fix for ISPN-3873.
> Invalidation commands now have a topology id, so they can wait for the initial topology to be installed on a joiner. However, that means EntryWrappingInterceptor also checks the topology id, and if it has changed it will throw an OutdatedTopologyException. The exception is propagated all the way to the caller.
> OutdatedTopologyExceptions are not useful in invalidation mode, since the invalidation is always sent to the entire cluster. So EntryWrappingInterceptor should ignore the topology id in invalidation mode.
> {noformat}
> Tests run: 4052, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 313.505 sec <<< FAILURE!testInvalidationDuringStateTransfer(org.infinispan.statetransfer.NonTxStateTransferInvalidationTest) Time elapsed: 0.004 sec <<< FAILURE!java.util.concurrent.ExecutionException: org.infinispan.remoting.RemoteException: ISPN000217: Received exception from NonTxStateTransferInvalidationTest-NodeB-5833, see cause for remote stack trace
> at java.util.concurrent.FutureTask.report(FutureTask.java:122)
> at java.util.concurrent.FutureTask.get(FutureTask.java:202)
> at org.infinispan.commons.util.concurrent.NotifyingFutureImpl.get(NotifyingFutureImpl.java:84)
> at org.infinispan.statetransfer.NonTxStateTransferInvalidationTest.testInvalidationDuringStateTransfer(NonTxStateTransferInvalidationTest.java:115)
> Caused by: org.infinispan.remoting.RemoteException: ISPN000217: Received exception from NonTxStateTransferInvalidationTest-NodeB-5833, see cause for remote stack trace
> at org.infinispan.remoting.transport.AbstractTransport.checkResponse(AbstractTransport.java:41)
> at org.infinispan.remoting.transport.AbstractTransport.parseResponseAndAddToResponseList(AbstractTransport.java:66)
> at org.infinispan.remoting.transport.jgroups.JGroupsTransport.invokeRemotely(JGroupsTransport.java:547)
> at org.infinispan.remoting.rpc.RpcManagerImpl.invokeRemotely(RpcManagerImpl.java:280)
> at org.infinispan.interceptors.InvalidationInterceptor.invalidateAcrossCluster(InvalidationInterceptor.java:227)
> at org.infinispan.interceptors.InvalidationInterceptor.handleInvalidate(InvalidationInterceptor.java:143)
> at org.infinispan.interceptors.InvalidationInterceptor.visitPutKeyValueCommand(InvalidationInterceptor.java:80)
> at org.infinispan.commands.write.PutKeyValueCommand.acceptVisitor(PutKeyValueCommand.java:70)
> at org.infinispan.interceptors.base.CommandInterceptor.invokeNextInterceptor(CommandInterceptor.java:98)
> at org.infinispan.interceptors.EntryWrappingInterceptor.invokeNextAndApplyChanges(EntryWrappingInterceptor.java:326)
> at org.infinispan.interceptors.EntryWrappingInterceptor.setSkipRemoteGetsAndInvokeNextForDataCommand(EntryWrappingInterceptor.java:407)
> at org.infinispan.interceptors.EntryWrappingInterceptor.visitPutKeyValueCommand(EntryWrappingInterceptor.java:164)
> at org.infinispan.commands.write.PutKeyValueCommand.acceptVisitor(PutKeyValueCommand.java:70)
> at org.infinispan.interceptors.base.CommandInterceptor.invokeNextInterceptor(CommandInterceptor.java:98)
> at org.infinispan.interceptors.locking.AbstractLockingInterceptor.visitPutKeyValueCommand(AbstractLockingInterceptor.java:68)
> at org.infinispan.commands.write.PutKeyValueCommand.acceptVisitor(PutKeyValueCommand.java:70)
> at org.infinispan.interceptors.base.CommandInterceptor.invokeNextInterceptor(CommandInterceptor.java:98)
> at org.infinispan.interceptors.base.CommandInterceptor.handleDefault(CommandInterceptor.java:112)
> at org.infinispan.commands.AbstractVisitor.visitPutKeyValueCommand(AbstractVisitor.java:32)
> at org.infinispan.commands.write.PutKeyValueCommand.acceptVisitor(PutKeyValueCommand.java:70)
> at org.infinispan.interceptors.base.CommandInterceptor.invokeNextInterceptor(CommandInterceptor.java:98)
> at org.infinispan.interceptors.CacheMgmtInterceptor.updateStoreStatistics(CacheMgmtInterceptor.java:148)
> at org.infinispan.interceptors.CacheMgmtInterceptor.visitPutKeyValueCommand(CacheMgmtInterceptor.java:134)
> at org.infinispan.commands.write.PutKeyValueCommand.acceptVisitor(PutKeyValueCommand.java:70)
> at org.infinispan.interceptors.base.CommandInterceptor.invokeNextInterceptor(CommandInterceptor.java:98)
> at org.infinispan.interceptors.InvocationContextInterceptor.handleAll(InvocationContextInterceptor.java:110)
> at org.infinispan.interceptors.InvocationContextInterceptor.handleDefault(InvocationContextInterceptor.java:73)
> at org.infinispan.commands.AbstractVisitor.visitPutKeyValueCommand(AbstractVisitor.java:32)
> at org.infinispan.commands.write.PutKeyValueCommand.acceptVisitor(PutKeyValueCommand.java:70)
> at org.infinispan.interceptors.InterceptorChain.invoke(InterceptorChain.java:333)
> at org.infinispan.CacheImpl.executeCommandAndCommitIfNeeded(CacheImpl.java:1403)
> at org.infinispan.CacheImpl.putInternal(CacheImpl.java:881)
> at org.infinispan.CacheImpl.access$100(CacheImpl.java:106)
> at org.infinispan.CacheImpl$2.call(CacheImpl.java:1015) ... 4 more
> Caused by: org.infinispan.statetransfer.OutdatedTopologyException: Cache topology changed while the command was executing: expected 2, got 3
> at org.infinispan.interceptors.EntryWrappingInterceptor.invokeNextAndApplyChanges(EntryWrappingInterceptor.java:347)
> at org.infinispan.interceptors.EntryWrappingInterceptor.setSkipRemoteGetsAndInvokeNextForDataCommand(EntryWrappingInterceptor.java:407)
> at org.infinispan.interceptors.EntryWrappingInterceptor.visitInvalidateCommand(EntryWrappingInterceptor.java:139)
> at org.infinispan.commands.write.InvalidateCommand.acceptVisitor(InvalidateCommand.java:118)
> at org.infinispan.interceptors.base.CommandInterceptor.invokeNextInterceptor(CommandInterceptor.java:98)
> at org.infinispan.interceptors.locking.AbstractLockingInterceptor.visitInvalidateCommand(AbstractLockingInterceptor.java:87)
> at org.infinispan.commands.write.InvalidateCommand.acceptVisitor(InvalidateCommand.java:118)
> at org.infinispan.interceptors.base.CommandInterceptor.invokeNextInterceptor(CommandInterceptor.java:98)
> at org.infinispan.interceptors.base.CommandInterceptor.handleDefault(CommandInterceptor.java:112)
> at org.infinispan.commands.AbstractVisitor.visitInvalidateCommand(AbstractVisitor.java:111)
> at org.infinispan.commands.write.InvalidateCommand.acceptVisitor(InvalidateCommand.java:118)
> at org.infinispan.interceptors.base.CommandInterceptor.invokeNextInterceptor(CommandInterceptor.java:98)
> at org.infinispan.interceptors.base.CommandInterceptor.handleDefault(CommandInterceptor.java:112)
> at org.infinispan.commands.AbstractVisitor.visitInvalidateCommand(AbstractVisitor.java:111)
> at org.infinispan.commands.write.InvalidateCommand.acceptVisitor(InvalidateCommand.java:118)
> at org.infinispan.interceptors.base.CommandInterceptor.invokeNextInterceptor(CommandInterceptor.java:98)
> at org.infinispan.interceptors.InvocationContextInterceptor.handleAll(InvocationContextInterceptor.java:110)
> at org.infinispan.interceptors.InvocationContextInterceptor.handleDefault(InvocationContextInterceptor.java:73)
> at org.infinispan.commands.AbstractVisitor.visitInvalidateCommand(AbstractVisitor.java:111)
> at org.infinispan.commands.write.InvalidateCommand.acceptVisitor(InvalidateCommand.java:118)
> at org.infinispan.interceptors.InterceptorChain.invoke(InterceptorChain.java:333)
> at org.infinispan.commands.remote.BaseRpcInvokingCommand.processVisitableCommand(BaseRpcInvokingCommand.java:39)
> at org.infinispan.commands.remote.SingleRpcCommand.perform(SingleRpcCommand.java:48)
> at org.infinispan.remoting.InboundInvocationHandlerImpl.handleInternal(InboundInvocationHandlerImpl.java:95)
> at org.infinispan.remoting.InboundInvocationHandlerImpl.access$000(InboundInvocationHandlerImpl.java:50)
> at org.infinispan.remoting.InboundInvocationHandlerImpl$2.run(InboundInvocationHandlerImpl.java:178) ... 3 moreResults :
> Failed tests: NonTxStateTransferInvalidationTest.testInvalidationDuringStateTransfer:115 » Execution
> {noformat}
--
This message was sent by Atlassian JIRA
(v6.4.11#64026)
8 years, 3 months
[JBoss JIRA] (ISPN-6857) OutdatedTopologyException in clustered invalidation cache because StateTransferInterceptor not in the chain
by Tristan Tarrant (JIRA)
[ https://issues.jboss.org/browse/ISPN-6857?page=com.atlassian.jira.plugin.... ]
Tristan Tarrant updated ISPN-6857:
----------------------------------
Status: Resolved (was: Pull Request Sent)
Fix Version/s: (was: 8.1.5.Final)
Resolution: Done
> OutdatedTopologyException in clustered invalidation cache because StateTransferInterceptor not in the chain
> -----------------------------------------------------------------------------------------------------------
>
> Key: ISPN-6857
> URL: https://issues.jboss.org/browse/ISPN-6857
> Project: Infinispan
> Issue Type: Bug
> Affects Versions: 8.1.0.Final
> Reporter: Marek Posolda
> Assignee: Dan Berindei
> Fix For: 9.0.0.Final, 8.2.5.Final
>
> Attachments: OutdatedTopologyExceptionReproducerTest.java
>
>
> I have the following setup:
> - 2 nodes in cluster with mode INVALIDATION_SYNC. No-transaction cache.
> - Node1 is started
> - Called "cache.remove" on some key on node1. At the same time, node2 is starting, which is causing topology change.
> - The "cache.remove" call on node1 is throwing OutdatedTopologyException.
> I found the cause is that StateTransferInterceptor is not added in InterceptorChain during INVALIDATION mode. It's just available during REPLICATION or DISTRIBUTED modes - https://github.com/infinispan/infinispan/blob/master/core/src/main/java/o...
> Indeed when I manually added StateTransferInterceptor to my invalidation cache:
> {code}
> invalidationConfigBuilder.customInterceptors()
> .addInterceptor()
> .before(NonTransactionalLockingInterceptor.class)
> .interceptorClass(StateTransferInterceptor.class);
> {code}
>
> I can see that issue is gone as OutdatedTopologyException is catched and command is retried with new topology.
> I am attaching the Java unit test for reproducing issue. On my laptop when I run it, I can almost always simulate the issue.
--
This message was sent by Atlassian JIRA
(v6.4.11#64026)
8 years, 3 months
[JBoss JIRA] (ISPN-7026) CacheClusterJoinTest.testIsCoordinator random failures
by Dan Berindei (JIRA)
[ https://issues.jboss.org/browse/ISPN-7026?page=com.atlassian.jira.plugin.... ]
Dan Berindei updated ISPN-7026:
-------------------------------
Description:
{{CacheClusterJoinTest.testIsCoordinator}} assumes that once {{JGroupsTransport}} received a view, {{JGroupsTransport.isCoordinator()}} returns the correct value.
However, only {{JGroupsTransfer.viewAccepted}} is synchronized, {{isCoordinator()}} is not, so the main thread can see {{isCoordinator() == false}} even after {{getMembers().get(0) == self}}.
{noformat}
19:32:27,555 DEBUG (Incoming-1,Test-NodeD-36891:[]) [JGroupsTransport] New view accepted: [Test-NodeD-36891|2] (1) [Test-NodeD-36891]
19:32:27,556 ERROR (testng-Test:[]) [TestSuiteProgress] Test failed: org.infinispan.api.CacheClusterJoinTest.testIsCoordinator
java.lang.AssertionError
at org.infinispan.api.CacheClusterJoinTest.testIsCoordinator(CacheClusterJoinTest.java:65) ~[test-classes/:?]
19:32:27,555 DEBUG (Incoming-1,Test-NodeD-36891:[]) [JGroupsTransport] Joined: [], Left: [Test-NodeC-35712]
{noformat}
It would be nice if {{isCoordinator()}}, {{getCoordinator()}}, and {{getMembers()}} were more in sync, even though the view can always change between two calls. The simplest way to do this would be to implement {{isCoordinator()}} and {{getCoordinator()}} on top of {{getMembers()}} and remove their fields, since they're not use in performance-sensitive code.
was:
{{CacheClusterJoinTest.testIsCoordinator}} assumes that once {{JGroupsTransport}} received a view, {{JGroupsTransport.isCoordinator()}} returns the correct value.
However, only {{JGroupsTransfer.viewAccepted}} is synchronized, {{isCoordinator()}} is not, so the main thread can see {{isCoordinator() == false}} even after {{getMembers().get(0) == self}}.
It would be nice if {{isCoordinator()}}, {{getCoordinator()}}, and {{getMembers()}} were more in sync, even though the view can always change between two calls. The simplest way to do this would be to implement {{isCoordinator()}} and {{getCoordinator()}} on top of {{getMembers()}} and remove their fields, since they're not use in performance-sensitive code.
> CacheClusterJoinTest.testIsCoordinator random failures
> ------------------------------------------------------
>
> Key: ISPN-7026
> URL: https://issues.jboss.org/browse/ISPN-7026
> Project: Infinispan
> Issue Type: Bug
> Components: Core, Test Suite - Core
> Affects Versions: 9.0.0.Alpha4
> Reporter: Dan Berindei
> Assignee: Dan Berindei
> Labels: testsuite_stability
> Fix For: 9.0.0.Beta1
>
>
> {{CacheClusterJoinTest.testIsCoordinator}} assumes that once {{JGroupsTransport}} received a view, {{JGroupsTransport.isCoordinator()}} returns the correct value.
> However, only {{JGroupsTransfer.viewAccepted}} is synchronized, {{isCoordinator()}} is not, so the main thread can see {{isCoordinator() == false}} even after {{getMembers().get(0) == self}}.
> {noformat}
> 19:32:27,555 DEBUG (Incoming-1,Test-NodeD-36891:[]) [JGroupsTransport] New view accepted: [Test-NodeD-36891|2] (1) [Test-NodeD-36891]
> 19:32:27,556 ERROR (testng-Test:[]) [TestSuiteProgress] Test failed: org.infinispan.api.CacheClusterJoinTest.testIsCoordinator
> java.lang.AssertionError
> at org.infinispan.api.CacheClusterJoinTest.testIsCoordinator(CacheClusterJoinTest.java:65) ~[test-classes/:?]
> 19:32:27,555 DEBUG (Incoming-1,Test-NodeD-36891:[]) [JGroupsTransport] Joined: [], Left: [Test-NodeC-35712]
> {noformat}
> It would be nice if {{isCoordinator()}}, {{getCoordinator()}}, and {{getMembers()}} were more in sync, even though the view can always change between two calls. The simplest way to do this would be to implement {{isCoordinator()}} and {{getCoordinator()}} on top of {{getMembers()}} and remove their fields, since they're not use in performance-sensitive code.
--
This message was sent by Atlassian JIRA
(v6.4.11#64026)
8 years, 3 months
[JBoss JIRA] (ISPN-7026) CacheClusterJoinTest.testIsCoordinator random failures
by Dan Berindei (JIRA)
[ https://issues.jboss.org/browse/ISPN-7026?page=com.atlassian.jira.plugin.... ]
Dan Berindei updated ISPN-7026:
-------------------------------
Status: Open (was: New)
> CacheClusterJoinTest.testIsCoordinator random failures
> ------------------------------------------------------
>
> Key: ISPN-7026
> URL: https://issues.jboss.org/browse/ISPN-7026
> Project: Infinispan
> Issue Type: Bug
> Components: Core, Test Suite - Core
> Affects Versions: 9.0.0.Alpha4
> Reporter: Dan Berindei
> Assignee: Dan Berindei
> Labels: testsuite_stability
> Fix For: 9.0.0.Beta1
>
>
> {{CacheClusterJoinTest.testIsCoordinator}} assumes that once {{JGroupsTransport}} received a view, {{JGroupsTransport.isCoordinator()}} returns the correct value.
> However, only {{JGroupsTransfer.viewAccepted}} is synchronized, {{isCoordinator()}} is not, so the main thread can see {{isCoordinator() == false}} even after {{getMembers().get(0) == self}}.
> It would be nice if {{isCoordinator()}}, {{getCoordinator()}}, and {{getMembers()}} were more in sync, even though the view can always change between two calls. The simplest way to do this would be to implement {{isCoordinator()}} and {{getCoordinator()}} on top of {{getMembers()}} and remove their fields, since they're not use in performance-sensitive code.
--
This message was sent by Atlassian JIRA
(v6.4.11#64026)
8 years, 3 months