From cc4dad216b111f4961ab02660f9dd17415342378 Mon Sep 17 00:00:00 2001 From: Emre Berk Kaya <75899391+emrberk@users.noreply.github.com> Date: Fri, 19 Apr 2024 21:32:52 +0300 Subject: [PATCH] Remove clientStats from MemberState (#1253) `clientStats` string in `MemberState` does not provide any additional information other than `enterprise` and `clusterConnectionTimestamp` compared to `ClientEndpointDTO`. In MC side, we parse this long string in every `MemberState` retrieval, and this adds additional overhead to our `TimedMemberState` consumption. For metrics processing, `ClientStats` does not provide additional information as well, because we consume these metrics directly from the member. For this purpose, this PR introduces `enterprise` and `clusterConnectionTimestamp` fields in `ClientStatsDTO`, removes `clientStats` from `TimedMemberState`, and adds connection start timestamp to `Connection` (we had it only in `ClientConnection`, and now it's extended to `ServerConnection` as well). Fixes https://hazelcast.atlassian.net/browse/MC-2620 Breaking changes (list specific methods/types/messages): * API * TimedMemberState * Connection Checklist: - [X] Labels (`Team:`, `Type:`, `Source:`, `Module:`) and Milestone set - [X] Add `Add to Release Notes` label if changes should be mentioned in release notes or `Not Release Notes content` if changes are not relevant for release notes - [X] Request reviewers if possible - [ ] New public APIs have `@Nonnull/@Nullable` annotations - [ ] New public APIs have `@since` tags in Javadoc - [ ] Send backports/forwardports if fix needs to be applied to past/future releases GitOrigin-RevId: 9e30d555e2d7c1250a3b42cd22090aa46daf6448 --- .../hazelcast/client/impl/ClientEndpoint.java | 12 +++++++++ .../client/impl/ClientEndpointImpl.java | 25 ++++++++++++++++++- .../TpcChannelClientConnectionAdapter.java | 6 +++++ .../management/TimedMemberStateFactory.java | 17 ------------- .../management/dto/ClientEndPointDTO.java | 14 +++++++++++ .../monitor/impl/MemberStateImpl.java | 20 +-------------- .../hazelcast/internal/nio/Connection.java | 7 ++++++ .../server/tcp/TcpServerConnection.java | 6 +++++ .../monitor/impl/MemberStateImplTest.java | 14 +++++------ .../server/DroppingServerConnection.java | 5 ++++ .../internal/server/FirewallingServer.java | 5 ++++ .../mocknetwork/MockServerConnection.java | 7 ++++++ 12 files changed, 94 insertions(+), 44 deletions(-) diff --git a/hazelcast/src/main/java/com/hazelcast/client/impl/ClientEndpoint.java b/hazelcast/src/main/java/com/hazelcast/client/impl/ClientEndpoint.java index 3a38894972b4..8a739ab7f424 100644 --- a/hazelcast/src/main/java/com/hazelcast/client/impl/ClientEndpoint.java +++ b/hazelcast/src/main/java/com/hazelcast/client/impl/ClientEndpoint.java @@ -82,6 +82,8 @@ public interface ClientEndpoint extends Client, DynamicMetricsProvider { ServerConnection getConnection(); + long getConnectionStartTime(); + void setLoginContext(LoginContext lc); void authenticated(UUID clientUuid, Credentials credentials, String clientVersion, @@ -102,6 +104,16 @@ void authenticated(UUID clientUuid, Credentials credentials, String clientVersio */ void setClientVersion(String version); + /** + * @return true if the client uses the enterprise build of Hazelcast + */ + boolean isEnterprise(); + + /** + * @param enterprise indicates whether the client uses the enterprise build or not + */ + void setEnterprise(Boolean enterprise); + /** * Updates to the latest client statistics. * diff --git a/hazelcast/src/main/java/com/hazelcast/client/impl/ClientEndpointImpl.java b/hazelcast/src/main/java/com/hazelcast/client/impl/ClientEndpointImpl.java index b1904c702021..693744c17f84 100644 --- a/hazelcast/src/main/java/com/hazelcast/client/impl/ClientEndpointImpl.java +++ b/hazelcast/src/main/java/com/hazelcast/client/impl/ClientEndpointImpl.java @@ -65,12 +65,12 @@ public final class ClientEndpointImpl implements ClientEndpoint { private final ConcurrentMap removeListenerActions = new ConcurrentHashMap<>(); private final SocketAddress socketAddress; private final long creationTime; - private LoginContext loginContext; private UUID clientUuid; private Credentials credentials; private volatile boolean authenticated; private String clientVersion; + private Boolean enterprise; private final AtomicReference statsRef = new AtomicReference<>(); private String clientName; private Set labels; @@ -84,6 +84,7 @@ public ClientEndpointImpl(ClientEngine clientEngine, NodeEngine nodeEngine, Serv this.connection = connection; this.socketAddress = connection.getRemoteSocketAddress(); this.clientVersion = "Unknown"; + this.enterprise = false; this.creationTime = System.currentTimeMillis(); } @@ -92,6 +93,11 @@ public ServerConnection getConnection() { return connection; } + @Override + public long getConnectionStartTime() { + return connection.getStartTime(); + } + @Override public UUID getUuid() { return clientUuid; @@ -139,8 +145,25 @@ public void setClientVersion(String version) { clientVersion = version; } + @Override + public boolean isEnterprise() { + return enterprise; + } + + @Override + public void setEnterprise(Boolean enterprise) { + this.enterprise = enterprise; + } + @Override public void setClientStatistics(ClientStatistics stats) { + boolean setBefore = getClientStatistics() != null; + if (!setBefore) { + String clientAttributes = stats.clientAttributes(); + if (clientAttributes.contains("enterprise=true")) { + setEnterprise(true); + } + } statsRef.set(stats); } diff --git a/hazelcast/src/main/java/com/hazelcast/client/impl/connection/tcp/TpcChannelClientConnectionAdapter.java b/hazelcast/src/main/java/com/hazelcast/client/impl/connection/tcp/TpcChannelClientConnectionAdapter.java index 420254d7a145..ed823556ea78 100644 --- a/hazelcast/src/main/java/com/hazelcast/client/impl/connection/tcp/TpcChannelClientConnectionAdapter.java +++ b/hazelcast/src/main/java/com/hazelcast/client/impl/connection/tcp/TpcChannelClientConnectionAdapter.java @@ -32,6 +32,7 @@ public class TpcChannelClientConnectionAdapter implements ClientConnection { + private final long startTime = System.currentTimeMillis(); private final Channel channel; public TpcChannelClientConnectionAdapter(Channel channel) { @@ -119,6 +120,11 @@ public long lastWriteTimeMillis() { throw new UnsupportedOperationException("Not supported for TPC channels"); } + @Override + public long getStartTime() { + return startTime; + } + @Nullable @Override public InetSocketAddress getRemoteSocketAddress() { diff --git a/hazelcast/src/main/java/com/hazelcast/internal/management/TimedMemberStateFactory.java b/hazelcast/src/main/java/com/hazelcast/internal/management/TimedMemberStateFactory.java index f44814e7072a..27ba81a9089e 100644 --- a/hazelcast/src/main/java/com/hazelcast/internal/management/TimedMemberStateFactory.java +++ b/hazelcast/src/main/java/com/hazelcast/internal/management/TimedMemberStateFactory.java @@ -18,7 +18,6 @@ import com.hazelcast.cache.impl.CacheService; import com.hazelcast.client.impl.ClientEndpoint; -import com.hazelcast.client.impl.statistics.ClientStatistics; import com.hazelcast.cluster.Address; import com.hazelcast.cluster.Member; import com.hazelcast.cluster.impl.MemberImpl; @@ -73,11 +72,9 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.UUID; import java.util.concurrent.TimeUnit; import static com.hazelcast.config.ConfigAccessor.getActiveMemberNetworkConfig; -import static com.hazelcast.internal.util.MapUtil.createHashMap; import static com.hazelcast.internal.util.SetUtil.createHashSet; /** @@ -183,26 +180,12 @@ private void createMemberState(MemberStateImpl memberState, createNodeState(memberState); createHotRestartState(memberState); createClusterHotRestartStatus(memberState); - - memberState.setClientStats(getClientAttributes(node.getClientEngine().getClientStatistics())); } protected void setCPMemberUuid(MemberStateImpl memberState) { memberState.setCpMemberUuid(null); } - private Map getClientAttributes(Map allClientStatistics) { - Map statsMap = createHashMap(allClientStatistics.size()); - for (Map.Entry entry : allClientStatistics.entrySet()) { - UUID uuid = entry.getKey(); - ClientStatistics statistics = entry.getValue(); - if (statistics != null) { - statsMap.put(uuid, statistics.clientAttributes()); - } - } - return statsMap; - } - private void createHotRestartState(MemberStateImpl memberState) { final HotRestartService hotRestartService = instance.node.getNodeExtension().getHotRestartService(); boolean hotBackupEnabled = hotRestartService.isHotBackupEnabled(); diff --git a/hazelcast/src/main/java/com/hazelcast/internal/management/dto/ClientEndPointDTO.java b/hazelcast/src/main/java/com/hazelcast/internal/management/dto/ClientEndPointDTO.java index edb38d497dab..78cc75eb7e46 100644 --- a/hazelcast/src/main/java/com/hazelcast/internal/management/dto/ClientEndPointDTO.java +++ b/hazelcast/src/main/java/com/hazelcast/internal/management/dto/ClientEndPointDTO.java @@ -30,6 +30,8 @@ import java.util.UUID; import static com.hazelcast.internal.util.JsonUtil.getArray; +import static com.hazelcast.internal.util.JsonUtil.getBoolean; +import static com.hazelcast.internal.util.JsonUtil.getLong; import static com.hazelcast.internal.util.JsonUtil.getString; /** @@ -46,7 +48,10 @@ public class ClientEndPointDTO implements JsonSerializable { public String address; public String clientType; public String clientVersion; + public boolean enterprise; + public boolean statsEnabled; public String name; + public long clusterConnectionTimestamp; public Set labels; /** @@ -66,7 +71,10 @@ public ClientEndPointDTO(ClientEndpoint clientEndpoint) { this.uuid = clientEndpoint.getUuid(); this.clientType = clientEndpoint.getClientType(); this.clientVersion = clientEndpoint.getClientVersion(); + this.enterprise = clientEndpoint.isEnterprise(); + this.statsEnabled = clientEndpoint.getClientStatistics() != null; this.name = clientEndpoint.getName(); + this.clusterConnectionTimestamp = clientEndpoint.getConnectionStartTime(); this.labels = clientEndpoint.getLabels(); InetSocketAddress socketAddress = clientEndpoint.getSocketAddress(); @@ -84,11 +92,14 @@ public JsonObject toJson() { root.add("address", address); root.add("clientType", clientType); root.add("clientVersion", clientVersion); + root.add("enterprise", enterprise); + root.add("statsEnabled", statsEnabled); root.add("name", name); JsonArray labelsObject = Json.array(); for (String label : labels) { labelsObject.add(label); } + root.add("clusterConnectionTimestamp", clusterConnectionTimestamp); root.add("labels", labelsObject); root.add("ipAddress", ipAddress); root.add("canonicalHostName", canonicalHostName); @@ -101,7 +112,10 @@ public void fromJson(JsonObject json) { address = getString(json, "address"); clientType = getString(json, "clientType"); clientVersion = getString(json, "clientVersion"); + enterprise = getBoolean(json, "enterprise"); + statsEnabled = getBoolean(json, "statsEnabled"); name = getString(json, "name"); + clusterConnectionTimestamp = getLong(json, "clusterConnectionTimestamp"); JsonArray labelsArray = getArray(json, "labels"); labels = new HashSet<>(); for (JsonValue labelValue : labelsArray) { diff --git a/hazelcast/src/main/java/com/hazelcast/internal/monitor/impl/MemberStateImpl.java b/hazelcast/src/main/java/com/hazelcast/internal/monitor/impl/MemberStateImpl.java index 3a1a52754461..40ddf6499c65 100644 --- a/hazelcast/src/main/java/com/hazelcast/internal/monitor/impl/MemberStateImpl.java +++ b/hazelcast/src/main/java/com/hazelcast/internal/monitor/impl/MemberStateImpl.java @@ -73,7 +73,6 @@ public class MemberStateImpl implements MemberState { private Set flakeIdGeneratorsWithStats = emptySet(); private Set userCodeNamespacesWithStats = emptySet(); private Collection clients = emptySet(); - private Map clientStats = emptyMap(); private MemberPartitionState memberPartitionState = new MemberPartitionStateImpl(); private LocalOperationStats operationStats = new LocalOperationStatsImpl(); private NodeState nodeState = new NodeStateImpl(); @@ -277,14 +276,6 @@ public void setClusterHotRestartStatus(ClusterHotRestartStatusDTO clusterHotRest this.clusterHotRestartStatus = clusterHotRestartStatus; } - public Map getClientStats() { - return clientStats; - } - - public void setClientStats(Map clientStats) { - this.clientStats = clientStats; - } - @Override public JsonObject toJson() { final JsonObject root = new JsonObject(); @@ -339,11 +330,6 @@ public JsonObject toJson() { root.add("hotRestartState", hotRestartState.toJson()); root.add("clusterHotRestartStatus", clusterHotRestartStatus.toJson()); - JsonObject clientStatsObject = new JsonObject(); - for (Map.Entry entry : clientStats.entrySet()) { - clientStatsObject.add(entry.getKey().toString(), entry.getValue()); - } - root.add("clientStats", clientStatsObject); return root; } @@ -509,10 +495,6 @@ public void fromJson(JsonObject json) { clusterHotRestartStatus = new ClusterHotRestartStatusDTO(); clusterHotRestartStatus.fromJson(jsonClusterHotRestartStatus); } - clientStats = new HashMap<>(); - for (JsonObject.Member next : getObject(json, "clientStats")) { - clientStats.put(UUID.fromString(next.getName()), next.getValue().asString()); - } } @Override @@ -522,6 +504,7 @@ public String toString() { + ", uuid=" + uuid + ", cpMemberUuid=" + cpMemberUuid + ", name=" + name + + ", clients=" + clients + ", mapsWithStats=" + mapsWithStats + ", multiMapsWithStats=" + multiMapsWithStats + ", replicatedMapsWithStats=" + replicatedMapsWithStats @@ -541,7 +524,6 @@ public String toString() { + ", nodeState=" + nodeState + ", hotRestartState=" + hotRestartState + ", clusterHotRestartStatus=" + clusterHotRestartStatus - + ", clientStats=" + clientStats + '}'; } } diff --git a/hazelcast/src/main/java/com/hazelcast/internal/nio/Connection.java b/hazelcast/src/main/java/com/hazelcast/internal/nio/Connection.java index ddb0bcd9aa9a..9fe488f65814 100644 --- a/hazelcast/src/main/java/com/hazelcast/internal/nio/Connection.java +++ b/hazelcast/src/main/java/com/hazelcast/internal/nio/Connection.java @@ -70,6 +70,13 @@ public interface Connection { */ long lastWriteTimeMillis(); + /** + * Returns the clock time in milliseconds of the initialization of this connection. + * + * @return the clock time of the initialization of this connection. + */ + long getStartTime(); + /** * Returns the address of the endpoint this Connection is connected to, or * null if it is unconnected. diff --git a/hazelcast/src/main/java/com/hazelcast/internal/server/tcp/TcpServerConnection.java b/hazelcast/src/main/java/com/hazelcast/internal/server/tcp/TcpServerConnection.java index 984b66f9ad02..50a08ac54ffa 100644 --- a/hazelcast/src/main/java/com/hazelcast/internal/server/tcp/TcpServerConnection.java +++ b/hazelcast/src/main/java/com/hazelcast/internal/server/tcp/TcpServerConnection.java @@ -71,6 +71,8 @@ public class TcpServerConnection implements ServerConnection { // indicate whether connection handshake is in progress/done (true) or not yet initiated (when false) private final AtomicBoolean handshake = new AtomicBoolean(); + private final long startTime = System.currentTimeMillis(); + private final ILogger logger; // Flag that indicates if the connection is accepted on this member (server-side) @@ -162,6 +164,10 @@ public void setConnectionType(String connectionType) { } } + public long getStartTime() { + return startTime; + } + public TcpServerConnectionManager getConnectionManager() { return connectionManager; } diff --git a/hazelcast/src/test/java/com/hazelcast/internal/monitor/impl/MemberStateImplTest.java b/hazelcast/src/test/java/com/hazelcast/internal/monitor/impl/MemberStateImplTest.java index ff22dad0a306..282a7f861441 100644 --- a/hazelcast/src/test/java/com/hazelcast/internal/monitor/impl/MemberStateImplTest.java +++ b/hazelcast/src/test/java/com/hazelcast/internal/monitor/impl/MemberStateImplTest.java @@ -78,6 +78,7 @@ public void testSerialization() CacheStatisticsImpl cacheStatistics = new CacheStatisticsImpl(Clock.currentTimeMillis()); cacheStatistics.increaseCacheHits(5); UUID clientUuid = UUID.randomUUID(); + long connectionTimestamp = System.currentTimeMillis(); Collection clients = new ArrayList<>(); ClientEndPointDTO client = new ClientEndPointDTO(); @@ -85,7 +86,10 @@ public void testSerialization() client.address = "localhost"; client.clientType = "undefined"; client.clientVersion = "5.2"; + client.enterprise = true; + client.statsEnabled = true; client.name = "aClient"; + client.clusterConnectionTimestamp = connectionTimestamp; client.labels = new HashSet<>(Collections.singletonList("label")); client.ipAddress = "10.176.167.34"; client.canonicalHostName = "ip-10-176-167-34.ec2.internal"; @@ -100,9 +104,6 @@ public void testSerialization() final String backupDirectory = "/hot/backup/dir"; final HotRestartStateImpl hotRestartState = new HotRestartStateImpl(backupTaskStatus, true, backupDirectory); - Map clientStats = new HashMap<>(); - clientStats.put(clientUuid, "someStats"); - Map endpoints = new HashMap<>(); endpoints.put(EndpointQualifier.MEMBER, new Address("127.0.0.1", 5701)); endpoints.put(EndpointQualifier.resolve(ProtocolType.WAN, "MyWAN"), new Address("127.0.0.1", 5901)); @@ -132,7 +133,6 @@ public void testSerialization() memberState.setClients(clients); memberState.setNodeState(state); memberState.setHotRestartState(hotRestartState); - memberState.setClientStats(clientStats); MemberStateImpl deserialized = new MemberStateImpl(); deserialized.fromJson(memberState.toJson()); @@ -162,7 +162,10 @@ public void testSerialization() assertEquals(clientUuid, client.uuid); assertEquals("localhost", client.address); assertEquals("undefined", client.clientType); + assertTrue(client.enterprise); + assertTrue(client.statsEnabled); assertEquals("aClient", client.name); + assertEquals(connectionTimestamp, client.clusterConnectionTimestamp); assertContains(client.labels, "label"); assertEquals("10.176.167.34", client.ipAddress); assertEquals("ip-10-176-167-34.ec2.internal", client.canonicalHostName); @@ -184,8 +187,5 @@ public void testSerialization() assertEquals(-1, clusterHotRestartStatus.getRemainingValidationTimeMillis()); assertEquals(-1, clusterHotRestartStatus.getRemainingDataLoadTimeMillis()); assertTrue(clusterHotRestartStatus.getMemberHotRestartStatusMap().isEmpty()); - - Map deserializedClientStats = deserialized.getClientStats(); - assertEquals("someStats", deserializedClientStats.get(clientUuid)); } } diff --git a/hazelcast/src/test/java/com/hazelcast/internal/server/DroppingServerConnection.java b/hazelcast/src/test/java/com/hazelcast/internal/server/DroppingServerConnection.java index f0af0965dd65..452d55f9a442 100644 --- a/hazelcast/src/test/java/com/hazelcast/internal/server/DroppingServerConnection.java +++ b/hazelcast/src/test/java/com/hazelcast/internal/server/DroppingServerConnection.java @@ -93,6 +93,11 @@ public long lastWriteTimeMillis() { return timestamp; } + @Override + public long getStartTime() { + return timestamp; + } + @Override public void close(String msg, Throwable cause) { if (!isAlive.compareAndSet(true, false)) { diff --git a/hazelcast/src/test/java/com/hazelcast/internal/server/FirewallingServer.java b/hazelcast/src/test/java/com/hazelcast/internal/server/FirewallingServer.java index 4b1648cc9cb3..535392c1128b 100644 --- a/hazelcast/src/test/java/com/hazelcast/internal/server/FirewallingServer.java +++ b/hazelcast/src/test/java/com/hazelcast/internal/server/FirewallingServer.java @@ -373,6 +373,11 @@ public long lastWriteTimeMillis() { return delegate.lastWriteTimeMillis(); } + @Override + public long getStartTime() { + return delegate.getStartTime(); + } + @Override public InetSocketAddress getRemoteSocketAddress() { return delegate.getRemoteSocketAddress(); diff --git a/hazelcast/src/test/java/com/hazelcast/test/mocknetwork/MockServerConnection.java b/hazelcast/src/test/java/com/hazelcast/test/mocknetwork/MockServerConnection.java index b725fc0e5ca3..55257b90fb83 100644 --- a/hazelcast/src/test/java/com/hazelcast/test/mocknetwork/MockServerConnection.java +++ b/hazelcast/src/test/java/com/hazelcast/test/mocknetwork/MockServerConnection.java @@ -60,6 +60,8 @@ public class MockServerConnection implements ServerConnection { private final ConcurrentMap attributeMap = new ConcurrentHashMap(); + private final long startTime = System.currentTimeMillis(); + public MockServerConnection( Address localAddress, Address remoteAddress, @@ -177,6 +179,11 @@ public long lastWriteTimeMillis() { return System.currentTimeMillis(); } + @Override + public long getStartTime() { + return startTime; + } + public void close(String msg, Throwable cause) { try { if (!alive.compareAndSet(true, false)) {