Hello, guys. Anton, Yakov, can you, please, share your wisdom, and make final review of IGNITE-425
Task: https://issues.apache.org/jira/browse/IGNITE-425 PR: https://github.com/apache/ignite/pull/2372 2017-09-18 18:33 GMT+03:00 Николай Ижиков <nizhikov....@gmail.com>: > So, resuming: > > 1) My solution reduces network communication. > As far as I know, a lot of people want to have this feature at Ignite 2.x. > It's impossible to gain perfect API right now, it will take months to gain > it. > My solution ready right now!, let's merge it and refactor whole Continuous > Query API at 3.0. > > 2) Current API is bad, and, yes, my changes make it a little bit > complicated. > But, complication minimized as possible and profit much bigger that > complication. > > 2017-09-18 17:39 GMT+03:00 Николай Ижиков <nizhikov....@gmail.com>: > >> Vladimir, >> >> Here is a short summary what is wrong with continuous query.... >> >> >> OK. >> I agree - this is problems of current API. >> >> How we can fix it by not merging ContinuousQueryWIthTransformer? >> How we can quickly design, discuss and implement new API? >> Because at the moment there is no any ticket to start working at. >> Moreover we can't throw ContinuousQuery away until 3.0 version. >> >> > What is worse, this interface is inconsistent with JCache event >> listeners, which distinguish between create, update and delete events. >> >> Can't agree with you. >> >> 1. As far as I know jcache doesn't have any Transformer conception. >> 2. We can distinguish create, update and delete events in transformer and >> we can push that knowledge to listener. >> >> >>> What I see in your PR is that we add one more confusing concept - in >>> addition to wrongly named "local listener" now we will also have >>> "TransformedEventListener". >>> >> >> I think usage of jcache API in some Ignite-specific classes is one more >> issue of existing ContinuousQuery. >> I think we must use Ignite only API for Ignite only features and use some >> wrappers to provide external API support. >> >> For these reasons I would still prefer to think of better continuous >>> queries design first instead of making current API even more complicated. >>> >> >> I think the main reason for some feature is to provide value to the user. >> Transformers adds value to a product because usage of transformer can >> lead to significant performance win. >> >> >>> Vladimir. >>> >>> On Mon, Sep 18, 2017 at 4:04 PM, Николай Ижиков <nizhikov....@gmail.com> >>> wrote: >>> >>> > Igniters, >>> > >>> > I discussed API of ContinuousQuery and ContinuousQueryWithTransformer >>> with >>> > Anton Vinogradov one more time. >>> > >>> > Since users who use regular ContinuousQuery already knows pros. and >>> cons of >>> > using initialQuery and to not to complicate API more and more until >>> 3.0 we >>> > agreed that best choice is to stay with existing initialQuery that >>> return >>> > Cache.Entry<K, V> for ContinuousQueryWithTransformer. >>> > >>> > Notice that initialQuery is not required and can be null. >>> > >>> > Thoughts? >>> > >>> > 2017-09-15 1:45 GMT+03:00 Denis Magda <dma...@apache.org>: >>> > >>> > > Vladimir, >>> > > >>> > > If the API is so bad then it might take much more time to make up and >>> > roll >>> > > out the new. Plus, there should be a community member who is ready to >>> > take >>> > > it over. My suggestion would be to accept this contribution and >>> initiate >>> > an >>> > > activity towards the new API if you like. >>> > > >>> > > Personally, I considered this API as one of the most vivid we have >>> basing >>> > > on my practical usage experience. I was aware of initial query’s >>> pitfalls >>> > > but isn’t it something we can put on paper? >>> > > >>> > > — >>> > > Denis >>> > > >>> > > > On Sep 12, 2017, at 6:04 AM, Vladimir Ozerov <voze...@gridgain.com >>> > >>> > > wrote: >>> > > > >>> > > > My opinion is that our query API is big piece of ... you know, >>> > especially >>> > > > ContinuousQuery. A lot of concepts and features are mixed in a >>> single >>> > > > entity, what makes it hard to understand and use. Let's finally >>> > deprecate >>> > > > ContinuousQuery and design nice and consistent API. E.g.: >>> > > > >>> > > > interface IgniteCache { >>> > > > UUID addListener(CacheEntryListener listener) >>> > > > void removeListener(UUID listenerId); >>> > > > } >>> > > > >>> > > > This method set's a listener on all nodes which will process event >>> > > locally, >>> > > > no network communication. Now if you want semantics similar to >>> existing >>> > > > continuous queries, you use special entry listener type: >>> > > > >>> > > > class ContinuousQueryCacheEntryListener implements >>> CacheEntryListener >>> > { >>> > > > ContinuousQueryRemoteFilter rmtFilter; >>> > > > ContinuousQueryRemoteTransformer rmtTransformer; >>> > > > ContinuousQueryLocalCallback locCb; >>> > > > } >>> > > > >>> > > > Last, "initial query" concept should be dropped from "continuous >>> query" >>> > > > feature completely. It doesn't guarantee any kind of atomicity or >>> > > > visibility wrt to cache events, so it adds no value. The same >>> behavior >>> > > > could be achieved as follows: >>> > > > >>> > > > cache.addListener(...) >>> > > > QueryCursor cursor = cache.query(initialQuery); >>> > > > >>> > > > Vladimir. >>> > > > >>> > > > >>> > > > On Tue, Sep 12, 2017 at 3:35 PM, Yakov Zhdanov < >>> yzhda...@apache.org> >>> > > wrote: >>> > > > >>> > > >> Dmitry, can you please take a look at public API change. >>> > > >> >>> > > >> Ticket - https://issues.apache.org/jira/browse/IGNITE-425 >>> > > >> PR - https://github.com/apache/ignite/pull/2372 >>> > > >> >>> > > >> Issues: >>> > > >> 1. Do you see any other option other than creating separate >>> class? As >>> > > for >>> > > >> me I don't. >>> > > >> 2. In a new class we still have initial query which uses <K, V> >>> types >>> > > which >>> > > >> is questionable. >>> > > >> >>> > > >> Igniters, please share your thoughts as well. Public API is the >>> face >>> > of >>> > > our >>> > > >> product we need to make it as convenient and consistent as we can. >>> > > >> >>> > > >> --Yakov >>> > > >> >>> > > >>> > > >>> > >>> > >>> > -- >>> > Nikolay Izhikov >>> > nizhikov....@gmail.com >>> > >>> >> >> >> >> -- >> Nikolay Izhikov >> nizhikov....@gmail.com >> > > > > -- > Nikolay Izhikov > nizhikov....@gmail.com > -- Nikolay Izhikov nizhikov....@gmail.com
