6:03 p.m.
I'd love to brainstorm about the clear() operation and what it means
on Infinispan.
I'm not sure to what extent, but it seems that clear() is designed to
work in a TX, or even create an implicit transaction if needed, but
I'm not understanding how that can work.
Obviously a clear() operation isn't listing all keys explicitly. Which
implies that it's undefined on which keys it's going to operate when
it's fired.. that seems like terribly wrong in a distributed key/value
store as we can't possibly freeze the global state and somehow define
a set of keys which are going to be affected, while an explicit
enumeration is needed to acquire the needed locks.
It might give a nice safe feeling that, when invoking a clear()
operation in a transaction, I can still abort the transaction to make
it cancel the operation; that's the only good part I can think of: we
can cancel it.
I don't think it has anything to do with consistency though? To make
sure you're effectively involving all replicas of all entries in a
consistent way, a lock would need to be acquired on each affected key,
which again implies a need to enumerate all keys, including the
unknown keys which might be hiding in a CacheStore: it's not enough to
broadcast the clear() operation to all nodes and have them simply wipe
their local state as that's never going to deal correctly
(consistently) with in-flight transactions working on different nodes
at different times (I guess enabling Total Order could help but you'd
need to make it mandatory).
So let's step back a second and consider what is the use case for
clear() ? I suspect it's primarily a method needed during testing, or
maybe cleanup before a backup is restored (operations), maybe a
manually activated JMX operation to clear the cache in exceptional
cases.
I don't think there would ever be a need for a clear() operation to
interact with other transactions, so I'd rather make it illegal to
invoke a clear() inside a transaction, or simply ignore the
transactional scope and have an immediate and distributed effect.
I'm likely missing something. What terrible consequences would this have?
Cheers,
Sanne
10:50 p.m.
Looking at ISPN-3557, I think that is resolved by ISPN-3560. It was
just a bug in the commands not using the context values properly. I
will look closer tomorrow, but I am pretty sure it is resolved (can
just add tests for PR).
I am guessing you are talking about ISPN-3558. Either way, I agree
Cache.clear in a tx is problematic. When I first thought of clear I
immediately think of atomicity (having the entire cache cleared at
some point), which currently this is not guaranteed. This is due to
only acquiring locks/clearing the entries in the cache when clear is
invoked and can change before the tx is actually committed. I was
talking with Dan briefly about it, and obviously it would be too late
to change anything for 6.0, but I think it is worth it to discuss it.
The best way I can think to make it more consistent would be when you
have a clear is that it could somehow block other insertions that it
didn't know about when invoked (maybe a new value lock) or somehow be
notified of insertions since the clear was done to know what keys to
remove additionally when it is committed. Either way doesn't seem
appealing to me.
Not supporting clear in a tx context is appealing. I think changing
clear to be outside of the current tx (like putForExternalRead) would
cause quite a bit of confusion instead. To suggest either here would
be best to know what use cases we have, which you know a lot better
than I.
- Will
On Tue, Oct 1, 2013 at 4:03 PM, Sanne Grinovero <sanne(a)infinispan.org> wrote:
I'd love to brainstorm about the clear() operation and what it
means
on Infinispan.
I'm not sure to what extent, but it seems that clear() is designed to
work in a TX, or even create an implicit transaction if needed, but
I'm not understanding how that can work.
Obviously a clear() operation isn't listing all keys explicitly. Which
implies that it's undefined on which keys it's going to operate when
it's fired.. that seems like terribly wrong in a distributed key/value
store as we can't possibly freeze the global state and somehow define
a set of keys which are going to be affected, while an explicit
enumeration is needed to acquire the needed locks.
It might give a nice safe feeling that, when invoking a clear()
operation in a transaction, I can still abort the transaction to make
it cancel the operation; that's the only good part I can think of: we
can cancel it.
I don't think it has anything to do with consistency though? To make
sure you're effectively involving all replicas of all entries in a
consistent way, a lock would need to be acquired on each affected key,
which again implies a need to enumerate all keys, including the
unknown keys which might be hiding in a CacheStore: it's not enough to
broadcast the clear() operation to all nodes and have them simply wipe
their local state as that's never going to deal correctly
(consistently) with in-flight transactions working on different nodes
at different times (I guess enabling Total Order could help but you'd
need to make it mandatory).
So let's step back a second and consider what is the use case for
clear() ? I suspect it's primarily a method needed during testing, or
maybe cleanup before a backup is restored (operations), maybe a
manually activated JMX operation to clear the cache in exceptional
cases.
I don't think there would ever be a need for a clear() operation to
interact with other transactions, so I'd rather make it illegal to
invoke a clear() inside a transaction, or simply ignore the
transactional scope and have an immediate and distributed effect.
I'm likely missing something. What terrible consequences would this have?
Cheers,
Sanne
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
4:48 a.m.
On 10/02/2013 12:03 AM, Sanne Grinovero wrote:
I'd love to brainstorm about the clear() operation and what it
means
on Infinispan.
I'm not sure to what extent, but it seems that clear() is designed to
work in a TX, or even create an implicit transaction if needed, but
I'm not understanding how that can work.
Obviously a clear() operation isn't listing all keys explicitly.
(offtopic) like entrySet(), keySet(), values() and iterator() when
executed in a transactional context. however, I think that this
operations are more difficult to handle, because they have to return
consistent data...
Which
implies that it's undefined on which keys it's going to operate when
it's fired.. that seems like terribly wrong in a distributed key/value
store as we can't possibly freeze the global state and somehow define
a set of keys which are going to be affected, while an explicit
enumeration is needed to acquire the needed locks.
you may not need to explicit acquire the lock for the affected key. you
can use a global lock that is share acquired for write transactions
without clear operation and exclusively acquire for transactions with
clear operation (exactly what I use for Total Order). also, the clear
transactions (i.e. the transactions with clear operation) can also only
acquired the global lock.
It might give a nice safe feeling that, when invoking a clear()
operation in a transaction, I can still abort the transaction to make
it cancel the operation; that's the only good part I can think of: we
can cancel it.
yep
I don't think it has anything to do with consistency though? To make
sure you're effectively involving all replicas of all entries in a
consistent way, a lock would need to be acquired on each affected key,
which again implies a need to enumerate all keys, including the
unknown keys which might be hiding in a CacheStore: it's not enough to
broadcast the clear() operation to all nodes and have them simply wipe
their local state as that's never going to deal correctly
(consistently) with in-flight transactions working on different nodes
at different times (I guess enabling Total Order could help but you'd
need to make it mandatory).
see the comment above about the locks... may not be the most efficient,
but handles the problem.
So let's step back a second and consider what is the use case
for
clear() ? I suspect it's primarily a method needed during testing, or
maybe cleanup before a backup is restored (operations), maybe a
manually activated JMX operation to clear the cache in exceptional
cases.
I'm not aware of the uses cases of a clear() operation, but use it as
JMX operation can be a problem in transactional caches (depending the
semantic you have in mind).
If it is going to clear all the caches (i.e. from all replicas/nodes),
then we have to find a way to set an order with the current transactions
or we will have something some nodes delivering a transaction (say tx1)
before the clear() and other nodes delivering tx1 after the clear(),
leading to an inconsistent state.
(offtopic) I'm wondering if we have that problem in the current
implementation if tx1 is only insert new keys... in that case, we have
no lock to serialize an order between the clear() and tx1...
I don't think there would ever be a need for a clear() operation to
interact with other transactions, so I'd rather make it illegal to
invoke a clear() inside a transaction, or simply ignore the
transactional scope and have an immediate and distributed effect.
I think the second solution can make inconsistency data ("or simply
ignore the transactional scope and have an immediate and distributed
effect")
I'm likely missing something. What terrible consequences would this have?
Cheers,
Sanne
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
4:29 a.m.
On Wed, Oct 2, 2013 at 12:48 PM, Pedro Ruivo <pedro(a)infinispan.org> wrote:
On 10/02/2013 12:03 AM, Sanne Grinovero wrote:
> I'd love to brainstorm about the clear() operation and what it means
> on Infinispan.
>
> I'm not sure to what extent, but it seems that clear() is designed to
> work in a TX, or even create an implicit transaction if needed, but
> I'm not understanding how that can work.
I think it's pretty straight-forward to define the semantics of operations
after clear(): they should assume that every key in the cache was removed.
True, like Pedro mentioned, the current implementation allows subsequent
operations in the same tx to see keys inserted by other txs. But I don't
think fixing that would be harder than ensuring that if a tx inserts key k
owned by nodes A and B, clear() from outside the tx either deletes k on A
and B or doesn't delete k on either node.
>
> Obviously a clear() operation isn't listing all keys explicitly.
(offtopic) like entrySet(), keySet(), values() and iterator() when
executed in a transactional context. however, I think that this
operations are more difficult to handle, because they have to return
consistent data...
I agree, defining how entrySet() and the others should work in a tx seems
harder than defining clear().
Will, I'm guessing your fix for ISPN-3560 still allows entrySet() after
clear() to see entries inserted by another tx in the meantime?
> Which
> implies that it's undefined on which keys it's going to operate when
> it's fired.. that seems like terribly wrong in a distributed key/value
> store as we can't possibly freeze the global state and somehow define
> a set of keys which are going to be affected, while an explicit
> enumeration is needed to acquire the needed locks.
you may not need to explicit acquire the lock for the affected key. you
can use a global lock that is share acquired for write transactions
without clear operation and exclusively acquire for transactions with
clear operation (exactly what I use for Total Order). also, the clear
transactions (i.e. the transactions with clear operation) can also only
acquired the global lock.
+1 for the global lock. It may slow things down a bit in the general case,
because every tx will have to acquire it in shared mode, but it will
guarantee consistency for clear().
It still won't help entrySet() and its brethren...
>
> It might give a nice safe feeling that, when invoking a clear()
> operation in a transaction, I can still abort the transaction to make
> it cancel the operation; that's the only good part I can think of: we
> can cancel it.
yep
Wouldn't it be harder to integrate it with Total Order if it didn't have a
tx?
But most of all, I think we have to keep it because it's a part of the Map
interface and a part of JCache's Cache interface.
>
> I don't think it has anything to do with consistency though? To make
> sure you're effectively involving all replicas of all entries in a
> consistent way, a lock would need to be acquired on each affected key,
> which again implies a need to enumerate all keys, including the
> unknown keys which might be hiding in a CacheStore: it's not enough to
> broadcast the clear() operation to all nodes and have them simply wipe
> their local state as that's never going to deal correctly
> (consistently) with in-flight transactions working on different nodes
> at different times (I guess enabling Total Order could help but you'd
> need to make it mandatory).
>
see the comment above about the locks... may not be the most efficient,
but handles the problem.
Note that we need to iterate the keys anyway, in order to fire the
CacheEntryRemoved listeners. Even if we optimized it skip the iteration
when there are no registered listeners, it would still have to handle the
general case.
> So let's step back a second and consider what is the use
case for
> clear() ? I suspect it's primarily a method needed during testing, or
> maybe cleanup before a backup is restored (operations), maybe a
> manually activated JMX operation to clear the cache in exceptional
> cases.
I'm not aware of the uses cases of a clear() operation, but use it as
JMX operation can be a problem in transactional caches (depending the
semantic you have in mind).
If it is going to clear all the caches (i.e. from all replicas/nodes),
then we have to find a way to set an order with the current transactions
or we will have something some nodes delivering a transaction (say tx1)
before the clear() and other nodes delivering tx1 after the clear(),
leading to an inconsistent state.
(offtopic) I'm wondering if we have that problem in the current
implementation if tx1 is only insert new keys... in that case, we have
no lock to serialize an order between the clear() and tx1...
I'm pretty sure we do have this problem. We definitely need to add some
tests for clear concurrent with other write operations.
In my view, as long as we're going to expose a clear() operation in some
way, it's going to be easiest to include it in a tx in transactional
caches. Exposing it in some other way is not going to make it any more
consistent, although prohibiting other operations in the same tx may make
the code a bit simpler.
>
> I don't think there would ever be a need for a clear() operation to
> interact with other transactions, so I'd rather make it illegal to
> invoke a clear() inside a transaction, or simply ignore the
> transactional scope and have an immediate and distributed effect.
We can't control whether other transactions are invoked concurrently with
clear(), so we can't make that illegal... If you mean interaction with
other operations in the same transaction, I agree that it's not absolutely
necessary to allow them, but I think most of that work has already been
done.
I think the second solution can make inconsistency data ("or
simply
ignore the transactional scope and have an immediate and distributed
effect")
Maybe that's the idea? Sanne, do you mean that if clear() were exposed via
a different API, it wouldn't have to provide the same consistency
guarantees as regular operations?
>
> I'm likely missing something. What terrible consequences would this have?
>
> Cheers,
> Sanne
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
5:57 a.m.
On 3 October 2013 10:29, Dan Berindei <dan.berindei(a)gmail.com> wrote:
On Wed, Oct 2, 2013 at 12:48 PM, Pedro Ruivo <pedro(a)infinispan.org> wrote:
>
>
>
> On 10/02/2013 12:03 AM, Sanne Grinovero wrote:
> > I'd love to brainstorm about the clear() operation and what it means
> > on Infinispan.
> >
> > I'm not sure to what extent, but it seems that clear() is designed to
> > work in a TX, or even create an implicit transaction if needed, but
> > I'm not understanding how that can work.
I think it's pretty straight-forward to define the semantics of operations
after clear(): they should assume that every key in the cache was removed.
True, like Pedro mentioned, the current implementation allows subsequent
operations in the same tx to see keys inserted by other txs. But I don't
think fixing that would be harder than ensuring that if a tx inserts key k
owned by nodes A and B, clear() from outside the tx either deletes k on A
and B or doesn't delete k on either node.
>
> >
> > Obviously a clear() operation isn't listing all keys explicitly.
>
> (offtopic) like entrySet(), keySet(), values() and iterator() when
> executed in a transactional context. however, I think that this
> operations are more difficult to handle, because they have to return
> consistent data...
I agree, defining how entrySet() and the others should work in a tx seems
harder than defining clear().
I agree, those other nasty beasts are next on my list: see below..
But we're not arguing about which method deserves the gold medal for
complex design ;-)
Will, I'm guessing your fix for ISPN-3560 still allows entrySet()
after
clear() to see entries inserted by another tx in the meantime?
>
> > Which
> > implies that it's undefined on which keys it's going to operate when
> > it's fired.. that seems like terribly wrong in a distributed key/value
> > store as we can't possibly freeze the global state and somehow define
> > a set of keys which are going to be affected, while an explicit
> > enumeration is needed to acquire the needed locks.
>
> you may not need to explicit acquire the lock for the affected key. you
> can use a global lock that is share acquired for write transactions
> without clear operation and exclusively acquire for transactions with
> clear operation (exactly what I use for Total Order). also, the clear
> transactions (i.e. the transactions with clear operation) can also only
> acquired the global lock.
+1 for the global lock. It may slow things down a bit in the general case,
because every tx will have to acquire it in shared mode, but it will
guarantee consistency for clear().
I had thought of that too, but it will slow down all operations, at
which benefit?
To have a nice transactional clear() operation which guarantees consistency?
The goal of this thread is to challenge if that makes sense for real use cases,
it seems in fact we would highly prefer a non transactional version,
and considering that we can't run a single Cache in both modes anymore
that's a problem.
Side note on the implementation complexity:
even the AsyncCacheStore decorator code is made extremely more tricky
just to handle clear() appropriately, to keep it in the correct order
of execution
compared to the other operations, while still allowing
"coaleshing" of other operations, which essentially implies we allow
reordering of operations as long as they affect independent keys.. where clear()
is of course a jolly which requires it's own execution path, its own global lock
and generally makes it far more complex.
It still won't help entrySet() and its brethren...
>
> >
> > It might give a nice safe feeling that, when invoking a clear()
> > operation in a transaction, I can still abort the transaction to make
> > it cancel the operation; that's the only good part I can think of: we
> > can cancel it.
>
> yep
Wouldn't it be harder to integrate it with Total Order if it didn't have a
tx?
But most of all, I think we have to keep it because it's a part of the Map
interface and a part of JCache's Cache interface.
>
>
> >
> > I don't think it has anything to do with consistency though? To make
> > sure you're effectively involving all replicas of all entries in a
> > consistent way, a lock would need to be acquired on each affected key,
> > which again implies a need to enumerate all keys, including the
> > unknown keys which might be hiding in a CacheStore: it's not enough to
> > broadcast the clear() operation to all nodes and have them simply wipe
> > their local state as that's never going to deal correctly
> > (consistently) with in-flight transactions working on different nodes
> > at different times (I guess enabling Total Order could help but you'd
> > need to make it mandatory).
> >
>
> see the comment above about the locks... may not be the most efficient,
> but handles the problem.
Note that we need to iterate the keys anyway, in order to fire the
CacheEntryRemoved listeners. Even if we optimized it skip the iteration when
there are no registered listeners, it would still have to handle the general
case.
>
> > So let's step back a second and consider what is the use case for
> > clear() ? I suspect it's primarily a method needed during testing, or
> > maybe cleanup before a backup is restored (operations), maybe a
> > manually activated JMX operation to clear the cache in exceptional
> > cases.
>
> I'm not aware of the uses cases of a clear() operation, but use it as
> JMX operation can be a problem in transactional caches (depending the
> semantic you have in mind).
>
> If it is going to clear all the caches (i.e. from all replicas/nodes),
> then we have to find a way to set an order with the current transactions
> or we will have something some nodes delivering a transaction (say tx1)
> before the clear() and other nodes delivering tx1 after the clear(),
> leading to an inconsistent state.
>
> (offtopic) I'm wondering if we have that problem in the current
> implementation if tx1 is only insert new keys... in that case, we have
> no lock to serialize an order between the clear() and tx1...
I'm pretty sure we do have this problem. We definitely need to add some
tests for clear concurrent with other write operations.
In my view, as long as we're going to expose a clear() operation in some
way, it's going to be easiest to include it in a tx in transactional caches.
Exposing it in some other way is not going to make it any more consistent,
although prohibiting other operations in the same tx may make the code a bit
simpler.
>
>
> >
> > I don't think there would ever be a need for a clear() operation to
> > interact with other transactions, so I'd rather make it illegal to
> > invoke a clear() inside a transaction, or simply ignore the
> > transactional scope and have an immediate and distributed effect.
We can't control whether other transactions are invoked concurrently with
clear(), so we can't make that illegal... If you mean interaction with other
operations in the same transaction, I agree that it's not absolutely
necessary to allow them, but I think most of that work has already been
done.
>
> I think the second solution can make inconsistency data ("or simply
> ignore the transactional scope and have an immediate and distributed
> effect")
Maybe that's the idea? Sanne, do you mean that if clear() were exposed via a
different API, it wouldn't have to provide the same consistency guarantees
as regular operations?
Yes that's the main point that we need to clarify.
Note that this discussion started because of a problem we're having in
the integration
of Infinispan as Hibernate 2nd level cache, which is a quite mature
contract with
some very clear ideas of how a cache needs to behave.
After more discussions yesterday, I think I'm understanding (Galder
will likely know more
but is currently away) that ideally a cache should be able to provide
both a transactional
clear() operation and an immediately-affecting one.
The only need for the transactional one is to be able to undo the
operation in case the
user's transaction is rolled back: we don't even support this case
today since it's suspending
the transaction to run the clear() operation in an independent transaction.
But the transactional clear() operation is far less important than
having a clear "evict all now"
operation. That's one of the basic operations a good mannered Cache
should provide:
remember that as a cache it's always a valid answer to return "null",
it's just suboptimal.
Returning the wrong answer instead, can have critical consequences, so
it's expected to
provide a way for the admin (JMX) or driving application to
immediately wipe the state;
I would argue that even a glocal lock's timing implications are
undesirable in such a case.
Focusing on the identify of Infinispan as a Key/Value store, I'd
consider it almost mandatory
for the client to know - to explicitly list - the keys it's interested
into, and be completely oblivious
to the entries which you're unable to list. For example, think of a
cache shared by multiple tenants
or multiple apps, which use keys which for some reason can't return
true to an equality check
(for example because they're coming from different classloaders).
In such a perspective, any operation which affects (or enumerates)
keys is a bad practice and
should imho be banned from the user API, unless for administration or
monitoring purposes.
I think this is quite clearly expressed already by all the warning in
javadoc on methods like
org.infinispan.Cache.keySet()
org.infinispan.Cache.size()
...
They all list a warning about needing to be used only for debug
purposes: I don't think we need
to waste time into making these transitionally consistent, as people
shouldn't be using them.
If a transactional clear() operation is possible without overhead,
then the 2nd level cache code
would benefit from it, but it's a borderline decision and its code
could be refactored to not need this,
at least that's my understanding so far and I guess we need Galder for
that to be clarified.
Sanne
>
>
> >
> > I'm likely missing something. What terrible consequences would this
> > have?
> >
> > Cheers,
> > Sanne
> > _______________________________________________
> > infinispan-dev mailing list
> > infinispan-dev(a)lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
> >
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
8:07 a.m.
On Thu, Oct 3, 2013 at 1:57 PM, Sanne Grinovero <sanne(a)infinispan.org>wrote:
On 3 October 2013 10:29, Dan Berindei <dan.berindei(a)gmail.com>
wrote:
>
> On Wed, Oct 2, 2013 at 12:48 PM, Pedro Ruivo <pedro(a)infinispan.org>
wrote:
>>
>>
>>
>> On 10/02/2013 12:03 AM, Sanne Grinovero wrote:
>> > I'd love to brainstorm about the clear() operation and what it means
>> > on Infinispan.
>> >
>> > I'm not sure to what extent, but it seems that clear() is designed to
>> > work in a TX, or even create an implicit transaction if needed, but
>> > I'm not understanding how that can work.
>
>
> I think it's pretty straight-forward to define the semantics of
operations
> after clear(): they should assume that every key in the cache was
removed.
>
> True, like Pedro mentioned, the current implementation allows subsequent
> operations in the same tx to see keys inserted by other txs. But I don't
> think fixing that would be harder than ensuring that if a tx inserts key
k
> owned by nodes A and B, clear() from outside the tx either deletes k on A
> and B or doesn't delete k on either node.
>
>
>>
>> >
>> > Obviously a clear() operation isn't listing all keys explicitly.
>>
>> (offtopic) like entrySet(), keySet(), values() and iterator() when
>> executed in a transactional context. however, I think that this
>> operations are more difficult to handle, because they have to return
>> consistent data...
>
>
> I agree, defining how entrySet() and the others should work in a tx seems
> harder than defining clear().
I agree, those other nasty beasts are next on my list: see below..
But we're not arguing about which method deserves the gold medal for
complex design ;-)
> Will, I'm guessing your fix for ISPN-3560 still allows entrySet() after
> clear() to see entries inserted by another tx in the meantime?
>
>>
>> > Which
>> > implies that it's undefined on which keys it's going to operate
when
>> > it's fired.. that seems like terribly wrong in a distributed key/value
>> > store as we can't possibly freeze the global state and somehow define
>> > a set of keys which are going to be affected, while an explicit
>> > enumeration is needed to acquire the needed locks.
>>
>> you may not need to explicit acquire the lock for the affected key. you
>> can use a global lock that is share acquired for write transactions
>> without clear operation and exclusively acquire for transactions with
>> clear operation (exactly what I use for Total Order). also, the clear
>> transactions (i.e. the transactions with clear operation) can also only
>> acquired the global lock.
>
>
> +1 for the global lock. It may slow things down a bit in the general
case,
> because every tx will have to acquire it in shared mode, but it will
> guarantee consistency for clear().
I had thought of that too, but it will slow down all operations, at
which benefit?
To have a nice transactional clear() operation which guarantees
consistency?
For me, the most important reason is to support the JCache API.
The goal of this thread is to challenge if that makes sense for real
use
cases,
it seems in fact we would highly prefer a non transactional version,
and considering that we can't run a single Cache in both modes anymore
that's a problem.
Side note on the implementation complexity:
even the AsyncCacheStore decorator code is made extremely more tricky
just to handle clear() appropriately, to keep it in the correct order
of execution
compared to the other operations, while still allowing
"coaleshing" of other operations, which essentially implies we allow
reordering of operations as long as they affect independent keys.. where
clear()
is of course a jolly which requires it's own execution path, its own
global lock
and generally makes it far more complex.
Off-topic: I'm not very familiar with the async cache store code, but I
really don't think coalescing writes is good for consistency. In case of a
crash, you'd have lots of incomplete transactions on disk.
>
> It still won't help entrySet() and its brethren...
>
>>
>> >
>> > It might give a nice safe feeling that, when invoking a clear()
>> > operation in a transaction, I can still abort the transaction to make
>> > it cancel the operation; that's the only good part I can think of: we
>> > can cancel it.
>>
>> yep
>
>
> Wouldn't it be harder to integrate it with Total Order if it didn't have
a
> tx?
>
> But most of all, I think we have to keep it because it's a part of the
Map
> interface and a part of JCache's Cache interface.
>
>>
>>
>> >
>> > I don't think it has anything to do with consistency though? To make
>> > sure you're effectively involving all replicas of all entries in a
>> > consistent way, a lock would need to be acquired on each affected key,
>> > which again implies a need to enumerate all keys, including the
>> > unknown keys which might be hiding in a CacheStore: it's not enough to
>> > broadcast the clear() operation to all nodes and have them simply wipe
>> > their local state as that's never going to deal correctly
>> > (consistently) with in-flight transactions working on different nodes
>> > at different times (I guess enabling Total Order could help but you'd
>> > need to make it mandatory).
>> >
>>
>> see the comment above about the locks... may not be the most efficient,
>> but handles the problem.
>
>
> Note that we need to iterate the keys anyway, in order to fire the
> CacheEntryRemoved listeners. Even if we optimized it skip the iteration
when
> there are no registered listeners, it would still have to handle the
general
> case.
>
>>
>> > So let's step back a second and consider what is the use case for
>> > clear() ? I suspect it's primarily a method needed during testing, or
>> > maybe cleanup before a backup is restored (operations), maybe a
>> > manually activated JMX operation to clear the cache in exceptional
>> > cases.
>>
>> I'm not aware of the uses cases of a clear() operation, but use it as
>> JMX operation can be a problem in transactional caches (depending the
>> semantic you have in mind).
>>
>> If it is going to clear all the caches (i.e. from all replicas/nodes),
>> then we have to find a way to set an order with the current transactions
>> or we will have something some nodes delivering a transaction (say tx1)
>> before the clear() and other nodes delivering tx1 after the clear(),
>> leading to an inconsistent state.
>>
>> (offtopic) I'm wondering if we have that problem in the current
>> implementation if tx1 is only insert new keys... in that case, we have
>> no lock to serialize an order between the clear() and tx1...
>
>
> I'm pretty sure we do have this problem. We definitely need to add some
> tests for clear concurrent with other write operations.
>
> In my view, as long as we're going to expose a clear() operation in some
> way, it's going to be easiest to include it in a tx in transactional
caches.
> Exposing it in some other way is not going to make it any more
consistent,
> although prohibiting other operations in the same tx may make the code a
bit
> simpler.
>
>>
>>
>> >
>> > I don't think there would ever be a need for a clear() operation to
>> > interact with other transactions, so I'd rather make it illegal to
>> > invoke a clear() inside a transaction, or simply ignore the
>> > transactional scope and have an immediate and distributed effect.
>
>
> We can't control whether other transactions are invoked concurrently with
> clear(), so we can't make that illegal... If you mean interaction with
other
> operations in the same transaction, I agree that it's not absolutely
> necessary to allow them, but I think most of that work has already been
> done.
>
>>
>> I think the second solution can make inconsistency data ("or simply
>> ignore the transactional scope and have an immediate and distributed
>> effect")
>
>
> Maybe that's the idea? Sanne, do you mean that if clear() were exposed
via a
> different API, it wouldn't have to provide the same consistency
guarantees
> as regular operations?
Yes that's the main point that we need to clarify.
Note that this discussion started because of a problem we're having in
the integration
of Infinispan as Hibernate 2nd level cache, which is a quite mature
contract with
some very clear ideas of how a cache needs to behave.
After more discussions yesterday, I think I'm understanding
(Galder
will likely know more
but is currently away) that ideally a cache should be able to provide
both a transactional
clear() operation and an immediately-affecting one.
The only need for the transactional one is to be able to undo the
operation in case the
user's transaction is rolled back: we don't even support this case
today since it's suspending
the transaction to run the clear() operation in an independent transaction.
But the transactional clear() operation is far less important than
having a clear "evict all now"
operation. That's one of the basic operations a good mannered Cache
should provide:
remember that as a cache it's always a valid answer to return "null",
it's just suboptimal.
Returning the wrong answer instead, can have critical consequences, so
it's expected to
provide a way for the admin (JMX) or driving application to
immediately wipe the state;
I would argue that even a glocal lock's timing implications are
undesirable in such a case.
TBH I'm not sure how would such an "evict all now" operation would work in
a distributed cache. Could you post a link to the Hibernate docs?
The JCache javadoc for clear() doesn't say anything about its interaction
with transactions.
Focusing on the identify of Infinispan as a Key/Value store, I'd
consider it almost mandatory
for the client to know - to explicitly list - the keys it's interested
into, and be completely oblivious
to the entries which you're unable to list. For example, think of a
cache shared by multiple tenants
or multiple apps, which use keys which for some reason can't return
true to an equality check
(for example because they're coming from different classloaders).
In such a perspective, any operation which affects (or enumerates)
keys is a bad practice and
should imho be banned from the user API, unless for administration or
monitoring purposes.
Our Map/Reduce API explicitly supports the user not listing the keys he's
interested in.
And we don't support multi-tenancy in a single cache, every cache must have
a single ClassLoader.
I think this is quite clearly expressed already by all the warning in
javadoc on methods like
org.infinispan.Cache.keySet()
org.infinispan.Cache.size()
...
They all list a warning about needing to be used only for debug
purposes: I don't think we need
to waste time into making these transitionally consistent, as people
shouldn't be using them.
The JCache API still has a clear() method and an iterator() method, even
though their Cache interface doesn't extend Map. That makes me think that
there may be legitimate uses for them, after all. And some users may never
see our javadocs, if they use the JCache API.
If a transactional clear() operation is possible without overhead,
then the 2nd level cache code
would benefit from it, but it's a borderline decision and its code
could be refactored to not need this,
at least that's my understanding so far and I guess we need Galder for
that to be clarified.
Sanne
>
>>
>>
>> >
>> > I'm likely missing something. What terrible consequences would this
>> > have?
>> >
>> > Cheers,
>> > Sanne
>> > _______________________________________________
>> > infinispan-dev mailing list
>> > infinispan-dev(a)lists.jboss.org
>> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>> >
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev(a)lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
6:21 a.m.
Guys, there's some context to this topic that Sanne has left out.
This discussion origins comes from Scott Marlow who's found a bug in a Hibernate 2LC
test. The test itself can be found in [1].
Basically, the test loads a couple of entities into the 2LC, and then calls evictAll in
the cache that evicts all entities from the 2LC. Then, it expects the size of the cache to
be 0 and entities to be gone from the 2LC.
The 2LC has a peculiar way to deal with evictEntityRegions or evictAll, it doesn't
just call clear() and hope for the best. Instead, it marks the cache as invalid, and next
time someone queries the cache, it clears it in a separate transaction. This is done
cluster wide too by sending messages to other node that the cache region is invalid.
The problem here is that because it's done in a separate transaction, it doesn't
have an impact on the on going transaction, so the assertions after evict fail. IOW, we
implemented evict as a "eventually clear", where eventually all transactions
will stop using the cached data, but any current transactions will still see data. We did
this for performance reasons and to avoids the issues Sanne mentiones below.
The problem is that we didn't envision this evict method to be used in such way. I
always assumed this would be run outside of a transaction. IOW, you'd use this when
you've updated the database separately (i.e. script, PL/SQL) and you want to clear the
2LC. I always thought that'd be something run via JMX or separate from another
transaction.
But, as Scott has found out, the TCK has some particular expectations of what happens
after a evictEntityRegions or evictAll call within a transaction :(
With this in mind, let me comment on the specifics below:
[1]
https://github.com/scottmarlow/wildfly/blob/b61dc1dddb1f70847d1d82206a366...
On Oct 2, 2013, at 1:03 AM, Sanne Grinovero <sanne(a)infinispan.org> wrote:
I'd love to brainstorm about the clear() operation and what it
means
on Infinispan.
I'm not sure to what extent, but it seems that clear() is designed to
work in a TX, or even create an implicit transaction if needed, but
I'm not understanding how that can work.
Obviously a clear() operation isn't listing all keys explicitly. Which
implies that it's undefined on which keys it's going to operate when
it's fired.. that seems like terribly wrong in a distributed key/value
store as we can't possibly freeze the global state and somehow define
a set of keys which are going to be affected, while an explicit
enumeration is needed to acquire the needed locks.
It might give a nice safe feeling that, when invoking a clear()
operation in a transaction, I can still abort the transaction to make
it cancel the operation; that's the only good part I can think of: we
can cancel it.
I don't think it has anything to do with consistency though? To make
sure you're effectively involving all replicas of all entries in a
consistent way, a lock would need to be acquired on each affected key,
which again implies a need to enumerate all keys, including the
unknown keys which might be hiding in a CacheStore: it's not enough to
broadcast the clear() operation to all nodes and have them simply wipe
their local state as that's never going to deal correctly
(consistently) with in-flight transactions working on different nodes
at different times (I guess enabling Total Order could help but you'd
need to make it mandatory).
So let's step back a second and consider what is the use case for
clear() ? I suspect it's primarily a method needed during testing, or
maybe cleanup before a backup is restored (operations), maybe a
manually activated JMX operation to clear the cache in exceptional
cases.
^ Well, that's precisely the problem of second level cache evictEntityRegions or
evictAll. In some scenarios, the clear is expected to affect the ongoing transaction too,
see above use case from the TCK.
I don't think there would ever be a need for a clear() operation
to
interact with other transactions, so I'd rather make it illegal to
invoke a clear() inside a transaction,
-1, unless you have other ideas to deal with evict calls that I mentioned above that
satisfy what the TCK is after.
or simply ignore the
transactional scope and have an immediate and distributed effect.
That'd be more inline with what's needed above.
I'm likely missing something. What terrible consequences would
this have?
p.s. For the 2LC use case, I'm trying to see if the clear() could be done in the same
transaction and see if that helps, but no luck yet as that causes other failures.
Cheers,
Sanne
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
galder(a)redhat.com
twitter.com/galderz
Project Lead, Escalante
http://escalante.io
Engineer, Infinispan
http://infinispan.org
6:33 a.m.
On Oct 16, 2013, at 1:21 PM, Galder Zamarreño <galder(a)redhat.com> wrote:
Guys, there's some context to this topic that Sanne has left
out.
This discussion origins comes from Scott Marlow who's found a bug in a Hibernate 2LC
test. The test itself can be found in [1].
Basically, the test loads a couple of entities into the 2LC, and then calls evictAll in
the cache that evicts all entities from the 2LC. Then, it expects the size of the cache to
be 0 and entities to be gone from the 2LC.
The 2LC has a peculiar way to deal with evictEntityRegions or evictAll, it doesn't
just call clear() and hope for the best. Instead, it marks the cache as invalid, and next
time someone queries the cache, it clears it in a separate transaction. This is done
cluster wide too by sending messages to other node that the cache region is invalid.
The problem here is that because it's done in a separate transaction, it doesn't
have an impact on the on going transaction, so the assertions after evict fail. IOW, we
implemented evict as a "eventually clear", where eventually all transactions
will stop using the cached data, but any current transactions will still see data. We did
this for performance reasons and to avoids the issues Sanne mentiones below.
The problem is that we didn't envision this evict method to be used in such way. I
always assumed this would be run outside of a transaction. IOW, you'd use this when
you've updated the database separately (i.e. script, PL/SQL) and you want to clear the
2LC. I always thought that'd be something run via JMX or separate from another
transaction.
But, as Scott has found out, the TCK has some particular expectations of what happens
after a evictEntityRegions or evictAll call within a transaction :(
With this in mind, let me comment on the specifics below:
[1]
https://github.com/scottmarlow/wildfly/blob/b61dc1dddb1f70847d1d82206a366...
On Oct 2, 2013, at 1:03 AM, Sanne Grinovero <sanne(a)infinispan.org> wrote:
> I'd love to brainstorm about the clear() operation and what it means
> on Infinispan.
>
> I'm not sure to what extent, but it seems that clear() is designed to
> work in a TX, or even create an implicit transaction if needed, but
> I'm not understanding how that can work.
>
> Obviously a clear() operation isn't listing all keys explicitly. Which
> implies that it's undefined on which keys it's going to operate when
> it's fired.. that seems like terribly wrong in a distributed key/value
> store as we can't possibly freeze the global state and somehow define
> a set of keys which are going to be affected, while an explicit
> enumeration is needed to acquire the needed locks.
>
> It might give a nice safe feeling that, when invoking a clear()
> operation in a transaction, I can still abort the transaction to make
> it cancel the operation; that's the only good part I can think of: we
> can cancel it.
>
> I don't think it has anything to do with consistency though? To make
> sure you're effectively involving all replicas of all entries in a
> consistent way, a lock would need to be acquired on each affected key,
> which again implies a need to enumerate all keys, including the
> unknown keys which might be hiding in a CacheStore: it's not enough to
> broadcast the clear() operation to all nodes and have them simply wipe
> their local state as that's never going to deal correctly
> (consistently) with in-flight transactions working on different nodes
> at different times (I guess enabling Total Order could help but you'd
> need to make it mandatory).
>
> So let's step back a second and consider what is the use case for
> clear() ? I suspect it's primarily a method needed during testing, or
> maybe cleanup before a backup is restored (operations), maybe a
> manually activated JMX operation to clear the cache in exceptional
> cases.
^ Well, that's precisely the problem of second level cache evictEntityRegions or
evictAll. In some scenarios, the clear is expected to affect the ongoing transaction too,
see above use case from the TCK.
> I don't think there would ever be a need for a clear() operation to
> interact with other transactions, so I'd rather make it illegal to
> invoke a clear() inside a transaction,
-1, unless you have other ideas to deal with evict calls that I mentioned above that
satisfy what the TCK is after.
Btw, I'm investigating other ways to solve this particular issue, maybe by tracking
which transaction made the evict call in the 2LC regions, and if any get/size calls come
after that, return nothing, instead of calling clear, and just leave the clear for when a
brand new transaction (different to the one that called evictAll) runs.
> or simply ignore the
> transactional scope and have an immediate and distributed effect.
That'd be more inline with what's needed above.
> I'm likely missing something. What terrible consequences would this have?
p.s. For the 2LC use case, I'm trying to see if the clear() could be done in the same
transaction and see if that helps, but no luck yet as that causes other failures.
>
> Cheers,
> Sanne
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
galder(a)redhat.com
twitter.com/galderz
Project Lead, Escalante
http://escalante.io
Engineer, Infinispan
http://infinispan.org
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
galder(a)redhat.com
twitter.com/galderz
Project Lead, Escalante
http://escalante.io
Engineer, Infinispan
http://infinispan.org
7:33 a.m.
On Wed, Oct 16, 2013 at 2:33 PM, Galder Zamarreño <galder(a)redhat.com> wrote:
On Oct 16, 2013, at 1:21 PM, Galder Zamarreño <galder(a)redhat.com> wrote:
> Guys, there's some context to this topic that Sanne has left out.
>
> This discussion origins comes from Scott Marlow who's found a bug in a
Hibernate 2LC test. The test itself can be found in [1].
>
> Basically, the test loads a couple of entities into the 2LC, and then
calls evictAll in the cache that evicts all entities from the 2LC. Then, it
expects the size of the cache to be 0 and entities to be gone from the 2LC.
>
> The 2LC has a peculiar way to deal with evictEntityRegions or evictAll,
it doesn't just call clear() and hope for the best. Instead, it marks the
cache as invalid, and next time someone queries the cache, it clears it in
a separate transaction. This is done cluster wide too by sending messages
to other node that the cache region is invalid.
>
> The problem here is that because it's done in a separate transaction, it
doesn't have an impact on the on going transaction, so the assertions after
evict fail. IOW, we implemented evict as a "eventually clear", where
eventually all transactions will stop using the cached data, but any
current transactions will still see data. We did this for performance
reasons and to avoids the issues Sanne mentiones below.
>
> The problem is that we didn't envision this evict method to be used in
such way. I always assumed this would be run outside of a transaction. IOW,
you'd use this when you've updated the database separately (i.e. script,
PL/SQL) and you want to clear the 2LC. I always thought that'd be something
run via JMX or separate from another transaction.
>
> But, as Scott has found out, the TCK has some particular expectations of
what happens after a evictEntityRegions or evictAll call within a
transaction :(
>
> With this in mind, let me comment on the specifics below:
>
> [1]
https://github.com/scottmarlow/wildfly/blob/b61dc1dddb1f70847d1d82206a366...
>
> On Oct 2, 2013, at 1:03 AM, Sanne Grinovero <sanne(a)infinispan.org>
wrote:
>
>> I'd love to brainstorm about the clear() operation and what it means
>> on Infinispan.
>>
>> I'm not sure to what extent, but it seems that clear() is designed to
>> work in a TX, or even create an implicit transaction if needed, but
>> I'm not understanding how that can work.
>>
>> Obviously a clear() operation isn't listing all keys explicitly. Which
>> implies that it's undefined on which keys it's going to operate when
>> it's fired.. that seems like terribly wrong in a distributed key/value
>> store as we can't possibly freeze the global state and somehow define
>> a set of keys which are going to be affected, while an explicit
>> enumeration is needed to acquire the needed locks.
>>
>> It might give a nice safe feeling that, when invoking a clear()
>> operation in a transaction, I can still abort the transaction to make
>> it cancel the operation; that's the only good part I can think of: we
>> can cancel it.
>>
>> I don't think it has anything to do with consistency though? To make
>> sure you're effectively involving all replicas of all entries in a
>> consistent way, a lock would need to be acquired on each affected key,
>> which again implies a need to enumerate all keys, including the
>> unknown keys which might be hiding in a CacheStore: it's not enough to
>> broadcast the clear() operation to all nodes and have them simply wipe
>> their local state as that's never going to deal correctly
>> (consistently) with in-flight transactions working on different nodes
>> at different times (I guess enabling Total Order could help but you'd
>> need to make it mandatory).
>>
>> So let's step back a second and consider what is the use case for
>> clear() ? I suspect it's primarily a method needed during testing, or
>> maybe cleanup before a backup is restored (operations), maybe a
>> manually activated JMX operation to clear the cache in exceptional
>> cases.
>
> ^ Well, that's precisely the problem of second level cache
evictEntityRegions or evictAll. In some scenarios, the clear is expected to
affect the ongoing transaction too, see above use case from the TCK.
>
>> I don't think there would ever be a need for a clear() operation to
>> interact with other transactions, so I'd rather make it illegal to
>> invoke a clear() inside a transaction,
>
> -1, unless you have other ideas to deal with evict calls that I
mentioned above that satisfy what the TCK is after.
Btw, I'm investigating other ways to solve this particular issue, maybe by
tracking which transaction made the evict call in the 2LC regions, and if
any get/size calls come after that, return nothing, instead of calling
clear, and just leave the clear for when a brand new transaction (different
to the one that called evictAll) runs.
If I understood the requirements correctly, it might be simpler to call
clear() twice: once inside the tx, and once while the tx is suspended. That
way, both the running tx and the other txs see an empty cache.
>
>> or simply ignore the
>> transactional scope and have an immediate and distributed effect.
>
> That'd be more inline with what's needed above.
>
>> I'm likely missing something. What terrible consequences would this
have?
>
> p.s. For the 2LC use case, I'm trying to see if the clear() could be
done in the same transaction and see if that helps, but no luck yet as that
causes other failures.
>
>>
>> Cheers,
>> Sanne
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev(a)lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> --
> Galder Zamarreño
> galder(a)redhat.com
> twitter.com/galderz
>
> Project Lead, Escalante
> http://escalante.io
>
> Engineer, Infinispan
> http://infinispan.org
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
galder(a)redhat.com
twitter.com/galderz
Project Lead, Escalante
http://escalante.io
Engineer, Infinispan
http://infinispan.org
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
8:11 a.m.
On 16 October 2013 12:21, Galder Zamarreño <galder(a)redhat.com> wrote:
Guys, there's some context to this topic that Sanne has left out.
This discussion origins comes from Scott Marlow who's found a bug in a Hibernate 2LC
test. The test itself can be found in [1].
Basically, the test loads a couple of entities into the 2LC, and then calls evictAll in
the cache that evicts all entities from the 2LC. Then, it expects the size of the cache to
be 0 and entities to be gone from the 2LC.
The 2LC has a peculiar way to deal with evictEntityRegions or evictAll, it doesn't
just call clear() and hope for the best. Instead, it marks the cache as invalid, and next
time someone queries the cache, it clears it in a separate transaction. This is done
cluster wide too by sending messages to other node that the cache region is invalid.
The problem here is that because it's done in a separate transaction, it doesn't
have an impact on the on going transaction, so the assertions after evict fail. IOW, we
implemented evict as a "eventually clear", where eventually all transactions
will stop using the cached data, but any current transactions will still see data. We did
this for performance reasons and to avoids the issues Sanne mentiones below.
The problem is that we didn't envision this evict method to be used in such way. I
always assumed this would be run outside of a transaction. IOW, you'd use this when
you've updated the database separately (i.e. script, PL/SQL) and you want to clear the
2LC. I always thought that'd be something run via JMX or separate from another
transaction.
That's indeed one of the use cases for a clear operation, but in this
case - which I'd classify as maintenance/manual control - I don't
think the user even expects a transaction to happen.
It could also happen by explicit invocation of user code, but I think
we're in the same use case.
But, as Scott has found out, the TCK has some particular expectations
of what happens after a evictEntityRegions or evictAll call within a transaction :(
To clarify, that's not just a TCK weirdness that we need to "hack
around" to pass it, it's an important use case, and as you say very
different than the "manual" control.
The scenario is that the user is invoking some batch update statement
on the database which potentially changes a set of entities, whose ids
might be unknown. To make a simple example, say the statement adds one
day to the age of all users and you want such a statement to run on
each midnight.
Such statements exist and are quite common (probably within the scope
of a better use case), and the point is that in such cases, and
evictAll(users) need to happen, but it should happen in the scope of
the transaction: isolation-wise, the current Session shouldn't be
loading any more stale data - and IF it loads from the DB fresh data,
this newly cached entries need to be confined to the current TX, and
the changes need to be made visible to other nodes and Sessions only
at commit: in other words, ideally you should roll back the eviction
from happening if the statement is rolled back.
Still considering it's a cache, with "ideally" I mean that it would
still be correct - although suboptimal - to evict all caches
immediately (out of transaction): worse case other sessions will have
slightly more cache misses, but they would still be correct.
So to summarise: Hibernate expects to be able to use it both in a
transactional mode and in a non-transactional mode. Steve Ebersole
pointed out that he would be open to discuss using two differently
named methods to make the expectation clear, as it currently just
relies on the fact that a transaction is active or not.
Dan has a great point about JCache, still we'd need to investigate
what the spec is saying about transactionality of such a method, it
would surprise me if it mandated transactionality?
I'll call myself out on how you're all thinking to implement it, as
long as the requirements described above are clear: I think they make
much sense for any general purpose cache and it's not just a weirdness
of the TCK designer or Hibernate. So the trouble for us is that
Infinispan no longer can operate on a cache mixing transactional and
non-transactional operations. That looks like something that needs to
be addressed, and considering the general use case for a clear I think
it would suffice to clearly mark clear as a non-transactional
operation.
BTW If I really need to clear it.. I don't care about it to be
consistently applied "all or nothing", it would be much more of a sane
decision to apply it at least where it worked fine. Being this an XA
operation seems nonsense.
That said, in the way a distributed key value store which aims at
bolting on eventual consistency & co, I would consider it damn
dangerous to embark in designing operations whose scope is not
restricted to a well defined set of keys. I tried to pitch my concern
at that level rather than jumping in the details of how to make the
TCK work. Conceptually, I think it's very tricky as you won't be able
to track any form of causality between this operation and subsequent
write operations. At some point you'll need to introduce tombstone
markers, and you need a to define what is your "keyset universe" to
implement such a method.
Cheers,
Sanne
With this in mind, let me comment on the specifics below:
[1]
https://github.com/scottmarlow/wildfly/blob/b61dc1dddb1f70847d1d82206a366...
On Oct 2, 2013, at 1:03 AM, Sanne Grinovero <sanne(a)infinispan.org> wrote:
> I'd love to brainstorm about the clear() operation and what it means
> on Infinispan.
>
> I'm not sure to what extent, but it seems that clear() is designed to
> work in a TX, or even create an implicit transaction if needed, but
> I'm not understanding how that can work.
>
> Obviously a clear() operation isn't listing all keys explicitly. Which
> implies that it's undefined on which keys it's going to operate when
> it's fired.. that seems like terribly wrong in a distributed key/value
> store as we can't possibly freeze the global state and somehow define
> a set of keys which are going to be affected, while an explicit
> enumeration is needed to acquire the needed locks.
>
> It might give a nice safe feeling that, when invoking a clear()
> operation in a transaction, I can still abort the transaction to make
> it cancel the operation; that's the only good part I can think of: we
> can cancel it.
>
> I don't think it has anything to do with consistency though? To make
> sure you're effectively involving all replicas of all entries in a
> consistent way, a lock would need to be acquired on each affected key,
> which again implies a need to enumerate all keys, including the
> unknown keys which might be hiding in a CacheStore: it's not enough to
> broadcast the clear() operation to all nodes and have them simply wipe
> their local state as that's never going to deal correctly
> (consistently) with in-flight transactions working on different nodes
> at different times (I guess enabling Total Order could help but you'd
> need to make it mandatory).
>
> So let's step back a second and consider what is the use case for
> clear() ? I suspect it's primarily a method needed during testing, or
> maybe cleanup before a backup is restored (operations), maybe a
> manually activated JMX operation to clear the cache in exceptional
> cases.
^ Well, that's precisely the problem of second level cache evictEntityRegions or
evictAll. In some scenarios, the clear is expected to affect the ongoing transaction too,
see above use case from the TCK.
> I don't think there would ever be a need for a clear() operation to
> interact with other transactions, so I'd rather make it illegal to
> invoke a clear() inside a transaction,
-1, unless you have other ideas to deal with evict calls that I mentioned above that
satisfy what the TCK is after.
> or simply ignore the
> transactional scope and have an immediate and distributed effect.
That'd be more inline with what's needed above.
> I'm likely missing something. What terrible consequences would this have?
p.s. For the 2LC use case, I'm trying to see if the clear() could be done in the same
transaction and see if that helps, but no luck yet as that causes other failures.
>
> Cheers,
> Sanne
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
galder(a)redhat.com
twitter.com/galderz
Project Lead, Escalante
http://escalante.io
Engineer, Infinispan
http://infinispan.org
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
9:15 a.m.
On Oct 16, 2013, at 3:11 PM, Sanne Grinovero <sanne(a)infinispan.org> wrote:
On 16 October 2013 12:21, Galder Zamarreño <galder(a)redhat.com>
wrote:
>
> Guys, there's some context to this topic that Sanne has left out.
>
> This discussion origins comes from Scott Marlow who's found a bug in a Hibernate
2LC test. The test itself can be found in [1].
>
> Basically, the test loads a couple of entities into the 2LC, and then calls evictAll
in the cache that evicts all entities from the 2LC. Then, it expects the size of the cache
to be 0 and entities to be gone from the 2LC.
>
> The 2LC has a peculiar way to deal with evictEntityRegions or evictAll, it
doesn't just call clear() and hope for the best. Instead, it marks the cache as
invalid, and next time someone queries the cache, it clears it in a separate transaction.
This is done cluster wide too by sending messages to other node that the cache region is
invalid.
>
> The problem here is that because it's done in a separate transaction, it
doesn't have an impact on the on going transaction, so the assertions after evict
fail. IOW, we implemented evict as a "eventually clear", where eventually all
transactions will stop using the cached data, but any current transactions will still see
data. We did this for performance reasons and to avoids the issues Sanne mentiones below.
>
> The problem is that we didn't envision this evict method to be used in such way.
I always assumed this would be run outside of a transaction. IOW, you'd use this when
you've updated the database separately (i.e. script, PL/SQL) and you want to clear the
2LC. I always thought that'd be something run via JMX or separate from another
transaction.
That's indeed one of the use cases for a clear operation, but in this
case - which I'd classify as maintenance/manual control - I don't
think the user even expects a transaction to happen.
It could also happen by explicit invocation of user code, but I think
we're in the same use case.
> But, as Scott has found out, the TCK has some particular expectations of what happens
after a evictEntityRegions or evictAll call within a transaction :(
To clarify, that's not just a TCK weirdness that we need to "hack
around" to pass it, it's an important use case, and as you say very
different than the "manual" control.
The scenario is that the user is invoking some batch update statement
on the database which potentially changes a set of entities, whose ids
might be unknown. To make a simple example, say the statement adds one
day to the age of all users and you want such a statement to run on
each midnight.
Such statements exist and are quite common (probably within the scope
of a better use case), and the point is that in such cases, and
evictAll(users) need to happen, but it should happen in the scope of
the transaction: isolation-wise, the current Session shouldn't be
loading any more stale data - and IF it loads from the DB fresh data,
this newly cached entries need to be confined to the current TX, and
the changes need to be made visible to other nodes and Sessions only
at commit: in other words, ideally you should roll back the eviction
from happening if the statement is rolled back.
Still considering it's a cache, with "ideally" I mean that it would
still be correct - although suboptimal - to evict all caches
immediately (out of transaction): worse case other sessions will have
slightly more cache misses, but they would still be correct.
So to summarise: Hibernate expects to be able to use it both in a
transactional mode and in a non-transactional mode. Steve Ebersole
pointed out that he would be open to discuss using two differently
named methods to make the expectation clear, as it currently just
relies on the fact that a transaction is active or not.
Dan has a great point about JCache, still we'd need to investigate
what the spec is saying about transactionality of such a method, it
would surprise me if it mandated transactionality?
^ Transactions have been left out of the spec [1].
I'll call myself out on how you're all thinking to implement it, as
long as the requirements described above are clear: I think they make
much sense for any general purpose cache and it's not just a weirdness
of the TCK designer or Hibernate. So the trouble for us is that
Infinispan no longer can operate on a cache mixing transactional and
non-transactional operations. That looks like something that needs to
be addressed, and considering the general use case for a clear I think
it would suffice to clearly mark clear as a non-transactional
operation.
BTW If I really need to clear it.. I don't care about it to be
consistently applied "all or nothing", it would be much more of a sane
decision to apply it at least where it worked fine. Being this an XA
operation seems nonsense.
That said, in the way a distributed key value store which aims at
bolting on eventual consistency & co, I would consider it damn
dangerous to embark in designing operations whose scope is not
restricted to a well defined set of keys. I tried to pitch my concern
at that level rather than jumping in the details of how to make the
TCK work. Conceptually, I think it's very tricky as you won't be able
to track any form of causality between this operation and subsequent
write operations. At some point you'll need to introduce tombstone
markers, and you need a to define what is your "keyset universe" to
implement such a method.
[1] https://groups.google.com/forum/?fromgroups=#!topic/jsr107/bEHJ5m10HT4
Cheers,
Sanne
>
> With this in mind, let me comment on the specifics below:
>
> [1]
https://github.com/scottmarlow/wildfly/blob/b61dc1dddb1f70847d1d82206a366...
>
> On Oct 2, 2013, at 1:03 AM, Sanne Grinovero <sanne(a)infinispan.org> wrote:
>
>> I'd love to brainstorm about the clear() operation and what it means
>> on Infinispan.
>>
>> I'm not sure to what extent, but it seems that clear() is designed to
>> work in a TX, or even create an implicit transaction if needed, but
>> I'm not understanding how that can work.
>>
>> Obviously a clear() operation isn't listing all keys explicitly. Which
>> implies that it's undefined on which keys it's going to operate when
>> it's fired.. that seems like terribly wrong in a distributed key/value
>> store as we can't possibly freeze the global state and somehow define
>> a set of keys which are going to be affected, while an explicit
>> enumeration is needed to acquire the needed locks.
>>
>> It might give a nice safe feeling that, when invoking a clear()
>> operation in a transaction, I can still abort the transaction to make
>> it cancel the operation; that's the only good part I can think of: we
>> can cancel it.
>>
>> I don't think it has anything to do with consistency though? To make
>> sure you're effectively involving all replicas of all entries in a
>> consistent way, a lock would need to be acquired on each affected key,
>> which again implies a need to enumerate all keys, including the
>> unknown keys which might be hiding in a CacheStore: it's not enough to
>> broadcast the clear() operation to all nodes and have them simply wipe
>> their local state as that's never going to deal correctly
>> (consistently) with in-flight transactions working on different nodes
>> at different times (I guess enabling Total Order could help but you'd
>> need to make it mandatory).
>>
>> So let's step back a second and consider what is the use case for
>> clear() ? I suspect it's primarily a method needed during testing, or
>> maybe cleanup before a backup is restored (operations), maybe a
>> manually activated JMX operation to clear the cache in exceptional
>> cases.
>
> ^ Well, that's precisely the problem of second level cache evictEntityRegions or
evictAll. In some scenarios, the clear is expected to affect the ongoing transaction too,
see above use case from the TCK.
>
>> I don't think there would ever be a need for a clear() operation to
>> interact with other transactions, so I'd rather make it illegal to
>> invoke a clear() inside a transaction,
>
> -1, unless you have other ideas to deal with evict calls that I mentioned above that
satisfy what the TCK is after.
>
>> or simply ignore the
>> transactional scope and have an immediate and distributed effect.
>
> That'd be more inline with what's needed above.
>
>> I'm likely missing something. What terrible consequences would this have?
>
> p.s. For the 2LC use case, I'm trying to see if the clear() could be done in the
same transaction and see if that helps, but no luck yet as that causes other failures.
>
>>
>> Cheers,
>> Sanne
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev(a)lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
> --
> Galder Zamarreño
> galder(a)redhat.com
> twitter.com/galderz
>
> Project Lead, Escalante
> http://escalante.io
>
> Engineer, Infinispan
> http://infinispan.org
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
galder(a)redhat.com
twitter.com/galderz
Project Lead, Escalante
http://escalante.io
Engineer, Infinispan
http://infinispan.org
9:29 a.m.
On 16 October 2013 15:15, Galder Zamarreño <galder(a)redhat.com> wrote:
On Oct 16, 2013, at 3:11 PM, Sanne Grinovero <sanne(a)infinispan.org> wrote:
> On 16 October 2013 12:21, Galder Zamarreño <galder(a)redhat.com> wrote:
>>
>> Guys, there's some context to this topic that Sanne has left out.
>>
>> This discussion origins comes from Scott Marlow who's found a bug in a
Hibernate 2LC test. The test itself can be found in [1].
>>
>> Basically, the test loads a couple of entities into the 2LC, and then calls
evictAll in the cache that evicts all entities from the 2LC. Then, it expects the size of
the cache to be 0 and entities to be gone from the 2LC.
>>
>> The 2LC has a peculiar way to deal with evictEntityRegions or evictAll, it
doesn't just call clear() and hope for the best. Instead, it marks the cache as
invalid, and next time someone queries the cache, it clears it in a separate transaction.
This is done cluster wide too by sending messages to other node that the cache region is
invalid.
>>
>> The problem here is that because it's done in a separate transaction, it
doesn't have an impact on the on going transaction, so the assertions after evict
fail. IOW, we implemented evict as a "eventually clear", where eventually all
transactions will stop using the cached data, but any current transactions will still see
data. We did this for performance reasons and to avoids the issues Sanne mentiones below.
>>
>> The problem is that we didn't envision this evict method to be used in such
way. I always assumed this would be run outside of a transaction. IOW, you'd use this
when you've updated the database separately (i.e. script, PL/SQL) and you want to
clear the 2LC. I always thought that'd be something run via JMX or separate from
another transaction.
>
> That's indeed one of the use cases for a clear operation, but in this
> case - which I'd classify as maintenance/manual control - I don't
> think the user even expects a transaction to happen.
> It could also happen by explicit invocation of user code, but I think
> we're in the same use case.
>
>
>> But, as Scott has found out, the TCK has some particular expectations of what
happens after a evictEntityRegions or evictAll call within a transaction :(
>
> To clarify, that's not just a TCK weirdness that we need to "hack
> around" to pass it, it's an important use case, and as you say very
> different than the "manual" control.
> The scenario is that the user is invoking some batch update statement
> on the database which potentially changes a set of entities, whose ids
> might be unknown. To make a simple example, say the statement adds one
> day to the age of all users and you want such a statement to run on
> each midnight.
> Such statements exist and are quite common (probably within the scope
> of a better use case), and the point is that in such cases, and
> evictAll(users) need to happen, but it should happen in the scope of
> the transaction: isolation-wise, the current Session shouldn't be
> loading any more stale data - and IF it loads from the DB fresh data,
> this newly cached entries need to be confined to the current TX, and
> the changes need to be made visible to other nodes and Sessions only
> at commit: in other words, ideally you should roll back the eviction
> from happening if the statement is rolled back.
>
> Still considering it's a cache, with "ideally" I mean that it would
> still be correct - although suboptimal - to evict all caches
> immediately (out of transaction): worse case other sessions will have
> slightly more cache misses, but they would still be correct.
>
> So to summarise: Hibernate expects to be able to use it both in a
> transactional mode and in a non-transactional mode. Steve Ebersole
> pointed out that he would be open to discuss using two differently
> named methods to make the expectation clear, as it currently just
> relies on the fact that a transaction is active or not.
>
> Dan has a great point about JCache, still we'd need to investigate
> what the spec is saying about transactionality of such a method, it
> would surprise me if it mandated transactionality?
^ Transactions have been left out of the spec [1].
Very wise ;-)
So that opens up any option for us.
If we can, I'd just avoid creating the implicit transaction. That
seems to be the troublesome part and also I suspect it would be an
easy fix: if there is no transaction, don't create one; if there is
one, participate.
On the fact that some clear() operations could be transactional.. I
think that opens a pandora box in future but since it's the optimal
behaviour also for the 2LC and it seems to be already correctly
implemented I'd vote to keep that for the moment.
Sanne
>
> I'll call myself out on how you're all thinking to implement it, as
> long as the requirements described above are clear: I think they make
> much sense for any general purpose cache and it's not just a weirdness
> of the TCK designer or Hibernate. So the trouble for us is that
> Infinispan no longer can operate on a cache mixing transactional and
> non-transactional operations. That looks like something that needs to
> be addressed, and considering the general use case for a clear I think
> it would suffice to clearly mark clear as a non-transactional
> operation.
>
> BTW If I really need to clear it.. I don't care about it to be
> consistently applied "all or nothing", it would be much more of a sane
> decision to apply it at least where it worked fine. Being this an XA
> operation seems nonsense.
>
> That said, in the way a distributed key value store which aims at
> bolting on eventual consistency & co, I would consider it damn
> dangerous to embark in designing operations whose scope is not
> restricted to a well defined set of keys. I tried to pitch my concern
> at that level rather than jumping in the details of how to make the
> TCK work. Conceptually, I think it's very tricky as you won't be able
> to track any form of causality between this operation and subsequent
> write operations. At some point you'll need to introduce tombstone
> markers, and you need a to define what is your "keyset universe" to
> implement such a method.
[1] https://groups.google.com/forum/?fromgroups=#!topic/jsr107/bEHJ5m10HT4
>
> Cheers,
> Sanne
>
>>
>> With this in mind, let me comment on the specifics below:
>>
>> [1]
https://github.com/scottmarlow/wildfly/blob/b61dc1dddb1f70847d1d82206a366...
>>
>> On Oct 2, 2013, at 1:03 AM, Sanne Grinovero <sanne(a)infinispan.org> wrote:
>>
>>> I'd love to brainstorm about the clear() operation and what it means
>>> on Infinispan.
>>>
>>> I'm not sure to what extent, but it seems that clear() is designed to
>>> work in a TX, or even create an implicit transaction if needed, but
>>> I'm not understanding how that can work.
>>>
>>> Obviously a clear() operation isn't listing all keys explicitly. Which
>>> implies that it's undefined on which keys it's going to operate when
>>> it's fired.. that seems like terribly wrong in a distributed key/value
>>> store as we can't possibly freeze the global state and somehow define
>>> a set of keys which are going to be affected, while an explicit
>>> enumeration is needed to acquire the needed locks.
>>>
>>> It might give a nice safe feeling that, when invoking a clear()
>>> operation in a transaction, I can still abort the transaction to make
>>> it cancel the operation; that's the only good part I can think of: we
>>> can cancel it.
>>>
>>> I don't think it has anything to do with consistency though? To make
>>> sure you're effectively involving all replicas of all entries in a
>>> consistent way, a lock would need to be acquired on each affected key,
>>> which again implies a need to enumerate all keys, including the
>>> unknown keys which might be hiding in a CacheStore: it's not enough to
>>> broadcast the clear() operation to all nodes and have them simply wipe
>>> their local state as that's never going to deal correctly
>>> (consistently) with in-flight transactions working on different nodes
>>> at different times (I guess enabling Total Order could help but you'd
>>> need to make it mandatory).
>>>
>>> So let's step back a second and consider what is the use case for
>>> clear() ? I suspect it's primarily a method needed during testing, or
>>> maybe cleanup before a backup is restored (operations), maybe a
>>> manually activated JMX operation to clear the cache in exceptional
>>> cases.
>>
>> ^ Well, that's precisely the problem of second level cache evictEntityRegions
or evictAll. In some scenarios, the clear is expected to affect the ongoing transaction
too, see above use case from the TCK.
>>
>>> I don't think there would ever be a need for a clear() operation to
>>> interact with other transactions, so I'd rather make it illegal to
>>> invoke a clear() inside a transaction,
>>
>> -1, unless you have other ideas to deal with evict calls that I mentioned above
that satisfy what the TCK is after.
>>
>>> or simply ignore the
>>> transactional scope and have an immediate and distributed effect.
>>
>> That'd be more inline with what's needed above.
>>
>>> I'm likely missing something. What terrible consequences would this
have?
>>
>> p.s. For the 2LC use case, I'm trying to see if the clear() could be done in
the same transaction and see if that helps, but no luck yet as that causes other
failures.
>>
>>>
>>> Cheers,
>>> Sanne
>>> _______________________________________________
>>> infinispan-dev mailing list
>>> infinispan-dev(a)lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>>
>>
>> --
>> Galder Zamarreño
>> galder(a)redhat.com
>> twitter.com/galderz
>>
>> Project Lead, Escalante
>> http://escalante.io
>>
>> Engineer, Infinispan
>> http://infinispan.org
>>
>>
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev(a)lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev
--
Galder Zamarreño
galder(a)redhat.com
twitter.com/galderz
Project Lead, Escalante
http://escalante.io
Engineer, Infinispan
http://infinispan.org
_______________________________________________
infinispan-dev mailing list
infinispan-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/infinispan-dev
4552
days inactive
4567
days old
infinispan-dev@lists.jboss.org
11 comments
5 participants
participants (5)
-
Dan Berindei -
Galder Zamarreño -
Pedro Ruivo -
Sanne Grinovero -
William Burns