Vova, > Public API is the face of our product. > We cannot and do not want to change it in a rush. > It is not a big problem if we spend additional week or month for API design
Fully agreed. I'm not trying to speed up changes, all I try is separate two discussions: * ticket implementation based on existing API * design of new API > It is much better *than* extend*ing* confusing behavior *of *already confusing and overengineered API Ignite already have a ContinuousQuery It's a matter of fact. Ticket goal is provide some useful feature to the user. I think it a good thing. Can you list confusions that added by ticket implementation? 2017-09-12 19:47 GMT+03:00 Vladimir Ozerov <voze...@gridgain.com>: > I meant "It is much better *than* extend*ing* confusing behavior *of > *already > confusing and overengineered API." > > On Tue, Sep 12, 2017 at 7:35 PM, Vladimir Ozerov <voze...@gridgain.com> > wrote: > > > Nikolay, > > > > Public API is the face of our product. We cannot and do not want to > change > > it in a rush. This ticket was in a backlog for more than 2 years. It is > not > > a big problem if we spend additional week or month for API design. It is > > much better to extend confusing behavior on already confusing and > > overengineered API. > > > > On Tue, Sep 12, 2017 at 6:47 PM, Николай Ижиков <nizhikov....@gmail.com> > > wrote: > > > >> Vova > >> > >> > I propose to deprecate current continuous queries and develop new API. > >> > This should not break anything. > >> > >> If the community agrees that *whole* continuous query API is bad - it > OK. > >> Let's develop new API. > >> > >> But developing new public API and implementing it is a very long > process. > >> One can see it based on this thread :) > >> > >> I think my implementation [1] of transformers for ContinuousQuery make > >> things better for a user because remote transformers can lead to > >> significant performance win. > >> > >> > Adding transformers on top of current API will make it total mess. > >> > >> I propose two things: > >> > >> 1. Continue discussion of current task [2] with scope limited by current > >> API. > >> 2. Start a discussion and work on new API. I think we can start with > >> listing things that make current API bad. I can drive such a discussion. > >> > >> [1] https://github.com/apache/ignite/pull/2372 > >> [2] https://issues.apache.org/jira/browse/IGNITE-425 > >> > >> > >> 2017-09-12 17:55 GMT+03:00 Dmitriy Setrakyan <dsetrak...@apache.org>: > >> > >> > Vladimir, are their factories for the proposed listeners? > >> > > >> > On Tue, Sep 12, 2017 at 7:52 AM, Alexey Goncharuk < > >> > alexey.goncha...@gmail.com> wrote: > >> > > >> > > Vladimir, > >> > > > >> > > Can you please clarify how the proposed API will work? > >> > > > >> > > 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. > >> > > > >> > > > >> > > Do I understand correctly that CacheEntryListener will have a method > >> like > >> > > onEvent() which will accept the cache event? > >> > > > >> > > > >> > > > 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; > >> > > > } > >> > > > > >> > > > > >> > > This becomes confusing: while the ContinuousQueryCacheEntryListener > >> > itself > >> > > has the onEvent() method, which is supposed to be called on event > >> nodes, > >> > it > >> > > also has a rmtFilter, which will also be called on event nodes. Will > >> the > >> > > onEvent() then invoked on the listener anyway, regardless of the > >> filter > >> > > result? Finally, the listener will have a local callback field, > which > >> > will > >> > > be called on the originating node. This sounds way more tricky to me > >> than > >> > > the current API. > >> > > > >> > > > >> > > > 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); > >> > > > > >> > > > > >> > > Agree with this. > >> > > > >> > > >> > >> > >> > >> -- > >> Nikolay Izhikov > >> nizhikov....@gmail.com > >> > > > > > -- Nikolay Izhikov nizhikov....@gmail.com
