timoninmaxim commented on a change in pull request #9081:
URL: https://github.com/apache/ignite/pull/9081#discussion_r685260622
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheDistributedQueryManager.java
##########
@@ -101,12 +104,30 @@
/** Event listener. */
private GridLocalEventListener lsnr;
+ /** Requester of cache query result pages. */
+ private CacheQueryPageRequester pageRequester;
+
/** {@inheritDoc} */
@Override public void start0() throws IgniteCheckedException {
super.start0();
assert cctx.config().getCacheMode() != LOCAL;
+ pageRequester = new CacheQueryPageRequester(cctx) {
+ /** {@inheritDoc} */
+ @Override protected void sendLocal(GridCacheQueryRequest req) {
Review comment:
I've tried to refactor it, but actually failed to make it better.
There are 3 events to send a request to other nodes: initial load, ask for
next page after finish previous, cancel query. Initiators of those events are
independent on each other and the only common place is `reducer`, that is
triggered and performs requests.
PageRequester's `sendRequest()` also invokes `sendLocal()` that depends much
on variables defined in `GridCacheDistributedQueryManager`. The initiators
doesn't have dependency on this manager. Then it's required to accumulate all
logic of sending request and local handling in single object (`pageRequester`).
Alternatives is make initiators depend on
`GridCacheDistributedQueryManager`, that makes code more spaghetti. Split
requester on request factory and request sender also doesn't make code clearer.
I think current solution is OK, it clear enough. The only bad place is
`sendLocal` that depends on the manager variables. I think we should not
refactor this manager now, as it is out of scope of this ticket. WDYT?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
