timoninmaxim commented on a change in pull request #9086:
URL: https://github.com/apache/ignite/pull/9086#discussion_r639555171
##########
File path:
modules/core/src/main/java/org/apache/ignite/internal/processors/cache/query/GridCacheQueryFutureAdapter.java
##########
@@ -264,25 +266,7 @@ protected void onNodeLeft(UUID evtNodeId) {
protected void enqueue(Collection<?> col) {
assert Thread.holdsLock(this);
- if (limitDisabled) {
- queue.add((Collection<R>)col);
-
- cnt.addAndGet(col.size());
- }
- else {
- if (capacity >= col.size()) {
- queue.add((Collection<R>)col);
- capacity -= col.size();
-
- cnt.addAndGet(col.size());
- }
- else if (capacity > 0) {
- queue.add(new ArrayList<>((Collection<R>)col).subList(0,
capacity));
- capacity = 0;
-
- cnt.addAndGet(capacity);
- }
- }
+ queue.add((Collection<R>)col);
Review comment:
Thanks for clarification, now understood your note. You're correct that
it will work safe on current state of master. It's because we don't have any
reduce of filter operations on received pages. But I did it by 2 reasons:
1. In parallel activity I add MergeSort algorithm, it merging data from
different pages. So there are cases when we get a data from single node, and it
covers a specified limit. But actually we need another page from other nodes to
compare. + Maybe there could be additional cases in future for other reduce or
filtering operations.
2. I don't like idea to make `enqueue()` responsible for finishing future,
as there is already a working mechanism of `onDone()` and `cancel()` based on
`hasNext()` result. Also now `enqueue()` creates a new collection from sublist
in case of limit applying on input collection, there is not much overhead but
it can be avoid by the same existing mechanism. I believe the less code the
better. So I decide remove this duplicated mechanism.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
