review comments

This commit is contained in:
rishabh singh 2024-11-06 16:26:19 +05:30
parent b199d57237
commit 518c2a7ee2
6 changed files with 19 additions and 20 deletions

View File

@ -123,8 +123,8 @@ Most metric values reset each emission period, as specified in `druid.monitoring
These metrics are reported from broker, historical and real-time nodes
|`mergeBuffer/pendingRequests`|Number of requests waiting to acquire a batch of buffers from the merge buffer pool.|This metric is only available if the `GroupByStatsMonitor` module is included.|Should be ideally 0, though a higher number isn't representative of a problem.|
|`mergeBuffer/usedCount`|Number of merge buffers used from the merge buffer pool.|This metric is only available if the `GroupByStatsMonitor` module is included.|Depends on the number of groupBy queries needing merge buffers.|
|`mergeBuffer/acquisitionCount`|Number of times groupBy queries acquired merge buffers.|This metric is only available if the `GroupByStatsMonitor` module is included.|Depends on the number of groupBy queries needing merge buffers.|
|`mergeBuffer/used`|Number of merge buffers used from the merge buffer pool.|This metric is only available if the `GroupByStatsMonitor` module is included.|Depends on the number of groupBy queries needing merge buffers.|
|`mergeBuffer/queries`|Number of groupBy queries that acquired a batch of buffers from the merge buffer pool.|This metric is only available if the `GroupByStatsMonitor` module is included.|Depends on the number of groupBy queries needing merge buffers.|
|`mergeBuffer/acquisitionTimeNs`|Total time in nanoseconds to acquire merge buffer for groupBy queries.|This metric is only available if the `GroupByStatsMonitor` module is included.|Varies|
|`groupBy/spilledQueries`|Number of groupBy queries that have spilled onto the disk.|This metric is only available if the `GroupByStatsMonitor` module is included.|Varies|
|`groupBy/spilledBytes`|Number of bytes spilled on the disk by the groupBy queries.|This metric is only available if the `GroupByStatsMonitor` module is included.|Varies|

View File

@ -65,7 +65,7 @@ public class GroupByStatsProvider
public static class AggregateStats
{
private long mergeBufferAcquisitionCount = 0;
private long mergeBufferQueries = 0;
private long mergeBufferAcquisitionTimeNs = 0;
private long spilledQueries = 0;
private long spilledBytes = 0;
@ -76,23 +76,23 @@ public class GroupByStatsProvider
}
public AggregateStats(
long mergeBufferAcquisitionCount,
long mergeBufferQueries,
long mergeBufferAcquisitionTimeNs,
long spilledQueries,
long spilledBytes,
long mergeDictionarySize
)
{
this.mergeBufferAcquisitionCount = mergeBufferAcquisitionCount;
this.mergeBufferQueries = mergeBufferQueries;
this.mergeBufferAcquisitionTimeNs = mergeBufferAcquisitionTimeNs;
this.spilledQueries = spilledQueries;
this.spilledBytes = spilledBytes;
this.mergeDictionarySize = mergeDictionarySize;
}
public long getMergeBufferAcquisitionCount()
public long getMergeBufferQueries()
{
return mergeBufferAcquisitionCount;
return mergeBufferQueries;
}
public long getMergeBufferAcquisitionTimeNs()
@ -118,7 +118,7 @@ public class GroupByStatsProvider
public void addQueryStats(PerQueryStats perQueryStats)
{
if (perQueryStats.getMergeBufferAcquisitionTimeNs() > 0) {
mergeBufferAcquisitionCount++;
mergeBufferQueries++;
mergeBufferAcquisitionTimeNs += perQueryStats.getMergeBufferAcquisitionTimeNs();
}
@ -134,14 +134,14 @@ public class GroupByStatsProvider
{
AggregateStats aggregateStats =
new AggregateStats(
mergeBufferAcquisitionCount,
mergeBufferQueries,
mergeBufferAcquisitionTimeNs,
spilledQueries,
spilledBytes,
mergeDictionarySize
);
this.mergeBufferAcquisitionCount = 0;
this.mergeBufferQueries = 0;
this.mergeBufferAcquisitionTimeNs = 0;
this.spilledQueries = 0;
this.spilledBytes = 0;
@ -164,7 +164,6 @@ public class GroupByStatsProvider
public void spilledBytes(long bytes)
{
spilledBytes.addAndGet(bytes);
}

View File

@ -13752,7 +13752,7 @@ public class GroupByQueryRunnerTest extends InitializedNullHandlingTest
GroupByStatsProvider.AggregateStats aggregateStats = statsProvider.getStatsSince();
Assert.assertEquals(1, aggregateStats.getSpilledQueries());
Assert.assertTrue(aggregateStats.getSpilledBytes() > 0);
Assert.assertEquals(1, aggregateStats.getMergeBufferAcquisitionCount());
Assert.assertEquals(1, aggregateStats.getMergeBufferQueries());
Assert.assertTrue(aggregateStats.getMergeBufferAcquisitionTimeNs() > 0);
if (!skipMergeDictionaryMetric) {
Assert.assertTrue(aggregateStats.getMergeDictionarySize() > 0);

View File

@ -51,7 +51,7 @@ public class GroupByStatsProviderTest
stats2.dictionarySize(400);
GroupByStatsProvider.AggregateStats aggregateStats = statsProvider.getStatsSince();
Assert.assertEquals(0L, aggregateStats.getMergeBufferAcquisitionCount());
Assert.assertEquals(0L, aggregateStats.getMergeBufferQueries());
Assert.assertEquals(0L, aggregateStats.getMergeBufferAcquisitionTimeNs());
Assert.assertEquals(0L, aggregateStats.getSpilledQueries());
Assert.assertEquals(0L, aggregateStats.getSpilledBytes());
@ -61,7 +61,7 @@ public class GroupByStatsProviderTest
statsProvider.closeQuery(id2);
aggregateStats = statsProvider.getStatsSince();
Assert.assertEquals(2, aggregateStats.getMergeBufferAcquisitionCount());
Assert.assertEquals(2, aggregateStats.getMergeBufferQueries());
Assert.assertEquals(1800L, aggregateStats.getMergeBufferAcquisitionTimeNs());
Assert.assertEquals(2L, aggregateStats.getSpilledQueries());
Assert.assertEquals(1600L, aggregateStats.getSpilledBytes());

View File

@ -51,12 +51,12 @@ public class GroupByStatsMonitor extends AbstractMonitor
emitter.emit(builder.setMetric("mergeBuffer/pendingRequests", mergeBufferPool.getPendingRequests()));
emitter.emit(builder.setMetric("mergeBuffer/usedCount", mergeBufferPool.getUsedResourcesCount()));
emitter.emit(builder.setMetric("mergeBuffer/used", mergeBufferPool.getUsedResourcesCount()));
GroupByStatsProvider.AggregateStats statsContainer = groupByStatsProvider.getStatsSince();
if (statsContainer.getMergeBufferAcquisitionCount() > 0) {
emitter.emit(builder.setMetric("mergeBuffer/acquisitionCount", statsContainer.getMergeBufferAcquisitionCount()));
if (statsContainer.getMergeBufferQueries() > 0) {
emitter.emit(builder.setMetric("mergeBuffer/queries", statsContainer.getMergeBufferQueries()));
emitter.emit(builder.setMetric(
"mergeBuffer/acquisitionTimeNs",
statsContainer.getMergeBufferAcquisitionTimeNs()

View File

@ -91,8 +91,8 @@ public class GroupByStatsMonitorTest
));
Assert.assertEquals(7, resultMap.size());
Assert.assertEquals(0, (long) resultMap.get("mergeBuffer/pendingRequests"));
Assert.assertEquals(0, (long) resultMap.get("mergeBuffer/usedCount"));
Assert.assertEquals(1, (long) resultMap.get("mergeBuffer/acquisitionCount"));
Assert.assertEquals(0, (long) resultMap.get("mergeBuffer/used"));
Assert.assertEquals(1, (long) resultMap.get("mergeBuffer/queries"));
Assert.assertEquals(100, (long) resultMap.get("mergeBuffer/acquisitionTimeNs"));
Assert.assertEquals(2, (long) resultMap.get("groupBy/spilledQueries"));
Assert.assertEquals(200, (long) resultMap.get("groupBy/spilledBytes"));
@ -113,7 +113,7 @@ public class GroupByStatsMonitorTest
boolean ret = monitor.doMonitor(emitter);
Assert.assertTrue(ret);
List<Number> numbers = emitter.getMetricValues("mergeBuffer/usedCount", Collections.emptyMap());
List<Number> numbers = emitter.getMetricValues("mergeBuffer/used", Collections.emptyMap());
Assert.assertEquals(1, numbers.size());
Assert.assertEquals(4, numbers.get(0).intValue());
}