You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by up...@apache.org on 2018/08/02 21:19:07 UTC

[geode] branch develop updated (4abb1d0 -> 555149d)

This is an automated email from the ASF dual-hosted git repository.

upthewaterspout pushed a change to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git.


    from 4abb1d0  GEODE-5513: Changes to build register interest initial snapshot from primary bucket (#2246)
     new cd3783a  GEODE-5493: fix publication of client statistics
     new 555149d  GEODE-5157: use distributedMember as client ID

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 ...niversalMembershipListenerAdapterDUnitTest.java |  13 --
 .../apache/geode/cache/client/internal/PutOp.java  |  34 +----
 .../cache/client/internal/ServerRegionProxy.java   |   5 +-
 .../geode/distributed/ConfigurationProperties.java |   2 +-
 .../geode/internal/admin/ClientStatsManager.java   |  62 +++------
 .../internal/beans/CacheServerBridge.java          |   9 +-
 ...ientStatisticsPublicationSecurityDUnitTest.java | 139 +++++++++++++++++++++
 .../commands/DescribeClientCommandDUnitTest.java   |   5 +-
 8 files changed, 174 insertions(+), 95 deletions(-)
 create mode 100644 geode-cq/src/distributedTest/java/org/apache/geode/cache/ClientStatisticsPublicationSecurityDUnitTest.java


[geode] 01/02: GEODE-5493: fix publication of client statistics

Posted by up...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

upthewaterspout pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git

commit cd3783a6a0c5597ccd6799d3833e864b05719f15
Author: Helena A. Bales <hb...@pivotal.io>
AuthorDate: Fri Jul 27 16:31:07 2018 -0700

    GEODE-5493: fix publication of client statistics
    
    Fix publication of client statistics with security enabled. Remove dead
    code related to retrieving old statistics that never worked. Add test
    for fix.
    
    Signed-off-by: Dan Smith <ds...@pivotal.io>
---
 .../apache/geode/cache/client/internal/PutOp.java  |  34 +----
 .../cache/client/internal/ServerRegionProxy.java   |   5 +-
 .../geode/distributed/ConfigurationProperties.java |   2 +-
 .../geode/internal/admin/ClientStatsManager.java   |  60 +++-----
 ...ientStatisticsPublicationSecurityDUnitTest.java | 154 +++++++++++++++++++++
 .../commands/DescribeClientCommandDUnitTest.java   |   2 +-
 6 files changed, 180 insertions(+), 77 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/PutOp.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/PutOp.java
index 8b0f0c4..e95c0e9 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/PutOp.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/PutOp.java
@@ -90,14 +90,14 @@ public class PutOp {
   }
 
   public static Object execute(ExecutablePool pool, String regionName, Object key, Object value,
-      byte[] deltaBytes, EntryEventImpl event, Operation operation, boolean requireOldValue,
-      Object expectedOldValue, Object callbackArg, boolean prSingleHopEnabled,
-      boolean isMetaRegionPutOp) {
+      byte[] deltaBytes, EntryEventImpl event, Operation operation,
+      boolean requireOldValue,
+      Object expectedOldValue, Object callbackArg,
+      boolean prSingleHopEnabled) {
 
     AbstractOp op = new PutOpImpl(regionName, key, value, deltaBytes, event, operation,
         requireOldValue, expectedOldValue, callbackArg, false/* donot send full obj; send delta */,
         prSingleHopEnabled);
-    ((PutOpImpl) op).setMetaRegionPutOp(isMetaRegionPutOp);
     return pool.execute(op);
   }
 
@@ -151,8 +151,6 @@ public class PutOp {
 
     private Object callbackArg;
 
-    private boolean isMetaRegionPutOp;
-
     private boolean prSingleHopEnabled;
 
     private boolean requireOldValue;
@@ -393,26 +391,6 @@ public class PutOp {
     }
 
     @Override
-    protected void sendMessage(Connection cnx) throws Exception {
-      if (!this.isMetaRegionPutOp) {
-        super.sendMessage(cnx);
-      } else {
-        getMessage().send(false);
-      }
-    }
-
-    @Override
-    protected void processSecureBytes(Connection cnx, Message message) throws Exception {
-      super.processSecureBytes(cnx, message);
-    }
-
-    @Override
-    protected boolean needsUserId() {
-      boolean ret = this.isMetaRegionPutOp ? false : super.needsUserId();
-      return ret;
-    }
-
-    @Override
     protected boolean isErrorResponse(int msgType) {
       return msgType == MessageType.PUT_DATA_ERROR;
     }
@@ -471,10 +449,6 @@ public class PutOp {
         return null;
       }
     }
-
-    void setMetaRegionPutOp(boolean bool) {
-      this.isMetaRegionPutOp = bool;
-    }
   }
 
 }
diff --git a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ServerRegionProxy.java b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ServerRegionProxy.java
index a552a08..e3609f2 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/client/internal/ServerRegionProxy.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/client/internal/ServerRegionProxy.java
@@ -130,8 +130,7 @@ public class ServerRegionProxy extends ServerProxy implements ServerRegionDataAc
       Object callbackArg, boolean isMetaRegionPutOp) {
     if (this.region == null) {
       return PutOp.execute(this.pool, this.regionName, key, value, deltaBytes, event,
-          Operation.CREATE, false, null, callbackArg, this.pool.getPRSingleHopEnabled(),
-          isMetaRegionPutOp);
+          Operation.CREATE, false, null, callbackArg, this.pool.getPRSingleHopEnabled());
     } else {
       return PutOp.execute(this.pool, this.region, key, value, deltaBytes, event, Operation.CREATE,
           false, null, callbackArg, this.pool.getPRSingleHopEnabled());
@@ -150,7 +149,7 @@ public class ServerRegionProxy extends ServerProxy implements ServerRegionDataAc
 
     if (this.region == null) {
       return PutOp.execute(this.pool, this.regionName, key, value, deltaBytes, event, operation,
-          requireOldValue, expectedOldValue, callbackArg, this.pool.getPRSingleHopEnabled(), false);
+          requireOldValue, expectedOldValue, callbackArg, this.pool.getPRSingleHopEnabled());
     } else {
       return PutOp.execute(this.pool, this.region, key, value, deltaBytes, event, operation,
           requireOldValue, expectedOldValue, callbackArg, this.pool.getPRSingleHopEnabled());
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java b/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
index 858b223..089a006 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/ConfigurationProperties.java
@@ -1175,7 +1175,7 @@ public interface ConfigurationProperties {
   /**
    * The static String definition of the <i>"log-file"</i> property <a name="log-file"/a>
    * </p>
-   * <U>Description</U>: Name of the file to write logging messages to. If the file name if ""
+   * <U>Description</U>: Name of the file to write logging messages to. If the file name is ""
    * (default) then messages are written to standard out.
    * </p>
    * <U>Default</U>: ""
diff --git a/geode-core/src/main/java/org/apache/geode/internal/admin/ClientStatsManager.java b/geode-core/src/main/java/org/apache/geode/internal/admin/ClientStatsManager.java
index c333906..2ea963c 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/admin/ClientStatsManager.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/admin/ClientStatsManager.java
@@ -16,25 +16,24 @@ package org.apache.geode.internal.admin;
 
 import java.util.Date;
 import java.util.Map;
-import java.util.Map.Entry;
 
 import org.apache.commons.lang.exception.ExceptionUtils;
+import org.apache.logging.log4j.Logger;
 
 import org.apache.geode.Statistics;
 import org.apache.geode.StatisticsType;
 import org.apache.geode.cache.CacheWriterException;
-import org.apache.geode.cache.Region;
 import org.apache.geode.cache.client.internal.PoolImpl;
 import org.apache.geode.cache.client.internal.ServerRegionProxy;
 import org.apache.geode.distributed.DistributedSystemDisconnectedException;
 import org.apache.geode.distributed.internal.InternalDistributedSystem;
-import org.apache.geode.i18n.LogWriterI18n;
 import org.apache.geode.internal.admin.remote.ClientHealthStats;
 import org.apache.geode.internal.cache.EntryEventImpl;
 import org.apache.geode.internal.cache.EventID;
 import org.apache.geode.internal.cache.GemFireCacheImpl;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.i18n.LocalizedStrings;
+import org.apache.geode.internal.logging.LogService;
 import org.apache.geode.internal.offheap.annotations.Released;
 
 /**
@@ -59,6 +58,8 @@ public class ClientStatsManager {
    */
   private static Statistics vmStats = null;
 
+  private static final Logger logger = LogService.getLogger();
+
   /**
    * This method publishes the client stats using the admin region.
    *
@@ -69,9 +70,9 @@ public class ClientStatsManager {
     if (!initializeStatistics(currentCache)) {
       return; // handles null case too
     }
-    LogWriterI18n logger = currentCache.getLoggerI18n();
-    if (logger.fineEnabled())
-      logger.fine("Entering ClientStatsManager#publishClientStats...");
+    if (logger.isDebugEnabled()) {
+      logger.debug("Entering ClientStatsManager#publishClientStats...");
+    }
 
     ClientHealthStats stats = getClientHealthStats(currentCache, pool);
 
@@ -96,7 +97,7 @@ public class ClientStatsManager {
       pool.getCancelCriterion().checkCancelInProgress(cwx);
       currentCache.getCancelCriterion().checkCancelInProgress(cwx);
       // TODO: Need to analyze these exception scenarios.
-      logger.warning(
+      logger.warn(
           LocalizedStrings.ClientStatsManager_FAILED_TO_SEND_CLIENT_HEALTH_STATS_TO_CACHESERVER,
           cwx);
     } catch (Exception e) {
@@ -105,8 +106,8 @@ public class ClientStatsManager {
       logger.info(LocalizedStrings.ClientStatsManager_FAILED_TO_PUBLISH_CLIENT_STATISTICS, e);
     }
 
-    if (logger.fineEnabled()) {
-      logger.fine("Exiting ClientStatsManager#publishClientStats.");
+    if (logger.isDebugEnabled()) {
+      logger.debug("Exiting ClientStatsManager#publishClientStats.");
     }
   }
 
@@ -125,7 +126,7 @@ public class ClientStatsManager {
     if (currentCache == null) {
       return false;
     }
-    LogWriterI18n logger = currentCache.getLoggerI18n();
+
     InternalDistributedSystem ds = (InternalDistributedSystem) currentCache.getDistributedSystem();
     if (currentCache.isClosed()) {
       return false;
@@ -135,7 +136,7 @@ public class ClientStatsManager {
     lastInitializedCache = currentCache;
 
     if (restart) {
-      if (logger.infoEnabled()) {
+      if (logger.isInfoEnabled()) {
         logger.info(
             LocalizedStrings.ClientStatsManager_CLIENTSTATSMANAGER_INTIALIZING_THE_STATISTICS);
       }
@@ -165,14 +166,14 @@ public class ClientStatsManager {
 
     // Validate that cache has changed before logging the warning, thus logging it once per cache
     if (cachePerfStats == null && restart) {
-      logger.warning(LocalizedStrings.ClientStatsManager_CLIENTSTATSMANAGER_0_ARE_NOT_AVAILABLE,
-          "CachePerfStats");
+      logger.warn(LocalizedStrings.ClientStatsManager_CLIENTSTATSMANAGER_0_ARE_NOT_AVAILABLE
+          .toLocalizedString("CachePerfStats"));
     }
 
     // Validate that cache has changed before logging the warning, thus logging it once per cache
     if (vmStats == null && restart) {
-      logger.warning(LocalizedStrings.ClientStatsManager_CLIENTSTATSMANAGER_0_ARE_NOT_AVAILABLE,
-          "VMStats");
+      logger.warn(LocalizedStrings.ClientStatsManager_CLIENTSTATSMANAGER_0_ARE_NOT_AVAILABLE
+          .toLocalizedString("VMStats"));
     }
 
     return true;
@@ -189,7 +190,6 @@ public class ClientStatsManager {
       return null;
     }
     ClientHealthStats stats = new ClientHealthStats();
-    LogWriterI18n logger = currentCache.getLoggerI18n();
 
     int gets = -1;
     int puts = -1;
@@ -226,36 +226,12 @@ public class ClientStatsManager {
       String poolStatsStr = "MinConnections=" + pool.getMinConnections() + ";MaxConnections="
           + pool.getMaxConnections() + ";Redundancy=" + pool.getSubscriptionRedundancy() + ";CQS="
           + pool.getQueryService().getCqs().length;
-      logger.info(LocalizedStrings.DEBUG,
-          "ClientHealthStats for poolName " + poolName + " poolStatsStr=" + poolStatsStr);
+      logger.debug("ClientHealthStats for poolName " + poolName + " poolStatsStr=" + poolStatsStr);
 
       newPoolStats.put(poolName, poolStatsStr);
 
-      // consider old stats
-      Region clientHealthMonitoringRegion = ClientHealthMonitoringRegion.getInstance(currentCache);
-
-      if (clientHealthMonitoringRegion != null) {
-        InternalDistributedSystem ds =
-            (InternalDistributedSystem) currentCache.getDistributedSystem();
-        ClientHealthStats oldStats =
-            (ClientHealthStats) clientHealthMonitoringRegion.get(ds.getMemberId());
-        logger.info(LocalizedStrings.DEBUG, "getClientHealthStats got oldStats  " + oldStats);
-        if (oldStats != null) {
-          Map<String, String> oldPoolStats = oldStats.getPoolStats();
-          logger.info(LocalizedStrings.DEBUG,
-              "getClientHealthStats got oldPoolStats  " + oldPoolStats);
-          if (oldPoolStats != null) {
-            for (Entry<String, String> entry : oldPoolStats.entrySet()) {
-              if (!poolName.equals(entry.getKey())) {
-                stats.getPoolStats().put(entry.getKey(), entry.getValue());
-              }
-            }
-          }
-        }
-      }
-
     } catch (Exception e) {
-      logger.fine("Exception in getting pool stats in  getClientHealthStats "
+      logger.debug("Exception in getting pool stats in  getClientHealthStats "
           + ExceptionUtils.getStackTrace(e));
     }
 
diff --git a/geode-cq/src/distributedTest/java/org/apache/geode/cache/ClientStatisticsPublicationSecurityDUnitTest.java b/geode-cq/src/distributedTest/java/org/apache/geode/cache/ClientStatisticsPublicationSecurityDUnitTest.java
new file mode 100644
index 0000000..2a66b42
--- /dev/null
+++ b/geode-cq/src/distributedTest/java/org/apache/geode/cache/ClientStatisticsPublicationSecurityDUnitTest.java
@@ -0,0 +1,154 @@
+/*
+ * 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.geode.cache;
+
+import static org.apache.geode.cache.client.ClientRegionShortcut.CACHING_PROXY;
+import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_CLIENT_AUTH_INIT;
+import static org.apache.geode.test.dunit.Invoke.invokeInEveryVM;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Properties;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.logging.log4j.Logger;
+import org.awaitility.Awaitility;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.cache.client.ClientCache;
+import org.apache.geode.cache.client.ClientCacheFactory;
+import org.apache.geode.cache.client.Pool;
+import org.apache.geode.cache.client.PoolFactory;
+import org.apache.geode.cache.client.PoolManager;
+import org.apache.geode.cache.client.internal.PoolImpl;
+import org.apache.geode.distributed.ConfigurationProperties;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.internal.admin.ClientStatsManager;
+import org.apache.geode.internal.cache.InternalCache;
+import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.internal.net.SocketCreator;
+import org.apache.geode.management.CacheServerMXBean;
+import org.apache.geode.management.ClientHealthStatus;
+import org.apache.geode.management.ManagementService;
+import org.apache.geode.management.internal.SystemManagementService;
+import org.apache.geode.management.internal.cli.i18n.CliStrings;
+import org.apache.geode.security.templates.UserPasswordAuthInit;
+import org.apache.geode.test.dunit.rules.ClientVM;
+import org.apache.geode.test.dunit.rules.ClusterStartupRule;
+import org.apache.geode.test.dunit.rules.MemberVM;
+import org.apache.geode.test.junit.categories.ClientServerTest;
+import org.apache.geode.test.junit.rules.GfshCommandRule;
+
+@Category({ClientServerTest.class})
+public class ClientStatisticsPublicationSecurityDUnitTest {
+  private static MemberVM locator, server;
+
+  @Rule
+  public final GfshCommandRule gfsh = new GfshCommandRule();
+
+  @Rule
+  public final ClusterStartupRule cluster = new ClusterStartupRule();
+
+  private static final Logger log = LogService.getLogger();
+
+  @Before
+  public void before() {
+    invokeInEveryVM(() -> {
+      // TODO: Remove this once GEODE-5157 is fixed
+      SocketCreator.use_client_host_name = false;
+    });
+
+    Properties locatorProps = new Properties();
+    locatorProps.setProperty(CliStrings.START_LOCATOR__MEMBER_NAME, "locator1");
+    locatorProps.setProperty("security-manager", "org.apache.geode.examples.SimpleSecurityManager");
+    locator = cluster.startLocatorVM(0, locatorProps);
+
+    Properties serverProps = new Properties();
+    serverProps.setProperty(CliStrings.START_SERVER__NAME, "server1");
+    serverProps.setProperty(CliStrings.START_SERVER__LOCATORS,
+        "localhost[" + locator.getPort() + "]");
+    serverProps.setProperty("security-manager", "org.apache.geode.examples.SimpleSecurityManager");
+    serverProps.setProperty("security-username", "cluster");
+    serverProps.setProperty("security-password", "cluster");
+    server = cluster.startServerVM(1, serverProps);
+    server.invoke(() -> {
+      InternalCache cache = InternalDistributedSystem.getConnectedInstance().getCache();
+
+      cache.createRegionFactory(RegionShortcut.REPLICATE).create("regionName");
+    });
+    locator.waitUntilRegionIsReadyOnExactlyThisManyServers("/regionName", 1);
+  }
+
+  @Test
+  public void testClientCanPublishStatisticsWithSecurity() throws Exception {
+    final String regionName = "regionName";
+
+    // TODO: Remove this once GEODE-5157 is fixed
+    SocketCreator.use_client_host_name = false;
+
+    ClientCache client = new ClientCacheFactory()
+        .set(ConfigurationProperties.LOG_LEVEL, "fine")
+        .set(ConfigurationProperties.LOG_FILE, "")
+        .set("security-manager", "org.apache.geode.examples.SimpleSecurityManager")
+        .set("security-username", "data")
+        .set("security-password", "data")
+        .set(SECURITY_CLIENT_AUTH_INIT, UserPasswordAuthInit.class.getName())
+        .create();
+    PoolFactory poolFactory = PoolManager.createFactory();
+    poolFactory.addServer("localhost", server.getPort());
+    poolFactory.setStatisticInterval(1000);
+    poolFactory.setSubscriptionEnabled(true);
+    Pool pool = poolFactory.create("poolName");
+
+
+    Region region =
+        client.createClientRegionFactory(CACHING_PROXY).setPoolName("poolName").create(regionName);
+
+    checkClientHealthStatus(0);
+
+    region.put("key", "value");
+
+    ClientStatsManager.publishClientStats((PoolImpl) pool);
+
+    checkClientHealthStatus(1, "poolName");
+
+    client.close(false);
+  }
+
+  private void checkClientHealthStatus(int expectedNumPuts,
+      String... expectedPoolStatKeys) {
+    final int serverPort = server.getPort();
+    server.invoke(() -> {
+      Awaitility.waitAtMost(1, TimeUnit.MINUTES).until(() -> {
+        Cache cache = ClusterStartupRule.getCache();
+        SystemManagementService service =
+            (SystemManagementService) ManagementService.getExistingManagementService(cache);
+        CacheServerMXBean serviceMBean = service.getJMXAdapter().getClientServiceMXBean(serverPort);
+        try {
+          String clientId = serviceMBean.getClientIds()[0];
+          ClientHealthStatus status = serviceMBean.showClientStats(clientId);
+          assertThat(status.getNumOfPuts()).isEqualTo(expectedNumPuts);
+          assertThat(status.getPoolStats().keySet())
+              .containsExactlyInAnyOrder(expectedPoolStatKeys);
+        } catch (Exception e) {
+          throw new RuntimeException(e);
+        }
+
+      });
+    });
+  }
+}
diff --git a/geode-cq/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DescribeClientCommandDUnitTest.java b/geode-cq/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DescribeClientCommandDUnitTest.java
index fb8d45f..fdf9a6c 100644
--- a/geode-cq/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DescribeClientCommandDUnitTest.java
+++ b/geode-cq/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DescribeClientCommandDUnitTest.java
@@ -151,7 +151,7 @@ public class DescribeClientCommandDUnitTest {
     }
 
     assertThat(data.get(CliStrings.DESCRIBE_CLIENT_COLUMN_PUTS)).isEqualTo("2");
-    assertThat(data.get(CliStrings.DESCRIBE_CLIENT_COLUMN_LISTENER_CALLS)).isEqualTo("1");
+    assertThat(data.get(CliStrings.DESCRIBE_CLIENT_COLUMN_LISTENER_CALLS)).isEqualTo("0");
     assertThat(data.get(CliStrings.DESCRIBE_CLIENT_COLUMN_DURABLE)).isEqualTo("No");
     assertThat(Integer.parseInt(data.get(CliStrings.DESCRIBE_CLIENT_COLUMN_THREADS)))
         .isGreaterThan(0);


[geode] 02/02: GEODE-5157: use distributedMember as client ID

Posted by up...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

upthewaterspout pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git

commit 555149dc9f6c9c283e3eac96d3656842ee188723
Author: Helena A. Bales <hb...@pivotal.io>
AuthorDate: Wed Aug 1 15:36:03 2018 -0700

    GEODE-5157: use distributedMember as client ID
    
    Changed the client to put stats information in the monitoring region
    using the DistributedMember instead of the toString of
    DistributedMember. On the server, if that fails, the toString of
    DistributedMember is used, for backward compatibility.
    
    This closes #2218
---
 .../UniversalMembershipListenerAdapterDUnitTest.java    | 13 -------------
 .../apache/geode/internal/admin/ClientStatsManager.java |  2 +-
 .../management/internal/beans/CacheServerBridge.java    |  9 ++++++++-
 .../ClientStatisticsPublicationSecurityDUnitTest.java   | 17 +----------------
 .../cli/commands/DescribeClientCommandDUnitTest.java    |  3 ---
 5 files changed, 10 insertions(+), 34 deletions(-)

diff --git a/geode-core/src/distributedTest/java/org/apache/geode/management/UniversalMembershipListenerAdapterDUnitTest.java b/geode-core/src/distributedTest/java/org/apache/geode/management/UniversalMembershipListenerAdapterDUnitTest.java
index 43ba849..f3c5040 100644
--- a/geode-core/src/distributedTest/java/org/apache/geode/management/UniversalMembershipListenerAdapterDUnitTest.java
+++ b/geode-core/src/distributedTest/java/org/apache/geode/management/UniversalMembershipListenerAdapterDUnitTest.java
@@ -27,7 +27,6 @@ import static org.apache.geode.distributed.internal.DistributionConfig.RESTRICT_
 import static org.apache.geode.internal.AvailablePort.SOCKET;
 import static org.apache.geode.internal.AvailablePort.getRandomAvailablePort;
 import static org.apache.geode.test.dunit.IgnoredException.addIgnoredException;
-import static org.apache.geode.test.dunit.Invoke.invokeInEveryVM;
 import static org.apache.geode.test.dunit.NetworkUtils.getServerHostName;
 import static org.apache.geode.test.dunit.Wait.pause;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -46,7 +45,6 @@ import java.util.concurrent.atomic.AtomicReference;
 
 import org.awaitility.core.ConditionTimeoutException;
 import org.junit.After;
-import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 
@@ -65,7 +63,6 @@ import org.apache.geode.distributed.Role;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.cache.tier.InternalClientMembership;
 import org.apache.geode.internal.cache.tier.sockets.ServerConnection;
-import org.apache.geode.internal.net.SocketCreator;
 import org.apache.geode.management.membership.ClientMembership;
 import org.apache.geode.management.membership.ClientMembershipEvent;
 import org.apache.geode.management.membership.ClientMembershipListener;
@@ -120,16 +117,6 @@ public class UniversalMembershipListenerAdapterDUnitTest extends ClientServerTes
   public DistributedRestoreSystemProperties distributedRestoreSystemProperties =
       new DistributedRestoreSystemProperties();
 
-  @Before
-  public void setUp() throws Exception {
-    SocketCreator.resolve_dns = false;
-    SocketCreator.use_client_host_name = false;
-    invokeInEveryVM(() -> {
-      SocketCreator.resolve_dns = false;
-      SocketCreator.use_client_host_name = false;
-    });
-  }
-
   @After
   public void tearDown() throws Exception {
     disconnectAllFromDS();
diff --git a/geode-core/src/main/java/org/apache/geode/internal/admin/ClientStatsManager.java b/geode-core/src/main/java/org/apache/geode/internal/admin/ClientStatsManager.java
index 2ea963c..e827127 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/admin/ClientStatsManager.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/admin/ClientStatsManager.java
@@ -87,7 +87,7 @@ public class ClientStatsManager {
       EntryEventImpl event = new EntryEventImpl((Object) null);
       try {
         event.setEventId(eventId);
-        regionProxy.putForMetaRegion(ds.getMemberId(), stats, null, event, null, true);
+        regionProxy.putForMetaRegion(ds.getDistributedMember(), stats, null, event, null, true);
       } finally {
         event.release();
       }
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/beans/CacheServerBridge.java b/geode-core/src/main/java/org/apache/geode/management/internal/beans/CacheServerBridge.java
index 5324d0a..36bc96e 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/beans/CacheServerBridge.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/beans/CacheServerBridge.java
@@ -38,6 +38,7 @@ import org.apache.geode.cache.server.CacheServer;
 import org.apache.geode.cache.server.ServerLoad;
 import org.apache.geode.cache.server.ServerLoadProbe;
 import org.apache.geode.cache.server.internal.ServerMetricsImpl;
+import org.apache.geode.distributed.DistributedMember;
 import org.apache.geode.internal.Version;
 import org.apache.geode.internal.admin.ClientHealthMonitoringRegion;
 import org.apache.geode.internal.admin.remote.ClientHealthStats;
@@ -501,6 +502,7 @@ public class CacheServerBridge extends ServerBridge {
 
     Region clientHealthMonitoringRegion = ClientHealthMonitoringRegion.getInstance(this.cache);
     String clientName = proxyId.getDSMembership();
+    DistributedMember clientMemberId = proxyId.getDistributedMember();
     status.setClientId(connInfo.toString());
     status.setName(clientName);
     status.setHostName(connInfo.getHostName());
@@ -517,7 +519,12 @@ public class CacheServerBridge extends ServerBridge {
       status.setSubscriptionEnabled(false);
     }
 
-    ClientHealthStats stats = (ClientHealthStats) clientHealthMonitoringRegion.get(clientName);
+    ClientHealthStats stats = (ClientHealthStats) clientHealthMonitoringRegion.get(clientMemberId);
+    if (stats == null) {
+      // Clients older than Geode 1.8 put strings in the region, rather than ids
+      // See GEODE-5157
+      stats = (ClientHealthStats) clientHealthMonitoringRegion.get(clientName);
+    }
 
     if (stats != null) {
       status.setCpus(stats.getCpus());
diff --git a/geode-cq/src/distributedTest/java/org/apache/geode/cache/ClientStatisticsPublicationSecurityDUnitTest.java b/geode-cq/src/distributedTest/java/org/apache/geode/cache/ClientStatisticsPublicationSecurityDUnitTest.java
index 2a66b42..3d77702 100644
--- a/geode-cq/src/distributedTest/java/org/apache/geode/cache/ClientStatisticsPublicationSecurityDUnitTest.java
+++ b/geode-cq/src/distributedTest/java/org/apache/geode/cache/ClientStatisticsPublicationSecurityDUnitTest.java
@@ -16,7 +16,6 @@ package org.apache.geode.cache;
 
 import static org.apache.geode.cache.client.ClientRegionShortcut.CACHING_PROXY;
 import static org.apache.geode.distributed.ConfigurationProperties.SECURITY_CLIENT_AUTH_INIT;
-import static org.apache.geode.test.dunit.Invoke.invokeInEveryVM;
 import static org.assertj.core.api.Assertions.assertThat;
 
 import java.util.Properties;
@@ -34,20 +33,16 @@ import org.apache.geode.cache.client.ClientCacheFactory;
 import org.apache.geode.cache.client.Pool;
 import org.apache.geode.cache.client.PoolFactory;
 import org.apache.geode.cache.client.PoolManager;
-import org.apache.geode.cache.client.internal.PoolImpl;
 import org.apache.geode.distributed.ConfigurationProperties;
 import org.apache.geode.distributed.internal.InternalDistributedSystem;
-import org.apache.geode.internal.admin.ClientStatsManager;
 import org.apache.geode.internal.cache.InternalCache;
 import org.apache.geode.internal.logging.LogService;
-import org.apache.geode.internal.net.SocketCreator;
 import org.apache.geode.management.CacheServerMXBean;
 import org.apache.geode.management.ClientHealthStatus;
 import org.apache.geode.management.ManagementService;
 import org.apache.geode.management.internal.SystemManagementService;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
 import org.apache.geode.security.templates.UserPasswordAuthInit;
-import org.apache.geode.test.dunit.rules.ClientVM;
 import org.apache.geode.test.dunit.rules.ClusterStartupRule;
 import org.apache.geode.test.dunit.rules.MemberVM;
 import org.apache.geode.test.junit.categories.ClientServerTest;
@@ -67,11 +62,6 @@ public class ClientStatisticsPublicationSecurityDUnitTest {
 
   @Before
   public void before() {
-    invokeInEveryVM(() -> {
-      // TODO: Remove this once GEODE-5157 is fixed
-      SocketCreator.use_client_host_name = false;
-    });
-
     Properties locatorProps = new Properties();
     locatorProps.setProperty(CliStrings.START_LOCATOR__MEMBER_NAME, "locator1");
     locatorProps.setProperty("security-manager", "org.apache.geode.examples.SimpleSecurityManager");
@@ -97,9 +87,6 @@ public class ClientStatisticsPublicationSecurityDUnitTest {
   public void testClientCanPublishStatisticsWithSecurity() throws Exception {
     final String regionName = "regionName";
 
-    // TODO: Remove this once GEODE-5157 is fixed
-    SocketCreator.use_client_host_name = false;
-
     ClientCache client = new ClientCacheFactory()
         .set(ConfigurationProperties.LOG_LEVEL, "fine")
         .set(ConfigurationProperties.LOG_FILE, "")
@@ -118,12 +105,10 @@ public class ClientStatisticsPublicationSecurityDUnitTest {
     Region region =
         client.createClientRegionFactory(CACHING_PROXY).setPoolName("poolName").create(regionName);
 
-    checkClientHealthStatus(0);
+    checkClientHealthStatus(0, "poolName");
 
     region.put("key", "value");
 
-    ClientStatsManager.publishClientStats((PoolImpl) pool);
-
     checkClientHealthStatus(1, "poolName");
 
     client.close(false);
diff --git a/geode-cq/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DescribeClientCommandDUnitTest.java b/geode-cq/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DescribeClientCommandDUnitTest.java
index fdf9a6c..96554bb 100644
--- a/geode-cq/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DescribeClientCommandDUnitTest.java
+++ b/geode-cq/src/distributedTest/java/org/apache/geode/management/internal/cli/commands/DescribeClientCommandDUnitTest.java
@@ -40,7 +40,6 @@ import org.apache.geode.cache.client.ClientRegionShortcut;
 import org.apache.geode.cache.query.CqAttributesFactory;
 import org.apache.geode.cache.query.QueryService;
 import org.apache.geode.internal.cache.GemFireCacheImpl;
-import org.apache.geode.internal.net.SocketCreator;
 import org.apache.geode.management.cli.Result;
 import org.apache.geode.management.internal.cli.i18n.CliStrings;
 import org.apache.geode.management.internal.cli.result.CommandResult;
@@ -193,8 +192,6 @@ public class DescribeClientCommandDUnitTest {
       cf.setPoolStatisticInterval(100);
       cf.setPoolSubscriptionRedundancy(1);
       cf.setPoolMinConnections(1);
-      // TODO: Remove this once GEODE-5157 is fixed
-      SocketCreator.use_client_host_name = false;
     };
 
     Properties clientProps = new Properties();