Igniters, Igor raised an interesting point in the PR:
Should we limit the number of Continuous Queries together with other queries according to ClientConnectorConfiguration.maxOpenCursorsPerConn? Or should we have a separate limit? Technically, Ignite returns a QueryCursor, but it is very different from other cursors. On Fri, Jul 17, 2020 at 11:25 AM Pavel Tupitsyn <ptupit...@apache.org> wrote: > 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 >> > > > >>> > > > > >> > > > >>> > > > >> > > > >>> > > >> > > > >>> > >> > > > >>> >> > > > >> >> > > > >> > > >> > >> >
