Hi, Re: Porting over http://opensource.atlassian.com/projects/hibernate/browse/HHH-3818 to Infinispan 2nd level cache. For HHH-3818, Brian used the cache as a bus to notify that the cache contents where invalid. To do so, he implemented cache entry modified and invalidated callbacks that react to cache modifications or invalidations where the FQN is an internal fqn which contains the local address. Now, the trick he uses for the invalidation is that on startup or whenever there's a view change, he establishes these internal FQNs in each node for each of the members in the cluster, i.e. fqn=/test/com/foo/test/ENTITY/NODE/127.0.0.1:39104 So, in an invalidation scenario, if you a node puts something on /test/com/foo/test/ENTITY/NODE/127.0.0.1:39104, the other nodes have at least /test/com/foo/test/ENTITY/NODE/127.0.0.1:39104 created on their caches which means that the invalidation callback works fine cos the FQN exists in other nodes. Now, in Infinispan's case, bearing in mind that there're no FQNs, I could set up such keys when there's a view change too. However, I was thinking of another alternative. Currently, cache invalidation callback fails on Infinispan because when you do a put on key=127.0.0.1:39104, the other nodes do not contain the key=127.0.0.1:39104, so no invalidation callback is received. So, my suggestion is the following: Why not have a callback saying: "I attempted to do an invalidation on k=x but k=x did not exist". Maybe something like CacheEntryNotInvalidated callback? It looks to me that it'd be relatively simply to implement such notification and would simplify the code on the Infinispan 2nd level cache provider.

On 10/28/2009 05:50 PM, Manik Surtani wrote: > > On 28 Oct 2009, at 09:29, Galder Zamarreno wrote: > >> Hi, >> >> Re: Porting over >> http://opensource.atlassian.com/projects/hibernate/browse/HHH-3818 >> to >> Infinispan 2nd level cache. >> >> For HHH-3818, Brian used the cache as a bus to notify that the cache >> contents where invalid. To do so, he implemented cache entry >> modified >> and invalidated callbacks that react to cache modifications or >> invalidations where the FQN is an internal fqn which contains the >> local >> address. >> >> Now, the trick he uses for the invalidation is that on startup or >> whenever there's a view change, he establishes these internal FQNs >> in >> each node for each of the members in the cluster, i.e. >> fqn=/test/com/foo/test/ENTITY/NODE/127.0.0.1:39104 >> >> So, in an invalidation scenario, if you a node puts something on >> /test/com/foo/test/ENTITY/NODE/127.0.0.1:39104, the other nodes have >> at >> least /test/com/foo/test/ENTITY/NODE/127.0.0.1:39104 created on >> their >> caches which means that the invalidation callback works fine cos the >> FQN >> exists in other nodes. >> >> Now, in Infinispan's case, bearing in mind that there're no FQNs, I >> could set up such keys when there's a view change too. However, I >> was >> thinking of another alternative. >> >> Currently, cache invalidation callback fails on Infinispan because >> when >> you do a put on key=127.0.0.1:39104, the other nodes do not contain >> the >> key=127.0.0.1:39104, so no invalidation callback is received. So, my >> suggestion is the following: >> >> Why not have a callback saying: "I attempted to do an invalidation >> on >> k=x but k=x did not exist". Maybe something like >> CacheEntryNotInvalidated callback? It looks to me that it'd be >> relatively simply to implement such notification and would simplify >> the >> code on the Infinispan 2nd level cache provider. > > Hmm, where would you impl this callback? As a part of the core > Infinispan notification system? The implementation would be somewhere in between RemoveCommand and InvalidateCommand. In RemoveCommand, after logging: log.trace("Nothing to remove since the entry is null or we have a null entry"); you'd have the possibility to notify that attempt was made to remove something but it wasn't there. I'm not sure whether it'd make a lot of sense to implement it for RemoveCommand so, we either add a notify method afterwards and make InvalidateCommand only provide a impl for it, or override perform() in IC, or somewhere in between, i.e. wrap code within the if (e == null || e.isNull()) into a method that is overriden by IC. Thoughts?

On 28 Oct 2009, at 17:04, Galder Zamarreno wrote: > > > On 10/28/2009 05:50 PM, Manik Surtani wrote: >> >> On 28 Oct 2009, at 09:29, Galder Zamarreno wrote: >> >>> Hi, >>> >>> Re: Porting over >>> http://opensource.atlassian.com/projects/hibernate/browse/HHH-3818 >>> to >>> Infinispan 2nd level cache. >>> >>> For HHH-3818, Brian used the cache as a bus to notify that the cache >>> contents where invalid. To do so, he implemented cache entry >>> modified >>> and invalidated callbacks that react to cache modifications or >>> invalidations where the FQN is an internal fqn which contains the >>> local >>> address. >>> >>> Now, the trick he uses for the invalidation is that on startup or >>> whenever there's a view change, he establishes these internal FQNs >>> in >>> each node for each of the members in the cluster, i.e. >>> fqn=/test/com/foo/test/ENTITY/NODE/127.0.0.1:39104 >>> >>> So, in an invalidation scenario, if you a node puts something on >>> /test/com/foo/test/ENTITY/NODE/127.0.0.1:39104, the other nodes have >>> at >>> least /test/com/foo/test/ENTITY/NODE/127.0.0.1:39104 created on >>> their >>> caches which means that the invalidation callback works fine cos the >>> FQN >>> exists in other nodes. >>> >>> Now, in Infinispan's case, bearing in mind that there're no FQNs, I >>> could set up such keys when there's a view change too. However, I >>> was >>> thinking of another alternative. >>> >>> Currently, cache invalidation callback fails on Infinispan because >>> when >>> you do a put on key=127.0.0.1:39104, the other nodes do not contain >>> the >>> key=127.0.0.1:39104, so no invalidation callback is received. So, my >>> suggestion is the following: >>> >>> Why not have a callback saying: "I attempted to do an invalidation >>> on >>> k=x but k=x did not exist". Maybe something like >>> CacheEntryNotInvalidated callback? It looks to me that it'd be >>> relatively simply to implement such notification and would simplify >>> the >>> code on the Infinispan 2nd level cache provider. >> >> Hmm, where would you impl this callback? As a part of the core >> Infinispan notification system? > > The implementation would be somewhere in between RemoveCommand and > InvalidateCommand. In RemoveCommand, after logging: > > log.trace("Nothing to remove since the entry is null or we > have a null entry"); > > you'd have the possibility to notify that attempt was made to remove > something but it wasn't there. I'm not sure whether it'd make a lot of > sense to implement it for RemoveCommand so, we either add a notify > method afterwards and make InvalidateCommand only provide a impl for > it, > or override perform() in IC, or somewhere in between, i.e. wrap code > within the if (e == null || e.isNull()) into a method that is > overriden > by IC. > > Thoughts? Hmm, not sure how generally useful this is, so as to include it as a part of the core distro. Adds bloat and complication to the notification/listener API. -- Manik Surtani manik(a)jboss.org Lead, Infinispan Lead, JBoss Cache http://www.infinispan.org http://www.jbosscache.org _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev

On Nov 3, 2009, at 9:35 PM, Galder Zamarreno wrote: > Manik, I might still need the notification requested below. > > Brian, I'd like to know your thoughts on this since it might influence > your work for > http://opensource.atlassian.com/projects/hibernate/browse/HHH-4521 > (actually, I think it does! see bottom) > Yep. I'm working on HHH-4521 now and I expect this to be a problem when I run the test for it. > If we were to handle evict(key) in such way that we only set the > invalid > state for that key, the invalidation event received remotely would > need > to somehow carry the key that needs to be invalidated. Now, this means > that without my proposed notification, you'd need to have a key > present > in the cache that is composed of address:key, since a key with simply > address would not be sufficient to figure out which key to invalidate. > > For evict all, this is much simpler cos key can simply be address and > each node can initialise on startup from their local address (or upon > viewChange). This means that when an evict all occurs, all address > instances reside in the cache, so invalidation works fine. > > However, for evict(key), it simply is not feasible to have an > address:key key for each key present in the cache in case it has to be > evicted, and without that, the cache invalidation listener event won't > get fired. Please also remember that the invalidation event in > Infinispan only carries the key (obviously!), so I cannot use the > value > part to add the key for example. > Yeah. For JBC I was going to remove the key from the Fqn and just use it as the value, but then realized this wouldn't work w/ invalidation. > My preferred solution here would be to have a notification like the > one > suggested below. That way I don't need to have address:key for every > cluster member and key in the cache, and I can see when a particular > key > was invalidated by an address. > > Other solutions include: > - treat evict(key) as evict all: that way the key does not have to > travel to other nodes since they clear the entire cache. That would > be a > bit too extreme. > > Thinking through it, I think you'd have the same issue with JBC based > 2nd level cache: > > Brian, When you evict a key, you do: > Fqn f = Fqn.fromRelativeElements(region, Internal.NODE, member == > null > ? Internal.LOCAL : member, key); > cache.put(f, ITEM, DUMMY); > > So the FQN contains the key and for it to be invalidated, you need an > fqn with the member address and key! So, you'd have the same issue. > Yes, unless JBC emits the notification even if the node didn't exist locally. Which I doubt. TBH, this whole thing of using cache data as a mechanism to pass notifications is a hack. Much nicer IMHO would be a nice SPI / plug point to allow applications to extend the set of commands. Let an app create an ExtensionCommand; if that comes in off the channel pass it off to the handler the application registered. A worse IMHO option to the notification bus hack is to have the Hibernate-Infinispan/JBC integration create a separate JGroups channel for these messages. If a JGroups ChannelFactory + shared transport is used, that's just yuck. If ChannelFactory/shared transport aren't used, it's just unworkable.

On Nov 4, 2009, at 4:48 PM, Galder Zamarreno wrote: > > > On 11/04/2009 03:29 AM, Brian Stansberry wrote: >> >> On Nov 3, 2009, at 9:35 PM, Galder Zamarreno wrote: >> >>> Manik, I might still need the notification requested below. >>> >>> Brian, I'd like to know your thoughts on this since it might >>> influence >>> your work for >>> http://opensource.atlassian.com/projects/hibernate/browse/HHH-4521 >>> (actually, I think it does! see bottom) >>> >> >> Yep. I'm working on HHH-4521 now and I expect this to be a problem >> when I run the test for it. >> >>> If we were to handle evict(key) in such way that we only set the >>> invalid >>> state for that key, the invalidation event received remotely would >>> need >>> to somehow carry the key that needs to be invalidated. Now, this >>> means >>> that without my proposed notification, you'd need to have a key >>> present >>> in the cache that is composed of address:key, since a key with >>> simply >>> address would not be sufficient to figure out which key to >>> invalidate. >>> >>> For evict all, this is much simpler cos key can simply be address >>> and >>> each node can initialise on startup from their local address (or >>> upon >>> viewChange). This means that when an evict all occurs, all address >>> instances reside in the cache, so invalidation works fine. >>> >>> However, for evict(key), it simply is not feasible to have an >>> address:key key for each key present in the cache in case it has >>> to be >>> evicted, and without that, the cache invalidation listener event >>> won't >>> get fired. Please also remember that the invalidation event in >>> Infinispan only carries the key (obviously!), so I cannot use the >>> value >>> part to add the key for example. >>> >> >> Yeah. For JBC I was going to remove the key from the Fqn and just use >> it as the value, but then realized this wouldn't work w/ >> invalidation. >> >>> My preferred solution here would be to have a notification like the >>> one >>> suggested below. That way I don't need to have address:key for every >>> cluster member and key in the cache, and I can see when a particular >>> key >>> was invalidated by an address. >>> >>> Other solutions include: >>> - treat evict(key) as evict all: that way the key does not have to >>> travel to other nodes since they clear the entire cache. That would >>> be a >>> bit too extreme. >>> >>> Thinking through it, I think you'd have the same issue with JBC >>> based >>> 2nd level cache: >>> >>> Brian, When you evict a key, you do: >>> Fqn f = Fqn.fromRelativeElements(region, Internal.NODE, member == >>> null >>> ? Internal.LOCAL : member, key); >>> cache.put(f, ITEM, DUMMY); >>> >>> So the FQN contains the key and for it to be invalidated, you need >>> an >>> fqn with the member address and key! So, you'd have the same issue. >>> >> >> Yes, unless JBC emits the notification even if the node didn't exist >> locally. Which I doubt. >> >> TBH, this whole thing of using cache data as a mechanism to pass >> notifications is a hack. Much nicer IMHO would be a nice SPI / plug >> point to allow applications to extend the set of commands. Let an app >> create an ExtensionCommand; if that comes in off the channel pass it >> off to the handler the application registered. >> >> A worse IMHO option to the notification bus hack is to have the >> Hibernate-Infinispan/JBC integration create a separate JGroups >> channel >> for these messages. If a JGroups ChannelFactory + shared transport is >> used, that's just yuck. If ChannelFactory/shared transport aren't >> used, it's just unworkable. > > I agree that the best way to solve this would be for ISPN/JBC to allow > extension of commands but I think this requires a fair bit of thinking > and design to get it right. So, we'd need to design it, implement it > and > proof-test it with SPI impls for ISPN and JBC 2nd level caches. > Agreed. (BTW I have other use cases, i.e. session management. ;) )

> That's why I suggested the emit notification when a remote > invalidation > does not found the node/key in the node where is trying to do the > invalidation. I think this could be a good short term solution while > we > fix the real issue. I don't think it'd require a lot of code. The main > issue with it is that such notification is a public API, so once we > put > it out, we have to carry on maintaining it. > > WDYT? > Can a boolean property be added to CacheEntryInvalidatedEvent? public boolean isKeyLocallyStored() or some better name.

BTW what happens with DIST and no L1 cache? Is an invalidation message even sent out?

> I also agree that an extra JGroups channel wouldn't be the best thing > here. It looks like an overkill to me. > > > -- > Galder Zamarreño > Sr. Software Engineer > Infinispan, JBoss Cache > _______________________________________________ > 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

On 4 Nov 2009, at 09:04, Brian Stansberry <brian.stansberry(a)redhat.com&gt; wrote: > > On Nov 4, 2009, at 4:48 PM, Galder Zamarreno wrote: > >> >> >> On 11/04/2009 03:29 AM, Brian Stansberry wrote: >>> >>> On Nov 3, 2009, at 9:35 PM, Galder Zamarreno wrote: >>> >>>> Manik, I might still need the notification requested below. >>>> >>>> Brian, I'd like to know your thoughts on this since it might >>>> influence >>>> your work for >>>> http://opensource.atlassian.com/projects/hibernate/browse/HHH-4521 >>>> (actually, I think it does! see bottom) >>>> >>> >>> Yep. I'm working on HHH-4521 now and I expect this to be a problem >>> when I run the test for it. >>> >>>> If we were to handle evict(key) in such way that we only set the >>>> invalid >>>> state for that key, the invalidation event received remotely would >>>> need >>>> to somehow carry the key that needs to be invalidated. Now, this >>>> means >>>> that without my proposed notification, you'd need to have a key >>>> present >>>> in the cache that is composed of address:key, since a key with >>>> simply >>>> address would not be sufficient to figure out which key to >>>> invalidate. >>>> >>>> For evict all, this is much simpler cos key can simply be address >>>> and >>>> each node can initialise on startup from their local address (or >>>> upon >>>> viewChange). This means that when an evict all occurs, all address >>>> instances reside in the cache, so invalidation works fine. >>>> >>>> However, for evict(key), it simply is not feasible to have an >>>> address:key key for each key present in the cache in case it has >>>> to be >>>> evicted, and without that, the cache invalidation listener event >>>> won't >>>> get fired. Please also remember that the invalidation event in >>>> Infinispan only carries the key (obviously!), so I cannot use the >>>> value >>>> part to add the key for example. >>>> >>> >>> Yeah. For JBC I was going to remove the key from the Fqn and just >>> use >>> it as the value, but then realized this wouldn't work w/ >>> invalidation. >>> >>>> My preferred solution here would be to have a notification like the >>>> one >>>> suggested below. That way I don't need to have address:key for >>>> every >>>> cluster member and key in the cache, and I can see when a >>>> particular >>>> key >>>> was invalidated by an address. >>>> >>>> Other solutions include: >>>> - treat evict(key) as evict all: that way the key does not have to >>>> travel to other nodes since they clear the entire cache. That would >>>> be a >>>> bit too extreme. >>>> >>>> Thinking through it, I think you'd have the same issue with JBC >>>> based >>>> 2nd level cache: >>>> >>>> Brian, When you evict a key, you do: >>>> Fqn f = Fqn.fromRelativeElements(region, Internal.NODE, member == >>>> null >>>> ? Internal.LOCAL : member, key); >>>> cache.put(f, ITEM, DUMMY); >>>> >>>> So the FQN contains the key and for it to be invalidated, you need >>>> an >>>> fqn with the member address and key! So, you'd have the same issue. >>>> >>> >>> Yes, unless JBC emits the notification even if the node didn't exist >>> locally. Which I doubt. >>> >>> TBH, this whole thing of using cache data as a mechanism to pass >>> notifications is a hack. Much nicer IMHO would be a nice SPI / plug >>> point to allow applications to extend the set of commands. Let an >>> app >>> create an ExtensionCommand; if that comes in off the channel pass it >>> off to the handler the application registered. >>> >>> A worse IMHO option to the notification bus hack is to have the >>> Hibernate-Infinispan/JBC integration create a separate JGroups >>> channel >>> for these messages. If a JGroups ChannelFactory + shared transport >>> is >>> used, that's just yuck. If ChannelFactory/shared transport aren't >>> used, it's just unworkable. >> >> I agree that the best way to solve this would be for ISPN/JBC to >> allow >> extension of commands but I think this requires a fair bit of >> thinking >> and design to get it right. So, we'd need to design it, implement it >> and >> proof-test it with SPI impls for ISPN and JBC 2nd level caches. >> > > Agreed. (BTW I have other use cases, i.e. session management. ;) ) > >> That's why I suggested the emit notification when a remote >> invalidation >> does not found the node/key in the node where is trying to do the >> invalidation. I think this could be a good short term solution while >> we >> fix the real issue. I don't think it'd require a lot of code. The >> main >> issue with it is that such notification is a public API, so once we >> put >> it out, we have to carry on maintaining it. >> >> WDYT? >> > > Can a boolean property be added to CacheEntryInvalidatedEvent? > > public boolean isKeyLocallyStored() > > or some better name. Perhaps isSuccessful() ? > > BTW what happens with DIST and no L1 cache? Is an invalidation message > even sent out? No. Dist and no L1 will not result in any invalidation messages. But afaik the infinispan hibernate provider doesn't use dist, right Galder?

>> I also agree that an extra JGroups channel wouldn't be the best thing >> here. It looks like an overkill to me. >> >> >> -- >> Galder Zamarreño >> Sr. Software Engineer >> Infinispan, JBoss Cache >> _______________________________________________ >> 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

On 11/04/2009 10:04 AM, Brian Stansberry wrote: > > On Nov 4, 2009, at 4:48 PM, Galder Zamarreno wrote: > >> >> >> On 11/04/2009 03:29 AM, Brian Stansberry wrote: >>> >>> </snip> >>> >>> Yes, unless JBC emits the notification even if the node didn't exist >>> locally. Which I doubt. >>> >>> TBH, this whole thing of using cache data as a mechanism to pass >>> notifications is a hack. Much nicer IMHO would be a nice SPI / plug >>> point to allow applications to extend the set of commands. Let an app >>> create an ExtensionCommand; if that comes in off the channel pass it >>> off to the handler the application registered. >>> >>> A worse IMHO option to the notification bus hack is to have the >>> Hibernate-Infinispan/JBC integration create a separate JGroups >>> channel >>> for these messages. If a JGroups ChannelFactory + shared transport is >>> used, that's just yuck. If ChannelFactory/shared transport aren't >>> used, it's just unworkable. >> >> I agree that the best way to solve this would be for ISPN/JBC to allow >> extension of commands but I think this requires a fair bit of thinking >> and design to get it right. So, we'd need to design it, implement it >> and >> proof-test it with SPI impls for ISPN and JBC 2nd level caches. >> > > Agreed. (BTW I have other use cases, i.e. session management. ;) ) I'm gonna open a JIRA for this at least in ISPN.