Skip to content

Commit 70edf83

Browse files
authored
[test] Fix TestDeleteStoreDeletesRealtimeTopic for correctness (#701)
This commit addresses a potential issue with resource management in `TestDeleteStoreDeletesRealtimeTopic`. The previous code prematurely closed the `TopicManagerRepository` within a try-with-resources block, causing early closure of internal topic managers and its associated PubSub clients. To resolve this, we close the `TopicManagerRepository` only when its local topic manager is no longer required. This change ensures proper resource management.
1 parent 270349c commit 70edf83

File tree

1 file changed

+10
-14
lines changed

1 file changed

+10
-14
lines changed

internal/venice-test-common/src/integrationTest/java/com/linkedin/venice/controller/TestDeleteStoreDeletesRealtimeTopic.java

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import static com.linkedin.venice.utils.IntegrationTestPushUtils.makeStoreHybrid;
66
import static com.linkedin.venice.utils.IntegrationTestPushUtils.sendStreamingRecord;
77
import static org.testng.Assert.assertEquals;
8+
import static org.testng.Assert.assertTrue;
89

910
import com.linkedin.venice.client.store.AvroGenericStoreClient;
1011
import com.linkedin.venice.client.store.ClientConfig;
@@ -15,7 +16,6 @@
1516
import com.linkedin.venice.exceptions.VeniceException;
1617
import com.linkedin.venice.integration.utils.ServiceFactory;
1718
import com.linkedin.venice.integration.utils.VeniceClusterWrapper;
18-
import com.linkedin.venice.kafka.TopicManager;
1919
import com.linkedin.venice.kafka.TopicManagerRepository;
2020
import com.linkedin.venice.meta.Version;
2121
import com.linkedin.venice.pubsub.PubSubTopicRepository;
@@ -42,26 +42,22 @@ public class TestDeleteStoreDeletesRealtimeTopic {
4242
private VeniceClusterWrapper venice = null;
4343
private AvroGenericStoreClient client = null;
4444
private ControllerClient controllerClient = null;
45-
private TopicManager topicManager = null;
45+
private TopicManagerRepository topicManagerRepository = null;
4646
private String storeName = null;
4747

48-
private PubSubTopicRepository pubSubTopicRepository = new PubSubTopicRepository();
48+
private final PubSubTopicRepository pubSubTopicRepository = new PubSubTopicRepository();
4949

5050
@BeforeClass
5151
public void setUp() {
5252
venice = ServiceFactory.getVeniceCluster();
5353
controllerClient =
5454
ControllerClient.constructClusterControllerClient(venice.getClusterName(), venice.getRandomRouterURL());
55-
56-
try (TopicManagerRepository topicManagerRepository = IntegrationTestPushUtils.getTopicManagerRepo(
55+
topicManagerRepository = IntegrationTestPushUtils.getTopicManagerRepo(
5756
DEFAULT_KAFKA_OPERATION_TIMEOUT_MS,
5857
100,
5958
0l,
6059
venice.getPubSubBrokerWrapper(),
61-
pubSubTopicRepository)) {
62-
topicManager = topicManagerRepository.getTopicManager();
63-
}
64-
60+
pubSubTopicRepository);
6561
storeName = Utils.getUniqueString("hybrid-store");
6662
venice.getNewStore(storeName);
6763
makeStoreHybrid(venice, storeName, 100L, 5L);
@@ -71,7 +67,7 @@ public void setUp() {
7167

7268
@AfterClass
7369
public void cleanUp() {
74-
Utils.closeQuietlyWithErrorLogged(topicManager);
70+
Utils.closeQuietlyWithErrorLogged(topicManagerRepository);
7571
Utils.closeQuietlyWithErrorLogged(client);
7672
Utils.closeQuietlyWithErrorLogged(venice);
7773
Utils.closeQuietlyWithErrorLogged(controllerClient);
@@ -109,7 +105,7 @@ public void deletingHybridStoreDeletesRealtimeTopic() {
109105

110106
// verify realtime topic exists
111107
PubSubTopic rtTopic = pubSubTopicRepository.getTopic(Version.composeRealTimeTopic(storeName));
112-
Assert.assertTrue(topicManager.containsTopicAndAllPartitionsAreOnline(rtTopic));
108+
assertTrue(topicManagerRepository.getTopicManager().containsTopicAndAllPartitionsAreOnline(rtTopic));
113109

114110
// disable store
115111
TestUtils.assertCommand(
@@ -130,11 +126,11 @@ public void deletingHybridStoreDeletesRealtimeTopic() {
130126
// verify realtime topic does not exist
131127
PubSubTopic realTimeTopicName = pubSubTopicRepository.getTopic(Version.composeRealTimeTopic(storeName));
132128
try {
133-
boolean isTruncated = topicManager.isTopicTruncated(realTimeTopicName, 60000);
134-
Assert.assertTrue(
129+
boolean isTruncated = topicManagerRepository.getTopicManager().isTopicTruncated(realTimeTopicName, 60000);
130+
assertTrue(
135131
isTruncated,
136132
"Real-time buffer topic should be truncated: " + realTimeTopicName + " but retention is set to: "
137-
+ topicManager.getTopicRetention(realTimeTopicName) + ".");
133+
+ topicManagerRepository.getTopicManager().getTopicRetention(realTimeTopicName) + ".");
138134
LOGGER.info("Confirmed truncation of real-time topic: {}", realTimeTopicName);
139135
} catch (PubSubTopicDoesNotExistException e) {
140136
LOGGER

0 commit comments

Comments
 (0)