feat(histogram): add meaningful exception when histograms add buckets…

… with different sizes (#1880) * Add meaningful exception when histograms adding buckets with different sizes. * Update memory/src/main/scala/filodb.memory/format/vectors/Histogram.scala Co-authored-by: yu-shipit <[email protected]> --------- Co-authored-by: Brian-Yu <[email protected]> Co-authored-by: yu-shipit <[email protected]>
filodb · Nov 12, 2024 · be75fb4 · be75fb4
1 parent b7455b0
commit be75fb4
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 1 deletion.
diff --git a/memory/src/main/scala/filodb.memory/format/vectors/Histogram.scala b/memory/src/main/scala/filodb.memory/format/vectors/Histogram.scala
@@ -154,7 +154,12 @@ final case class LongHistogram(buckets: HistogramBuckets, values: Array[Long]) e
    * Adds the buckets from other into this LongHistogram
    */
   final def add(other: LongHistogram): Unit = {
-    assert(other.buckets == buckets)
+    if (other.buckets != buckets) {
+      throw new IllegalArgumentException(
+           s"Mismatch in bucket sizes. Cannot add histograms with different bucket configurations. " +
+             s"Expected: ${buckets}, Found: ${other.buckets}"
+      )
+    }
     cforRange { 0 until numBuckets } { b =>
       values(b) += other.values(b)
     }

diff --git a/memory/src/test/scala/filodb.memory/format/vectors/HistogramTest.scala b/memory/src/test/scala/filodb.memory/format/vectors/HistogramTest.scala
@@ -62,6 +62,25 @@ class HistogramTest extends NativeVectorTest {
   }
 
   describe("Histogram") {
+      it("should add two histograms with the same bucket scheme correctly") {
+        val hist1 = LongHistogram(bucketScheme, Array(1, 2, 3, 4, 5, 6, 7, 8))
+        val hist2 = LongHistogram(bucketScheme, Array(2, 4, 6, 8, 10, 12, 14, 18))
+        hist1.add(hist2)
+
+        hist1.values shouldEqual Array(3, 6, 9, 12, 15, 18, 21, 26)
+      }
+
+      it("should not add histograms with different bucket schemes") {
+        val hist1 = LongHistogram(bucketScheme, Array(1, 2, 3, 4, 5, 6, 7, 8))
+        val histWithDifferentBuckets = LongHistogram(customScheme, Array(1, 2, 3, 4, 5, 6, 7))
+
+        val thrown = intercept[IllegalArgumentException] {
+          hist1.add(histWithDifferentBuckets)
+        }
+
+        thrown.getMessage shouldEqual s"Mismatch in bucket sizes. Cannot add histograms with different bucket configurations. " +
+          s"Expected: ${hist1.buckets}, Found: ${histWithDifferentBuckets.buckets}"
+      }
     it("should calculate quantile correctly") {
       mutableHistograms.zip(quantile50Result).foreach { case (h, res) =>
         val quantile = h.quantile(0.50)