Browse Source

Use MB rather than GB to calculate max boolean clauses (#90309)

Calling JvmStats.jvmStats().getMem().getHeapMax().getGb() rounds down
to 0 if the heap is less than 1Gb, which yields a max boolean clause of only
1024. Calculating using Mb instead will give a more proportionate value.

Fixes #86136
Alan Woodward 3 years ago
parent
commit
9d11e2168e

+ 6 - 0
docs/changelog/90309.yaml

@@ -0,0 +1,6 @@
+pr: 90309
+summary: Use MB rather than GB to calculate max boolean clauses
+area: Search
+type: bug
+issues:
+ - 86136

+ 7 - 7
server/src/main/java/org/elasticsearch/search/SearchUtils.java

@@ -17,22 +17,22 @@ public final class SearchUtils {
 
     public static int calculateMaxClauseValue(ThreadPool threadPool) {
         int searchThreadPoolSize = threadPool.info(ThreadPool.Names.SEARCH).getMax();
-        long heapSize = JvmStats.jvmStats().getMem().getHeapMax().getGb();
+        long heapSize = JvmStats.jvmStats().getMem().getHeapMax().getMb();
         return calculateMaxClauseValue(searchThreadPoolSize, heapSize);
     }
 
-    static int calculateMaxClauseValue(long threadPoolSize, double heapInGb) {
-        if (threadPoolSize <= 0 || heapInGb <= 0) {
+    static int calculateMaxClauseValue(long threadPoolSize, long heapInMb) {
+        if (threadPoolSize <= 0 || heapInMb <= 0) {
             return DEFAULT_MAX_CLAUSE_COUNT;
         }
         // In a worst-case scenario, each clause may end up using up to 16k of memory
         // to load postings, positions, offsets, impacts, etc. So we calculate the
         // maximum number of clauses we can support in a single thread pool by
-        // dividing the heap by 16k (or the equivalent, multiplying the heap in GB by
-        // 64k), and then divide that by the number of possible concurrent search
+        // dividing the heap by 16k (or the equivalent, multiplying the heap in MB by
+        // 64), and then divide that by the number of possible concurrent search
         // threads.
-        int maxClauseCount = (int) (heapInGb * 65_536 / threadPoolSize);
-        return Math.max(DEFAULT_MAX_CLAUSE_COUNT, maxClauseCount);
+        long maxClauseCount = (heapInMb * 64 / threadPoolSize);
+        return Math.max(DEFAULT_MAX_CLAUSE_COUNT, (int) maxClauseCount);
     }
 
     private SearchUtils() {}

+ 5 - 5
server/src/test/java/org/elasticsearch/search/SearchUtilsTests.java

@@ -15,19 +15,19 @@ public class SearchUtilsTests extends ESTestCase {
     public void testConfigureMaxClauses() {
 
         // Heap below 1 Gb
-        assertEquals(8192, SearchUtils.calculateMaxClauseValue(4, 0.5));
+        assertEquals(16368, SearchUtils.calculateMaxClauseValue(4, 1023));
 
         // Number of processors not available
-        assertEquals(1024, SearchUtils.calculateMaxClauseValue(-1, 1));
+        assertEquals(1024, SearchUtils.calculateMaxClauseValue(-1, 1024));
 
         // Insanely high configured search thread pool size
-        assertEquals(1024, SearchUtils.calculateMaxClauseValue(1024, 1));
+        assertEquals(1024, SearchUtils.calculateMaxClauseValue(1024, 1024));
 
         // 1Gb heap, 8 processors
-        assertEquals(5041, SearchUtils.calculateMaxClauseValue(13, 1));
+        assertEquals(5041, SearchUtils.calculateMaxClauseValue(13, 1024));
 
         // 30Gb heap, 48 processors
-        assertEquals(26932, SearchUtils.calculateMaxClauseValue(73, 30));
+        assertEquals(26932, SearchUtils.calculateMaxClauseValue(73, 30 * 1024));
     }
 
 }