Andrey, I think getEntry(…) can be done quickly. Adding another serialization mechanism for cache entries together with a configuration flag may take additional time. I would suggest, again, starting with getEntry method and then moving down to queries.
D. On Mon, Dec 21, 2015 at 1:04 PM, Andrey Kornev <andrewkor...@hotmail.com> wrote: > Personally, I'm firmly in the old-farts camp -- I like things consistent. > :))) My motto is: get things right first, and then make them fast. > > As for the queries, I don't understand why do you think that supporting > the version would mean adding lots of branching code? One would only have 2 > implementations of Ignite's CacheEntry - one with the version field and > another -- without. The latter would just throw an > UnsupportedOperationException or something like that when user calls > CacheEntry.version(). Then, I imagine there would be only one place in the > code where the decision as to which implementation to use would have to be > made. Based on the value of Query's includeEntryVersion flag the code > would instantiate either one implementation class or another. That's it. > Am I oversimplifying? > > For my use case, I'm actually very interested in adding the entry version > support to the queries. The getEntry() came out of the realization that > Ignite doesn't currently provide any way to atomically obtain a cache value > along with its version. Truth be told, the EntryProcessor could also be > used as a last resort workaround, but given its current rather unfortunate > implementation, we decided against it. > > Regards > Andrey > > > From: dsetrak...@apache.org > > Date: Mon, 21 Dec 2015 12:15:50 -0800 > > Subject: Re: Missing ways to get access to Ignite CacheEntry > > To: dev@ignite.apache.org > > > > Andrey, > > > > You got me thinking on whether I belong to the playful performance kids > > category, or to the old-fart consistency crowd :) > > > > I agree on the getEntry(…) suggestion, and I think it would be fairly > easy > > to implement, even for the new community members. As far as including > > version into query results, it seems like a larger undertaking, and seems > > like it may pollute the existing implementation with additional hacky > code > > branches. > > > > I suggest adding a ticket for the getEntry(…) method only for now. What > do > > you think? > > > > D. > > > > On Mon, Dec 21, 2015 at 10:19 AM, Andrey Kornev < > andrewkor...@hotmail.com> > > wrote: > > > > > Couple of points. > > > > > > First, if it were up to me, I'd keep the API simple & consistent by > > > always providing the version as defined by Ignite's CacheEntry > interface, > > > regardless whether it's the query who produces the entry or a > specialized > > > method like getEntry(). Which node is going to consume the data -- a > > > "client" or "server" -- should not matter either. In this case, > however a > > > concern about perceived performance losses due to the version > overhead has > > > apparently trumped any API integrity and consistency concerns. > > > > > > Second, given the first point, here's what I think we could do to keep > > > everybody (the performance kids and the consistency/integrity > old-farts) > > > happy. ))) > > > > > > 1) getEntry(K) unconditionally always returns a valid version > regardless > > > whether it's called from a client or server. In fact, the only reason > to > > > ever use this method is to get the entry version. I'd even suggest > going > > > one step further and have the getEntry() method return Ignite's > CacheEntry > > > interface rather than JCache's Cache.Entry (only to be immediately > > > unwrapped to Ignite's CacheEntry). > > > > > > 2) for queries, provide an option on the Ignite's Query class (say, > > > setIncludeEntryVersion(boolean)) that would control whether the query > > > should send the version back. By default, it'd be "false" as to have no > > > impact on performance. More advanced users ;) would still be able to > enjoy > > > API consistency by setting that option to "true". > > > > > > Deal? > > > Andrey > > > > > > > From: dsetrak...@apache.org > > > > Date: Mon, 21 Dec 2015 08:30:02 -0800 > > > > Subject: Re: Missing ways to get access to Ignite CacheEntry > > > > To: dev@ignite.apache.org > > > > > > > > On Mon, Dec 21, 2015 at 12:36 AM, Denis Magda <dma...@gridgain.com> > > > wrote: > > > > > > > > > > > > > > > > > > > On 12/21/2015 11:15 AM, Dmitriy Setrakyan wrote: > > > > > > > > > >> On Mon, Dec 21, 2015 at 12:11 AM, Denis Magda < > dma...@gridgain.com> > > > > >> wrote: > > > > >> > > > > >> Why it shouldn't work? > > > > >>> If client receives an entry that has a reference to the entry > with > > > > >>> version, transferred from a server as well as a part of the > response, > > > > >>> then > > > > >>> unwrap(...) should work. > > > > >>> Isn't this feasible to implement? > > > > >>> > > > > >>> The reason is consistency. If getEntry(…) can be unwrapped on the > > > client > > > > >> side, then all the query results should be un-wrappable as well, > which > > > > >> means that we have to always carry the cache version, even if user > > > does > > > > >> not > > > > >> need it. > > > > >> > > > > > The proposal is to add getEntry(...) method exactly for the cases > when > > > > > someone needs an entry with a version on a client. > > > > > In all other cases (queries, etc.) the version won't be a part of a > > > > > response. > > > > > > > > > > Right, this is inconsistent but if to document everything well > then the > > > > > user will only benefit from the new getEntry(...) method. > > > > > CacheEntry with version is already can be unwrapped from specific > > > places > > > > > that are listed in its documentation. We will just broaden the > list. > > > > > > > > > > > > > Romain, Andrey, can you please comment? > > > > > > > >
