Skip to content

Commit c150276

Browse files
authored
[test] Speed up test framework shutdown and fix several flaky tests (linkedin#2485)
Deflake and harden multiple integration tests, parallelize cluster shutdown, and remove dead test code. Wrap several assertions in waitForNonDeterministicAssertion to tolerate eventual consistency between router and storage engine, changelog inclusive seeks, incremental push status visibility, and meta store readiness. Add retryOnThrowable and bounded get timeouts to prevent hanging futures. Increase timeouts for empty push, version activation, participant readiness, and offline job start for CI resilience. Fix resource leaks by closing ControllerClient, D2 client, and other closeables in @AfterClass/@AfterMethod. Switch selected tests to @test(singleThreaded = true) for isolation, restore lost config overrides for schema-refresh scenarios, and remove obsolete view-based tests. Refactor VeniceClusterWrapper and related multi-cluster wrappers to shut down independent components concurrently using dedicated cached thread pools, avoiding ForkJoinPool starvation. Keep ZooKeeper shutdown sequenced after PubSub broker. Fix race in native replication repush test by waiting for version creation in all child DCs before issuing kill. Misc cleanups: reduce poll interval for faster feedback, demote noisy logs to debug, remove unused imports and dead code.
1 parent 1560deb commit c150276

16 files changed

Lines changed: 752 additions & 1169 deletions

clients/da-vinci-client/src/main/java/com/linkedin/davinci/kafka/consumer/LeaderFollowerStoreIngestionTask.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1006,7 +1006,7 @@ private boolean canSwitchToLeaderTopic(PartitionConsumptionState pcs) {
10061006
// critical for cluster operation and we want faster leader transitions there. Once DoL is proven stable
10071007
// in production, this extra check can be removed to get the full performance benefit.
10081008
if (!isSystemStore && !canSwitchToLeaderTopicLegacy(pcs)) {
1009-
LOGGER.info(
1009+
LOGGER.debug(
10101010
"DoL mechanism complete for replica: {} but legacy time-based check not yet satisfied. Waiting for legacy check to pass.",
10111011
pcs.getReplicaId());
10121012
return false;
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
package com.linkedin.venice.consumer;
2+
3+
import static com.linkedin.venice.ConfigKeys.CLUSTER_NAME;
4+
import static com.linkedin.venice.ConfigKeys.KAFKA_BOOTSTRAP_SERVERS;
5+
import static com.linkedin.venice.ConfigKeys.ZOOKEEPER_ADDRESS;
6+
import static com.linkedin.venice.integration.utils.VeniceControllerWrapper.D2_SERVICE_NAME;
7+
8+
import com.linkedin.davinci.consumer.ChangelogClientConfig;
9+
import com.linkedin.venice.client.store.AvroSpecificStoreClient;
10+
import com.linkedin.venice.client.store.ClientConfig;
11+
import com.linkedin.venice.client.store.ClientFactory;
12+
import com.linkedin.venice.common.VeniceSystemStoreType;
13+
import com.linkedin.venice.controllerapi.ControllerClient;
14+
import com.linkedin.venice.controllerapi.UpdateStoreQueryParams;
15+
import com.linkedin.venice.integration.utils.PubSubBrokerWrapper;
16+
import com.linkedin.venice.integration.utils.VeniceClusterWrapper;
17+
import com.linkedin.venice.integration.utils.VeniceRouterWrapper;
18+
import com.linkedin.venice.integration.utils.VeniceTwoLayerMultiRegionMultiClusterWrapper;
19+
import com.linkedin.venice.integration.utils.ZkServerWrapper;
20+
import com.linkedin.venice.meta.Version;
21+
import com.linkedin.venice.system.store.MetaStoreDataType;
22+
import com.linkedin.venice.systemstore.schemas.StoreMetaKey;
23+
import com.linkedin.venice.systemstore.schemas.StoreMetaValue;
24+
import com.linkedin.venice.utils.TestUtils;
25+
import java.util.Collections;
26+
import java.util.List;
27+
import java.util.Properties;
28+
import java.util.concurrent.TimeUnit;
29+
import org.apache.logging.log4j.Logger;
30+
import org.testng.Assert;
31+
32+
33+
/**
34+
* Shared utility methods for changelog consumer integration tests.
35+
*/
36+
public class ChangelogConsumerTestUtils {
37+
private ChangelogConsumerTestUtils() {
38+
}
39+
40+
public static Properties buildConsumerProperties(
41+
VeniceTwoLayerMultiRegionMultiClusterWrapper multiRegionWrapper,
42+
PubSubBrokerWrapper localKafka,
43+
String clusterName,
44+
ZkServerWrapper localZkServer) {
45+
Properties consumerProperties = new Properties();
46+
consumerProperties.putAll(multiRegionWrapper.getPubSubClientProperties());
47+
consumerProperties.put(KAFKA_BOOTSTRAP_SERVERS, localKafka.getAddress());
48+
consumerProperties.put(CLUSTER_NAME, clusterName);
49+
consumerProperties.put(ZOOKEEPER_ADDRESS, localZkServer.getAddress());
50+
return consumerProperties;
51+
}
52+
53+
public static ChangelogClientConfig buildBaseChangelogClientConfig(
54+
Properties consumerProperties,
55+
String localD2ZkHosts,
56+
long versionSwapDetectionIntervalSeconds) {
57+
return new ChangelogClientConfig().setConsumerProperties(consumerProperties)
58+
.setControllerD2ServiceName(D2_SERVICE_NAME)
59+
.setD2ServiceName(VeniceRouterWrapper.CLUSTER_DISCOVERY_D2_SERVICE_NAME)
60+
.setLocalD2ZkHosts(localD2ZkHosts)
61+
.setControllerRequestRetryCount(3)
62+
.setVersionSwapDetectionIntervalTimeInSeconds(versionSwapDetectionIntervalSeconds);
63+
}
64+
65+
public static UpdateStoreQueryParams buildDefaultStoreParams() {
66+
return new UpdateStoreQueryParams().setActiveActiveReplicationEnabled(true)
67+
.setHybridRewindSeconds(500)
68+
.setHybridOffsetLagThreshold(8)
69+
.setChunkingEnabled(true)
70+
.setNativeReplicationEnabled(true)
71+
.setPartitionCount(3);
72+
}
73+
74+
public static void waitForMetaSystemStoreToBeReady(
75+
String storeName,
76+
ControllerClient controllerClient,
77+
VeniceClusterWrapper clusterWrapper) {
78+
String metaSystemStoreName = VeniceSystemStoreType.META_STORE.getSystemStoreName(storeName);
79+
TestUtils.waitForNonDeterministicPushCompletion(
80+
Version.composeKafkaTopic(metaSystemStoreName, 1),
81+
controllerClient,
82+
90,
83+
TimeUnit.SECONDS);
84+
clusterWrapper.refreshAllRouterMetaData();
85+
String routerUrl = clusterWrapper.getRandomRouterURL();
86+
try (AvroSpecificStoreClient<StoreMetaKey, StoreMetaValue> metaStoreClient =
87+
ClientFactory.getAndStartSpecificAvroClient(
88+
ClientConfig.defaultSpecificClientConfig(metaSystemStoreName, StoreMetaValue.class)
89+
.setVeniceURL(routerUrl))) {
90+
StoreMetaKey storeClusterConfigKey =
91+
MetaStoreDataType.STORE_CLUSTER_CONFIG.getStoreMetaKey(Collections.singletonMap("KEY_STORE_NAME", storeName));
92+
TestUtils.waitForNonDeterministicAssertion(90, TimeUnit.SECONDS, false, true, () -> {
93+
StoreMetaValue value = metaStoreClient.get(storeClusterConfigKey).get(30, TimeUnit.SECONDS);
94+
Assert.assertNotNull(value, "Meta store should return non-null value for STORE_CLUSTER_CONFIG");
95+
Assert.assertNotNull(value.storeClusterConfig, "storeClusterConfig should not be null");
96+
});
97+
}
98+
}
99+
100+
public static void cleanupAfterTest(
101+
List<AutoCloseable> testCloseables,
102+
List<String> testStoresToDelete,
103+
ControllerClient parentControllerClient,
104+
Logger logger) {
105+
for (int i = testCloseables.size() - 1; i >= 0; i--) {
106+
try {
107+
testCloseables.get(i).close();
108+
} catch (Exception e) {
109+
logger.warn("Failed to close resource during test cleanup", e);
110+
}
111+
}
112+
testCloseables.clear();
113+
114+
for (String storeName: testStoresToDelete) {
115+
try {
116+
parentControllerClient.disableAndDeleteStore(storeName);
117+
} catch (Exception e) {
118+
logger.warn("Failed to delete store {} during test cleanup", storeName, e);
119+
}
120+
}
121+
testStoresToDelete.clear();
122+
}
123+
}

0 commit comments

Comments
 (0)