The pull request is ready for review. On Fri, Jul 17, 2020 at 4:11 AM Igor Sapego <isap...@apache.org> wrote:
> I've reviewed changes made to IEP and they look good to me. > > Best Regards, > Igor > > > On Wed, Jul 15, 2020 at 1:03 PM Pavel Tupitsyn <ptupit...@apache.org> > wrote: > > > Alex, > > > > You are correct, OP_RESOURCE_CLOSE is enough. > > Removed the extra op. > > > > > If client closes CQ it doesn't want to receive any new events. Why > can't > > we > > > just ignore events for this CQ after that moment? > > I don't think that our protocol should involve ignoring messages. > > If the client stops the query, the server should guarantee that no events > > will be sent > > to the client after the OP_RESOURCE_CLOSE response. > > > > I had some concerns about this guarantee, but after reviewing > GridNioServer > > logic, > > the current implementation with OP_RESOURCE_CLOSE seems to be fine. > > > > > > > > On Wed, Jul 15, 2020 at 10:09 AM Alex Plehanov <plehanov.a...@gmail.com> > > wrote: > > > > > Pavel, > > > > > > > OP_QUERY_CONTINUOUS_END_NOTIFICATION is another client -> server > > message > > > I think you mean "server -> client" here. > > > > > > But I still didn't get why do we need it. > > > I've briefly looked to the POC implementation and, as far as I > > understand, > > > OP_QUERY_CONTINUOUS_END_NOTIFICATION can be sent only when > > > OP_RESOURCE_CLOSE is received by server (client closes the CQ > > explicitly). > > > If client closes CQ it doesn't want to receive any new events. Why > can't > > we > > > just ignore events for this CQ after that moment? > > > Also, in current implementation OP_QUERY_CONTINUOUS_END_NOTIFICATION is > > > sent before OP_RESOURCE_CLOSE response, so OP_RESOURCE_CLOSE response > can > > > be used the same way as OP_QUERY_CONTINUOUS_END_NOTIFICATION. > > > > > > Such notification (or something more generalized like > OP_RESOURCE_CLOSED) > > > can be helpful if CQ is closed by someone else (for example if > > > administrator call QueryMXBean.cancelContinuous), but AFAIK now we > don't > > > have callbacks for this action on user side. > > > > > > > > > ср, 15 июл. 2020 г. в 01:26, Pavel Tupitsyn <ptupit...@apache.org>: > > > > > > > Igniters, > > > > > > > > I've made an important change to the IEP (and the POC): > > > > OP_QUERY_CONTINUOUS_END_NOTIFICATION is another client -> server > > message > > > > that notifies the client that the query has stopped and no more > events > > > > should be expected. > > > > > > > > This is important because client can't immediately stop listening > > > > for OP_QUERY_CONTINUOUS_EVENT_NOTIFICATION > > > > after sending OP_RESOURCE_CLOSE - some more events can be present in > > one > > > of > > > > the buffers/queues of the server and/or the OS. > > > > > > > > Let me know if this makes sense. > > > > > > > > On Tue, Jul 14, 2020 at 3:25 PM Pavel Tupitsyn <ptupit...@apache.org > > > > > > wrote: > > > > > > > > > I've removed Initial Query from the POC and IEP (left a note there > > > about > > > > > the decision). > > > > > > > > > > Since there are no other comments and concerns, I'll move on with > the > > > > > final implementation. > > > > > > > > > > On Fri, Jul 10, 2020 at 4:14 PM Pavel Tupitsyn < > ptupit...@apache.org > > > > > > > > wrote: > > > > > > > > > >> Igor, Alex, > > > > >> > > > > >> I was aware of the duplicates issue with the initial query, but I > > did > > > > not > > > > >> give it a second thought. > > > > >> > > > > >> Now I see that Vladimir was right - Initial query seems to be > > > pointless, > > > > >> since users can > > > > >> achieve the same by simply invoking the regular query. > > > > >> > > > > >> I will remove Initial Query from the IEP and POC next week if > there > > > are > > > > >> no objections by then. > > > > >> > > > > >> > > > > >> On Fri, Jul 10, 2020 at 3:58 PM Alex Plehanov < > > > plehanov.a...@gmail.com> > > > > >> wrote: > > > > >> > > > > >>> Igor, Pavel, > > > > >>> > > > > >>> Here is discussion about removal: [1] > > > > >>> > > > > >>> [1] : > > > > >>> > > > > >>> > > > > > > > > > > http://apache-ignite-developers.2346864.n4.nabble.com/ContinuousQueryWithTransformer-implementation-questions-2-td21418i20.html#a22041 > > > > >>> > > > > >>> пт, 10 июл. 2020 г. в 17:44, Igor Sapego <isap...@apache.org>: > > > > >>> > > > > >>> > Can not find proposal to remove them, so maybe it was not on > > > devlist, > > > > >>> > but here is discussion about the problem: [1] > > > > >>> > > > > > >>> > [1] - > > > > >>> > > > > > >>> > > > > > >>> > > > > > > > > > > http://apache-ignite-developers.2346864.n4.nabble.com/Continuous-queries-and-duplicates-td39444.html > > > > >>> > > > > > >>> > Best Regards, > > > > >>> > Igor > > > > >>> > > > > > >>> > > > > > >>> > On Fri, Jul 10, 2020 at 3:27 PM Pavel Tupitsyn < > > > ptupit...@apache.org > > > > > > > > > >>> > wrote: > > > > >>> > > > > > >>> > > > What's about "stop" message? How can user unsubscribe from > > > > >>> receiving > > > > >>> > > notifications? > > > > >>> > > OP_RESOURCE_CLOSE is used for that. I've updated the IEP in > an > > > > >>> attempt to > > > > >>> > > make this cleaner. > > > > >>> > > > > > > >>> > > > I've seen discussions on removing initial query from > > > continuous > > > > >>> > queries > > > > >>> > > Interesting, I'm not aware of this. Can you please link those > > > > >>> > discussions? > > > > >>> > > > > > > >>> > > On Fri, Jul 10, 2020 at 2:04 PM Igor Sapego < > > isap...@apache.org> > > > > >>> wrote: > > > > >>> > > > > > > >>> > > > Pavel, > > > > >>> > > > > > > > >>> > > > What's about "stop" message? How can user unsubscribe from > > > > >>> receiving > > > > >>> > > > notifications? > > > > >>> > > > > > > > >>> > > > Also, I believe I've seen discussions on removing initial > > query > > > > >>> from > > > > >>> > > > continuous queries, > > > > >>> > > > as there are not any guarantees about getting consistent > > > results > > > > >>> with > > > > >>> > > them. > > > > >>> > > > Should > > > > >>> > > > we avoid adding them in thin protocol maybe? It would also > > > > simplify > > > > >>> > > > protocol a lot. > > > > >>> > > > > > > > >>> > > > Best Regards, > > > > >>> > > > Igor > > > > >>> > > > > > > > >>> > > > > > > > >>> > > > On Tue, Jun 30, 2020 at 2:39 PM Pavel Tupitsyn < > > > > >>> ptupit...@apache.org> > > > > >>> > > > wrote: > > > > >>> > > > > > > > >>> > > > > Igniters, > > > > >>> > > > > > > > > >>> > > > > Let's discuss Thin Client Continuous Queries, > > > > >>> > > > > I've prepared an IEP [1] and a PoC [2]. > > > > >>> > > > > > > > > >>> > > > > [1] > > > > >>> > > > > > > > > >>> > > > > > > > > >>> > > > > > > > >>> > > > > > > >>> > > > > > >>> > > > > > > > > > > https://cwiki.apache.org/confluence/display/IGNITE/IEP-50%3A+Thin+Client+Continuous+Queries > > > > >>> > > > > > > > > >>> > > > > [2] https://github.com/apache/ignite/pull/7966 > > > > >>> > > > > > > > > >>> > > > > > > > >>> > > > > > > >>> > > > > > >>> > > > > >> > > > > > > > > > >
