From 63222d6f102e989cd61dfc327297f0412f9c99a0 Mon Sep 17 00:00:00 2001 From: Mikhail Antonov Date: Wed, 23 Mar 2016 20:07:13 -0700 Subject: [PATCH] HBASE-15524 Fix NPE in client-side metrics --- .../apache/hadoop/hbase/client/AsyncProcess.java | 38 ++++++++++++++++++---- .../hadoop/hbase/client/MetricsConnection.java | 4 ++- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncProcess.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncProcess.java index 9a67990..a646687 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncProcess.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncProcess.java @@ -1175,9 +1175,15 @@ class AsyncProcess { byte[] row = e.getValue().iterator().next().getAction().getRow(); // Do not use the exception for updating cache because it might be coming from // any of the regions in the MultiAction. - if (tableName != null) { - connection.updateCachedLocations(tableName, regionName, row, - ClientExceptionsUtil.isMetaClearingException(t) ? null : t, server); + try { + if (tableName != null) { + connection.updateCachedLocations(tableName, regionName, row, + ClientExceptionsUtil.isMetaClearingException(t) ? null : t, server); + } + } catch (Throwable ex) { + // That should never happen, but if it did, we want to make sure + // we still process errors + LOG.error("Couldn't update cached region locations: " + ex); } for (Action action : e.getValue()) { Retry retry = manageError( @@ -1298,8 +1304,14 @@ class AsyncProcess { // Register corresponding failures once per server/once per region. if (!regionFailureRegistered) { regionFailureRegistered = true; - connection.updateCachedLocations( + try { + connection.updateCachedLocations( tableName, regionName, row.getRow(), result, server); + } catch (Throwable ex) { + // That should never happen, but if it did, we want to make sure + // we still process errors + LOG.error("Couldn't update cached region locations: " + ex); + } } if (failureCount == 0) { errorsByServer.reportServerError(server); @@ -1319,8 +1331,14 @@ class AsyncProcess { } else { if (AsyncProcess.this.connection.getConnectionMetrics() != null) { - AsyncProcess.this.connection.getConnectionMetrics(). - updateServerStats(server, regionName, result); + try { + AsyncProcess.this.connection.getConnectionMetrics(). + updateServerStats(server, regionName, result); + } catch (Throwable ex) { + // That should never happen, but if it did, we want to make sure + // we still process errors + LOG.error("Couldn't update server stats/metrics: " + ex); + } } // update the stats about the region, if its a user table. We don't want to slow down @@ -1366,8 +1384,14 @@ class AsyncProcess { // for every possible exception that comes through, however. connection.clearCaches(server); } else { - connection.updateCachedLocations( + try { + connection.updateCachedLocations( tableName, region, actions.get(0).getAction().getRow(), throwable, server); + } catch (Throwable ex) { + // That should never happen, but if it did, we want to make sure + // we still process errors + LOG.error("Couldn't update cached region locations: " + ex); + } } failureCount += actions.size(); diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java index b6efdb9..dc56246 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java @@ -61,6 +61,7 @@ public class MetricsConnection { private static final String MEMLOAD_BASE = "memstoreLoad_"; private static final String HEAP_BASE = "heapOccupancy_"; private static final String CACHE_BASE = "cacheDroppingExceptions_"; + private static final String UNKNOWN_EXCEPTION = "UnknownException"; private static final String CLIENT_SVC = ClientService.getDescriptor().getName(); /** A container class for collecting details about the RPC call as it percolates. */ @@ -455,7 +456,8 @@ public class MetricsConnection { } public void incrCacheDroppingExceptions(Object exception) { - getMetric(CACHE_BASE + exception.getClass().getSimpleName(), + getMetric(CACHE_BASE + + (exception == null? UNKNOWN_EXCEPTION : exception.getClass().getSimpleName()), cacheDroppingExceptions, counterFactory).inc(); } } -- 2.8.0-rc2