ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Николай Ижиков <nizhikov....@gmail.com>
Subject Re: ContinuousQueryWithTransformer implementation questions - 2
Date Wed, 13 Sep 2017 15:03:45 GMT
Guys.

I'm new in the project so can someone more experienced tell me:

What's is wrong with the current implementation of ContinuousQuery?

1. initialQuery is useless - OK, understood.

What else is wrong?


2017-09-12 20:02 GMT+03:00 Николай Ижиков <nizhikov.dev@gmail.com>:

> 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 <vozerov@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 <vozerov@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.dev@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 <dsetrakyan@apache.org>:
>> >>
>> >> > Vladimir, are their factories for the proposed listeners?
>> >> >
>> >> > On Tue, Sep 12, 2017 at 7:52 AM, Alexey Goncharuk <
>> >> > alexey.goncharuk@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 ContinuousQueryCacheEntryListe
>> ner
>> >> > 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.dev@gmail.com
>> >>
>> >
>> >
>>
>
>
>
> --
> Nikolay Izhikov
> NIzhikov.dev@gmail.com
>



-- 
Nikolay Izhikov
NIzhikov.dev@gmail.com

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message