From e24f7f4675d5f4159f858985a3a3923673c04e4e Mon Sep 17 00:00:00 2001 From: Andrew Purtell Date: Tue, 25 Jul 2017 18:37:53 -0700 Subject: [PATCH] HBASE-18054 log when we add/remove failed servers in client (Ali) --- .../hbase/ipc/BlockingRpcConnection.java | 2 +- .../hadoop/hbase/ipc/FailedServers.java | 10 ++- .../hadoop/hbase/ipc/NettyRpcConnection.java | 2 +- .../hbase/ipc/TestFailedServersLog.java | 77 +++++++++++++++++++ .../hadoop/hbase/ipc/TestHBaseClient.java | 11 +-- 5 files changed, 94 insertions(+), 8 deletions(-) create mode 100644 hbase-client/src/test/java/org/apache/hadoop/hbase/ipc/TestFailedServersLog.java diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/BlockingRpcConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/BlockingRpcConnection.java index 1012ad03500..df6a7b5c141 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/BlockingRpcConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/BlockingRpcConnection.java @@ -488,7 +488,7 @@ class BlockingRpcConnection extends RpcConnection implements Runnable { closeSocket(); IOException e = ExceptionUtil.asInterrupt(t); if (e == null) { - this.rpcClient.failedServers.addToFailedServers(remoteId.address); + this.rpcClient.failedServers.addToFailedServers(remoteId.address, t); if (t instanceof LinkageError) { // probably the hbase hadoop version does not match the running hadoop version e = new DoNotRetryIOException(t); diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/FailedServers.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/FailedServers.java index 238b7a3fdbd..4f567c44cdf 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/FailedServers.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/FailedServers.java @@ -23,6 +23,8 @@ import java.util.Iterator; import java.util.LinkedList; import java.util.Map; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; @@ -36,6 +38,7 @@ public class FailedServers { private final Map failedServers = new HashMap(); private long latestExpiry = 0; private final int recheckServersTimeout; + private static final Log LOG = LogFactory.getLog(FailedServers.class); public FailedServers(Configuration conf) { this.recheckServersTimeout = conf.getInt( @@ -45,10 +48,15 @@ public class FailedServers { /** * Add an address to the list of the failed servers list. */ - public synchronized void addToFailedServers(InetSocketAddress address) { + public synchronized void addToFailedServers(InetSocketAddress address, Throwable throwable) { final long expiry = EnvironmentEdgeManager.currentTime() + recheckServersTimeout; this.failedServers.put(address.toString(), expiry); this.latestExpiry = expiry; + if (LOG.isDebugEnabled()) { + LOG.debug( + "Added failed server with address " + address.toString() + " to list caused by " + + throwable.toString()); + } } /** diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcConnection.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcConnection.java index 204b812f17d..c879990f940 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcConnection.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/NettyRpcConnection.java @@ -266,7 +266,7 @@ class NettyRpcConnection extends RpcConnection { Channel ch = future.channel(); if (!future.isSuccess()) { failInit(ch, toIOE(future.cause())); - rpcClient.failedServers.addToFailedServers(remoteId.address); + rpcClient.failedServers.addToFailedServers(remoteId.address, future.cause()); return; } ch.writeAndFlush(connectionHeaderPreamble.retainedDuplicate()); diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/ipc/TestFailedServersLog.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/ipc/TestFailedServersLog.java new file mode 100644 index 00000000000..03e3ca5039f --- /dev/null +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/ipc/TestFailedServersLog.java @@ -0,0 +1,77 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license + * agreements. See the NOTICE file distributed with this work for additional information regarding + * copyright ownership. The ASF licenses this file to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. You may obtain a + * copy of the License at http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable + * law or agreed to in writing, software distributed under the License is distributed on an "AS IS" + * BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License + * for the specific language governing permissions and limitations under the License. + */ + +package org.apache.hadoop.hbase.ipc; + +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; + +import java.net.InetSocketAddress; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.testclassification.ClientTests; +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.apache.log4j.Appender; +import org.apache.log4j.Level; +import org.apache.log4j.LogManager; +import org.apache.log4j.spi.LoggingEvent; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.runner.RunWith; +import org.mockito.ArgumentCaptor; +import org.mockito.Captor; +import org.mockito.Mock; +import org.mockito.Mockito; +import org.mockito.runners.MockitoJUnitRunner; + +@RunWith(MockitoJUnitRunner.class) +@Category({ ClientTests.class, SmallTests.class }) +public class TestFailedServersLog { + static final int TEST_PORT = 9999; + private InetSocketAddress addr; + + @Mock + private Appender mockAppender; + + @Captor + private ArgumentCaptor captorLoggingEvent; + + @Before + public void setup() { + LogManager.getRootLogger().addAppender(mockAppender); + } + + @After + public void teardown() { + LogManager.getRootLogger().removeAppender(mockAppender); + } + + @Test + public void testAddToFailedServersLogging() { + Throwable nullException = new NullPointerException(); + + FailedServers fs = new FailedServers(new Configuration()); + addr = new InetSocketAddress(TEST_PORT); + + fs.addToFailedServers(addr, nullException); + + Mockito.verify(mockAppender).doAppend((LoggingEvent) captorLoggingEvent.capture()); + LoggingEvent loggingEvent = (LoggingEvent) captorLoggingEvent.getValue(); + assertThat(loggingEvent.getLevel(), is(Level.DEBUG)); + assertEquals("Added failed server with address " + addr.toString() + " to list caused by " + + nullException.toString(), + loggingEvent.getRenderedMessage()); + } + +} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestHBaseClient.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestHBaseClient.java index 26488cf9477..b2bccd6e7da 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestHBaseClient.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/ipc/TestHBaseClient.java @@ -37,6 +37,7 @@ public class TestHBaseClient { ManualEnvironmentEdge ee = new ManualEnvironmentEdge(); EnvironmentEdgeManager.injectEdge( ee ); FailedServers fs = new FailedServers(new Configuration()); + Throwable testThrowable = new Throwable();//throwable already tested in TestFailedServers.java InetSocketAddress ia = InetSocketAddress.createUnresolved("bad", 12); InetSocketAddress ia2 = InetSocketAddress.createUnresolved("bad", 12); // same server as ia @@ -46,7 +47,7 @@ public class TestHBaseClient { Assert.assertFalse( fs.isFailedServer(ia) ); - fs.addToFailedServers(ia); + fs.addToFailedServers(ia,testThrowable); Assert.assertTrue( fs.isFailedServer(ia) ); Assert.assertTrue( fs.isFailedServer(ia2) ); @@ -58,9 +59,9 @@ public class TestHBaseClient { Assert.assertFalse( fs.isFailedServer(ia) ); Assert.assertFalse( fs.isFailedServer(ia2) ); - fs.addToFailedServers(ia); - fs.addToFailedServers(ia3); - fs.addToFailedServers(ia4); + fs.addToFailedServers(ia,testThrowable); + fs.addToFailedServers(ia3,testThrowable); + fs.addToFailedServers(ia4,testThrowable); Assert.assertTrue( fs.isFailedServer(ia) ); Assert.assertTrue( fs.isFailedServer(ia2) ); @@ -74,7 +75,7 @@ public class TestHBaseClient { Assert.assertFalse( fs.isFailedServer(ia4) ); - fs.addToFailedServers(ia3); + fs.addToFailedServers(ia3,testThrowable); Assert.assertFalse( fs.isFailedServer(ia4) ); } }