ContinuousQueryWithTransformer implementation questions - 2

[yzhdanov]

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

[Vladimir Ozerov]

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 <[hidden email]> 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]

Hello, Vladimir.


class ContinuousQueryCacheEntryListener implements CacheEntryListener {
    ContinuousQueryRemoteFilter rmtFilter;
    ContinuousQueryRemoteTransformer rmtTransformer;
    ContinuousQueryLocalCallback locCb;
}

I think class you proposed should be separated in two because of generic(as
I do in my pull request with existing ContinuousQuery).

If we setup ContinuousQuery *without* transformer we got CacheEntryEvent<K,
V> in listener.

If we setup ContinuousQuery *with* transformer we got T in listener -
object of type T produced by transformer.


2017-09-12 16:04 GMT+03:00 Vladimir Ozerov <[hidden email]>:

> 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 <[hidden email]>
> 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
[hidden email]

[yzhdanov]

Agree with Vladimir. However, this will break API compatibility. So, at
this point this is impossible.

--Yakov

[Nikolay Izhikov]

Yakov.

> 2. In a new class we still have initial query which uses <K, V> types
which
is questionable.

After some discussion with a colleague, I think I got your point.

If I understand your question right:

Why does someone want to execute an initial query that returns <K, V>?
If one use ContinuousQueryWithTransformer that transform <K, V> to <T>.

I think you right and we have to transform initialQuery results as well.

However, at the moment remote transformer supported only for a ScanQuery
[1]: "Currently transformers are supported ONLY for ScanQuery
<https://ignite.apache.org/releases/latest/javadoc/org/apache/ignite/cache/query/ScanQuery.html>
."

So I propose next:

1. Change ContinuousQueryWithTransformer to support only ScanQuery as
initial query.
If in future we will want to support all kinds of Query we can change
from setInitialQuery(ScanQuery) to setInitialQuery(Query) without breaking
API.
2. Run initial scan query with the same transformer as ContinuousQuery.

Thoughts?

[1] https://ignite.apache.org/releases/latest/javadoc/org/
apache/ignite/IgniteCache.html#query(org.apache.ignite.
cache.query.Query,%20org.apache.ignite.lang.IgniteClosure)


2017-09-12 15:35 GMT+03:00 Yakov Zhdanov <[hidden email]>:

> 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
[hidden email]

[Vladimir Ozerov]

Yakov,

I propose to deprecate current continuous queries and develop new API. This
should not break anything. Adding transformers on top of current API will
make it total mess.

On Tue, Sep 12, 2017 at 5:39 PM, Николай Ижиков <[hidden email]>
wrote:

> Yakov.
>
> > 2. In a new class we still have initial query which uses <K, V> types
> which
> is questionable.
>
> After some discussion with a colleague, I think I got your point.
>
> If I understand your question right:
>
> Why does someone want to execute an initial query that returns <K, V>?
> If one use ContinuousQueryWithTransformer that transform <K, V> to <T>.
>
> I think you right and we have to transform initialQuery results as well.
>
> However, at the moment remote transformer supported only for a ScanQuery
> [1]: "Currently transformers are supported ONLY for ScanQuery
> <https://ignite.apache.org/releases/latest/javadoc/org/
> apache/ignite/cache/query/ScanQuery.html>
> ."
>
> So I propose next:
>
> 1. Change ContinuousQueryWithTransformer to support only ScanQuery as
> initial query.
> If in future we will want to support all kinds of Query we can change
> from setInitialQuery(ScanQuery) to setInitialQuery(Query) without breaking
> API.
> 2. Run initial scan query with the same transformer as ContinuousQuery.
>
> Thoughts?
>
> [1] https://ignite.apache.org/releases/latest/javadoc/org/
> apache/ignite/IgniteCache.html#query(org.apache.ignite.
> cache.query.Query,%20org.apache.ignite.lang.IgniteClosure)
>
>
> 2017-09-12 15:35 GMT+03:00 Yakov Zhdanov <[hidden email]>:
>
> > 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
> [hidden email]
>

[Alexey Goncharuk]

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.

[dsetrakyan]

Vladimir, are their factories for the proposed listeners?

On Tue, Sep 12, 2017 at 7:52 AM, Alexey Goncharuk <
[hidden email]> 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]

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 <[hidden email]>:

> Vladimir, are their factories for the proposed listeners?
>
> On Tue, Sep 12, 2017 at 7:52 AM, Alexey Goncharuk <
> [hidden email]> 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
[hidden email]

[Vladimir Ozerov]

Alex,

The source of our confusion in current API is that we called "filter" what
actually should be "listener". I propose to set has the simplest API
possible. Note that CacheEntryListener is a part of jcache API. Call of
this method will set passed listener on remote nodes. Nothing more.

UUID listen(CacheEntryListener listener)

Next, I propose to treat our current continuous queries implementation as a
specific implementation of CacheEntryListener interface. This
implementation will filter/transform events remotely, and then transfer
them to some "local" callback. This is it.


On Tue, Sep 12, 2017 at 5:52 PM, Alexey Goncharuk <
[hidden email]> 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.
>

[Vladimir Ozerov]

Dima,

We definitely can have factories if we want.

On Tue, Sep 12, 2017 at 5:55 PM, Dmitriy Setrakyan <[hidden email]>
wrote:

> Vladimir, are their factories for the proposed listeners?
>
> On Tue, Sep 12, 2017 at 7:52 AM, Alexey Goncharuk <
> [hidden email]> 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.
> >
>

[Vladimir Ozerov]

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, Николай Ижиков <[hidden email]>
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 <[hidden email]>:
>
> > Vladimir, are their factories for the proposed listeners?
> >
> > On Tue, Sep 12, 2017 at 7:52 AM, Alexey Goncharuk <
> > [hidden email]> 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
> [hidden email]
>

[Vladimir Ozerov]

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 <[hidden email]>
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, Николай Ижиков <[hidden email]>
> 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 <[hidden email]>:
>>
>> > Vladimir, are their factories for the proposed listeners?
>> >
>> > On Tue, Sep 12, 2017 at 7:52 AM, Alexey Goncharuk <
>> > [hidden email]> 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
>> [hidden email]
>>
>
>

[Nikolay Izhikov]

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 <[hidden email]>:

> 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 <[hidden email]>
> 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, Николай Ижиков <[hidden email]>
> > 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 <[hidden email]>:
> >>
> >> > Vladimir, are their factories for the proposed listeners?
> >> >
> >> > On Tue, Sep 12, 2017 at 7:52 AM, Alexey Goncharuk <
> >> > [hidden email]> 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
> >> [hidden email]
> >>
> >
> >
>

--
Nikolay Izhikov
[hidden email]

[Nikolay Izhikov]

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 Николай Ижиков <[hidden email]>:

> 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 <[hidden email]>:
>
>> 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 <[hidden email]>
>> 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, Николай Ижиков <[hidden email]
>> >
>> > 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 <[hidden email]>:
>> >>
>> >> > Vladimir, are their factories for the proposed listeners?
>> >> >
>> >> > On Tue, Sep 12, 2017 at 7:52 AM, Alexey Goncharuk <
>> >> > [hidden email]> 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
>> >> [hidden email]
>> >>
>> >
>> >
>>
>
>
>
> --
> Nikolay Izhikov
> [hidden email]
>

--
Nikolay Izhikov
[hidden email]

[Nikolay Izhikov]

Hello, Vova.

Can you, please, share your knowledge: Why we have abandon ContinuousQuery?

> 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 Николай Ижиков <[hidden email]
> <mailto:[hidden email]>>:
>
>     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 <[hidden email]
>     <mailto:[hidden email]>>:
>
>         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
>         <[hidden email] <mailto:[hidden email]>>
>         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, Николай Ижиков
>         <[hidden email] <mailto:[hidden email]>>
>          > 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
>         <https://github.com/apache/ignite/pull/2372>
>          >> [2] https://issues.apache.org/jira/browse/IGNITE-425
>         <https://issues.apache.org/jira/browse/IGNITE-425>
>          >>
>          >>
>          >> 2017-09-12 17:55 GMT+03:00 Dmitriy Setrakyan
>         <[hidden email] <mailto:[hidden email]>>:
>          >>
>          >> > Vladimir, are their factories for the proposed listeners?
>          >> >
>          >> > On Tue, Sep 12, 2017 at 7:52 AM, Alexey Goncharuk <
>          >> > [hidden email]
>         <mailto:[hidden email]>> 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
>          >> [hidden email] <mailto:[hidden email]>
>          >>
>          >
>          >
>
>
>
>
>     --
>     Nikolay Izhikov
>     [hidden email] <mailto:[hidden email]>
>
>
>
>
> --
> Nikolay Izhikov
> [hidden email] <mailto:[hidden email]>

[dmagda]

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 <[hidden email]> 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 <[hidden email]> 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]

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 <[hidden email]>:

> 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 <[hidden email]>
> 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 <[hidden email]>
> 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
[hidden email]

[Vladimir Ozerov]

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, Николай Ижиков <[hidden email]>
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 <[hidden email]>:
>
> > 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 <[hidden email]>
> > 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 <[hidden email]>
> > 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
> [hidden email]
>

[Nikolay Izhikov]

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, Николай Ижиков <[hidden email]>
> 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 <[hidden email]>:
> >
> > > 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 <[hidden email]>
> > > 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 <[hidden email]>
> > > 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
> > [hidden email]
> >
>

--
Nikolay Izhikov
[hidden email]