[GitHub] [ignite] Berkof commented on a change in pull request #9544: IGNITE-15844 BusyExecutor allow task be executed after deactive/activate.

[GitBox <gi...@apache.org>]

Berkof commented on a change in pull request #9544:
URL: https://github.com/apache/ignite/pull/9544#discussion_r741626151



##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/stat/BusyExecutor.java
##########
@@ -119,12 +150,40 @@ public boolean busyRun(Runnable r) {
      * Task surrounded with try/catch and if it's complete with any exception - resulting future will return
      *
      * @param r Task to execute.
-     * @return Completable future.
+     * @return Completable future with executed flag in result.
      */
     public CompletableFuture<Boolean> submit(Runnable r) {
+        GridBusyLock lock = busyLock;
+
+        CompletableFuture<Boolean> res = new CompletableFuture<>();
+
+        pool.execute(() -> res.complete(busyRun(r, lock)));
+
+        return res;
+    }
+
+    /**
+     * Submit cancellable task to execute in thread pool.
+     *
+     * @param ct Task to execute.
+     * @return Completable future with executed flag in result.
+     */
+    public CompletableFuture<Boolean> submit(CancellableTask ct) {
+        GridBusyLock lock = busyLock;
+
         CompletableFuture<Boolean> res = new CompletableFuture<>();
+        
+        if (busyRun(() -> cancellableTasks.put(ct, ct), lock)) {

Review comment:
       Removed. It is unnecessary to even try to synchronize it.

##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/stat/BusyExecutor.java
##########
@@ -135,6 +194,25 @@ public boolean busyRun(Runnable r) {
      * @param r Task to execute.
      */
     public void execute(Runnable r) {
-        pool.execute(() -> busyRun(r));
+        GridBusyLock lock = busyLock;
+
+        pool.execute(() -> busyRun(r, lock));
+    }
+
+    /**
+     * Execute cancellable task in thread pool under busy lock. Track task to cancel on executor stop.
+     *
+     * @param ct Cancellable task to execute.
+     */
+    public void execute(CancellableTask ct) {
+        GridBusyLock lock = busyLock;
+
+        if (busyRun(() -> cancellableTasks.put(ct, ct), lock)) {
+            pool.execute(() -> {
+                busyRun(ct, lock);
+
+                cancellableTasks.remove(ct);
+            });
+        }

Review comment:
       -1 line of code
   +1 garbage object: CompletableFuture.
   Lets save objects. Or maybe we should remove execute methods?

##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/stat/BusyExecutor.java
##########
@@ -35,52 +39,65 @@
     private final String name;
 
     /** Active flag (used to skip commands in inactive cluster.) */
-    private volatile boolean active;
+    private final AtomicBoolean active = new AtomicBoolean(false);

Review comment:
       Good point, thanks.

##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/stat/BusyExecutor.java
##########
@@ -35,52 +39,65 @@
     private final String name;
 
     /** Active flag (used to skip commands in inactive cluster.) */
-    private volatile boolean active;
+    private final AtomicBoolean active = new AtomicBoolean(false);
 
     /** Lock protection of started gathering during deactivation. */
-    private final GridBusyLock busyLock = new GridBusyLock();
+    private volatile GridBusyLock busyLock = new GridBusyLock();
 
     /** Executor pool. */
     private final IgniteThreadPoolExecutor pool;
 
+    /** Cancellable tasks. */
+    private final ConcurrentMap<CancellableTask, Object> cancellableTasks = new ConcurrentHashMap<>();

Review comment:
       CancellableTask does not implement Comparable so we can't use ConcurrentSkipListSet.
   There is no ConcurrentHashSet structure in ApacheIgnite now.
   But there is GridConcurrentHashSet (actually it is just a wrapper around ConcurrentHashMap and, unfortunately, it creates extra objects, but I replace ConcurrentHashMap with it.

##########
File path: modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/stat/BusyExecutor.java
##########
@@ -135,6 +194,25 @@ public boolean busyRun(Runnable r) {
      * @param r Task to execute.
      */
     public void execute(Runnable r) {
-        pool.execute(() -> busyRun(r));
+        GridBusyLock lock = busyLock;
+
+        pool.execute(() -> busyRun(r, lock));

Review comment:
       -1 line of code
   +1 garbage object: CompletableFuture.
   Lets save objects.
   




-- 
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: notifications-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org