[infinispan-dev] ISPN-293 getAsync impl requires more reengineering

classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|

[infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Galder Zamarreno
Hi,

Re: https://issues.jboss.org/browse/ISPN-293

I have an issue with my implementation that simply wraps the realRemoteGet in DistributionInterceptor around a Callable:

Assume that cache is configured with Distribution(numOwners=1, l1=enabled), no transactions, and we have a cluster of 2 nodes:

- [main-thread] Put k0 in a cache that should own it.
- [main-thread] Do a getAsync for k0 from a node that does not own it:
- [async-thread] This leads to a remote get which updates the L1 and updates the context created by the main-thread and putting the updated entry in there (however, this thread does not release the locks)
- [main-thread] Next up, we put a new key, i.e. k1, in the node that didn't own k0 (the node where we updated the L1 cache):
- Now this thread has used the same context that the async thread used, so when it comes to releasing locks, it finds two entries in the context, the one locked by the async-thread and one for the main-thread and it fails with java.lang.IllegalMonitorStateException

Now, what's happening here is the async-thread is acquiring the lock but not releasing it because the release happens in the DistLockInterceptor/LockingInterceptor, so a different interceptor to where the lock is being acquired. So, in theory, the solution would be for DistLockInterceptor to wrap the invokeNext() and afterCall() for when an async get so that all "remote get", "l1 update", and "release locks", happen in a separate thread. However, for this to work, it will need to figure out whether a remoteGet() will be necessary in the first place, otherwise is useless. Whether the remoteGet should happen is determined by this code in DistributionInterceptor:

if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) {

Also, if DistLockInterceptor does this check, we need to make sure DistributionInterceptor does not do it again, otherwise it's a waste. I think this might work although it does require some further reengineering.

I'm gonna try to implement this but wondering whether anyone can see any potential flaws here, or if anyone has any better ideas :)

Cheers,
--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Galder Zamarreno
Actually, the set of checks is not just limited to the one I mentioned, there's other checks to make depending if the get is a result of get or the result of a write that needs the previous value.

The other thing I was considering is whether the L1 updated could be moved to DistLockInterceptor so that lock acquisition/release was symmetrical in the sense that it was done both from same interceptor, but that is even more complicated in my view.

On Feb 9, 2011, at 9:14 AM, Galder Zamarreño wrote:

> Hi,
>
> Re: https://issues.jboss.org/browse/ISPN-293
>
> I have an issue with my implementation that simply wraps the realRemoteGet in DistributionInterceptor around a Callable:
>
> Assume that cache is configured with Distribution(numOwners=1, l1=enabled), no transactions, and we have a cluster of 2 nodes:
>
> - [main-thread] Put k0 in a cache that should own it.
> - [main-thread] Do a getAsync for k0 from a node that does not own it:
> - [async-thread] This leads to a remote get which updates the L1 and updates the context created by the main-thread and putting the updated entry in there (however, this thread does not release the locks)
> - [main-thread] Next up, we put a new key, i.e. k1, in the node that didn't own k0 (the node where we updated the L1 cache):
> - Now this thread has used the same context that the async thread used, so when it comes to releasing locks, it finds two entries in the context, the one locked by the async-thread and one for the main-thread and it fails with java.lang.IllegalMonitorStateException
>
> Now, what's happening here is the async-thread is acquiring the lock but not releasing it because the release happens in the DistLockInterceptor/LockingInterceptor, so a different interceptor to where the lock is being acquired. So, in theory, the solution would be for DistLockInterceptor to wrap the invokeNext() and afterCall() for when an async get so that all "remote get", "l1 update", and "release locks", happen in a separate thread. However, for this to work, it will need to figure out whether a remoteGet() will be necessary in the first place, otherwise is useless. Whether the remoteGet should happen is determined by this code in DistributionInterceptor:
>
> if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) {
>
> Also, if DistLockInterceptor does this check, we need to make sure DistributionInterceptor does not do it again, otherwise it's a waste. I think this might work although it does require some further reengineering.
>
> I'm gonna try to implement this but wondering whether anyone can see any potential flaws here, or if anyone has any better ideas :)
>
> Cheers,
> --
> Galder Zamarreño
> Sr. Software Engineer
> Infinispan, JBoss Cache
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Mircea Markus
In reply to this post by Galder Zamarreno

On 9 Feb 2011, at 08:14, Galder Zamarreño wrote:

> Hi,
>
> Re: https://issues.jboss.org/browse/ISPN-293
>
> I have an issue with my implementation that simply wraps the realRemoteGet in DistributionInterceptor around a Callable:
>
> Assume that cache is configured with Distribution(numOwners=1, l1=enabled), no transactions, and we have a cluster of 2 nodes:
>
> - [main-thread] Put k0 in a cache that should own it.
> - [main-thread] Do a getAsync for k0 from a node that does not own it:
> - [async-thread] This leads to a remote get which updates the L1 and updates the context created by the main-thread and putting the updated entry in there (however, this thread does not release the locks)
Can't you use a different InvocationContext instance for this? But even with the same IC, wondering why IC doesn't get cleaned up: InvocationContextInterceptor.handleAll calls ic.reset() in its finally block. Are you starting the async thread after InvocationContextInterceptor?

> - [main-thread] Next up, we put a new key, i.e. k1, in the node that didn't own k0 (the node where we updated the L1 cache):
> - Now this thread has used the same context that the async thread used,
> so when it comes to releasing locks, it finds two entries in the context, the one locked by the async-thread and one for the main-thread and it fails with java.lang.IllegalMonitorStateException
>
> Now, what's happening here is the async-thread is acquiring the lock but not releasing it because the release happens in the DistLockInterceptor/LockingInterceptor, so a different interceptor to where the lock is being acquired. So, in theory, the solution would be for DistLockInterceptor to wrap the invokeNext() and afterCall() for when an async get so that all "remote get", "l1 update", and "release locks", happen in a separate thread. However, for this to work, it will need to figure out whether a remoteGet() will be necessary in the first place, otherwise is useless. Whether the remoteGet should happen is determined by this code in DistributionInterceptor:
>
> if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) {
>
> Also, if DistLockInterceptor does this check, we need to make sure DistributionInterceptor does not do it again, otherwise it's a waste. I think this might work although it does require some further reengineering.
>
> I'm gonna try to implement this but wondering whether anyone can see any potential flaws here, or if anyone has any better ideas :)
I might be missing something but don't we already do the same thing for asyncPut operationd and can't we follow the exact same pattern?

>
> Cheers,
> --
> Galder Zamarreño
> Sr. Software Engineer
> Infinispan, JBoss Cache
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Sanne Grinovero
In reply to this post by Galder Zamarreno
2011/2/9 Galder Zamarreño <[hidden email]>:

> Hi,
>
> Re: https://issues.jboss.org/browse/ISPN-293
>
> I have an issue with my implementation that simply wraps the realRemoteGet in DistributionInterceptor around a Callable:
>
> Assume that cache is configured with Distribution(numOwners=1, l1=enabled), no transactions, and we have a cluster of 2 nodes:
>
> - [main-thread] Put k0 in a cache that should own it.
> - [main-thread] Do a getAsync for k0 from a node that does not own it:
> - [async-thread] This leads to a remote get which updates the L1 and updates the context created by the main-thread and putting the updated entry in there (however, this thread does not release the locks)
> - [main-thread] Next up, we put a new key, i.e. k1, in the node that didn't own k0 (the node where we updated the L1 cache):
> - Now this thread has used the same context that the async thread used, so when it comes to releasing locks, it finds two entries in the context, the one locked by the async-thread and one for the main-thread and it fails with java.lang.IllegalMonitorStateException

Hi Galder, please note I'm working on ISPN-919 : my task in this case
is to have the IllegalMonitorStateException ignored/swallowed; my
patch is trivial, my pull request was delayed yesterday just because I
couldn't create a proper test and I'm still figuring out which
scenarios trigger my stacktraces.

I don't think this eases/solves you problem, as I guess you still will
need to do all lock releases in the separate thread, but remember this
behaviour might change pretty soon.

For Hibernate's purpose, I'd expect this asyncGet to share the same
transaction as [main-thread] and release locks / commit at the same
time - I hope that's possible?

Regards,
Sanne


>
> Now, what's happening here is the async-thread is acquiring the lock but not releasing it because the release happens in the DistLockInterceptor/LockingInterceptor, so a different interceptor to where the lock is being acquired. So, in theory, the solution would be for DistLockInterceptor to wrap the invokeNext() and afterCall() for when an async get so that all "remote get", "l1 update", and "release locks", happen in a separate thread. However, for this to work, it will need to figure out whether a remoteGet() will be necessary in the first place, otherwise is useless. Whether the remoteGet should happen is determined by this code in DistributionInterceptor:
>
> if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) {
>
> Also, if DistLockInterceptor does this check, we need to make sure DistributionInterceptor does not do it again, otherwise it's a waste. I think this might work although it does require some further reengineering.
>
> I'm gonna try to implement this but wondering whether anyone can see any potential flaws here, or if anyone has any better ideas :)
>
> Cheers,
> --
> Galder Zamarreño
> Sr. Software Engineer
> Infinispan, JBoss Cache
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Galder Zamarreno
In reply to this post by Mircea Markus

On Feb 9, 2011, at 10:54 AM, Mircea Markus wrote:

>
> On 9 Feb 2011, at 08:14, Galder Zamarreño wrote:
>
>> Hi,
>>
>> Re: https://issues.jboss.org/browse/ISPN-293
>>
>> I have an issue with my implementation that simply wraps the realRemoteGet in DistributionInterceptor around a Callable:
>>
>> Assume that cache is configured with Distribution(numOwners=1, l1=enabled), no transactions, and we have a cluster of 2 nodes:
>>
>> - [main-thread] Put k0 in a cache that should own it.
>> - [main-thread] Do a getAsync for k0 from a node that does not own it:
>> - [async-thread] This leads to a remote get which updates the L1 and updates the context created by the main-thread and putting the updated entry in there (however, this thread does not release the locks)
> Can't you use a different InvocationContext instance for this? But even with the same IC, wondering why IC doesn't get cleaned up: InvocationContextInterceptor.handleAll calls ic.reset() in its finally block. Are you starting the async thread after InvocationContextInterceptor?

The async thread does get started after InvocationContextInterceptor because you only want to start it if you have to go remote with the get and right now you only now that in DI. This is consistent with the other async* operations, the only thing that's made sync is when you actually have to go remote:

    * Asynchronous version of {@link #put(Object, Object, long, TimeUnit, long, TimeUnit)}.  This method does not block
    * on remote calls, even if your cache mode is synchronous.  Has no benefit over {@link #put(Object, Object, long,
    * TimeUnit, long, TimeUnit)} if used in LOCAL mode.

Now, using a different IC for when you know that the async thread needs to go remote *and* you need to store in L1 might be a viable option. However, my worry here goes back to locks. So, you'd have created a new IC, then acquired the locks on X, then reset the IC without clearing the locks. Would the fact that you didn't clear the locks cause issues? Hmmmm....

>> - [main-thread] Next up, we put a new key, i.e. k1, in the node that didn't own k0 (the node where we updated the L1 cache):
>> - Now this thread has used the same context that the async thread used,
>> so when it comes to releasing locks, it finds two entries in the context, the one locked by the async-thread and one for the main-thread and it fails with java.lang.IllegalMonitorStateException
>>
>> Now, what's happening here is the async-thread is acquiring the lock but not releasing it because the release happens in the DistLockInterceptor/LockingInterceptor, so a different interceptor to where the lock is being acquired. So, in theory, the solution would be for DistLockInterceptor to wrap the invokeNext() and afterCall() for when an async get so that all "remote get", "l1 update", and "release locks", happen in a separate thread. However, for this to work, it will need to figure out whether a remoteGet() will be necessary in the first place, otherwise is useless. Whether the remoteGet should happen is determined by this code in DistributionInterceptor:
>>
>> if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) {
>>
>> Also, if DistLockInterceptor does this check, we need to make sure DistributionInterceptor does not do it again, otherwise it's a waste. I think this might work although it does require some further reengineering.
>>
>> I'm gonna try to implement this but wondering whether anyone can see any potential flaws here, or if anyone has any better ideas :)
> I might be missing something but don't we already do the same thing for asyncPut operationd and can't we follow the exact same pattern?

No, it's a very different one. In the rest of async* ops, the thing that's made async is the actual sending of the replication, once you've already updated the local cache. So, you never find yourself with having to update the local cache in the async thread.

But the problem here is that after the async get, more stuff happens (L1 update), so as Manik says in the JIRA, you can't just do the remote get and forget to update the L1 cache. So, this forces you to do a put within the async thread.

I think the solution might involve a combination of my suggestion to make sure locks are clear *and* use a different context for the different thread (only when L1 is enabled)

>>
>> Cheers,
>> --
>> Galder Zamarreño
>> Sr. Software Engineer
>> Infinispan, JBoss Cache
>>
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Manik Surtani-2
In reply to this post by Galder Zamarreno
What about putting the entire call on a separate thread much earlier in the call stack? E.g., in the CacheDelegate? You get the added benefit of any cache loading happening offline as well. Plus a much simpler impl. :)

Sent from my iPhone

On 9 Feb 2011, at 08:14, Galder Zamarreño <[hidden email]> wrote:

> Hi,
>
> Re: https://issues.jboss.org/browse/ISPN-293
>
> I have an issue with my implementation that simply wraps the realRemoteGet in DistributionInterceptor around a Callable:
>
> Assume that cache is configured with Distribution(numOwners=1, l1=enabled), no transactions, and we have a cluster of 2 nodes:
>
> - [main-thread] Put k0 in a cache that should own it.
> - [main-thread] Do a getAsync for k0 from a node that does not own it:
> - [async-thread] This leads to a remote get which updates the L1 and updates the context created by the main-thread and putting the updated entry in there (however, this thread does not release the locks)
> - [main-thread] Next up, we put a new key, i.e. k1, in the node that didn't own k0 (the node where we updated the L1 cache):
> - Now this thread has used the same context that the async thread used, so when it comes to releasing locks, it finds two entries in the context, the one locked by the async-thread and one for the main-thread and it fails with java.lang.IllegalMonitorStateException
>
> Now, what's happening here is the async-thread is acquiring the lock but not releasing it because the release happens in the DistLockInterceptor/LockingInterceptor, so a different interceptor to where the lock is being acquired. So, in theory, the solution would be for DistLockInterceptor to wrap the invokeNext() and afterCall() for when an async get so that all "remote get", "l1 update", and "release locks", happen in a separate thread. However, for this to work, it will need to figure out whether a remoteGet() will be necessary in the first place, otherwise is useless. Whether the remoteGet should happen is determined by this code in DistributionInterceptor:
>
> if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) {
>
> Also, if DistLockInterceptor does this check, we need to make sure DistributionInterceptor does not do it again, otherwise it's a waste. I think this might work although it does require some further reengineering.
>
> I'm gonna try to implement this but wondering whether anyone can see any potential flaws here, or if anyone has any better ideas :)
>
> Cheers,
> --
> Galder Zamarreño
> Sr. Software Engineer
> Infinispan, JBoss Cache
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Mircea Markus

On 9 Feb 2011, at 12:34, Manik Surtani wrote:

> What about putting the entire call on a separate thread much earlier in the call stack? E.g., in the CacheDelegate? You get the added benefit of any cache loading happening offline as well. Plus a much simpler impl. :)
+1.

>
> Sent from my iPhone
>
> On 9 Feb 2011, at 08:14, Galder Zamarreño <[hidden email]> wrote:
>
>> Hi,
>>
>> Re: https://issues.jboss.org/browse/ISPN-293
>>
>> I have an issue with my implementation that simply wraps the realRemoteGet in DistributionInterceptor around a Callable:
>>
>> Assume that cache is configured with Distribution(numOwners=1, l1=enabled), no transactions, and we have a cluster of 2 nodes:
>>
>> - [main-thread] Put k0 in a cache that should own it.
>> - [main-thread] Do a getAsync for k0 from a node that does not own it:
>> - [async-thread] This leads to a remote get which updates the L1 and updates the context created by the main-thread and putting the updated entry in there (however, this thread does not release the locks)
>> - [main-thread] Next up, we put a new key, i.e. k1, in the node that didn't own k0 (the node where we updated the L1 cache):
>> - Now this thread has used the same context that the async thread used, so when it comes to releasing locks, it finds two entries in the context, the one locked by the async-thread and one for the main-thread and it fails with java.lang.IllegalMonitorStateException
>>
>> Now, what's happening here is the async-thread is acquiring the lock but not releasing it because the release happens in the DistLockInterceptor/LockingInterceptor, so a different interceptor to where the lock is being acquired. So, in theory, the solution would be for DistLockInterceptor to wrap the invokeNext() and afterCall() for when an async get so that all "remote get", "l1 update", and "release locks", happen in a separate thread. However, for this to work, it will need to figure out whether a remoteGet() will be necessary in the first place, otherwise is useless. Whether the remoteGet should happen is determined by this code in DistributionInterceptor:
>>
>> if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) {
>>
>> Also, if DistLockInterceptor does this check, we need to make sure DistributionInterceptor does not do it again, otherwise it's a waste. I think this might work although it does require some further reengineering.
>>
>> I'm gonna try to implement this but wondering whether anyone can see any potential flaws here, or if anyone has any better ideas :)
>>
>> Cheers,
>> --
>> Galder Zamarreño
>> Sr. Software Engineer
>> Infinispan, JBoss Cache
>>
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Galder Zamarreno
The problem that I see with that is getAsync() calls that should resolve locally will get the penalty of the thread context switching.

I think I have a workable solution without having to rely on your suggestion which passes the test suite. I'll send a pull request and you (Mircea+Manik) can have a look to it, and then we can decide :)

On Feb 9, 2011, at 2:32 PM, Mircea Markus wrote:

>
> On 9 Feb 2011, at 12:34, Manik Surtani wrote:
>
>> What about putting the entire call on a separate thread much earlier in the call stack? E.g., in the CacheDelegate? You get the added benefit of any cache loading happening offline as well. Plus a much simpler impl. :)
> +1.
>>
>> Sent from my iPhone
>>
>> On 9 Feb 2011, at 08:14, Galder Zamarreño <[hidden email]> wrote:
>>
>>> Hi,
>>>
>>> Re: https://issues.jboss.org/browse/ISPN-293
>>>
>>> I have an issue with my implementation that simply wraps the realRemoteGet in DistributionInterceptor around a Callable:
>>>
>>> Assume that cache is configured with Distribution(numOwners=1, l1=enabled), no transactions, and we have a cluster of 2 nodes:
>>>
>>> - [main-thread] Put k0 in a cache that should own it.
>>> - [main-thread] Do a getAsync for k0 from a node that does not own it:
>>> - [async-thread] This leads to a remote get which updates the L1 and updates the context created by the main-thread and putting the updated entry in there (however, this thread does not release the locks)
>>> - [main-thread] Next up, we put a new key, i.e. k1, in the node that didn't own k0 (the node where we updated the L1 cache):
>>> - Now this thread has used the same context that the async thread used, so when it comes to releasing locks, it finds two entries in the context, the one locked by the async-thread and one for the main-thread and it fails with java.lang.IllegalMonitorStateException
>>>
>>> Now, what's happening here is the async-thread is acquiring the lock but not releasing it because the release happens in the DistLockInterceptor/LockingInterceptor, so a different interceptor to where the lock is being acquired. So, in theory, the solution would be for DistLockInterceptor to wrap the invokeNext() and afterCall() for when an async get so that all "remote get", "l1 update", and "release locks", happen in a separate thread. However, for this to work, it will need to figure out whether a remoteGet() will be necessary in the first place, otherwise is useless. Whether the remoteGet should happen is determined by this code in DistributionInterceptor:
>>>
>>> if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) {
>>>
>>> Also, if DistLockInterceptor does this check, we need to make sure DistributionInterceptor does not do it again, otherwise it's a waste. I think this might work although it does require some further reengineering.
>>>
>>> I'm gonna try to implement this but wondering whether anyone can see any potential flaws here, or if anyone has any better ideas :)
>>>
>>> Cheers,
>>> --
>>> Galder Zamarreño
>>> Sr. Software Engineer
>>> Infinispan, JBoss Cache
>>>
>>>
>>> _______________________________________________
>>> infinispan-dev mailing list
>>> [hidden email]
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Galder Zamarreno
Hmmmm, I've thought this further. The penalty of the thread ctx switching is probably not that bad actually cos the point of getAsync() is the ability to paralelise rather than how quickly getAsync() returns.

On Feb 9, 2011, at 4:38 PM, Galder Zamarreño wrote:

> The problem that I see with that is getAsync() calls that should resolve locally will get the penalty of the thread context switching.
>
> I think I have a workable solution without having to rely on your suggestion which passes the test suite. I'll send a pull request and you (Mircea+Manik) can have a look to it, and then we can decide :)
>
> On Feb 9, 2011, at 2:32 PM, Mircea Markus wrote:
>
>>
>> On 9 Feb 2011, at 12:34, Manik Surtani wrote:
>>
>>> What about putting the entire call on a separate thread much earlier in the call stack? E.g., in the CacheDelegate? You get the added benefit of any cache loading happening offline as well. Plus a much simpler impl. :)
>> +1.
>>>
>>> Sent from my iPhone
>>>
>>> On 9 Feb 2011, at 08:14, Galder Zamarreño <[hidden email]> wrote:
>>>
>>>> Hi,
>>>>
>>>> Re: https://issues.jboss.org/browse/ISPN-293
>>>>
>>>> I have an issue with my implementation that simply wraps the realRemoteGet in DistributionInterceptor around a Callable:
>>>>
>>>> Assume that cache is configured with Distribution(numOwners=1, l1=enabled), no transactions, and we have a cluster of 2 nodes:
>>>>
>>>> - [main-thread] Put k0 in a cache that should own it.
>>>> - [main-thread] Do a getAsync for k0 from a node that does not own it:
>>>> - [async-thread] This leads to a remote get which updates the L1 and updates the context created by the main-thread and putting the updated entry in there (however, this thread does not release the locks)
>>>> - [main-thread] Next up, we put a new key, i.e. k1, in the node that didn't own k0 (the node where we updated the L1 cache):
>>>> - Now this thread has used the same context that the async thread used, so when it comes to releasing locks, it finds two entries in the context, the one locked by the async-thread and one for the main-thread and it fails with java.lang.IllegalMonitorStateException
>>>>
>>>> Now, what's happening here is the async-thread is acquiring the lock but not releasing it because the release happens in the DistLockInterceptor/LockingInterceptor, so a different interceptor to where the lock is being acquired. So, in theory, the solution would be for DistLockInterceptor to wrap the invokeNext() and afterCall() for when an async get so that all "remote get", "l1 update", and "release locks", happen in a separate thread. However, for this to work, it will need to figure out whether a remoteGet() will be necessary in the first place, otherwise is useless. Whether the remoteGet should happen is determined by this code in DistributionInterceptor:
>>>>
>>>> if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) {
>>>>
>>>> Also, if DistLockInterceptor does this check, we need to make sure DistributionInterceptor does not do it again, otherwise it's a waste. I think this might work although it does require some further reengineering.
>>>>
>>>> I'm gonna try to implement this but wondering whether anyone can see any potential flaws here, or if anyone has any better ideas :)
>>>>
>>>> Cheers,
>>>> --
>>>> Galder Zamarreño
>>>> Sr. Software Engineer
>>>> Infinispan, JBoss Cache
>>>>
>>>>
>>>> _______________________________________________
>>>> infinispan-dev mailing list
>>>> [hidden email]
>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>
>>> _______________________________________________
>>> infinispan-dev mailing list
>>> [hidden email]
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> --
> Galder Zamarreño
> Sr. Software Engineer
> Infinispan, JBoss Cache
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Mircea Markus
On 9 Feb 2011, at 17:35, Galder Zamarreño wrote:

> Hmmmm, I've thought this further. The penalty of the thread ctx switching is probably not that bad actually cos the point of getAsync() is the ability to paralelise rather than how quickly getAsync() returns.
Or a local get can be forced(flag) before giving control(if local value doesn't exist) to the new thread.

>
> On Feb 9, 2011, at 4:38 PM, Galder Zamarreño wrote:
>
>> The problem that I see with that is getAsync() calls that should resolve locally will get the penalty of the thread context switching.
>>
>> I think I have a workable solution without having to rely on your suggestion which passes the test suite. I'll send a pull request and you (Mircea+Manik) can have a look to it, and then we can decide :)
>>
>> On Feb 9, 2011, at 2:32 PM, Mircea Markus wrote:
>>
>>>
>>> On 9 Feb 2011, at 12:34, Manik Surtani wrote:
>>>
>>>> What about putting the entire call on a separate thread much earlier in the call stack? E.g., in the CacheDelegate? You get the added benefit of any cache loading happening offline as well. Plus a much simpler impl. :)
>>> +1.
>>>>
>>>> Sent from my iPhone
>>>>
>>>> On 9 Feb 2011, at 08:14, Galder Zamarreño <[hidden email]> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Re: https://issues.jboss.org/browse/ISPN-293
>>>>>
>>>>> I have an issue with my implementation that simply wraps the realRemoteGet in DistributionInterceptor around a Callable:
>>>>>
>>>>> Assume that cache is configured with Distribution(numOwners=1, l1=enabled), no transactions, and we have a cluster of 2 nodes:
>>>>>
>>>>> - [main-thread] Put k0 in a cache that should own it.
>>>>> - [main-thread] Do a getAsync for k0 from a node that does not own it:
>>>>> - [async-thread] This leads to a remote get which updates the L1 and updates the context created by the main-thread and putting the updated entry in there (however, this thread does not release the locks)
>>>>> - [main-thread] Next up, we put a new key, i.e. k1, in the node that didn't own k0 (the node where we updated the L1 cache):
>>>>> - Now this thread has used the same context that the async thread used, so when it comes to releasing locks, it finds two entries in the context, the one locked by the async-thread and one for the main-thread and it fails with java.lang.IllegalMonitorStateException
>>>>>
>>>>> Now, what's happening here is the async-thread is acquiring the lock but not releasing it because the release happens in the DistLockInterceptor/LockingInterceptor, so a different interceptor to where the lock is being acquired. So, in theory, the solution would be for DistLockInterceptor to wrap the invokeNext() and afterCall() for when an async get so that all "remote get", "l1 update", and "release locks", happen in a separate thread. However, for this to work, it will need to figure out whether a remoteGet() will be necessary in the first place, otherwise is useless. Whether the remoteGet should happen is determined by this code in DistributionInterceptor:
>>>>>
>>>>> if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) {
>>>>>
>>>>> Also, if DistLockInterceptor does this check, we need to make sure DistributionInterceptor does not do it again, otherwise it's a waste. I think this might work although it does require some further reengineering.
>>>>>
>>>>> I'm gonna try to implement this but wondering whether anyone can see any potential flaws here, or if anyone has any better ideas :)
>>>>>
>>>>> Cheers,
>>>>> --
>>>>> Galder Zamarreño
>>>>> Sr. Software Engineer
>>>>> Infinispan, JBoss Cache
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> infinispan-dev mailing list
>>>>> [hidden email]
>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>>
>>>> _______________________________________________
>>>> infinispan-dev mailing list
>>>> [hidden email]
>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>
>>>
>>> _______________________________________________
>>> infinispan-dev mailing list
>>> [hidden email]
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>
>> --
>> Galder Zamarreño
>> Sr. Software Engineer
>> Infinispan, JBoss Cache
>>
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> --
> Galder Zamarreño
> Sr. Software Engineer
> Infinispan, JBoss Cache
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Galder Zamarreno
That's a good optimisation Mircea, I've added it.

There's another important problem to solve here which is, how do you make getAsync() maintain the transaction semantics of the incoming thread? getAsync does not acquire locks except in the case where the remote get leads to a L1 put in which case a write lock is acquired on entry put on L1.

The main problem here is how to make the transaction propagate from one thread to the other? I tried to pass the current thread's transaction to the getAsync() thread and create an invocation context using it. This doesn't work cos LocalTxInvocationContext does not keep the current transaction, instead it keeps the localTransaction which for 1st invocation is null.

The Infinispan code relies on TransactionManager.getTransaction() which most likely relies on a ThreadLocal being set to the current transaction. Once you switch threads, this method returns null, and with the solution attempted solution I get:

Caused by: java.lang.IllegalStateException: This should only be called in an tx scope
        at org.infinispan.interceptors.TxInterceptor.enlist(TxInterceptor.java:193)
        at org.infinispan.interceptors.TxInterceptor.enlistReadAndInvokeNext(TxInterceptor.java:167)
        at org.infinispan.interceptors.TxInterceptor.visitGetKeyValueCommand(TxInterceptor.java:162)

So, what I've done is get the InvocationContextContainer to take the passed transaction and assign it to the local tx invocation context, and then use this transaction rather than the one from tm.getTransaction() in the enlist method.

I've created a pull request with this solution and you can find it in: https://github.com/infinispan/infinispan/pull/160

Note that I've made a couple of TODOs in AbstractTxInvocationContext in order to discuss potentially naming getTransaction/setTransaction differently and see if we wanna merge somehow with getRunningTransaction() available in TxInvocationContext.

Cheers,

On Feb 9, 2011, at 6:42 PM, Mircea Markus wrote:

> On 9 Feb 2011, at 17:35, Galder Zamarreño wrote:
>
>> Hmmmm, I've thought this further. The penalty of the thread ctx switching is probably not that bad actually cos the point of getAsync() is the ability to paralelise rather than how quickly getAsync() returns.
> Or a local get can be forced(flag) before giving control(if local value doesn't exist) to the new thread.
>>
>> On Feb 9, 2011, at 4:38 PM, Galder Zamarreño wrote:
>>
>>> The problem that I see with that is getAsync() calls that should resolve locally will get the penalty of the thread context switching.
>>>
>>> I think I have a workable solution without having to rely on your suggestion which passes the test suite. I'll send a pull request and you (Mircea+Manik) can have a look to it, and then we can decide :)
>>>
>>> On Feb 9, 2011, at 2:32 PM, Mircea Markus wrote:
>>>
>>>>
>>>> On 9 Feb 2011, at 12:34, Manik Surtani wrote:
>>>>
>>>>> What about putting the entire call on a separate thread much earlier in the call stack? E.g., in the CacheDelegate? You get the added benefit of any cache loading happening offline as well. Plus a much simpler impl. :)
>>>> +1.
>>>>>
>>>>> Sent from my iPhone
>>>>>
>>>>> On 9 Feb 2011, at 08:14, Galder Zamarreño <[hidden email]> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Re: https://issues.jboss.org/browse/ISPN-293
>>>>>>
>>>>>> I have an issue with my implementation that simply wraps the realRemoteGet in DistributionInterceptor around a Callable:
>>>>>>
>>>>>> Assume that cache is configured with Distribution(numOwners=1, l1=enabled), no transactions, and we have a cluster of 2 nodes:
>>>>>>
>>>>>> - [main-thread] Put k0 in a cache that should own it.
>>>>>> - [main-thread] Do a getAsync for k0 from a node that does not own it:
>>>>>> - [async-thread] This leads to a remote get which updates the L1 and updates the context created by the main-thread and putting the updated entry in there (however, this thread does not release the locks)
>>>>>> - [main-thread] Next up, we put a new key, i.e. k1, in the node that didn't own k0 (the node where we updated the L1 cache):
>>>>>> - Now this thread has used the same context that the async thread used, so when it comes to releasing locks, it finds two entries in the context, the one locked by the async-thread and one for the main-thread and it fails with java.lang.IllegalMonitorStateException
>>>>>>
>>>>>> Now, what's happening here is the async-thread is acquiring the lock but not releasing it because the release happens in the DistLockInterceptor/LockingInterceptor, so a different interceptor to where the lock is being acquired. So, in theory, the solution would be for DistLockInterceptor to wrap the invokeNext() and afterCall() for when an async get so that all "remote get", "l1 update", and "release locks", happen in a separate thread. However, for this to work, it will need to figure out whether a remoteGet() will be necessary in the first place, otherwise is useless. Whether the remoteGet should happen is determined by this code in DistributionInterceptor:
>>>>>>
>>>>>> if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) {
>>>>>>
>>>>>> Also, if DistLockInterceptor does this check, we need to make sure DistributionInterceptor does not do it again, otherwise it's a waste. I think this might work although it does require some further reengineering.
>>>>>>
>>>>>> I'm gonna try to implement this but wondering whether anyone can see any potential flaws here, or if anyone has any better ideas :)
>>>>>>
>>>>>> Cheers,
>>>>>> --
>>>>>> Galder Zamarreño
>>>>>> Sr. Software Engineer
>>>>>> Infinispan, JBoss Cache
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> infinispan-dev mailing list
>>>>>> [hidden email]
>>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>>>
>>>>> _______________________________________________
>>>>> infinispan-dev mailing list
>>>>> [hidden email]
>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>>
>>>>
>>>> _______________________________________________
>>>> infinispan-dev mailing list
>>>> [hidden email]
>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>
>>> --
>>> Galder Zamarreño
>>> Sr. Software Engineer
>>> Infinispan, JBoss Cache
>>>
>>>
>>> _______________________________________________
>>> infinispan-dev mailing list
>>> [hidden email]
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>
>> --
>> Galder Zamarreño
>> Sr. Software Engineer
>> Infinispan, JBoss Cache
>>
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Manik Surtani
How do other *Async() calls deal with jta, if at all?

On 10 Feb 2011, at 11:10, Galder Zamarreño wrote:

> That's a good optimisation Mircea, I've added it.
>
> There's another important problem to solve here which is, how do you make getAsync() maintain the transaction semantics of the incoming thread? getAsync does not acquire locks except in the case where the remote get leads to a L1 put in which case a write lock is acquired on entry put on L1.
>
> The main problem here is how to make the transaction propagate from one thread to the other? I tried to pass the current thread's transaction to the getAsync() thread and create an invocation context using it. This doesn't work cos LocalTxInvocationContext does not keep the current transaction, instead it keeps the localTransaction which for 1st invocation is null.
>
> The Infinispan code relies on TransactionManager.getTransaction() which most likely relies on a ThreadLocal being set to the current transaction. Once you switch threads, this method returns null, and with the solution attempted solution I get:
>
> Caused by: java.lang.IllegalStateException: This should only be called in an tx scope
> at org.infinispan.interceptors.TxInterceptor.enlist(TxInterceptor.java:193)
> at org.infinispan.interceptors.TxInterceptor.enlistReadAndInvokeNext(TxInterceptor.java:167)
> at org.infinispan.interceptors.TxInterceptor.visitGetKeyValueCommand(TxInterceptor.java:162)
>
> So, what I've done is get the InvocationContextContainer to take the passed transaction and assign it to the local tx invocation context, and then use this transaction rather than the one from tm.getTransaction() in the enlist method.
>
> I've created a pull request with this solution and you can find it in: https://github.com/infinispan/infinispan/pull/160
>
> Note that I've made a couple of TODOs in AbstractTxInvocationContext in order to discuss potentially naming getTransaction/setTransaction differently and see if we wanna merge somehow with getRunningTransaction() available in TxInvocationContext.
>
> Cheers,
>
> On Feb 9, 2011, at 6:42 PM, Mircea Markus wrote:
>
>> On 9 Feb 2011, at 17:35, Galder Zamarreño wrote:
>>
>>> Hmmmm, I've thought this further. The penalty of the thread ctx switching is probably not that bad actually cos the point of getAsync() is the ability to paralelise rather than how quickly getAsync() returns.
>> Or a local get can be forced(flag) before giving control(if local value doesn't exist) to the new thread.
>>>
>>> On Feb 9, 2011, at 4:38 PM, Galder Zamarreño wrote:
>>>
>>>> The problem that I see with that is getAsync() calls that should resolve locally will get the penalty of the thread context switching.
>>>>
>>>> I think I have a workable solution without having to rely on your suggestion which passes the test suite. I'll send a pull request and you (Mircea+Manik) can have a look to it, and then we can decide :)
>>>>
>>>> On Feb 9, 2011, at 2:32 PM, Mircea Markus wrote:
>>>>
>>>>>
>>>>> On 9 Feb 2011, at 12:34, Manik Surtani wrote:
>>>>>
>>>>>> What about putting the entire call on a separate thread much earlier in the call stack? E.g., in the CacheDelegate? You get the added benefit of any cache loading happening offline as well. Plus a much simpler impl. :)
>>>>> +1.
>>>>>>
>>>>>> Sent from my iPhone
>>>>>>
>>>>>> On 9 Feb 2011, at 08:14, Galder Zamarreño <[hidden email]> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Re: https://issues.jboss.org/browse/ISPN-293
>>>>>>>
>>>>>>> I have an issue with my implementation that simply wraps the realRemoteGet in DistributionInterceptor around a Callable:
>>>>>>>
>>>>>>> Assume that cache is configured with Distribution(numOwners=1, l1=enabled), no transactions, and we have a cluster of 2 nodes:
>>>>>>>
>>>>>>> - [main-thread] Put k0 in a cache that should own it.
>>>>>>> - [main-thread] Do a getAsync for k0 from a node that does not own it:
>>>>>>> - [async-thread] This leads to a remote get which updates the L1 and updates the context created by the main-thread and putting the updated entry in there (however, this thread does not release the locks)
>>>>>>> - [main-thread] Next up, we put a new key, i.e. k1, in the node that didn't own k0 (the node where we updated the L1 cache):
>>>>>>> - Now this thread has used the same context that the async thread used, so when it comes to releasing locks, it finds two entries in the context, the one locked by the async-thread and one for the main-thread and it fails with java.lang.IllegalMonitorStateException
>>>>>>>
>>>>>>> Now, what's happening here is the async-thread is acquiring the lock but not releasing it because the release happens in the DistLockInterceptor/LockingInterceptor, so a different interceptor to where the lock is being acquired. So, in theory, the solution would be for DistLockInterceptor to wrap the invokeNext() and afterCall() for when an async get so that all "remote get", "l1 update", and "release locks", happen in a separate thread. However, for this to work, it will need to figure out whether a remoteGet() will be necessary in the first place, otherwise is useless. Whether the remoteGet should happen is determined by this code in DistributionInterceptor:
>>>>>>>
>>>>>>> if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) {
>>>>>>>
>>>>>>> Also, if DistLockInterceptor does this check, we need to make sure DistributionInterceptor does not do it again, otherwise it's a waste. I think this might work although it does require some further reengineering.
>>>>>>>
>>>>>>> I'm gonna try to implement this but wondering whether anyone can see any potential flaws here, or if anyone has any better ideas :)
>>>>>>>
>>>>>>> Cheers,
>>>>>>> --
>>>>>>> Galder Zamarreño
>>>>>>> Sr. Software Engineer
>>>>>>> Infinispan, JBoss Cache
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> infinispan-dev mailing list
>>>>>>> [hidden email]
>>>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>>>>
>>>>>> _______________________________________________
>>>>>> infinispan-dev mailing list
>>>>>> [hidden email]
>>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> infinispan-dev mailing list
>>>>> [hidden email]
>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>>
>>>> --
>>>> Galder Zamarreño
>>>> Sr. Software Engineer
>>>> Infinispan, JBoss Cache
>>>>
>>>>
>>>> _______________________________________________
>>>> infinispan-dev mailing list
>>>> [hidden email]
>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>
>>> --
>>> Galder Zamarreño
>>> Sr. Software Engineer
>>> Infinispan, JBoss Cache
>>>
>>>
>>> _______________________________________________
>>> infinispan-dev mailing list
>>> [hidden email]
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> --
> Galder Zamarreño
> Sr. Software Engineer
> Infinispan, JBoss Cache
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Manik Surtani
[hidden email]
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Galder Zamarreno
The other *Async() limit themselves to making the RPC call in a separate thread, i.e. putAsync.

So, by the time they have to go remote to replicate something, they've already acquire the locks within the user thread and the transaction is in prepare state.

The PrepareCommand should contains a GlobalTransaction which transforms into a RemoteTransaction in the other node.

I don't see the need to transfer actual javax.transaction.Transaction information with these commands.

Btw, I've just been talking to Mircea and we're considering passing around the LocalTransaction between the caller and the getAsync thread rather than passing javax.transaction.Transaction which we don't have control over.

On Feb 10, 2011, at 7:09 PM, Manik Surtani wrote:

> How do other *Async() calls deal with jta, if at all?
>
> On 10 Feb 2011, at 11:10, Galder Zamarreño wrote:
>
>> That's a good optimisation Mircea, I've added it.
>>
>> There's another important problem to solve here which is, how do you make getAsync() maintain the transaction semantics of the incoming thread? getAsync does not acquire locks except in the case where the remote get leads to a L1 put in which case a write lock is acquired on entry put on L1.
>>
>> The main problem here is how to make the transaction propagate from one thread to the other? I tried to pass the current thread's transaction to the getAsync() thread and create an invocation context using it. This doesn't work cos LocalTxInvocationContext does not keep the current transaction, instead it keeps the localTransaction which for 1st invocation is null.
>>
>> The Infinispan code relies on TransactionManager.getTransaction() which most likely relies on a ThreadLocal being set to the current transaction. Once you switch threads, this method returns null, and with the solution attempted solution I get:
>>
>> Caused by: java.lang.IllegalStateException: This should only be called in an tx scope
>> at org.infinispan.interceptors.TxInterceptor.enlist(TxInterceptor.java:193)
>> at org.infinispan.interceptors.TxInterceptor.enlistReadAndInvokeNext(TxInterceptor.java:167)
>> at org.infinispan.interceptors.TxInterceptor.visitGetKeyValueCommand(TxInterceptor.java:162)
>>
>> So, what I've done is get the InvocationContextContainer to take the passed transaction and assign it to the local tx invocation context, and then use this transaction rather than the one from tm.getTransaction() in the enlist method.
>>
>> I've created a pull request with this solution and you can find it in: https://github.com/infinispan/infinispan/pull/160
>>
>> Note that I've made a couple of TODOs in AbstractTxInvocationContext in order to discuss potentially naming getTransaction/setTransaction differently and see if we wanna merge somehow with getRunningTransaction() available in TxInvocationContext.
>>
>> Cheers,
>>
>> On Feb 9, 2011, at 6:42 PM, Mircea Markus wrote:
>>
>>> On 9 Feb 2011, at 17:35, Galder Zamarreño wrote:
>>>
>>>> Hmmmm, I've thought this further. The penalty of the thread ctx switching is probably not that bad actually cos the point of getAsync() is the ability to paralelise rather than how quickly getAsync() returns.
>>> Or a local get can be forced(flag) before giving control(if local value doesn't exist) to the new thread.
>>>>
>>>> On Feb 9, 2011, at 4:38 PM, Galder Zamarreño wrote:
>>>>
>>>>> The problem that I see with that is getAsync() calls that should resolve locally will get the penalty of the thread context switching.
>>>>>
>>>>> I think I have a workable solution without having to rely on your suggestion which passes the test suite. I'll send a pull request and you (Mircea+Manik) can have a look to it, and then we can decide :)
>>>>>
>>>>> On Feb 9, 2011, at 2:32 PM, Mircea Markus wrote:
>>>>>
>>>>>>
>>>>>> On 9 Feb 2011, at 12:34, Manik Surtani wrote:
>>>>>>
>>>>>>> What about putting the entire call on a separate thread much earlier in the call stack? E.g., in the CacheDelegate? You get the added benefit of any cache loading happening offline as well. Plus a much simpler impl. :)
>>>>>> +1.
>>>>>>>
>>>>>>> Sent from my iPhone
>>>>>>>
>>>>>>> On 9 Feb 2011, at 08:14, Galder Zamarreño <[hidden email]> wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Re: https://issues.jboss.org/browse/ISPN-293
>>>>>>>>
>>>>>>>> I have an issue with my implementation that simply wraps the realRemoteGet in DistributionInterceptor around a Callable:
>>>>>>>>
>>>>>>>> Assume that cache is configured with Distribution(numOwners=1, l1=enabled), no transactions, and we have a cluster of 2 nodes:
>>>>>>>>
>>>>>>>> - [main-thread] Put k0 in a cache that should own it.
>>>>>>>> - [main-thread] Do a getAsync for k0 from a node that does not own it:
>>>>>>>> - [async-thread] This leads to a remote get which updates the L1 and updates the context created by the main-thread and putting the updated entry in there (however, this thread does not release the locks)
>>>>>>>> - [main-thread] Next up, we put a new key, i.e. k1, in the node that didn't own k0 (the node where we updated the L1 cache):
>>>>>>>> - Now this thread has used the same context that the async thread used, so when it comes to releasing locks, it finds two entries in the context, the one locked by the async-thread and one for the main-thread and it fails with java.lang.IllegalMonitorStateException
>>>>>>>>
>>>>>>>> Now, what's happening here is the async-thread is acquiring the lock but not releasing it because the release happens in the DistLockInterceptor/LockingInterceptor, so a different interceptor to where the lock is being acquired. So, in theory, the solution would be for DistLockInterceptor to wrap the invokeNext() and afterCall() for when an async get so that all "remote get", "l1 update", and "release locks", happen in a separate thread. However, for this to work, it will need to figure out whether a remoteGet() will be necessary in the first place, otherwise is useless. Whether the remoteGet should happen is determined by this code in DistributionInterceptor:
>>>>>>>>
>>>>>>>> if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) {
>>>>>>>>
>>>>>>>> Also, if DistLockInterceptor does this check, we need to make sure DistributionInterceptor does not do it again, otherwise it's a waste. I think this might work although it does require some further reengineering.
>>>>>>>>
>>>>>>>> I'm gonna try to implement this but wondering whether anyone can see any potential flaws here, or if anyone has any better ideas :)
>>>>>>>>
>>>>>>>> Cheers,
>>>>>>>> --
>>>>>>>> Galder Zamarreño
>>>>>>>> Sr. Software Engineer
>>>>>>>> Infinispan, JBoss Cache
>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> infinispan-dev mailing list
>>>>>>>> [hidden email]
>>>>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> infinispan-dev mailing list
>>>>>>> [hidden email]
>>>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> infinispan-dev mailing list
>>>>>> [hidden email]
>>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>>>
>>>>> --
>>>>> Galder Zamarreño
>>>>> Sr. Software Engineer
>>>>> Infinispan, JBoss Cache
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> infinispan-dev mailing list
>>>>> [hidden email]
>>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>>
>>>> --
>>>> Galder Zamarreño
>>>> Sr. Software Engineer
>>>> Infinispan, JBoss Cache
>>>>
>>>>
>>>> _______________________________________________
>>>> infinispan-dev mailing list
>>>> [hidden email]
>>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>>
>>>
>>> _______________________________________________
>>> infinispan-dev mailing list
>>> [hidden email]
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>
>> --
>> Galder Zamarreño
>> Sr. Software Engineer
>> Infinispan, JBoss Cache
>>
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> --
> Manik Surtani
> [hidden email]
> twitter.com/maniksurtani
>
> Lead, Infinispan
> http://www.infinispan.org
>
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Sanne Grinovero
2011/2/11 Galder Zamarreño <[hidden email]>:
> The other *Async() limit themselves to making the RPC call in a separate thread, i.e. putAsync.
>
> So, by the time they have to go remote to replicate something, they've already acquire the locks within the user thread and the transaction is in prepare state.
>
> The PrepareCommand should contains a GlobalTransaction which transforms into a RemoteTransaction in the other node.
>
> I don't see the need to transfer actual javax.transaction.Transaction information with these commands.

So basically if I abort a transaction during which I performed some
asyncPut() operations, they are not rolled back? I could live with
that, as long as we clearly document this.

>
> Btw, I've just been talking to Mircea and we're considering passing around the LocalTransaction between the caller and the getAsync thread rather than passing javax.transaction.Transaction which we don't have control over.

Jonathan mentioned a way to share transactions across threads using
JBoss TransactionManager; the main problem with this is that while
something similar might be supported by others: it's not standardized.

>
> On Feb 10, 2011, at 7:09 PM, Manik Surtani wrote:
>
>> How do other *Async() calls deal with jta, if at all?
>>
>> On 10 Feb 2011, at 11:10, Galder Zamarreño wrote:
>>
>>> That's a good optimisation Mircea, I've added it.
>>>
>>> There's another important problem to solve here which is, how do you make getAsync() maintain the transaction semantics of the incoming thread? getAsync does not acquire locks except in the case where the remote get leads to a L1 put in which case a write lock is acquired on entry put on L1.
>>>
>>> The main problem here is how to make the transaction propagate from one thread to the other? I tried to pass the current thread's transaction to the getAsync() thread and create an invocation context using it. This doesn't work cos LocalTxInvocationContext does not keep the current transaction, instead it keeps the localTransaction which for 1st invocation is null.
>>>
>>> The Infinispan code relies on TransactionManager.getTransaction() which most likely relies on a ThreadLocal being set to the current transaction. Once you switch threads, this method returns null, and with the solution attempted solution I get:
>>>
>>> Caused by: java.lang.IllegalStateException: This should only be called in an tx scope
>>>      at org.infinispan.interceptors.TxInterceptor.enlist(TxInterceptor.java:193)
>>>      at org.infinispan.interceptors.TxInterceptor.enlistReadAndInvokeNext(TxInterceptor.java:167)
>>>      at org.infinispan.interceptors.TxInterceptor.visitGetKeyValueCommand(TxInterceptor.java:162)
>>>
>>> So, what I've done is get the InvocationContextContainer to take the passed transaction and assign it to the local tx invocation context, and then use this transaction rather than the one from tm.getTransaction() in the enlist method.
>>>
>>> I've created a pull request with this solution and you can find it in: https://github.com/infinispan/infinispan/pull/160
>>>
>>> Note that I've made a couple of TODOs in AbstractTxInvocationContext in order to discuss potentially naming getTransaction/setTransaction differently and see if we wanna merge somehow with getRunningTransaction() available in TxInvocationContext.
>>>
>>> Cheers,
>>>
>>> On Feb 9, 2011, at 6:42 PM, Mircea Markus wrote:
>>>
>>>> On 9 Feb 2011, at 17:35, Galder Zamarreño wrote:
>>>>
>>>>> Hmmmm, I've thought this further. The penalty of the thread ctx switching is probably not that bad actually cos the point of getAsync() is the ability to paralelise rather than how quickly getAsync() returns.
>>>> Or a local get can be forced(flag) before giving control(if local value doesn't exist) to the new thread.
>>>>>
>>>>> On Feb 9, 2011, at 4:38 PM, Galder Zamarreño wrote:
>>>>>
>>>>>> The problem that I see with that is getAsync() calls that should resolve locally will get the penalty of the thread context switching.
>>>>>>
>>>>>> I think I have a workable solution without having to rely on your suggestion which passes the test suite. I'll send a pull request and you (Mircea+Manik) can have a look to it, and then we can decide :)
>>>>>>
>>>>>> On Feb 9, 2011, at 2:32 PM, Mircea Markus wrote:
>>>>>>
>>>>>>>
>>>>>>> On 9 Feb 2011, at 12:34, Manik Surtani wrote:
>>>>>>>
>>>>>>>> What about putting the entire call on a separate thread much earlier in the call stack? E.g., in the CacheDelegate? You get the added benefit of any cache loading happening offline as well. Plus a much simpler impl. :)
>>>>>>> +1.
>>>>>>>>
>>>>>>>> Sent from my iPhone
>>>>>>>>
>>>>>>>> On 9 Feb 2011, at 08:14, Galder Zamarreño <[hidden email]> wrote:
>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> Re: https://issues.jboss.org/browse/ISPN-293
>>>>>>>>>
>>>>>>>>> I have an issue with my implementation that simply wraps the realRemoteGet in DistributionInterceptor around a Callable:
>>>>>>>>>
>>>>>>>>> Assume that cache is configured with Distribution(numOwners=1, l1=enabled), no transactions, and we have a cluster of 2 nodes:
>>>>>>>>>
>>>>>>>>> - [main-thread] Put k0 in a cache that should own it.
>>>>>>>>> - [main-thread] Do a getAsync for k0 from a node that does not own it:
>>>>>>>>> - [async-thread] This leads to a remote get which updates the L1 and updates the context created by the main-thread and putting the updated entry in there (however, this thread does not release the locks)
>>>>>>>>> - [main-thread] Next up, we put a new key, i.e. k1, in the node that didn't own k0 (the node where we updated the L1 cache):
>>>>>>>>> - Now this thread has used the same context that the async thread used, so when it comes to releasing locks, it finds two entries in the context, the one locked by the async-thread and one for the main-thread and it fails with java.lang.IllegalMonitorStateException
>>>>>>>>>
>>>>>>>>> Now, what's happening here is the async-thread is acquiring the lock but not releasing it because the release happens in the DistLockInterceptor/LockingInterceptor, so a different interceptor to where the lock is being acquired. So, in theory, the solution would be for DistLockInterceptor to wrap the invokeNext() and afterCall() for when an async get so that all "remote get", "l1 update", and "release locks", happen in a separate thread. However, for this to work, it will need to figure out whether a remoteGet() will be necessary in the first place, otherwise is useless. Whether the remoteGet should happen is determined by this code in DistributionInterceptor:
>>>>>>>>>
>>>>>>>>> if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) {
>>>>>>>>>
>>>>>>>>> Also, if DistLockInterceptor does this check, we need to make sure DistributionInterceptor does not do it again, otherwise it's a waste. I think this might work although it does require some further reengineering.
>>>>>>>>>
>>>>>>>>> I'm gonna try to implement this but wondering whether anyone can see any potential flaws here, or if anyone has any better ideas :)
>>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>> --
>>>>>>>>> Galder Zamarreño
>>>>>>>>> Sr. Software Engineer
>>>>>>>>> Infinispan, JBoss Cache

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Mircea Markus

On 11 Feb 2011, at 11:20, Sanne Grinovero wrote:

> 2011/2/11 Galder Zamarreño <[hidden email]>:
>> The other *Async() limit themselves to making the RPC call in a separate thread, i.e. putAsync.
>>
>> So, by the time they have to go remote to replicate something, they've already acquire the locks within the user thread and the transaction is in prepare state.
>>
>> The PrepareCommand should contains a GlobalTransaction which transforms into a RemoteTransaction in the other node.
>>
>> I don't see the need to transfer actual javax.transaction.Transaction information with these commands.
>
> So basically if I abort a transaction during which I performed some
> asyncPut() operations, they are not rolled back? I could live with
> that, as long as we clearly document this.
I think this is not good and confusing: asyncGet are transactional and asyncPut are not.
>
>>
>> Btw, I've just been talking to Mircea and we're considering passing around the LocalTransaction between the caller and the getAsync thread rather than passing javax.transaction.Transaction which we don't have control over.
>
> Jonathan mentioned a way to share transactions across threads using
> JBoss TransactionManager;
It's not only the TM that needs to support  that: many resource adapters (infinispan being one of them) don't expect multiple threads working with the same transaction at the same time.
This is a nice feature though, and I do see the use of it.

> the main problem with this is that while
> something similar might be supported by others: it's not standardized.
>
>>
>> On Feb 10, 2011, at 7:09 PM, Manik Surtani wrote:
>>
>>> How do other *Async() calls deal with jta, if at all?
>>>
>>> On 10 Feb 2011, at 11:10, Galder Zamarreño wrote:
>>>
>>>> That's a good optimisation Mircea, I've added it.
>>>>
>>>> There's another important problem to solve here which is, how do you make getAsync() maintain the transaction semantics of the incoming thread? getAsync does not acquire locks except in the case where the remote get leads to a L1 put in which case a write lock is acquired on entry put on L1.
>>>>
>>>> The main problem here is how to make the transaction propagate from one thread to the other? I tried to pass the current thread's transaction to the getAsync() thread and create an invocation context using it. This doesn't work cos LocalTxInvocationContext does not keep the current transaction, instead it keeps the localTransaction which for 1st invocation is null.
>>>>
>>>> The Infinispan code relies on TransactionManager.getTransaction() which most likely relies on a ThreadLocal being set to the current transaction. Once you switch threads, this method returns null, and with the solution attempted solution I get:
>>>>
>>>> Caused by: java.lang.IllegalStateException: This should only be called in an tx scope
>>>>      at org.infinispan.interceptors.TxInterceptor.enlist(TxInterceptor.java:193)
>>>>      at org.infinispan.interceptors.TxInterceptor.enlistReadAndInvokeNext(TxInterceptor.java:167)
>>>>      at org.infinispan.interceptors.TxInterceptor.visitGetKeyValueCommand(TxInterceptor.java:162)
>>>>
>>>> So, what I've done is get the InvocationContextContainer to take the passed transaction and assign it to the local tx invocation context, and then use this transaction rather than the one from tm.getTransaction() in the enlist method.
>>>>
>>>> I've created a pull request with this solution and you can find it in: https://github.com/infinispan/infinispan/pull/160
>>>>
>>>> Note that I've made a couple of TODOs in AbstractTxInvocationContext in order to discuss potentially naming getTransaction/setTransaction differently and see if we wanna merge somehow with getRunningTransaction() available in TxInvocationContext.
>>>>
>>>> Cheers,
>>>>
>>>> On Feb 9, 2011, at 6:42 PM, Mircea Markus wrote:
>>>>
>>>>> On 9 Feb 2011, at 17:35, Galder Zamarreño wrote:
>>>>>
>>>>>> Hmmmm, I've thought this further. The penalty of the thread ctx switching is probably not that bad actually cos the point of getAsync() is the ability to paralelise rather than how quickly getAsync() returns.
>>>>> Or a local get can be forced(flag) before giving control(if local value doesn't exist) to the new thread.
>>>>>>
>>>>>> On Feb 9, 2011, at 4:38 PM, Galder Zamarreño wrote:
>>>>>>
>>>>>>> The problem that I see with that is getAsync() calls that should resolve locally will get the penalty of the thread context switching.
>>>>>>>
>>>>>>> I think I have a workable solution without having to rely on your suggestion which passes the test suite. I'll send a pull request and you (Mircea+Manik) can have a look to it, and then we can decide :)
>>>>>>>
>>>>>>> On Feb 9, 2011, at 2:32 PM, Mircea Markus wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> On 9 Feb 2011, at 12:34, Manik Surtani wrote:
>>>>>>>>
>>>>>>>>> What about putting the entire call on a separate thread much earlier in the call stack? E.g., in the CacheDelegate? You get the added benefit of any cache loading happening offline as well. Plus a much simpler impl. :)
>>>>>>>> +1.
>>>>>>>>>
>>>>>>>>> Sent from my iPhone
>>>>>>>>>
>>>>>>>>> On 9 Feb 2011, at 08:14, Galder Zamarreño <[hidden email]> wrote:
>>>>>>>>>
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Re: https://issues.jboss.org/browse/ISPN-293
>>>>>>>>>>
>>>>>>>>>> I have an issue with my implementation that simply wraps the realRemoteGet in DistributionInterceptor around a Callable:
>>>>>>>>>>
>>>>>>>>>> Assume that cache is configured with Distribution(numOwners=1, l1=enabled), no transactions, and we have a cluster of 2 nodes:
>>>>>>>>>>
>>>>>>>>>> - [main-thread] Put k0 in a cache that should own it.
>>>>>>>>>> - [main-thread] Do a getAsync for k0 from a node that does not own it:
>>>>>>>>>> - [async-thread] This leads to a remote get which updates the L1 and updates the context created by the main-thread and putting the updated entry in there (however, this thread does not release the locks)
>>>>>>>>>> - [main-thread] Next up, we put a new key, i.e. k1, in the node that didn't own k0 (the node where we updated the L1 cache):
>>>>>>>>>> - Now this thread has used the same context that the async thread used, so when it comes to releasing locks, it finds two entries in the context, the one locked by the async-thread and one for the main-thread and it fails with java.lang.IllegalMonitorStateException
>>>>>>>>>>
>>>>>>>>>> Now, what's happening here is the async-thread is acquiring the lock but not releasing it because the release happens in the DistLockInterceptor/LockingInterceptor, so a different interceptor to where the lock is being acquired. So, in theory, the solution would be for DistLockInterceptor to wrap the invokeNext() and afterCall() for when an async get so that all "remote get", "l1 update", and "release locks", happen in a separate thread. However, for this to work, it will need to figure out whether a remoteGet() will be necessary in the first place, otherwise is useless. Whether the remoteGet should happen is determined by this code in DistributionInterceptor:
>>>>>>>>>>
>>>>>>>>>> if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) {
>>>>>>>>>>
>>>>>>>>>> Also, if DistLockInterceptor does this check, we need to make sure DistributionInterceptor does not do it again, otherwise it's a waste. I think this might work although it does require some further reengineering.
>>>>>>>>>>
>>>>>>>>>> I'm gonna try to implement this but wondering whether anyone can see any potential flaws here, or if anyone has any better ideas :)
>>>>>>>>>>
>>>>>>>>>> Cheers,
>>>>>>>>>> --
>>>>>>>>>> Galder Zamarreño
>>>>>>>>>> Sr. Software Engineer
>>>>>>>>>> Infinispan, JBoss Cache
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Sanne Grinovero
In reply to this post by Sanne Grinovero
Just talked to Jonathan, please forget my previous doubts. He had to
leave, you might know this already, but in case this is what he said:

1)Make sure you wait for all Futures to be completed at the end of the
transaction, so that some don't leak out of the scope.

2)It should be enough to enlist the Command in the TM while creating
the Future - before changing thread - so that it carries the proper
XID.
After that, the receiving node is a different node, so it should be
treated as all gets already are: taking part of the remote
transaction.

A consequence of this is that an asyncPut is currently fine with
transactions, as from what you describe only the RPC is async.

sorry if this is obvious to you already,
Sanne

2011/2/11 Sanne Grinovero <[hidden email]>:

> 2011/2/11 Galder Zamarreño <[hidden email]>:
>> The other *Async() limit themselves to making the RPC call in a separate thread, i.e. putAsync.
>>
>> So, by the time they have to go remote to replicate something, they've already acquire the locks within the user thread and the transaction is in prepare state.
>>
>> The PrepareCommand should contains a GlobalTransaction which transforms into a RemoteTransaction in the other node.
>>
>> I don't see the need to transfer actual javax.transaction.Transaction information with these commands.
>
> So basically if I abort a transaction during which I performed some
> asyncPut() operations, they are not rolled back? I could live with
> that, as long as we clearly document this.
>
>>
>> Btw, I've just been talking to Mircea and we're considering passing around the LocalTransaction between the caller and the getAsync thread rather than passing javax.transaction.Transaction which we don't have control over.
>
> Jonathan mentioned a way to share transactions across threads using
> JBoss TransactionManager; the main problem with this is that while
> something similar might be supported by others: it's not standardized.
>
>>
>> On Feb 10, 2011, at 7:09 PM, Manik Surtani wrote:
>>
>>> How do other *Async() calls deal with jta, if at all?
>>>
>>> On 10 Feb 2011, at 11:10, Galder Zamarreño wrote:
>>>
>>>> That's a good optimisation Mircea, I've added it.
>>>>
>>>> There's another important problem to solve here which is, how do you make getAsync() maintain the transaction semantics of the incoming thread? getAsync does not acquire locks except in the case where the remote get leads to a L1 put in which case a write lock is acquired on entry put on L1.
>>>>
>>>> The main problem here is how to make the transaction propagate from one thread to the other? I tried to pass the current thread's transaction to the getAsync() thread and create an invocation context using it. This doesn't work cos LocalTxInvocationContext does not keep the current transaction, instead it keeps the localTransaction which for 1st invocation is null.
>>>>
>>>> The Infinispan code relies on TransactionManager.getTransaction() which most likely relies on a ThreadLocal being set to the current transaction. Once you switch threads, this method returns null, and with the solution attempted solution I get:
>>>>
>>>> Caused by: java.lang.IllegalStateException: This should only be called in an tx scope
>>>>      at org.infinispan.interceptors.TxInterceptor.enlist(TxInterceptor.java:193)
>>>>      at org.infinispan.interceptors.TxInterceptor.enlistReadAndInvokeNext(TxInterceptor.java:167)
>>>>      at org.infinispan.interceptors.TxInterceptor.visitGetKeyValueCommand(TxInterceptor.java:162)
>>>>
>>>> So, what I've done is get the InvocationContextContainer to take the passed transaction and assign it to the local tx invocation context, and then use this transaction rather than the one from tm.getTransaction() in the enlist method.
>>>>
>>>> I've created a pull request with this solution and you can find it in: https://github.com/infinispan/infinispan/pull/160
>>>>
>>>> Note that I've made a couple of TODOs in AbstractTxInvocationContext in order to discuss potentially naming getTransaction/setTransaction differently and see if we wanna merge somehow with getRunningTransaction() available in TxInvocationContext.
>>>>
>>>> Cheers,
>>>>
>>>> On Feb 9, 2011, at 6:42 PM, Mircea Markus wrote:
>>>>
>>>>> On 9 Feb 2011, at 17:35, Galder Zamarreño wrote:
>>>>>
>>>>>> Hmmmm, I've thought this further. The penalty of the thread ctx switching is probably not that bad actually cos the point of getAsync() is the ability to paralelise rather than how quickly getAsync() returns.
>>>>> Or a local get can be forced(flag) before giving control(if local value doesn't exist) to the new thread.
>>>>>>
>>>>>> On Feb 9, 2011, at 4:38 PM, Galder Zamarreño wrote:
>>>>>>
>>>>>>> The problem that I see with that is getAsync() calls that should resolve locally will get the penalty of the thread context switching.
>>>>>>>
>>>>>>> I think I have a workable solution without having to rely on your suggestion which passes the test suite. I'll send a pull request and you (Mircea+Manik) can have a look to it, and then we can decide :)
>>>>>>>
>>>>>>> On Feb 9, 2011, at 2:32 PM, Mircea Markus wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> On 9 Feb 2011, at 12:34, Manik Surtani wrote:
>>>>>>>>
>>>>>>>>> What about putting the entire call on a separate thread much earlier in the call stack? E.g., in the CacheDelegate? You get the added benefit of any cache loading happening offline as well. Plus a much simpler impl. :)
>>>>>>>> +1.
>>>>>>>>>
>>>>>>>>> Sent from my iPhone
>>>>>>>>>
>>>>>>>>> On 9 Feb 2011, at 08:14, Galder Zamarreño <[hidden email]> wrote:
>>>>>>>>>
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Re: https://issues.jboss.org/browse/ISPN-293
>>>>>>>>>>
>>>>>>>>>> I have an issue with my implementation that simply wraps the realRemoteGet in DistributionInterceptor around a Callable:
>>>>>>>>>>
>>>>>>>>>> Assume that cache is configured with Distribution(numOwners=1, l1=enabled), no transactions, and we have a cluster of 2 nodes:
>>>>>>>>>>
>>>>>>>>>> - [main-thread] Put k0 in a cache that should own it.
>>>>>>>>>> - [main-thread] Do a getAsync for k0 from a node that does not own it:
>>>>>>>>>> - [async-thread] This leads to a remote get which updates the L1 and updates the context created by the main-thread and putting the updated entry in there (however, this thread does not release the locks)
>>>>>>>>>> - [main-thread] Next up, we put a new key, i.e. k1, in the node that didn't own k0 (the node where we updated the L1 cache):
>>>>>>>>>> - Now this thread has used the same context that the async thread used, so when it comes to releasing locks, it finds two entries in the context, the one locked by the async-thread and one for the main-thread and it fails with java.lang.IllegalMonitorStateException
>>>>>>>>>>
>>>>>>>>>> Now, what's happening here is the async-thread is acquiring the lock but not releasing it because the release happens in the DistLockInterceptor/LockingInterceptor, so a different interceptor to where the lock is being acquired. So, in theory, the solution would be for DistLockInterceptor to wrap the invokeNext() and afterCall() for when an async get so that all "remote get", "l1 update", and "release locks", happen in a separate thread. However, for this to work, it will need to figure out whether a remoteGet() will be necessary in the first place, otherwise is useless. Whether the remoteGet should happen is determined by this code in DistributionInterceptor:
>>>>>>>>>>
>>>>>>>>>> if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) {
>>>>>>>>>>
>>>>>>>>>> Also, if DistLockInterceptor does this check, we need to make sure DistributionInterceptor does not do it again, otherwise it's a waste. I think this might work although it does require some further reengineering.
>>>>>>>>>>
>>>>>>>>>> I'm gonna try to implement this but wondering whether anyone can see any potential flaws here, or if anyone has any better ideas :)
>>>>>>>>>>
>>>>>>>>>> Cheers,
>>>>>>>>>> --
>>>>>>>>>> Galder Zamarreño
>>>>>>>>>> Sr. Software Engineer
>>>>>>>>>> Infinispan, JBoss Cache
>

_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Galder Zamarreno
In reply to this post by Sanne Grinovero

On Feb 11, 2011, at 12:20 PM, Sanne Grinovero wrote:

> 2011/2/11 Galder Zamarreño <[hidden email]>:
>> The other *Async() limit themselves to making the RPC call in a separate thread, i.e. putAsync.
>>
>> So, by the time they have to go remote to replicate something, they've already acquire the locks within the user thread and the transaction is in prepare state.
>>
>> The PrepareCommand should contains a GlobalTransaction which transforms into a RemoteTransaction in the other node.
>>
>> I don't see the need to transfer actual javax.transaction.Transaction information with these commands.
>
> So basically if I abort a transaction during which I performed some
> asyncPut() operations, they are not rolled back? I could live with
> that, as long as we clearly document this.

Of course it's rolled back. An async put within a transaction will only go remote when you actually commit the transaction. So if u rollback, so remote puts are done.

And to clarify, an asyncPut will synchronously hit the cache store unless the cache store is configured in async mode. If we want asyncPut() to be proper async, even for cache store access, further work would be needed. I varguely remember someone talking about this, could it have been Philippe Van Dyck? I can't find references to that right now.

>
>>
>> Btw, I've just been talking to Mircea and we're considering passing around the LocalTransaction between the caller and the getAsync thread rather than passing javax.transaction.Transaction which we don't have control over.
>
> Jonathan mentioned a way to share transactions across threads using
> JBoss TransactionManager; the main problem with this is that while
> something similar might be supported by others: it's not standardized.
>
>>
>> On Feb 10, 2011, at 7:09 PM, Manik Surtani wrote:
>>
>>> How do other *Async() calls deal with jta, if at all?
>>>
>>> On 10 Feb 2011, at 11:10, Galder Zamarreño wrote:
>>>
>>>> That's a good optimisation Mircea, I've added it.
>>>>
>>>> There's another important problem to solve here which is, how do you make getAsync() maintain the transaction semantics of the incoming thread? getAsync does not acquire locks except in the case where the remote get leads to a L1 put in which case a write lock is acquired on entry put on L1.
>>>>
>>>> The main problem here is how to make the transaction propagate from one thread to the other? I tried to pass the current thread's transaction to the getAsync() thread and create an invocation context using it. This doesn't work cos LocalTxInvocationContext does not keep the current transaction, instead it keeps the localTransaction which for 1st invocation is null.
>>>>
>>>> The Infinispan code relies on TransactionManager.getTransaction() which most likely relies on a ThreadLocal being set to the current transaction. Once you switch threads, this method returns null, and with the solution attempted solution I get:
>>>>
>>>> Caused by: java.lang.IllegalStateException: This should only be called in an tx scope
>>>>      at org.infinispan.interceptors.TxInterceptor.enlist(TxInterceptor.java:193)
>>>>      at org.infinispan.interceptors.TxInterceptor.enlistReadAndInvokeNext(TxInterceptor.java:167)
>>>>      at org.infinispan.interceptors.TxInterceptor.visitGetKeyValueCommand(TxInterceptor.java:162)
>>>>
>>>> So, what I've done is get the InvocationContextContainer to take the passed transaction and assign it to the local tx invocation context, and then use this transaction rather than the one from tm.getTransaction() in the enlist method.
>>>>
>>>> I've created a pull request with this solution and you can find it in: https://github.com/infinispan/infinispan/pull/160
>>>>
>>>> Note that I've made a couple of TODOs in AbstractTxInvocationContext in order to discuss potentially naming getTransaction/setTransaction differently and see if we wanna merge somehow with getRunningTransaction() available in TxInvocationContext.
>>>>
>>>> Cheers,
>>>>
>>>> On Feb 9, 2011, at 6:42 PM, Mircea Markus wrote:
>>>>
>>>>> On 9 Feb 2011, at 17:35, Galder Zamarreño wrote:
>>>>>
>>>>>> Hmmmm, I've thought this further. The penalty of the thread ctx switching is probably not that bad actually cos the point of getAsync() is the ability to paralelise rather than how quickly getAsync() returns.
>>>>> Or a local get can be forced(flag) before giving control(if local value doesn't exist) to the new thread.
>>>>>>
>>>>>> On Feb 9, 2011, at 4:38 PM, Galder Zamarreño wrote:
>>>>>>
>>>>>>> The problem that I see with that is getAsync() calls that should resolve locally will get the penalty of the thread context switching.
>>>>>>>
>>>>>>> I think I have a workable solution without having to rely on your suggestion which passes the test suite. I'll send a pull request and you (Mircea+Manik) can have a look to it, and then we can decide :)
>>>>>>>
>>>>>>> On Feb 9, 2011, at 2:32 PM, Mircea Markus wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> On 9 Feb 2011, at 12:34, Manik Surtani wrote:
>>>>>>>>
>>>>>>>>> What about putting the entire call on a separate thread much earlier in the call stack? E.g., in the CacheDelegate? You get the added benefit of any cache loading happening offline as well. Plus a much simpler impl. :)
>>>>>>>> +1.
>>>>>>>>>
>>>>>>>>> Sent from my iPhone
>>>>>>>>>
>>>>>>>>> On 9 Feb 2011, at 08:14, Galder Zamarreño <[hidden email]> wrote:
>>>>>>>>>
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> Re: https://issues.jboss.org/browse/ISPN-293
>>>>>>>>>>
>>>>>>>>>> I have an issue with my implementation that simply wraps the realRemoteGet in DistributionInterceptor around a Callable:
>>>>>>>>>>
>>>>>>>>>> Assume that cache is configured with Distribution(numOwners=1, l1=enabled), no transactions, and we have a cluster of 2 nodes:
>>>>>>>>>>
>>>>>>>>>> - [main-thread] Put k0 in a cache that should own it.
>>>>>>>>>> - [main-thread] Do a getAsync for k0 from a node that does not own it:
>>>>>>>>>> - [async-thread] This leads to a remote get which updates the L1 and updates the context created by the main-thread and putting the updated entry in there (however, this thread does not release the locks)
>>>>>>>>>> - [main-thread] Next up, we put a new key, i.e. k1, in the node that didn't own k0 (the node where we updated the L1 cache):
>>>>>>>>>> - Now this thread has used the same context that the async thread used, so when it comes to releasing locks, it finds two entries in the context, the one locked by the async-thread and one for the main-thread and it fails with java.lang.IllegalMonitorStateException
>>>>>>>>>>
>>>>>>>>>> Now, what's happening here is the async-thread is acquiring the lock but not releasing it because the release happens in the DistLockInterceptor/LockingInterceptor, so a different interceptor to where the lock is being acquired. So, in theory, the solution would be for DistLockInterceptor to wrap the invokeNext() and afterCall() for when an async get so that all "remote get", "l1 update", and "release locks", happen in a separate thread. However, for this to work, it will need to figure out whether a remoteGet() will be necessary in the first place, otherwise is useless. Whether the remoteGet should happen is determined by this code in DistributionInterceptor:
>>>>>>>>>>
>>>>>>>>>> if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) {
>>>>>>>>>>
>>>>>>>>>> Also, if DistLockInterceptor does this check, we need to make sure DistributionInterceptor does not do it again, otherwise it's a waste. I think this might work although it does require some further reengineering.
>>>>>>>>>>
>>>>>>>>>> I'm gonna try to implement this but wondering whether anyone can see any potential flaws here, or if anyone has any better ideas :)
>>>>>>>>>>
>>>>>>>>>> Cheers,
>>>>>>>>>> --
>>>>>>>>>> Galder Zamarreño
>>>>>>>>>> Sr. Software Engineer
>>>>>>>>>> Infinispan, JBoss Cache
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Galder Zamarreno

On Feb 14, 2011, at 3:00 PM, Galder Zamarreño wrote:

>
> On Feb 11, 2011, at 12:20 PM, Sanne Grinovero wrote:
>
>> 2011/2/11 Galder Zamarreño <[hidden email]>:
>>> The other *Async() limit themselves to making the RPC call in a separate thread, i.e. putAsync.
>>>
>>> So, by the time they have to go remote to replicate something, they've already acquire the locks within the user thread and the transaction is in prepare state.
>>>
>>> The PrepareCommand should contains a GlobalTransaction which transforms into a RemoteTransaction in the other node.
>>>
>>> I don't see the need to transfer actual javax.transaction.Transaction information with these commands.
>>
>> So basically if I abort a transaction during which I performed some
>> asyncPut() operations, they are not rolled back? I could live with
>> that, as long as we clearly document this.
>
> Of course it's rolled back. An async put within a transaction will only go remote when you actually commit the transaction. So if u rollback, so remote puts are done.

s/so/no: So if u rollback, no remote puts are done.

>
> And to clarify, an asyncPut will synchronously hit the cache store unless the cache store is configured in async mode. If we want asyncPut() to be proper async, even for cache store access, further work would be needed. I varguely remember someone talking about this, could it have been Philippe Van Dyck? I can't find references to that right now.
>
>>
>>>
>>> Btw, I've just been talking to Mircea and we're considering passing around the LocalTransaction between the caller and the getAsync thread rather than passing javax.transaction.Transaction which we don't have control over.
>>
>> Jonathan mentioned a way to share transactions across threads using
>> JBoss TransactionManager; the main problem with this is that while
>> something similar might be supported by others: it's not standardized.
>>
>>>
>>> On Feb 10, 2011, at 7:09 PM, Manik Surtani wrote:
>>>
>>>> How do other *Async() calls deal with jta, if at all?
>>>>
>>>> On 10 Feb 2011, at 11:10, Galder Zamarreño wrote:
>>>>
>>>>> That's a good optimisation Mircea, I've added it.
>>>>>
>>>>> There's another important problem to solve here which is, how do you make getAsync() maintain the transaction semantics of the incoming thread? getAsync does not acquire locks except in the case where the remote get leads to a L1 put in which case a write lock is acquired on entry put on L1.
>>>>>
>>>>> The main problem here is how to make the transaction propagate from one thread to the other? I tried to pass the current thread's transaction to the getAsync() thread and create an invocation context using it. This doesn't work cos LocalTxInvocationContext does not keep the current transaction, instead it keeps the localTransaction which for 1st invocation is null.
>>>>>
>>>>> The Infinispan code relies on TransactionManager.getTransaction() which most likely relies on a ThreadLocal being set to the current transaction. Once you switch threads, this method returns null, and with the solution attempted solution I get:
>>>>>
>>>>> Caused by: java.lang.IllegalStateException: This should only be called in an tx scope
>>>>>     at org.infinispan.interceptors.TxInterceptor.enlist(TxInterceptor.java:193)
>>>>>     at org.infinispan.interceptors.TxInterceptor.enlistReadAndInvokeNext(TxInterceptor.java:167)
>>>>>     at org.infinispan.interceptors.TxInterceptor.visitGetKeyValueCommand(TxInterceptor.java:162)
>>>>>
>>>>> So, what I've done is get the InvocationContextContainer to take the passed transaction and assign it to the local tx invocation context, and then use this transaction rather than the one from tm.getTransaction() in the enlist method.
>>>>>
>>>>> I've created a pull request with this solution and you can find it in: https://github.com/infinispan/infinispan/pull/160
>>>>>
>>>>> Note that I've made a couple of TODOs in AbstractTxInvocationContext in order to discuss potentially naming getTransaction/setTransaction differently and see if we wanna merge somehow with getRunningTransaction() available in TxInvocationContext.
>>>>>
>>>>> Cheers,
>>>>>
>>>>> On Feb 9, 2011, at 6:42 PM, Mircea Markus wrote:
>>>>>
>>>>>> On 9 Feb 2011, at 17:35, Galder Zamarreño wrote:
>>>>>>
>>>>>>> Hmmmm, I've thought this further. The penalty of the thread ctx switching is probably not that bad actually cos the point of getAsync() is the ability to paralelise rather than how quickly getAsync() returns.
>>>>>> Or a local get can be forced(flag) before giving control(if local value doesn't exist) to the new thread.
>>>>>>>
>>>>>>> On Feb 9, 2011, at 4:38 PM, Galder Zamarreño wrote:
>>>>>>>
>>>>>>>> The problem that I see with that is getAsync() calls that should resolve locally will get the penalty of the thread context switching.
>>>>>>>>
>>>>>>>> I think I have a workable solution without having to rely on your suggestion which passes the test suite. I'll send a pull request and you (Mircea+Manik) can have a look to it, and then we can decide :)
>>>>>>>>
>>>>>>>> On Feb 9, 2011, at 2:32 PM, Mircea Markus wrote:
>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 9 Feb 2011, at 12:34, Manik Surtani wrote:
>>>>>>>>>
>>>>>>>>>> What about putting the entire call on a separate thread much earlier in the call stack? E.g., in the CacheDelegate? You get the added benefit of any cache loading happening offline as well. Plus a much simpler impl. :)
>>>>>>>>> +1.
>>>>>>>>>>
>>>>>>>>>> Sent from my iPhone
>>>>>>>>>>
>>>>>>>>>> On 9 Feb 2011, at 08:14, Galder Zamarreño <[hidden email]> wrote:
>>>>>>>>>>
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> Re: https://issues.jboss.org/browse/ISPN-293
>>>>>>>>>>>
>>>>>>>>>>> I have an issue with my implementation that simply wraps the realRemoteGet in DistributionInterceptor around a Callable:
>>>>>>>>>>>
>>>>>>>>>>> Assume that cache is configured with Distribution(numOwners=1, l1=enabled), no transactions, and we have a cluster of 2 nodes:
>>>>>>>>>>>
>>>>>>>>>>> - [main-thread] Put k0 in a cache that should own it.
>>>>>>>>>>> - [main-thread] Do a getAsync for k0 from a node that does not own it:
>>>>>>>>>>> - [async-thread] This leads to a remote get which updates the L1 and updates the context created by the main-thread and putting the updated entry in there (however, this thread does not release the locks)
>>>>>>>>>>> - [main-thread] Next up, we put a new key, i.e. k1, in the node that didn't own k0 (the node where we updated the L1 cache):
>>>>>>>>>>> - Now this thread has used the same context that the async thread used, so when it comes to releasing locks, it finds two entries in the context, the one locked by the async-thread and one for the main-thread and it fails with java.lang.IllegalMonitorStateException
>>>>>>>>>>>
>>>>>>>>>>> Now, what's happening here is the async-thread is acquiring the lock but not releasing it because the release happens in the DistLockInterceptor/LockingInterceptor, so a different interceptor to where the lock is being acquired. So, in theory, the solution would be for DistLockInterceptor to wrap the invokeNext() and afterCall() for when an async get so that all "remote get", "l1 update", and "release locks", happen in a separate thread. However, for this to work, it will need to figure out whether a remoteGet() will be necessary in the first place, otherwise is useless. Whether the remoteGet should happen is determined by this code in DistributionInterceptor:
>>>>>>>>>>>
>>>>>>>>>>> if (ctx.isOriginLocal() && !(isMappedToLocalNode = dm.isLocal(key)) && isNotInL1(key)) {
>>>>>>>>>>>
>>>>>>>>>>> Also, if DistLockInterceptor does this check, we need to make sure DistributionInterceptor does not do it again, otherwise it's a waste. I think this might work although it does require some further reengineering.
>>>>>>>>>>>
>>>>>>>>>>> I'm gonna try to implement this but wondering whether anyone can see any potential flaws here, or if anyone has any better ideas :)
>>>>>>>>>>>
>>>>>>>>>>> Cheers,
>>>>>>>>>>> --
>>>>>>>>>>> Galder Zamarreño
>>>>>>>>>>> Sr. Software Engineer
>>>>>>>>>>> Infinispan, JBoss Cache
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> --
> Galder Zamarreño
> Sr. Software Engineer
> Infinispan, JBoss Cache
>
>
> _______________________________________________
> infinispan-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/infinispan-dev

--
Galder Zamarreño
Sr. Software Engineer
Infinispan, JBoss Cache


_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Manik Surtani
In reply to this post by Galder Zamarreno

On 14 Feb 2011, at 14:00, Galder Zamarreño wrote:

> And to clarify, an asyncPut will synchronously hit the cache store unless the cache store is configured in async mode. If we want asyncPut() to be proper async, even for cache store access, further work would be needed. I varguely remember someone talking about this, could it have been Philippe Van Dyck? I can't find references to that right now.

Async calls are just wrt. RPC for now.  In future we may move cache store writing to the async thread as well, but for now, its just RPC.

--
Manik Surtani
[hidden email]
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev
Reply | Threaded
Open this post in threaded view
|

Re: [infinispan-dev] ISPN-293 getAsync impl requires more reengineering

Manik Surtani
In reply to this post by Galder Zamarreno

On 14 Feb 2011, at 14:02, Galder Zamarreño wrote:

> s/so/no: So if u rollback, no remote puts are done.

Not necessarily.  :)  Since the RPCs are async, the prepare and rollback could be re-ordered (if the rollback comes after prepare).

But yeah, the Async() methods were not designed for transactions.
--
Manik Surtani
[hidden email]
twitter.com/maniksurtani

Lead, Infinispan
http://www.infinispan.org




_______________________________________________
infinispan-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/infinispan-dev