[ https://issues.jboss.org/browse/ISPN-761?page=com.atlassian.jira.plugin.s... ] Sanne Grinovero edited comment on ISPN-761 at 7/28/14 11:19 AM: ---------------------------------------------------------------- Continuation from previous comment {quote} <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. <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> 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 <sannegrinovero> if you use flags you need to deal with the consequences ;-) <wburns> sannegrinovero: :-) <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 <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. <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? <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 <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. <mmarkus> sannegrinovero: indeed <mmarkus> then back to the map.entries and map.keys problem <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} was (Author: william.burns): 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@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(a)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@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} -- This message was sent by Atlassian JIRA (v6.2.6#6264)