xiangfu0 commented on code in PR #20222:
URL: https://github.com/apache/pinot/pull/20222#discussion_r4537662042
##########
pinot-controller/src/test/java/org/apache/pinot/controller/util/TableMetadataReaderTest.java:
##########
@@ -120,14 +116,29 @@ public void testTableLevelMetadataWithoutReplication()
JsonNode metadata = getSegmentsMetadata(serverToSegments);
assertEquals(segmentNames(metadata), Set.of("seg0", "seg1", "seg2"));
- assertEquals(_legacyRequestCount.get(), 0);
+ }
+
+ /// A server whose table-level request fails must fail the whole call
instead of returning partial metadata.
+ @Test
+ public void testTableLevelMetadataFailsWhenServerReturnsError() {
+ Map<String, List<String>> serverToSegments = new LinkedHashMap<>();
+ serverToSegments.put("server0", List.of("seg0"));
+ serverToSegments.put("server1", List.of("seg1"));
+
+ assertThrows(RuntimeException.class, () ->
getSegmentsMetadata(serverToSegments, Set.of("server1")));
Review Comment:
Optional: this test also passes at the merge-base. The old fallback failed
there too, because `startFailingServer` returns 404 on the per-segment path. So
it guards against partial metadata, but it wouldn't catch the fallback being
reintroduced. Serving 200 on `/tables/{table}/segments/{seg}/metadata` from the
failing server, and checking the exception message, would pin both.
--
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]
