HLLSketchBuild: Intermittent errors at query time

[gianm]

Affected Version

Observed in Druid 0.13.0-incubating-iap9, an Imply build, but one that has no differences from 0.13.0-incubating in the extensions-core/datasketches tree. Btw, both use sketches-core 0.10.3.

Description

I ran this query 200 times and saw two errors on historicals:

{
  "queryType": "topN",
  "dataSource": "my-ds-name",
  "intervals": [
    "2019-03-01T22:45:26.000Z/2019-03-08T22:45:26.000Z"
  ],
  "granularity": {
    "type": "period",
    "period": "P1W",
    "origin": "2019-03-01T22:45:26.000Z"
  },
  "dimension": "geo.city",
  "threshold": 200,
  "metric": "distinctUserId",
  "aggregations": [
    {
      "name": "distinctUserId",
      "type": "HLLSketchBuild",
      "fieldName": "userId",
      "lgK": 12,
      "tgtHllType": "HLL_4"
    }
  ]
}

The first was:

java.lang.NullPointerException
        at com.yahoo.sketches.hll.DirectHll4Array$DirectHll4Iterator.value(DirectHll4Array.java:173) ~[?:?]
        at com.yahoo.sketches.hll.HllPairIterator.nextValid(HllPairIterator.java:75) ~[?:?]
        at com.yahoo.sketches.hll.Union.unionImpl(Union.java:374) ~[?:?]
        at com.yahoo.sketches.hll.Union.update(Union.java:264) ~[?:?]
        at org.apache.druid.query.aggregation.datasketches.hll.HllSketchAggregatorFactory.combine(HllSketchAggregatorFactory.java:123) ~[?:?]
        at org.apache.druid.query.aggregation.datasketches.hll.HllSketchBuildAggregatorFactory.combine(HllSketchBuildAggregatorFactory.java:40) ~[?:?]
        at org.apache.druid.query.aggregation.datasketches.hll.HllSketchAggregatorFactory.combine(HllSketchAggregatorFactory.java:44) ~[?:?]
        at org.apache.druid.query.topn.TopNBinaryFn.apply(TopNBinaryFn.java:103) ~[druid-processing-0.13.0-incubating-iap9.jar:0.13.0-incubating-iap9]

And the second was:

java.lang.NegativeArraySizeException
        at com.yahoo.sketches.hll.Hll4Array.<init>(Hll4Array.java:32) ~[?:?]
        at com.yahoo.sketches.hll.Conversions.convertToHll4(Conversions.java:20) ~[?:?]
        at com.yahoo.sketches.hll.AbstractHllArray.copyAs(AbstractHllArray.java:34) ~[?:?]
        at com.yahoo.sketches.hll.AbstractHllArray.copyAs(AbstractHllArray.java:17) ~[?:?]
        at com.yahoo.sketches.hll.HllSketch.copyAs(HllSketch.java:241) ~[?:?]
        at com.yahoo.sketches.hll.Union.getResult(Union.java:179) ~[?:?]
        at org.apache.druid.query.aggregation.datasketches.hll.HllSketchAggregatorFactory.combine(HllSketchAggregatorFactory.java:124) ~[?:?]
        at org.apache.druid.query.aggregation.datasketches.hll.HllSketchBuildAggregatorFactory.combine(HllSketchBuildAggregatorFactory.java:40) ~[?:?]
        at org.apache.druid.query.aggregation.datasketches.hll.HllSketchAggregatorFactory.combine(HllSketchAggregatorFactory.java:44) ~[?:?]
        at org.apache.druid.query.topn.TopNBinaryFn.apply(TopNBinaryFn.java:103) ~[druid-processing-0.13.0-incubating-iap9.jar:0.13.0-incubating-iap9]

[gianm]

I think this is happening because HllSketchBuildBufferAggregator's get(buf, position) method does not copy data out from buf to the heap. topN and groupBy v2 (which works similarly) both call get on BufferAggregators from segment processing threads, then transfer those objects to another thread for merging (in this case, TopNBinaryFn is doing that merge). Those objects cannot reference the underlying buffer, because if they do, they may find themselves reading garbage as the underlying buffer is reused.

We should update the javadocs for BufferAggregator to make this clear and audit existing implementations to make sure they aren't referencing the underlying buffers.

@AlexanderSaydakov @leerho have either of you seen something like this in one of your prod deployments? I would expect that you should if you are using topNs or groupBy v2's.

[leerho]

@gianm

We have not. Nevertheless, I'm sure you know this, but these sketches are not thread-safe. I hope that @jon-wei 's PR will solve this. :)