jtuglu1 commented on code in PR #18813:
URL: https://github.com/apache/druid/pull/18813#discussion_r2600712935
##########
extensions-contrib/spectator-histogram/src/main/java/org/apache/druid/spectator/histogram/SpectatorHistogramBufferAggregator.java:
##########
@@ -62,70 +57,41 @@ public void aggregate(ByteBuffer buffer, int position)
if (obj == null) {
return;
}
- SpectatorHistogram counts = histogramCache.get(buffer).get(position);
- if (obj instanceof SpectatorHistogram) {
- SpectatorHistogram other = (SpectatorHistogram) obj;
- counts.merge(other);
- } else if (obj instanceof Number) {
- counts.insert((Number) obj);
- } else {
- throw new IAE(
- "Expected a number or a long[], but received [%s] of type [%s]",
- obj,
- obj.getClass()
- );
- }
+ SpectatorHistogram counts = innerAggregator.get(buffer, position);
+ innerAggregator.merge(counts, obj);
}
@Override
public Object get(final ByteBuffer buffer, final int position)
{
- // histogramCache is an IdentityHashMap where the reference of buffer is
used for equality checks.
- // So the returned object isn't impacted by the changes in the buffer
object made by concurrent threads.
-
- SpectatorHistogram spectatorHistogram =
histogramCache.get(buffer).get(position);
- if (spectatorHistogram.isEmpty()) {
+ SpectatorHistogram histo = innerAggregator.get(buffer, position);
+ if (histo.isEmpty()) {
Review Comment:
I think that seems reasonable – it should get cleaned up both at
relocate/end. Not sure if we can do a read prior to any writes being committed
into the histogram? cc @bsyk
--
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]
