Jackie-Jiang commented on code in PR #9875:
URL: https://github.com/apache/pinot/pull/9875#discussion_r1041377162
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTestSet.java:
##########
@@ -606,6 +608,37 @@ public void testReset(TableType tableType)
}, 30_000L, "Failed to wait for all segments come back online");
}
+ public String reloadTableAndValidateResponse(String tableName, TableType
tableType, boolean forceDownload)
+ throws IOException {
+ String jobId = null;
+ String response =
+ sendPostRequest(_controllerRequestURLBuilder.forTableReload(tableName,
tableType, forceDownload), null);
+ String tableNameWithType =
TableNameBuilder.forType(tableType).tableNameWithType(tableName);
+ JsonNode tableLevelDetails =
+
JsonUtils.stringToJsonNode(StringEscapeUtils.unescapeJava(response.split(":
")[1])).get(tableNameWithType);
+ String isZKWriteSuccess =
tableLevelDetails.get("reloadJobMetaZKStorageStatus").asText();
+ assertEquals("SUCCESS", isZKWriteSuccess);
Review Comment:
(minor, not introduced in this PR)
```suggestion
assertEquals(isZKWriteSuccess, "SUCCESS");
```
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTestSet.java:
##########
@@ -606,6 +608,37 @@ public void testReset(TableType tableType)
}, 30_000L, "Failed to wait for all segments come back online");
}
+ public String reloadTableAndValidateResponse(String tableName, TableType
tableType, boolean forceDownload)
+ throws IOException {
+ String jobId = null;
+ String response =
+ sendPostRequest(_controllerRequestURLBuilder.forTableReload(tableName,
tableType, forceDownload), null);
+ String tableNameWithType =
TableNameBuilder.forType(tableType).tableNameWithType(tableName);
+ JsonNode tableLevelDetails =
+
JsonUtils.stringToJsonNode(StringEscapeUtils.unescapeJava(response.split(":
")[1])).get(tableNameWithType);
+ String isZKWriteSuccess =
tableLevelDetails.get("reloadJobMetaZKStorageStatus").asText();
+ assertEquals("SUCCESS", isZKWriteSuccess);
+ jobId = tableLevelDetails.get("reloadJobId").asText();
Review Comment:
(minor) Remove the declaration on line 613
```suggestion
String jobId = tableLevelDetails.get("reloadJobId").asText();
```
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/LLCRealtimeClusterIntegrationTest.java:
##########
@@ -206,7 +206,7 @@ public void setUp()
@Override
public void tearDown()
throws Exception {
- FileUtils.deleteDirectory(new File(CONSUMER_DIRECTORY));
+ FileUtils.deleteDirectory(new File(CONSUMER_DIRECTORY));
Review Comment:
(nit) revert
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/BaseClusterIntegrationTestSet.java:
##########
@@ -606,6 +608,37 @@ public void testReset(TableType tableType)
}, 30_000L, "Failed to wait for all segments come back online");
}
+ public String reloadTableAndValidateResponse(String tableName, TableType
tableType, boolean forceDownload)
+ throws IOException {
+ String jobId = null;
+ String response =
+ sendPostRequest(_controllerRequestURLBuilder.forTableReload(tableName,
tableType, forceDownload), null);
+ String tableNameWithType =
TableNameBuilder.forType(tableType).tableNameWithType(tableName);
+ JsonNode tableLevelDetails =
+
JsonUtils.stringToJsonNode(StringEscapeUtils.unescapeJava(response.split(":
")[1])).get(tableNameWithType);
+ String isZKWriteSuccess =
tableLevelDetails.get("reloadJobMetaZKStorageStatus").asText();
+ assertEquals("SUCCESS", isZKWriteSuccess);
+ jobId = tableLevelDetails.get("reloadJobId").asText();
+ String jobStatusResponse =
sendGetRequest(_controllerRequestURLBuilder.forControllerJobStatus(jobId));
+ JsonNode jobStatus = JsonUtils.stringToJsonNode(jobStatusResponse);
+
+ // Validate all fields are present
+ assertEquals(jobStatus.get("metadata").get("jobId").asText(), jobId);
+ assertEquals(jobStatus.get("metadata").get("jobType").asText(),
"RELOAD_ALL_SEGMENTS");
+ assertEquals(jobStatus.get("metadata").get("tableName").asText(),
tableNameWithType);
+ return jobId;
+ }
+
+ public boolean isReloadJobCompleted(String reloadJobId)
Review Comment:
We can remove the `OfflineClusterIntegrationTest.validateReloadJobSuccess()`
and replace all usage with `assertTrue(isReloadJobCompleted(jobId)`. Currently
we didn't really validate if the reload is done
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/LLCRealtimeClusterIntegrationTest.java:
##########
@@ -277,6 +277,48 @@ public void testReload()
testReload(false);
}
+ @Test
+ public void testEnableDisableDictionaryWithReload()
+ throws Exception {
+ String query = "SELECT * FROM myTable WHERE ActualElapsedTime > 0";
+ JsonNode queryResponse = postQuery(query);
+ long numTotalDocs = queryResponse.get("totalDocs").asLong();
+
+ // Enable dictionary.
+ TableConfig tableConfig = getRealtimeTableConfig();
+
tableConfig.getIndexingConfig().getNoDictionaryColumns().remove("ActualElapsedTime");
+ updateTableConfig(tableConfig);
+ String enableDictReloadId = reloadTableAndValidateResponse(getTableName(),
TableType.REALTIME, false);
+ TestUtils.waitForCondition(aVoid -> {
+ try {
+ return isReloadJobCompleted(enableDictReloadId);
+ } catch (Exception e) {
+ throw new RuntimeException(e);
+ }
+ }, 60_000L, "Failed to reload.");
+
+ queryResponse = postQuery(query);
+ long numTotalDocsAfterReload = queryResponse.get("totalDocs").asLong();
+ assertEquals(numTotalDocs, numTotalDocsAfterReload);
Review Comment:
We want to validate if total doc and query response is always correct during
the reload. We can also pick a query that can return different metadata based
on whether the column is dictionary encoded to ensure the changing index
actually happens. E.g. if we pick a value that doesn't exist, `SELECT COUNT(*)
FROM myTable WHERE ActualElapsedTime = <val>` will have docs scanned in filter
for raw index, but no scan for dictionary
--
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]
