yashmayya commented on code in PR #16096:
URL: https://github.com/apache/pinot/pull/16096#discussion_r2163013791
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TableRebalanceIntegrationTest.java:
##########
@@ -1726,4 +1556,82 @@ private void waitForTableEVISConverge(String tableName,
long timeoutMs) {
}
}, 1000L, timeoutMs, "Failed to converge EV and IS for table: " +
tableName);
}
+
+ /**
+ * Helper method to perform segment moving test with specified configuration.
+ * Changes the table tenant, executes rebalance with force commit, and
verifies if segments were committed.
+ */
+ void performSegmentMovingTest(RebalanceConfig rebalanceConfig, TableConfig
tableConfig, String newTenant,
+ boolean shouldCommit, long timeoutMs)
throws Exception {
+ performSegmentMovingTest(rebalanceConfig, tableConfig, newTenant,
shouldCommit, timeoutMs, false);
+ }
+
+ /**
+ * Helper method to perform segment moving test with EVIS convergence wait.
+ * Similar to performSegmentMovingTest but waits for external view/ideal
state convergence instead of rebalance completion.
+ */
+ void performSegmentMovingTestWithEVISConverge(RebalanceConfig
rebalanceConfig, TableConfig tableConfig,
Review Comment:
Unused?
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TableRebalanceIntegrationTest.java:
##########
@@ -1726,4 +1556,82 @@ private void waitForTableEVISConverge(String tableName,
long timeoutMs) {
}
}, 1000L, timeoutMs, "Failed to converge EV and IS for table: " +
tableName);
}
+
+ /**
+ * Helper method to perform segment moving test with specified configuration.
+ * Changes the table tenant, executes rebalance with force commit, and
verifies if segments were committed.
+ */
+ void performSegmentMovingTest(RebalanceConfig rebalanceConfig, TableConfig
tableConfig, String newTenant,
+ boolean shouldCommit, long timeoutMs)
throws Exception {
+ performSegmentMovingTest(rebalanceConfig, tableConfig, newTenant,
shouldCommit, timeoutMs, false);
+ }
+
+ /**
+ * Helper method to perform segment moving test with EVIS convergence wait.
+ * Similar to performSegmentMovingTest but waits for external view/ideal
state convergence instead of rebalance completion.
+ */
+ void performSegmentMovingTestWithEVISConverge(RebalanceConfig
rebalanceConfig, TableConfig tableConfig,
+ String newTenant,
boolean shouldCommit, long timeoutMs) throws Exception {
+ performSegmentMovingTest(rebalanceConfig, tableConfig, newTenant,
shouldCommit, timeoutMs, true);
+ }
+
+ /**
+ * Helper method to perform segment moving test with specified configuration.
+ * Changes the table tenant, executes rebalance with force commit, and
verifies if segments were committed.
+ *
+ * @param rebalanceConfig the rebalance configuration
+ * @param tableConfig the table configuration
+ * @param newTenant the new tenant to move segments to
+ * @param shouldCommit whether segments should be committed (affects
verification)
+ * @param timeoutMs timeout in milliseconds
+ * @param waitForEVISConverge if true, waits for external view/ideal state
convergence; if false, waits for rebalance completion
+ */
+ private void performSegmentMovingTest(RebalanceConfig rebalanceConfig,
TableConfig tableConfig, String newTenant,
Review Comment:
If this is identical to the one in the pauseless integration test class, I
think we can move it to `BaseClusterIntegrationTestSet` and share it across
both these classes.
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/TableRebalancePauselessIntegrationTest.java:
##########
@@ -207,171 +205,129 @@ public void testForceCommit()
rebalanceConfig.setMinAvailableReplicas(0);
rebalanceConfig.setDowntime(false);
rebalanceConfig.setIncludeConsuming(true);
- response = sendPostRequest(getRebalanceUrl(rebalanceConfig,
TableType.REALTIME));
- rebalanceResult = JsonUtils.stringToObject(response,
RebalanceResult.class);
+ // notice that this could get an HTTP 409 CONFLICT, when the test failed
due to the timeout waiting on the table
+ // to converge, and try to rebalance again here.
+ String response = sendPostRequest(getRebalanceUrl(rebalanceConfig,
TableType.REALTIME));
+ RebalanceResult rebalanceResult = JsonUtils.stringToObject(response,
RebalanceResult.class);
waitForRebalanceToComplete(rebalanceResult.getJobId(), 30000);
- } finally {
serverStarter0.stop();
serverStarter1.stop();
serverStarter2.stop();
serverStarter3.stop();
}
}
- @Test
- void testForceCommitStrictReplicaGroup()
- throws Exception {
- final String tenantA = "tenantA_strictRG";
- final String tenantB = "tenantB_strictRG";
-
- BaseServerStarter serverStarter0 = startOneServer(0);
- BaseServerStarter serverStarter1 = startOneServer(1);
- createServerTenant(tenantA, 0, 2);
-
- BaseServerStarter serverStarter2 = startOneServer(2);
- BaseServerStarter serverStarter3 = startOneServer(3);
- createServerTenant(tenantB, 0, 2);
- try {
- // Prepare the table to replicate segments across two servers on tenantA
- TableConfig tableConfig = getRealtimeTableConfig();
- tableConfig.setRoutingConfig(
- new RoutingConfig(null, null,
RoutingConfig.STRICT_REPLICA_GROUP_INSTANCE_SELECTOR_TYPE,
- false));
- tableConfig.setTenantConfig(new TenantConfig(getBrokerTenant(), tenantA,
null));
- tableConfig.getValidationConfig().setReplication("2");
- tableConfig.getValidationConfig().setPeerSegmentDownloadScheme("http");
-
- updateTableConfig(tableConfig);
-
- RebalanceConfig rebalanceConfig = new RebalanceConfig();
- rebalanceConfig.setDryRun(false);
- rebalanceConfig.setIncludeConsuming(true);
-
- String response = sendPostRequest(getRebalanceUrl(rebalanceConfig,
TableType.REALTIME));
- RebalanceResult rebalanceResult = JsonUtils.stringToObject(response,
RebalanceResult.class);
- RebalanceSummaryResult summary =
rebalanceResult.getRebalanceSummaryResult();
- assertEquals(
-
summary.getServerInfo().getNumServers().getExpectedValueAfterRebalance(),
- 2);
-
assertEquals(summary.getSegmentInfo().getConsumingSegmentToBeMovedSummary().getNumConsumingSegmentsToBeMoved(),
- 4);
+ private void waitForRebalanceToComplete(String rebalanceJobId, long
timeoutMs) {
Review Comment:
Let's move these common utils to one of the base classes (probably
`ControllerTest`) and re-use them instead of duplicating.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
