From 84eb868f983d7e7c2daddbe056d3889cbea0a3fb Mon Sep 17 00:00:00 2001 From: Nick Dimiduk Date: Fri, 4 Mar 2022 09:19:03 +0100 Subject: [PATCH] HBASE-26764 Implement generic exception support for TraceUtil methods over Callables and Runnables For the `TraceUtil` methods that accept `Callable` and `Runnable` types, make them generic over a child of `Throwable`. This allows us to consolidate the two method signatures into a single more flexible definition. Signed-off-by: Duo Zhang --- .../hbase/client/AsyncConnectionImpl.java | 12 +- .../hadoop/hbase/client/HRegionLocator.java | 12 +- .../apache/hadoop/hbase/client/HTable.java | 6 +- .../apache/hadoop/hbase/trace/TraceUtil.java | 117 ++++++++---------- .../hbase/regionserver/wal/AbstractFSWAL.java | 10 +- 5 files changed, 66 insertions(+), 91 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java index ee6b393b3d1..1eebcab4c93 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncConnectionImpl.java @@ -27,7 +27,6 @@ import static org.apache.hadoop.hbase.client.MetricsConnection.CLIENT_SIDE_METRI import static org.apache.hadoop.hbase.client.NonceGenerator.CLIENT_NONCES_ENABLED_KEY; import static org.apache.hadoop.hbase.trace.HBaseSemanticAttributes.SERVER_NAME_KEY; import static org.apache.hadoop.hbase.util.FutureUtils.addListener; - import io.opentelemetry.api.trace.Span; import java.io.IOException; import java.util.Optional; @@ -37,7 +36,6 @@ import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; -import java.util.function.Supplier; import org.apache.commons.io.IOUtils; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.AuthUtil; @@ -58,10 +56,8 @@ import org.apache.hadoop.security.UserGroupInformation; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; import org.slf4j.LoggerFactory; - import org.apache.hbase.thirdparty.com.google.common.util.concurrent.ThreadFactoryBuilder; import org.apache.hbase.thirdparty.io.netty.util.HashedWheelTimer; - import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.AdminService; import org.apache.hadoop.hbase.shaded.protobuf.generated.ClientProtos.ClientService; import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProtos; @@ -405,13 +401,7 @@ public class AsyncConnectionImpl implements AsyncConnection { @Override public Hbck getHbck(ServerName masterServer) { - return TraceUtil.trace(new Supplier() { - - @Override - public Hbck get() { - return getHbckInternal(masterServer); - } - }, "AsyncConnection.getHbck"); + return TraceUtil.trace(() -> getHbckInternal(masterServer), "AsyncConnection.getHbck"); } @Override diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HRegionLocator.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HRegionLocator.java index b62f090fbf4..11c9cc11b24 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HRegionLocator.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HRegionLocator.java @@ -157,14 +157,14 @@ public class HRegionLocator implements RegionLocator { return regions; } - private T tracedLocationFuture( - TraceUtil.IOExceptionCallable action, - Function> getRegionNames, + private R tracedLocationFuture( + TraceUtil.ThrowingCallable action, + Function> getRegionNames, Supplier spanSupplier - ) throws IOException { + ) throws T { final Span span = spanSupplier.get(); try (Scope ignored = span.makeCurrent()) { - final T result = action.call(); + final R result = action.call(); final List regionNames = getRegionNames.apply(result); if (!CollectionUtils.isEmpty(regionNames)) { span.setAttribute(REGION_NAMES_KEY, regionNames); @@ -172,7 +172,7 @@ public class HRegionLocator implements RegionLocator { span.setStatus(StatusCode.OK); span.end(); return result; - } catch (IOException e) { + } catch (Throwable e) { TraceUtil.setError(span, e); span.end(); throw e; diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java index f57ca091a35..856dbc6bf56 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HTable.java @@ -530,7 +530,7 @@ public class HTable implements Table { final Supplier supplier = new TableOperationSpanBuilder(connection) .setTableName(tableName) .setOperation(delete); - TraceUtil.traceWithIOException(() -> { + TraceUtil.trace(() -> { ClientServiceCallable callable = new ClientServiceCallable(this.connection, getName(), delete.getRow(), this.rpcControllerFactory.newController(), delete.getPriority()) { @@ -573,7 +573,7 @@ public class HTable implements Table { final Supplier supplier = new TableOperationSpanBuilder(connection) .setTableName(tableName) .setOperation(put); - TraceUtil.traceWithIOException(() -> { + TraceUtil.trace(() -> { validatePut(put); ClientServiceCallable callable = new ClientServiceCallable(this.connection, getName(), put.getRow(), @@ -1116,7 +1116,7 @@ public class HTable implements Table { .setName("HTable.close") .setTableName(tableName) .setSpanKind(SpanKind.INTERNAL); - TraceUtil.traceWithIOException(() -> { + TraceUtil.trace(() -> { if (this.closed) { return; } diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/trace/TraceUtil.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/trace/TraceUtil.java index a01962e9946..7aa7988017f 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/trace/TraceUtil.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/trace/TraceUtil.java @@ -24,8 +24,8 @@ import io.opentelemetry.api.trace.StatusCode; import io.opentelemetry.api.trace.Tracer; import io.opentelemetry.context.Context; import io.opentelemetry.context.Scope; -import java.io.IOException; import java.util.List; +import java.util.concurrent.Callable; import java.util.concurrent.CompletableFuture; import java.util.function.Supplier; import org.apache.hadoop.hbase.Version; @@ -84,7 +84,7 @@ public final class TraceUtil { Supplier spanSupplier ) { Span span = spanSupplier.get(); - try (Scope scope = span.makeCurrent()) { + try (Scope ignored = span.makeCurrent()) { CompletableFuture future = action.get(); endSpan(future, span); return future; @@ -97,7 +97,7 @@ public final class TraceUtil { public static CompletableFuture tracedFuture(Supplier> action, String spanName) { Span span = createSpan(spanName); - try (Scope scope = span.makeCurrent()) { + try (Scope ignored = span.makeCurrent()) { CompletableFuture future = action.get(); endSpan(future, span); return future; @@ -113,7 +113,7 @@ public final class TraceUtil { Supplier spanSupplier ) { Span span = spanSupplier.get(); - try (Scope scope = span.makeCurrent()) { + try (Scope ignored = span.makeCurrent()) { List> futures = action.get(); endSpan(CompletableFuture.allOf(futures.toArray(new CompletableFuture[0])), span); return futures; @@ -139,70 +139,27 @@ public final class TraceUtil { }); } - public static void trace(Runnable action, String spanName) { - trace(action, () -> createSpan(spanName)); - } - - public static void trace(Runnable action, Supplier creator) { - Span span = creator.get(); - try (Scope scope = span.makeCurrent()) { - action.run(); - span.setStatus(StatusCode.OK); - } catch (Throwable e) { - setError(span, e); - throw e; - } finally { - span.end(); - } - } - - public static T trace(Supplier action, String spanName) { - Span span = createSpan(spanName); - try (Scope scope = span.makeCurrent()) { - T ret = action.get(); - span.setStatus(StatusCode.OK); - return ret; - } catch (Throwable e) { - setError(span, e); - throw e; - } finally { - span.end(); - } - } - + /** + * A {@link Runnable} that may also throw. + * @param the type of {@link Throwable} that can be produced. + */ @FunctionalInterface - public interface IOExceptionCallable { - V call() throws IOException; + public interface ThrowingRunnable { + void run() throws T; } - public static T trace(IOExceptionCallable callable, String spanName) throws IOException { - return trace(callable, () -> createSpan(spanName)); + public static void trace( + final ThrowingRunnable runnable, + final String spanName) throws T { + trace(runnable, () -> createSpan(spanName)); } - public static T trace(IOExceptionCallable callable, Supplier creator) - throws IOException { - Span span = creator.get(); - try (Scope scope = span.makeCurrent()) { - T ret = callable.call(); - span.setStatus(StatusCode.OK); - return ret; - } catch (Throwable e) { - setError(span, e); - throw e; - } finally { - span.end(); - } - } - - @FunctionalInterface - public interface IOExceptionRunnable { - void run() throws IOException; - } - - public static void traceWithIOException(IOExceptionRunnable runnable, - Supplier creator) throws IOException { - Span span = creator.get(); - try (Scope scope = span.makeCurrent()) { + public static void trace( + final ThrowingRunnable runnable, + final Supplier spanSupplier + ) throws T { + Span span = spanSupplier.get(); + try (Scope ignored = span.makeCurrent()) { runnable.run(); span.setStatus(StatusCode.OK); } catch (Throwable e) { @@ -212,4 +169,38 @@ public final class TraceUtil { span.end(); } } + + /** + * A {@link Callable} that may also throw. + * @param the result type of method call. + * @param the type of {@link Throwable} that can be produced. + */ + @FunctionalInterface + public interface ThrowingCallable { + R call() throws T; + } + + public static R trace( + final ThrowingCallable callable, + final String spanName + ) throws T { + return trace(callable, () -> createSpan(spanName)); + } + + public static R trace( + final ThrowingCallable callable, + final Supplier spanSupplier + ) throws T { + Span span = spanSupplier.get(); + try (Scope ignored = span.makeCurrent()) { + final R ret = callable.call(); + span.setStatus(StatusCode.OK); + return ret; + } catch (Throwable e) { + setError(span, e); + throw e; + } finally { + span.end(); + } + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/AbstractFSWAL.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/AbstractFSWAL.java index c32191937a2..2707f929087 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/AbstractFSWAL.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/AbstractFSWAL.java @@ -561,18 +561,12 @@ public abstract class AbstractFSWAL implements WAL { @Override public final void sync(boolean forceSync) throws IOException { - TraceUtil.trace(() -> { - doSync(forceSync); - return null; - }, () -> createSpan("WAL.sync")); + TraceUtil.trace(() -> doSync(forceSync), () -> createSpan("WAL.sync")); } @Override public final void sync(long txid, boolean forceSync) throws IOException { - TraceUtil.trace(() -> { - doSync(txid, forceSync); - return null; - }, () -> createSpan("WAL.sync")); + TraceUtil.trace(() -> doSync(txid, forceSync), () -> createSpan("WAL.sync")); } protected abstract void doSync(boolean forceSync) throws IOException;