Skip to content

Commit

Permalink
Fix flaky ClientIndexStatsTest [HZ-4534] [HZ-4536] (#1009)
Browse files Browse the repository at this point in the history 
Avoid race conditions between async (via events) proxy initialization on
other members than the one who got
`InitializeDistributedObjectOperation` and cluster restart. Those races
are unfortunate but are not tested here.

Fixes HZ-4534, HZ-4536
Fixes: https://github.com/hazelcast/hazelcast-enterprise/issues/7063
Fixes: #26269
Fixes part of:
https://github.com/hazelcast/hazelcast-enterprise/issues/6550#issuecomment-1966769964

---------

Co-authored-by: ihsan demir <ihsan@hazelcast.com>
GitOrigin-RevId: c33a2000c268af7ff2d03f67e3e266ad637af3e6
  • Loading branch information
@k-jamroz @ihsandemir @actions-user
2 people authored and actions-user committed Mar 8, 2024
1 parent 3774050 commit 67e503c
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 13 deletions.
7 changes: 6 additions & 1 deletion ...hazelcast/spi/impl/proxyservice/impl/operations/InitializeDistributedObjectOperation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@
import com.hazelcast.nio.ObjectDataInput;
import com.hazelcast.nio.ObjectDataOutput;
import com.hazelcast.nio.serialization.IdentifiedDataSerializable;
import com.hazelcast.spi.impl.SpiDataSerializerHook;
import com.hazelcast.spi.impl.operationservice.Operation;
import com.hazelcast.spi.impl.proxyservice.ProxyService;
import com.hazelcast.spi.impl.SpiDataSerializerHook;

import java.io.IOException;

Expand Down Expand Up @@ -77,4 +77,9 @@ public int getFactoryId() {
public int getClassId() {
return SpiDataSerializerHook.DIST_OBJECT_INIT;
}

@Override
protected void toString(StringBuilder sb) {
sb.append(", name=").append(name);
}
}
43 changes: 31 additions & 12 deletions hazelcast/src/test/java/com/hazelcast/client/map/ClientIndexStatsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import com.hazelcast.config.IndexConfig;
import com.hazelcast.config.IndexType;
import com.hazelcast.config.MapConfig;
import com.hazelcast.core.DistributedObject;
import com.hazelcast.core.HazelcastInstance;
import com.hazelcast.internal.monitor.impl.LocalMapStatsImpl;
import com.hazelcast.internal.monitor.impl.PartitionedIndexStatsImpl;
Expand Down Expand Up @@ -109,7 +110,6 @@ protected void ensureSafeState() {
waitAllForSafeState(hazelcastFactory.getAllHazelcastInstances());
}

@SuppressWarnings("unchecked")
@Test
@Override
public void testQueryCounting_WhenPartitionPredicateIsUsed() {
Expand Down Expand Up @@ -143,34 +143,31 @@ public void shouldUseIndexFromClient_whenMemberProxyExists() {
@Test
@Ignore("HZ-4455")
public void shouldUseIndexFromClient_whenMemberProxyDestroyed() {
testIndexWithoutMapProxy((mapName) -> {
member1.getMap(mapName).destroy();
});
testIndexWithoutMapProxy((mapName) -> member1.getMap(mapName).destroy());
}

@Test
public void shouldUseIndexFromClient_whenClusterRestarted() {
warmUpPartitions(member1, member2);
testIndexWithoutMapProxy((mapName) -> restartCluster(true, ClusterState.ACTIVE));
testIndexWithoutMapProxy((mapName) -> restartCluster(true, ClusterState.ACTIVE, mapName));
}

@Test
public void shouldUseIndexFromClient_whenClusterRestartedForcefully() {
warmUpPartitions(member1, member2);
testIndexWithoutMapProxy((mapName) -> restartCluster(false, ClusterState.ACTIVE));
testIndexWithoutMapProxy((mapName) -> restartCluster(false, ClusterState.ACTIVE, mapName));
}

@Ignore("https://github.com/hazelcast/hazelcast-enterprise/issues/7063")
@Test
public void shouldUseIndexFromClient_whenClusterRestartedInPassiveState() {
warmUpPartitions(member1, member2);
testIndexWithoutMapProxy((mapName) -> restartCluster(true, ClusterState.PASSIVE));
testIndexWithoutMapProxy((mapName) -> restartCluster(true, ClusterState.PASSIVE, mapName));
}

@Test
public void shouldUseIndexFromClient_whenClusterRestartedInFrozenState() {
warmUpPartitions(member1, member2);
testIndexWithoutMapProxy((mapName) -> restartCluster(true, ClusterState.FROZEN));
testIndexWithoutMapProxy((mapName) -> restartCluster(true, ClusterState.FROZEN, mapName));
}

private void testIndexWithoutMapProxy(Consumer<String> actionBeforeTest) {
Expand Down Expand Up @@ -205,12 +202,34 @@ private void testIndexWithoutMapProxy(Consumer<String> actionBeforeTest) {
assertThat(stats().getIndexedQueryCount()).isEqualTo(100);
}

private void restartCluster(boolean graceful, ClusterState restartInState) {
private void assertMapProxyInitializedEventually(String indexMapName) {
assertTrueEventually(() -> {
// In some circumstances getDistributedObjects may not return all objects,
// but that should not happen in this test.
assertThat(member1.getDistributedObjects().stream().map(DistributedObject::getName))
.as("Should initialize proxy on member1")
.contains(indexMapName);
assertThat(member2.getDistributedObjects().stream().map(DistributedObject::getName))
.as("Should initialize proxy on member2")
.contains(indexMapName);
});
}

private void restartCluster(boolean graceful, ClusterState restartInState, String indexMapName) {
// Avoid race conditions between async proxy initialization on other members (via events)
// and cluster restart. Those races are unfortunate but are not tested here.
assertMapProxyInitializedEventually(indexMapName);

warmUpPartitions(member1, member2);
member1.getCluster().changeClusterState(restartInState);
member1 = restartMember(member1, member2, graceful);
member2 = restartMember(member2, member1, graceful);
member1.getCluster().changeClusterState(ClusterState.ACTIVE);

// Wait for proxy initialization by post join operation to avoid race condition.
// Note that indexes are initialized in many ways, proxy initialization being only one
// of them. So even without proxy indexes might work but with proxy they will for sure.
assertMapProxyInitializedEventually(indexMapName);
}

private HazelcastInstance restartMember(HazelcastInstance member, HazelcastInstance otherMember, boolean graceful) {
Expand Down Expand Up @@ -242,7 +261,7 @@ private static LocalMapStats combineStats(IMap map1, IMap map2) {
LocalMapStats stats1 = map1.getLocalMapStats();
LocalMapStats stats2 = map2.getLocalMapStats();

List<IndexRegistry> allIndexes = new ArrayList<IndexRegistry>();
List<IndexRegistry> allIndexes = new ArrayList<>();
allIndexes.addAll(getAllIndexes(map1));
allIndexes.addAll(getAllIndexes(map2));

Expand All @@ -254,7 +273,7 @@ private static LocalMapStats combineStats(IMap map1, IMap map2) {
combinedStats.setIndexedQueryCount(stats1.getIndexedQueryCount());

assertEquals(stats1.getIndexStats().size(), stats2.getIndexStats().size());
Map<String, PartitionedIndexStatsImpl> combinedIndexStatsMap = new HashMap<String, PartitionedIndexStatsImpl>();
Map<String, PartitionedIndexStatsImpl> combinedIndexStatsMap = new HashMap<>();
for (Map.Entry<String, LocalIndexStats> indexEntry : stats1.getIndexStats().entrySet()) {
LocalIndexStats indexStats1 = indexEntry.getValue();
LocalIndexStats indexStats2 = stats2.getIndexStats().get(indexEntry.getKey());
Expand Down

0 comments on commit 67e503c

Please sign in to comment.