Author: mduerig
Date: Mon Oct 17 10:02:55 2016
New Revision: 1765235

URL: http://svn.apache.org/viewvc?rev=1765235&view=rev
Log:
OAK-4919: Better feedback from method invocations on RevisionGCMBean

Modified:
    
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/management/ManagementOperation.java
    
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java

Modified: 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/management/ManagementOperation.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/management/ManagementOperation.java?rev=1765235&r1=1765234&r2=1765235&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/management/ManagementOperation.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/management/ManagementOperation.java
 Mon Oct 17 10:02:55 2016
@@ -20,6 +20,7 @@
 package org.apache.jackrabbit.oak.management;
 
 import static com.google.common.base.Objects.toStringHelper;
+import static com.google.common.base.Preconditions.checkNotNull;
 import static java.lang.Thread.currentThread;
 import static java.util.concurrent.TimeUnit.DAYS;
 import static java.util.concurrent.TimeUnit.HOURS;
@@ -48,6 +49,7 @@ import java.util.concurrent.FutureTask;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import javax.annotation.Nonnull;
 import javax.management.openmbean.CompositeData;
 import javax.management.openmbean.CompositeDataSupport;
 import javax.management.openmbean.CompositeType;
@@ -57,6 +59,8 @@ import javax.management.openmbean.Tabula
 import javax.management.openmbean.TabularDataSupport;
 import javax.management.openmbean.TabularType;
 
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -73,17 +77,40 @@ public class ManagementOperation<R> exte
     private static final AtomicInteger idGen = new AtomicInteger();
 
     protected final int id;
+
+    @Nonnull
     protected final String name;
 
+    @Nonnull
+    private final Supplier<String> statusMessage;
+
     /**
-     * Create a new {@code ManagementOperation} of the given name. The name
+     * Create a new {@code ManagementOperation} of the given name. The {@code 
name}
      * is an informal value attached to this instance.
      *
      * @param name  informal name
      * @param task  task to execute for this operation
      */
-    public static <R> ManagementOperation<R> newManagementOperation(String 
name, Callable<R> task) {
-        return new ManagementOperation<R>(name, task);
+    public static <R> ManagementOperation<R> newManagementOperation(
+            @Nonnull String name,
+            @Nonnull Callable<R> task) {
+        return new ManagementOperation<R>(name, Suppliers.ofInstance(""), 
task);
+    }
+
+    /**
+     * Create a new {@code ManagementOperation} of the given name. The {@code 
name}
+     * is an informal value attached to this instance.
+     *
+     * @param name           informal name
+     * @param statusMessage  an informal status message describing the status 
of the background
+     *                       operation at the time of invocation.
+     * @param task           task to execute for this operation
+     */
+    public static <R> ManagementOperation<R> newManagementOperation(
+            @Nonnull String name,
+            @Nonnull Supplier<String> statusMessage,
+            @Nonnull Callable<R> task) {
+        return new ManagementOperation<R>(name, statusMessage, task);
     }
 
     /**
@@ -93,8 +120,10 @@ public class ManagementOperation<R> exte
      * @param result result returned by the operation
      * @return  a {@code ManagementOperation} instance that is already done.
      */
+    @Nonnull
     public static <R> ManagementOperation<R> done(String name, final R result) 
{
-        return new ManagementOperation<R>("not started", new Callable<R>() {
+        return new ManagementOperation<R>("done", Suppliers.ofInstance(""),
+                new Callable<R>() {
             @Override
             public R call() throws Exception {
                 return result;
@@ -117,22 +146,27 @@ public class ManagementOperation<R> exte
 
             @Override
             public Status getStatus() {
-                return none(id, name + " not started");
+                return none(id, "NA");
             }
         };
     }
 
     /**
-     * Create a new {@code ManagementOperation} of the given name. The name
+     * Create a new {@code ManagementOperation} of the given name. The {@code 
name}
      * is an informal value attached to this instance.
-     *
-     * @param name  informal name
-     * @param task  task to execute for this operation
-     */
-    private ManagementOperation(String name, Callable<R> task) {
+     * @param name           informal name
+     * @param statusMessage  an informal status message describing the status 
of the background
+     *                       operation at the time of invocation.
+     * @param task           task to execute for this operation
+     */
+    private ManagementOperation(
+            @Nonnull String name,
+            @Nonnull Supplier<String> statusMessage,
+            @Nonnull Callable<R> task) {
         super(task);
         this.id = idGen.incrementAndGet();
-        this.name = name;
+        this.name = checkNotNull(name);
+        this.statusMessage = checkNotNull(statusMessage);
     }
 
     /**
@@ -150,6 +184,7 @@ public class ManagementOperation<R> exte
      * Informal name
      * @return  name of this operation
      */
+    @Nonnull
     public String getName() {
         return name;
     }
@@ -168,21 +203,23 @@ public class ManagementOperation<R> exte
      *
      * @return  the current status of this operation
      */
+    @Nonnull
     public Status getStatus() {
         if (isCancelled()) {
             return failed(id, name + " cancelled");
         } else if (isDone()) {
             try {
-                return succeeded(id, name + " succeeded: " + get());
+                R result = get();
+                return succeeded(id, name + " succeeded" + (result != null ? 
": " + result : ""));
             } catch (InterruptedException e) {
                 currentThread().interrupt();
-                return failed(id, name + " status unknown: " + e.getMessage());
+                return failed(id, name + " interrupted: " + e.getMessage());
             } catch (ExecutionException e) {
-                LOG.error(name + " failed", e.getCause());
+                LOG.error("{} failed", name, e.getCause());
                 return failed(id, name + " failed: " + 
e.getCause().getMessage());
             }
         } else {
-            return running(id, name + " running");
+            return running(id, name + " running: " + statusMessage.get());
         }
     }
 

Modified: 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java
URL: 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java?rev=1765235&r1=1765234&r2=1765235&view=diff
==============================================================================
--- 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java
 (original)
+++ 
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/RevisionGC.java
 Mon Oct 17 10:02:55 2016
@@ -20,6 +20,8 @@
 package org.apache.jackrabbit.oak.spi.state;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static 
org.apache.jackrabbit.oak.management.ManagementOperation.Status.failed;
+import static 
org.apache.jackrabbit.oak.management.ManagementOperation.Status.succeeded;
 import static org.apache.jackrabbit.oak.management.ManagementOperation.done;
 import static 
org.apache.jackrabbit.oak.management.ManagementOperation.newManagementOperation;
 
@@ -29,6 +31,8 @@ import java.util.concurrent.Executor;
 import javax.annotation.Nonnull;
 import javax.management.openmbean.CompositeData;
 
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
 import org.apache.jackrabbit.oak.management.ManagementOperation;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -42,11 +46,37 @@ public class RevisionGC implements Revis
     public static final String OP_NAME = "Revision garbage collection";
 
     @Nonnull
+    private ManagementOperation<Void> gcOp = done(OP_NAME, null);
+
+    @Nonnull
     private final Runnable runGC;
+
+    @Nonnull
     private final Runnable cancelGC;
+
+    @Nonnull
+    private final Supplier<String> statusMessage;
+
+    @Nonnull
     private final Executor executor;
 
-    private ManagementOperation<String> gcOp = done(OP_NAME, "");
+    /**
+     * @param runGC          Revision garbage collector
+     * @param cancelGC       Executor for cancelling the garbage collection 
task
+     * @param statusMessage  an informal status message describing the status 
of the background
+     *                       operation at the time of invocation.
+     * @param executor       Executor for initiating the garbage collection 
task
+     */
+    public RevisionGC(
+            @Nonnull Runnable runGC,
+            @Nonnull Runnable cancelGC,
+            @Nonnull Supplier<String> statusMessage,
+            @Nonnull Executor executor) {
+        this.runGC = checkNotNull(runGC);
+        this.cancelGC = checkNotNull(cancelGC);
+        this.statusMessage = checkNotNull(statusMessage);
+        this.executor = checkNotNull(executor);
+    }
 
     /**
      * @param runGC        Revision garbage collector
@@ -57,40 +87,43 @@ public class RevisionGC implements Revis
             @Nonnull Runnable runGC,
             @Nonnull Runnable cancelGC,
             @Nonnull Executor executor) {
-        this.runGC = checkNotNull(runGC);
-        this.cancelGC = checkNotNull(cancelGC);
-        this.executor = checkNotNull(executor);
+        this(runGC, cancelGC, Suppliers.ofInstance(""), executor);
     }
 
     @Nonnull
     @Override
     public CompositeData startRevisionGC() {
         if (gcOp.isDone()) {
-            gcOp = newManagementOperation(OP_NAME, new Callable<String>() {
+            gcOp = newManagementOperation(OP_NAME, statusMessage, new 
Callable<Void>() {
                 @Override
-                public String call() throws Exception {
+                public Void call() throws Exception {
                     runGC.run();
-                    return "Revision GC initiated";
+                    return null;
                 }
             });
             executor.execute(gcOp);
+            return succeeded(OP_NAME + " started").toCompositeData();
+        } else {
+            return failed(OP_NAME + " already running").toCompositeData();
         }
-        return getRevisionGCStatus();
     }
 
     @Nonnull
     @Override
     public CompositeData cancelRevisionGC() {
         if (!gcOp.isDone()) {
-            executor.execute(newManagementOperation(OP_NAME, new 
Callable<String>() {
+            executor.execute(newManagementOperation(OP_NAME, new 
Callable<Void>() {
                 @Override
-                public String call() throws Exception {
+                public Void call() throws Exception {
+                    gcOp.cancel(false);
                     cancelGC.run();
-                    return "Revision GC cancelled";
+                    return null;
                 }
             }));
+            return succeeded("Revision garbage collection 
cancelled").toCompositeData();
+        } else {
+            return failed(OP_NAME + " not running").toCompositeData();
         }
-        return getRevisionGCStatus();
     }
 
     @Nonnull


Reply via email to