ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vladimir Ozerov <voze...@gridgain.com>
Subject Re: ContinuousQueryWithTransformer implementation questions - 2
Date Mon, 18 Sep 2017 13:43:49 GMT
Nikolay,

Here is a short summary what is wrong with continuous query:
1) It should not have "initialQuery"
2) It should not be called through IgniteCache.query() method, as it has
nothing in common with other "query" types
3) Main thing: our listeners are *ALWAYS* executed on a node which
initiated the query.

p.3 is the major problem. JCache specification doesn't define where
listeners should be invoked. Should we have ability to execute them on data
nodes, there would be much less demand in transformers.

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". What is worse, this interface is inconsistent
with JCache event listeners, which distinguish between create, update and
delete events.

For these reasons I would still prefer to think of better continuous
queries design first instead of making current API even more complicated.

Vladimir.

On Mon, Sep 18, 2017 at 4:04 PM, Николай Ижиков <nizhikov.dev@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 <dmagda@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 <vozerov@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 <yzhdanov@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.dev@gmail.com
>

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