From 810473b2772b02dbdc9600261009a30e17d8cf26 Mon Sep 17 00:00:00 2001 From: Michael Stack Date: Thu, 19 Jul 2018 15:17:40 -0700 Subject: [PATCH] HBASE-20914 Trim Master memory usage Add (weak reference) interning of ServerNames. Correct Balancer regions x racks matrix. Make smaller defaults when creating ArrayDeques. --- .../org/apache/hadoop/hbase/ServerName.java | 17 ++++++++--- .../hbase/procedure2/ProcedureDeque.java | 4 +++ .../master/balancer/BaseLoadBalancer.java | 2 +- .../balancer/StochasticLoadBalancer.java | 3 +- .../apache/hadoop/hbase/TestServerName.java | 30 +++++++++++++++---- 5 files changed, 43 insertions(+), 13 deletions(-) diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/ServerName.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/ServerName.java index f79b17afea0..34ac1e54ed0 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/ServerName.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/ServerName.java @@ -27,9 +27,11 @@ import java.util.regex.Pattern; import org.apache.hadoop.hbase.net.Address; import org.apache.hadoop.hbase.util.Addressing; import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hbase.thirdparty.com.google.common.collect.Interner; +import org.apache.hbase.thirdparty.com.google.common.collect.Interners; +import org.apache.hbase.thirdparty.com.google.common.net.InetAddresses; import org.apache.yetus.audience.InterfaceAudience; -import org.apache.hbase.thirdparty.com.google.common.net.InetAddresses; /** @@ -99,6 +101,13 @@ public class ServerName implements Comparable, Serializable { private byte [] bytes; public static final List EMPTY_SERVER_LIST = new ArrayList<>(0); + /** + * Intern ServerNames. The Set of ServerNames is mostly-fixed changing slowly as Servers + * restart. Rather than create a new instance everytime, try and return existing instance + * if there is one. + */ + private static final Interner INTERN_POOL = Interners.newWeakInterner(); + protected ServerName(final String hostname, final int port, final long startcode) { this(Address.fromParts(hostname, port), startcode); } @@ -176,7 +185,7 @@ public class ServerName implements Comparable, Serializable { * a shared immutable object as an internal optimization. */ public static ServerName valueOf(final String hostname, final int port, final long startcode) { - return new ServerName(hostname, port, startcode); + return INTERN_POOL.intern(new ServerName(hostname, port, startcode)); } /** @@ -185,7 +194,7 @@ public class ServerName implements Comparable, Serializable { * a shared immutable object as an internal optimization. */ public static ServerName valueOf(final String serverName) { - return new ServerName(serverName); + return INTERN_POOL.intern(new ServerName(serverName)); } /** @@ -194,7 +203,7 @@ public class ServerName implements Comparable, Serializable { * a shared immutable object as an internal optimization. */ public static ServerName valueOf(final String hostAndPort, final long startCode) { - return new ServerName(hostAndPort, startCode); + return INTERN_POOL.intern(new ServerName(hostAndPort, startCode)); } @Override diff --git a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureDeque.java b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureDeque.java index 41b8ca9c297..617e3847c2b 100644 --- a/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureDeque.java +++ b/hbase-procedure/src/main/java/org/apache/hadoop/hbase/procedure2/ProcedureDeque.java @@ -31,4 +31,8 @@ import java.util.ArrayDeque; */ @InterfaceAudience.Private public class ProcedureDeque extends ArrayDeque { + public ProcedureDeque() { + // Default is 16 for a list that is rarely used; elements will resize if too small. + super(2); + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java index f32930fe4ef..51c2758c6e8 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java @@ -566,7 +566,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer { * and rack have the highest locality for region */ private void computeCachedLocalities() { - rackLocalities = new float[numRegions][numServers]; + rackLocalities = new float[numRegions][numRacks]; regionsToMostLocalEntities = new int[LocalityType.values().length][numRegions]; // Compute localities and find most local server per region diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java index ac5ad64b28d..ba080a32e61 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/StochasticLoadBalancer.java @@ -530,8 +530,7 @@ public class StochasticLoadBalancer extends BaseLoadBalancer { sm.getRegionMetrics().forEach((byte[] regionName, RegionMetrics rm) -> { Deque rLoads = oldLoads.get(Bytes.toString(regionName)); if (rLoads == null) { - // There was nothing there - rLoads = new ArrayDeque<>(); + rLoads = new ArrayDeque<>(numRegionLoadsToRemember + 1); } else if (rLoads.size() >= numRegionLoadsToRemember) { rLoads.remove(); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestServerName.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestServerName.java index 6ed7c6b2628..fcbe9c9e08e 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestServerName.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestServerName.java @@ -20,16 +20,19 @@ package org.apache.hadoop.hbase; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import java.util.HashSet; import java.util.Set; import java.util.regex.Pattern; + import org.apache.hadoop.hbase.testclassification.MiscTests; import org.apache.hadoop.hbase.testclassification.SmallTests; import org.apache.hadoop.hbase.util.Addressing; import org.apache.hadoop.hbase.util.Bytes; import org.junit.ClassRule; +import org.junit.Ignore; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -54,7 +57,7 @@ public class TestServerName { @Test public void testGetHostNameMinusDomain() { assertEquals("2607:f0d0:1002:51::4", - ServerName.getHostNameMinusDomain("2607:f0d0:1002:51::4")); + ServerName.getHostNameMinusDomain("2607:f0d0:1002:51::4")); assertEquals("2607:f0d0:1002:0051:0000:0000:0000:0004", ServerName.getHostNameMinusDomain("2607:f0d0:1002:0051:0000:0000:0000:0004")); assertEquals("1.1.1.1", ServerName.getHostNameMinusDomain("1.1.1.1")); @@ -86,10 +89,11 @@ public class TestServerName { ServerName.parseServerName("192.168.1.199:58102"); } - @Test public void testParseOfBytes() { + @Test + public void testParseOfBytes() { final String snStr = "www.EXAMPLE.org,1234,5678"; ServerName sn = ServerName.valueOf(snStr); - byte [] versionedBytes = sn.getVersionedBytes(); + byte[] versionedBytes = sn.getVersionedBytes(); ServerName parsedSn = ServerName.parseVersionedServerName(versionedBytes); assertEquals(sn.toString(), parsedSn.toString()); assertEquals(sn.getHostnameLowerCase(), parsedSn.getHostnameLowerCase()); @@ -97,7 +101,7 @@ public class TestServerName { assertEquals(sn.getStartcode(), parsedSn.getStartcode()); final String hostnamePortStr = sn.getAddress().toString(); - byte [] bytes = Bytes.toBytes(hostnamePortStr); + byte[] bytes = Bytes.toBytes(hostnamePortStr); parsedSn = ServerName.parseVersionedServerName(bytes); assertEquals(sn.getHostnameLowerCase(), parsedSn.getHostnameLowerCase()); assertEquals(sn.getPort(), parsedSn.getPort()); @@ -114,9 +118,9 @@ public class TestServerName { assertEquals(sn.hashCode(), sn2.hashCode()); assertNotSame(sn.hashCode(), sn3.hashCode()); assertEquals(sn.toString(), - ServerName.valueOf("www.example.org", 1234, 5678).toString()); + ServerName.valueOf("www.example.org", 1234, 5678).toString()); assertEquals(sn.toString(), - ServerName.valueOf("www.example.org:1234", 5678).toString()); + ServerName.valueOf("www.example.org:1234", 5678).toString()); assertEquals("www.example.org" + ServerName.SERVERNAME_SEPARATOR + "1234" + ServerName.SERVERNAME_SEPARATOR + "5678", sn.toString()); } @@ -132,5 +136,19 @@ public class TestServerName { assertTrue(upper.equals(lower)); assertTrue(ServerName.isSameAddress(lower, upper)); } + + @Test + public void testInterning() { + ServerName sn1 = ServerName.valueOf("www.example.org", 1234, 5671); + assertSame(sn1, ServerName.valueOf("www.example.org", 1234, 5671)); + } + + @Ignore // Enable and let fun for hours to make sure weak references working fine. + @Test + public void testInterningDoesWeakReferences() { + for (int i = 0; i < Integer.MAX_VALUE; i++) { + ServerName.valueOf("www.example.org", 1234, i++); + } + } }