Merge pull request #18355 from mikemccand/iterables_flatten

Iterables.flatten should not pre-cache the first iterator

core/src/main/java/org/elasticsearch/common/util/iterable/Iterables.java

@@ -54,6 +54,8 @@ public class Iterables {
         }
     }
 
+    /** Flattens the two level {@code Iterable} into a single {@code Iterable}.  Note that this pre-caches the values from the outer {@code
+     *  Iterable}, but not the values from the inner one. */
     public static <T> Iterable<T> flatten(Iterable<? extends Iterable<T>> inputs) {
         Objects.requireNonNull(inputs);
         return new FlattenedIterables<>(inputs);

core/src/main/java/org/elasticsearch/indices/IndexingMemoryController.java

@@ -93,7 +93,7 @@ public class IndexingMemoryController extends AbstractComponent implements Index
 
     private final ShardsIndicesStatusChecker statusChecker;
 
-    IndexingMemoryController(Settings settings, ThreadPool threadPool, Iterable<IndexShard>indexServices) {
+    IndexingMemoryController(Settings settings, ThreadPool threadPool, Iterable<IndexShard> indexServices) {
         this(settings, threadPool, indexServices, JvmInfo.jvmInfo().getMem().getHeapMax().bytes());
     }
 

core/src/main/java/org/elasticsearch/indices/IndicesService.java

@@ -181,7 +181,9 @@ public class IndicesService extends AbstractLifecycleComponent<IndicesService> i
         this.namedWriteableRegistry = namedWriteableRegistry;
         clusterSettings.addSettingsUpdateConsumer(IndexStoreConfig.INDICES_STORE_THROTTLE_TYPE_SETTING, indexStoreConfig::setRateLimitingType);
         clusterSettings.addSettingsUpdateConsumer(IndexStoreConfig.INDICES_STORE_THROTTLE_MAX_BYTES_PER_SEC_SETTING, indexStoreConfig::setRateLimitingThrottle);
-        indexingMemoryController = new IndexingMemoryController(settings, threadPool, Iterables.flatten(this));
+        indexingMemoryController = new IndexingMemoryController(settings, threadPool,
+                                                                // ensure we pull an iter with new shards - flatten makes a copy
+                                                                () -> Iterables.flatten(this).iterator());
         this.indexScopeSetting = indexScopedSettings;
         this.circuitBreakerService = circuitBreakerService;
         this.indicesFieldDataCache = new IndicesFieldDataCache(settings, new IndexFieldDataCache.Listener() {

core/src/test/java/org/elasticsearch/common/util/iterable/IterablesTests.java

@@ -19,12 +19,14 @@
 
 package org.elasticsearch.common.util.iterable;
 
-import org.elasticsearch.test.ESTestCase;
-
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Iterator;
+import java.util.List;
 import java.util.NoSuchElementException;
 
+import org.elasticsearch.test.ESTestCase;
+
 import static org.hamcrest.object.HasToString.hasToString;
 
 public class IterablesTests extends ESTestCase {
@@ -56,6 +58,34 @@ public class IterablesTests extends ESTestCase {
         test(iterable);
     }
 
+    public void testFlatten() {
+        List<List<Integer>> list = new ArrayList<>();
+        list.add(new ArrayList<>());
+
+        Iterable<Integer> allInts = Iterables.flatten(list);
+        int count = 0;
+        for(int x : allInts) {
+            count++;
+        }
+        assertEquals(0, count);
+        list.add(new ArrayList<>());
+        list.get(1).add(0);
+
+        // changes to the outer list are not seen since flatten pre-caches outer list on init:
+        count = 0;
+        for(int x : allInts) {
+            count++;
+        }
+        assertEquals(0, count);
+
+        // but changes to the original inner lists are seen:
+        list.get(0).add(0);
+        for(int x : allInts) {
+            count++;
+        }
+        assertEquals(1, count);
+    }
+
     private void test(Iterable<String> iterable) {
         try {
             Iterables.get(iterable, -1);
@@ -73,4 +103,4 @@ public class IterablesTests extends ESTestCase {
             assertThat(e, hasToString("java.lang.IndexOutOfBoundsException: 3"));
         }
     }
-}
+}