Fix withAutomaticUnit

Author: prdoyle

server/src/main/java/org/elasticsearch/common/unit/ByteSizeValue.java

@@ -56,17 +56,23 @@ public static ByteSizeValue of(long size, ByteSizeUnit unit) {
         return new ByteSizeValue(unit.toBytes(size), unit);
     }
 
+    /**
+     * @return {@link ByteSizeValue} whose {@link #preferredUnit} is a reasonable one for human consumption.
+     */
     public static ByteSizeValue withAutomaticUnit(long sizeInBytes) {
         if (sizeInBytes == 0) {
             return ZERO;
         }
-        for (int ordinal = ByteSizeUnit.values().length-1; ordinal >= 0; --ordinal) {
+
+        // We pick a unit such that sizeInBytes is a multiple of 1/4 of that unit.
+        // That preserves the exact given number of bytes without using more than 2 decimal places.
+        for (int ordinal = ByteSizeUnit.values().length - 1; ordinal >= 0; --ordinal) {
             ByteSizeUnit candidateUnit = ByteSizeUnit.values()[ordinal];
             if (candidateUnit == BYTES) {
                 // We handle this using ofBytes below
                 continue;
             }
-            if (sizeInBytes % candidateUnit.toBytes(1) == 0) {
+            if (sizeInBytes % (candidateUnit.toBytes(1) / 4) == 0) {
                 return new ByteSizeValue(sizeInBytes, candidateUnit);
             }
         }

server/src/test/java/org/elasticsearch/common/unit/ByteSizeValueTests.java

@@ -9,8 +9,6 @@
 
 package org.elasticsearch.common.unit;
 
-import com.carrotsearch.randomizedtesting.annotations.Repeat;
-
 import org.elasticsearch.ElasticsearchParseException;
 import org.elasticsearch.common.io.stream.Writeable.Reader;
 import org.elasticsearch.test.AbstractWireSerializingTestCase;
@@ -32,7 +30,6 @@
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 
-@Repeat(iterations = 1000)
 public class ByteSizeValueTests extends AbstractWireSerializingTestCase<ByteSizeValue> {
     public void testActualPeta() {
         MatcherAssert.assertThat(ByteSizeValue.of(4, PB).getBytes(), equalTo(4503599627370496L));
@@ -569,12 +566,34 @@ private static String percentToDecimal(int percent) {
     }
 
     public void testWithAutomaticUnit() {
-        long kibi = 1024;
-        long mebi = kibi * kibi;
-        assertEquals(new ByteSizeValue(kibi-1, BYTES), ByteSizeValue.withAutomaticUnit(kibi-1));
-        assertEquals(new ByteSizeValue(kibi, KB), ByteSizeValue.withAutomaticUnit(kibi));
-        assertEquals(new ByteSizeValue(kibi+1, BYTES), ByteSizeValue.withAutomaticUnit(kibi+1));
-        assertEquals(new ByteSizeValue(mebi-kibi, KB), ByteSizeValue.withAutomaticUnit(mebi-kibi));
+        for (var unit : ByteSizeUnit.values()) {
+            if (unit == BYTES) {
+                continue;
+            }
+            long bytes = unit.toBytes(1);
+            for (int numFourths = 1; numFourths <= 8; numFourths++) {
+                checkAutomaticUnitWithCornerCases(bytes * numFourths / 4, unit);
+            }
+        }
+    }
+
+    private void checkAutomaticUnitWithCornerCases(long sizeInBytes, ByteSizeUnit expectedUnit) {
+        assertEquals(
+            sizeInBytes + " should use " + expectedUnit,
+            new ByteSizeValue(sizeInBytes, expectedUnit),
+            ByteSizeValue.withAutomaticUnit(sizeInBytes)
+        );
+        // Plus or minus one byte, it should revert to bytes
+        assertEquals(
+            sizeInBytes - 1 + " should use " + BYTES,
+            new ByteSizeValue(sizeInBytes - 1, BYTES),
+            ByteSizeValue.withAutomaticUnit(sizeInBytes - 1)
+        );
+        assertEquals(
+            sizeInBytes + 1 + " should use " + BYTES,
+            new ByteSizeValue(sizeInBytes + 1, BYTES),
+            ByteSizeValue.withAutomaticUnit(sizeInBytes + 1)
+        );
     }
 
     @Override