xiangfu0 commented on code in PR #20015:
URL: https://github.com/apache/pinot/pull/20015#discussion_r4366214660
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/CustomDataQueryClusterIntegrationTest.java:
##########
@@ -115,9 +117,32 @@ public void setUp()
setUpTable();
waitForAllDocsLoaded(60_000);
+ waitForNoOptionalSegments(60_000);
LOGGER.warn("Finished setting up integration test class: {}",
getClass().getSimpleName());
}
+ /// Waits until the broker routes no optional segment for the table.
+ ///
+ /// The broker routes a new segment as optional until it sees the segment
ONLINE in the external view. The
+ /// single-stage engine still sends optional segments to the servers, which
query them once loaded, so
+ /// [#waitForAllDocsLoaded] can pass before the broker sees the last segment
ONLINE. The multi-stage engine skips
+ /// optional segments, so a multi-stage query issued at that point misses
them.
+ protected void waitForNoOptionalSegments(long timeoutMs) {
+ String routingTableUrl = getBrokerBaseApiUrl() +
"/debug/routingTableWithOptionalSegments/" + getTableName();
+ TestUtils.waitForCondition(() -> {
+ // Routing table per table type: server -> {"segments": [...],
"optionalSegments": [...]}
+ for (JsonNode serverToSegments :
JsonUtils.stringToJsonNode(sendGetRequest(routingTableUrl))) {
+ for (JsonNode segments : serverToSegments) {
+ if (!segments.get("optionalSegments").isEmpty()) {
Review Comment:
Agreed, no change: the broker always sets a list there, so
`optionalSegments` is always present.
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/CustomDataQueryClusterIntegrationTest.java:
##########
@@ -115,9 +117,32 @@ public void setUp()
setUpTable();
waitForAllDocsLoaded(60_000);
+ waitForNoOptionalSegments(60_000);
LOGGER.warn("Finished setting up integration test class: {}",
getClass().getSimpleName());
}
+ /// Waits until the broker routes no optional segment for the table.
+ ///
+ /// The broker routes a new segment as optional until it sees the segment
ONLINE in the external view. The
+ /// single-stage engine still sends optional segments to the servers, which
query them once loaded, so
+ /// [#waitForAllDocsLoaded] can pass before the broker sees the last segment
ONLINE. The multi-stage engine skips
+ /// optional segments, so a multi-stage query issued at that point misses
them.
+ protected void waitForNoOptionalSegments(long timeoutMs) {
+ String routingTableUrl = getBrokerBaseApiUrl() +
"/debug/routingTableWithOptionalSegments/" + getTableName();
+ TestUtils.waitForCondition(() -> {
+ // Routing table per table type: server -> {"segments": [...],
"optionalSegments": [...]}
+ for (JsonNode serverToSegments :
JsonUtils.stringToJsonNode(sendGetRequest(routingTableUrl))) {
+ for (JsonNode segments : serverToSegments) {
+ if (!segments.get("optionalSegments").isEmpty()) {
+ return false;
+ }
+ }
+ }
+ return true;
+ }, 100L, timeoutMs, "Broker still routes optional segments for table: " +
getTableName(),
+ Duration.ofMillis(timeoutMs / 10));
Review Comment:
Thanks, no change.
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/custom/CustomDataQueryClusterIntegrationTest.java:
##########
@@ -115,9 +117,32 @@ public void setUp()
setUpTable();
waitForAllDocsLoaded(60_000);
+ waitForNoOptionalSegments(60_000);
Review Comment:
Checked: `MultiTopicRealtimeClusterIntegrationTest` only queries through the
single-stage engine, which does query optional segments, so skipping this wait
doesn't expose it. Leaving it as is.
--
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]
