[infinispan-issues] [JBoss JIRA] (ISPN-761) Cache.keySet(), entrySet(), values(), size() ignore contents of cache loader
William Burns (JIRA)
jira-events at lists.jboss.org
Thu Jul 11 12:20:21 EDT 2013
[ https://issues.jboss.org/browse/ISPN-761?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12789180#comment-12789180 ]
William Burns commented on ISPN-761:
------------------------------------
Continuation from previous comment
{quote}
* mmarkus has quit (Quit: Leaving.)
<wburns> sannegrinovero: actually since we are on size now - the problem with just returning a count I have found is the Flag can mess with that optimization
<sannegrinovero> wburns: ? how can the Flag mess with a SELECT COUNT(*) ??
<wburns> sannegrinovero: passivation enabled you would add memory store and cache store size
<wburns> sannegrinovero: no passivation you would just add the cache store size
<wburns> sannegrinovero: however the no passivation if you use the flag the cache store being a superset is no longer true
<sannegrinovero> wburns: right.depending if it's passivation or not the math to be used is different, but not hard.
<sannegrinovero> (either you add the size of the container or you don't)
<wburns> sannegrinovero: that isn't true when the flag is used
<sannegrinovero> BTW this will never be an exact value, that's not possible. This is needed for statistical purposes only.
* pilhuhn has quit (Quit: Computer has gone to sleep.)
<sannegrinovero> wburns: which Flag are you talking about?
<wburns> sannegrinovero: SKIP_CACHE_STORE on a put operation for a new key
<wburns> sannegrinovero: the exact value portion is the discussion I was looking for, do we care about this case with the Flag
<wburns> sannegrinovero: cause the only way to be 100% correct is to get all the keys from the memory store and the cache store and return the size of the union
<wburns> in the no passivation one
<wburns> sannegrinovero: however if we ignore the flag usage then we can just return the size of the cache store
<sannegrinovero> wburns: well no we don't care
* mmarkus (~Adium at redhat/jboss/mmarkus) has joined #infinispan
* ChanServ gives channel operator status to mmarkus
<mmarkus> sannegrinovero: my connection dropped
<mmarkus> sannegrinovero: wburns: back now
<wburns> mmarkus: whoops you missed the good parts :P
<wburns> sannegrinovero: okay that was mmarkus feelings as well
<sannegrinovero> mmarkus: np I think the interesting discussion finished. I'll add it all to JIRA ok? So we don't lose this again as I'm sure it was discussed on the mailing list already.
<wburns> mmarkus and I I meant
<sannegrinovero> +1
* Alkpone has quit (Quit: Leaving.)
<sannegrinovero> if you use flags you need to deal with the consequences ;-)
<wburns> sannegrinovero: :-)
* tsykora__ has quit (Ping timeout: 264 seconds)
<mmarkus> sannegrinovero: wburns: thinking more about it I think sannegrinovero has a good point regarding keySet() and entries()
<mmarkus> for Cache.keySet() we want to return a Set that only allows invoking Set.iterator on it
<wburns> mmarkus: I agree with his points, but which that would affect 761 right now?
<mmarkus> and this iterator would only load data lazily
<mmarkus> sannegrinovero: ^^ I guess that's what you have in mind
<wburns> mmarkus: so basically like the Streams from Java 8?
<sannegrinovero> mmarkus: yea. I think we should work on this in tandem with ISPN-3290 : you have the opportunity now to add/adjust some methods there and should just do it.
<jbossbot> jira [ISPN-3290] Redesign CacheStore API [Open (Unresolved) Task, Critical, Mircea Markus] https://issues.jboss.org/browse/ISPN-3290
<sannegrinovero> mmarkus: TBH I don't think ISPN-761 is as important as Map/Reduce .. but it's the same problem so we'd better consider them both as "customers" for the above issue.
<jbossbot> jira [ISPN-761] Cache.keySet(),entrySet(),values(),size() ignore contents of cache loader [Pull Request Sent (Unresolved) Bug, Critical, William Burns] https://issues.jboss.org/browse/ISPN-761
<mmarkus> sannegrinovero: so we'd need an interable cache store
* rachmatowicz has quit (Quit: Ex-Chat)
<sannegrinovero> mmarkus: there is another important customer actually: CacheStore performance: when using DIST and starting up a new node you need to load all entries, but only _owned_ entities are of interest.
* rachmatowicz (~nrla at modemcable117.232-202-24.mc.videotron.ca) has joined #infinispan
<sannegrinovero> mmarkus: not sure if we had Iterable in mind. We definitely discussed this on ML and had several good ideas. need to find that.
<sannegrinovero> mmarkus: but agreed it definitely is something like that, conceptually.
<mmarkus> sannegrinovero: right
<mmarkus> sannegrinovero: not sure we'd be able to make all the stores "iterable" though
<mmarkus> sannegrinovero: I mean the underlying implementation to allow that
<wburns> mmarkus: should we do something inline with Java 8?
* hagarth has quit (Ping timeout: 276 seconds)
<sannegrinovero> mmarkus: that's why I'm starting to buy in your idea to move some CacheStore(s) out of the repository :P
<mmarkus> wburns: I think that would make sense
<mmarkus> sannegrinovero: well for those that are not iterable we can still stick to the existing approach
* jbossbot has quit (Read error: Operation timed out)
<sannegrinovero> mmarkus: some implementations might not be able to implement all optimisations (needs to be optional) and also we might want to update some CacheStores after the Infinispan 6.0 deadlines.. relax the release dates for non product components.
<sannegrinovero> mmarkus: +100
<sannegrinovero> mmarkus: but also don't bother updating all of them at the same time.
* pmuir (~pmuir at redhat/jboss/pmuir) has joined #infinispan
<pmuir> galderz: ping
<mmarkus> sannegrinovero: indeed
<mmarkus> then back to the map.entries and map.keys problem
* vchepeli has quit (Quit: Leaving.)
<wburns> mmarkus: sannegrinovero: why couldn't the non interable ones basically just be wrapped with an iterable interface and they are just documented as possible OOM stores :P
<sannegrinovero> mmarkus: I guess a good plan would be to start moving out the stores from the repo, so that then you can experiment with SPI changes without too much update overhead.
<mmarkus> wburns: read my mind :-)
<sannegrinovero> wburns: +1 not all cachestores are fit for this. Ideally we'd make a table on the doc listing which ones support certain "smarter" features and which ones are more limited.
<wburns> mmarkus: sannegrinovero: and actually we can do that very easily when moving to Java 8 when the stream methods are added to collections and they do all the hard work for us :)
<wburns> mmarkus: sannegrinovero: too bad we won't be on Java 8 until who knows when, haha
<sannegrinovero> wburns: right! but that will take some time. Hopefully quick enough for the various cachestores to update their APIs as well so that we can try and find similarities.
<sannegrinovero> mmarkus: wburns: cool discussions :) missed you guys.
<mmarkus> wburns: sannegrinovero: okay then. So the current implementation Will is working on in the scope of ISPN-3290 will make sense in the future as well, for the CS that are not iterable
<wburns> mmarkus: back into reality: what do want for tomorrow? :P
<mmarkus> once we add iterable cache stores, we'll be able to enhance the implementation for these stores to be much more memory efficient
<sannegrinovero> mmarkus: ok good point, it's not worthless code.
<mmarkus> wburns: +1 to move on with your impl for ISPN-761 and then extend it in the scope of ISPN-3290
<sannegrinovero> wburns: I think you should explore how the CacheStore SPI should look like. You need to study both your needs (Infinispan Core) and the implementation options you have (I'd start from the CacheStores we support)
<mmarkus> wburns: move on == document limitations and integrate it
{quote}
> Cache.keySet(),entrySet(),values(),size() ignore contents of cache loader
> -------------------------------------------------------------------------
>
> Key: ISPN-761
> URL: https://issues.jboss.org/browse/ISPN-761
> Project: Infinispan
> Issue Type: Bug
> Components: Loaders and Stores
> Affects Versions: 4.2.0.BETA1
> Reporter: Paul Ferraro
> Assignee: William Burns
> Priority: Critical
> Labels: onboard, potential_performance_hit
> Fix For: 6.0.0.Alpha1, 6.0.0.Final
>
>
> Passivated cache entries are not represented in values returned by the keySet(), entrySet(), values(), and size() Cache methods. This results in inconsistent behavior, since it is possible that a given cache key may not be contained in keySet() even though Cache.get(...) would return a non-null value if the entry was previously passivated.
> I think CacheLoaderInterceptor needs to implement the following methods:
> Object visitSizeCommand(InvocationContext ctx, SizeCommand command) throws Throwable
> Object visitValuesCommand(InvocationContext ctx, ValuesCommand command) throws Throwable
> Object visitEntrySetCommand(InvocationContext ctx, EntrySetCommand command) throws Throwable
> Object visitKeySetCommand(InvocationContext ctx, KeySetCommand command) throws Throwable
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
More information about the infinispan-issues
mailing list