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:09 UTC

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

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();