FrankChen021 commented on code in PR #19484:
URL: https://github.com/apache/druid/pull/19484#discussion_r3273985127
##########
server/src/test/java/org/apache/druid/segment/realtime/appenderator/StreamAppenderatorTest.java:
##########
@@ -2296,14 +2297,14 @@ public void testQueryBySegments() throws Exception
private void verifySinkMetrics(StubServiceEmitter emitter, Set<String>
segmentIds)
{
int segments = segmentIds.size();
- emitter.verifyEmitted("query/cpu/time", 1);
- Assert.assertEquals(segments,
emitter.getMetricEvents("query/segment/time").size());
- Assert.assertEquals(segments,
emitter.getMetricEvents("query/segmentAndCache/time").size());
- Assert.assertEquals(segments,
emitter.getMetricEvents("query/wait/time").size());
+ emitter.verifyEmitted(DefaultQueryMetrics.QUERY_CPU_TIME, 1);
+ Assert.assertEquals(segments,
emitter.getMetricEvents(DefaultQueryMetrics.QUERY_SEGMENT_TIME).size());
Review Comment:
[P3] Test helper does not cover the fixed metric-value regression
The modified helper still only checks that each metric name was emitted for
each segment. The old fall-through bug also emitted query/segment/time,
query/wait/time, and query/segmentAndCache/time with the right segment
dimensions, but overwrote their values through the wrong reporter, so this test
would still pass with the buggy implementation. Please add coverage that
verifies the reporter-to-accumulator mapping, otherwise the exact regression
fixed in SinkQuerySegmentWalker can return unnoticed.
--
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]
