Vladsz83 commented on code in PR #12589:
URL: https://github.com/apache/ignite/pull/12589#discussion_r2630996761
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/task/QueryTasksQueue.java:
##########
@@ -83,16 +90,22 @@ public int size() {
/** Add a task to the queue. */
public void addTask(QueryAwareTask task) {
+ Node node = new Node(task);
+
lock.lock();
try {
assert last.next == null : "Unexpected last.next: " + last.next;
- last = last.next = new Node(task);
+ last = last.next = node;
- cnt.getAndIncrement();
+ int tasksCnt = cnt.incrementAndGet();
- notEmpty.signal();
+ // Do not wake up new threads if it's enough free treads to
process the new task.
Review Comment:
What if `tasksCnt > freeThreadsCnt` but a couple of threads can porocess
tasks? For instance, 10 tasks and 1-2 free threads. Why not allow them to pick
up a task?
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/task/QueryTasksQueue.java:
##########
@@ -160,8 +182,9 @@ public void unblockQuery(QueryKey qryKey) {
assert removed;
- if (cnt.get() > 0)
- notEmpty.signal();
Review Comment:
We removed almost all signals. With the condition `if (tasksCnt >
freeThreadsCnt` in the task adding, it seems we might keep rest of the tasks
unprocessed until a new one comes. If it won't, some tasks wil lstay in the
queue unprocessed. 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]
