Fix to load indices status in batches so that the URL used to call Op…

…enSearch does not exceed maximum length (#21208) (#21269) * fix to load status in batches so that the url does not get too long * partitioning over the length of the names, added changelog * fix calculation error * fix calculation error * switching approach * switching approach * fixing test * refactorings * fix error during refactorings * adding more block types * Fixing max
Graylog2 · Jan 7, 2025 · ca99aa8 · ca99aa8
1 parent da799e6
commit ca99aa8
Show file tree

Hide file tree

Showing 6 changed files with 80 additions and 38 deletions.
diff --git a/changelog/unreleased/pr-21208.toml b/changelog/unreleased/pr-21208.toml
@@ -0,0 +1,4 @@
+type = "f"
+message = "batching request for index block status if the combined length of the indices exceed the max possible URL length "
+
+pulls = ["21208"]
diff --git a/...ge-elasticsearch7/src/main/java/org/graylog/storage/elasticsearch7/IndicesAdapterES7.java b/...ge-elasticsearch7/src/main/java/org/graylog/storage/elasticsearch7/IndicesAdapterES7.java
@@ -109,6 +109,11 @@ public class IndicesAdapterES7 implements IndicesAdapter {
     private final ClusterStateApi clusterStateApi;
     private final IndexTemplateAdapter indexTemplateAdapter;
 
+    // this is the maximum amount of bytes that the index list is supposed to fill in a request,
+    // it assumes that these don't need url encoding. If we exceed the maximum, we request settings for all indices
+    // and filter after wards
+    private final int MAX_INDICES_URL_LENGTH = 3000;
+
     @Inject
     public IndicesAdapterES7(ElasticsearchClient client,
                              StatsApi statsApi,
@@ -405,14 +410,17 @@ public IndicesBlockStatus getIndicesBlocksStatus(final List<String> indices) {
         if (indices == null || indices.isEmpty()) {
             throw new IllegalArgumentException("Expecting list of indices with at least one index present.");
         }
-        final GetSettingsRequest getSettingsRequest = new GetSettingsRequest()
-                .indices(indices.toArray(new String[]{}))
+
+        final GetSettingsRequest request = new GetSettingsRequest()
                 .indicesOptions(IndicesOptions.fromOptions(false, true, true, true))
-                .names(new String[]{});
+                .names("index.blocks.read", "index.blocks.write", "index.blocks.metadata", "index.blocks.read_only", "index.blocks.read_only_allow_delete");
+
+        final var maxLengthExceeded = String.join(",", indices).length() > MAX_INDICES_URL_LENGTH;
+        final GetSettingsRequest getSettingsRequest = maxLengthExceeded ? request : request.indices(indices.toArray(new String[]{}));
 
         return client.execute((c, requestOptions) -> {
             final GetSettingsResponse settingsResponse = c.indices().getSettings(getSettingsRequest, requestOptions);
-            return BlockSettingsParser.parseBlockSettings(settingsResponse);
+            return BlockSettingsParser.parseBlockSettings(settingsResponse, maxLengthExceeded ? Optional.of(indices) : Optional.empty());
         });
     }
 

diff --git a/...csearch7/src/main/java/org/graylog/storage/elasticsearch7/blocks/BlockSettingsParser.java b/...csearch7/src/main/java/org/graylog/storage/elasticsearch7/blocks/BlockSettingsParser.java
@@ -21,6 +21,9 @@
 import org.graylog.shaded.elasticsearch7.org.elasticsearch.common.settings.Settings;
 import org.graylog2.indexer.indices.blocks.IndicesBlockStatus;
 
+import java.util.Arrays;
+import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -29,23 +32,31 @@ public class BlockSettingsParser {
     static final String BLOCK_SETTINGS_PREFIX = "index.blocks.";
 
     public static IndicesBlockStatus parseBlockSettings(final GetSettingsResponse settingsResponse) {
-        IndicesBlockStatus result = new IndicesBlockStatus();
+        return parseBlockSettings(settingsResponse, Optional.empty());
+    }
+
+    public static IndicesBlockStatus parseBlockSettings(final GetSettingsResponse settingsResponse, final Optional<List<String>> indices) {
+        final IndicesBlockStatus result = new IndicesBlockStatus();
         final ImmutableOpenMap<String, Settings> indexToSettingsMap = settingsResponse.getIndexToSettings();
         final String[] indicesInResponse = indexToSettingsMap.keys().toArray(String.class);
-        for (String index : indicesInResponse) {
-            final Settings blockSettings = indexToSettingsMap.get(index).getByPrefix(BLOCK_SETTINGS_PREFIX);
-
-            if (!blockSettings.isEmpty()) {
-                final Set<String> blockSettingsNames = blockSettings.names();
-                final Set<String> blockSettingsSetToTrue = blockSettingsNames.stream()
-                        .filter(s -> blockSettings.getAsBoolean(s, false))
-                        .map(s -> BLOCK_SETTINGS_PREFIX + s)
-                        .collect(Collectors.toSet());
-                if (!blockSettingsSetToTrue.isEmpty()) {
-                    result.addIndexBlocks(index, blockSettingsSetToTrue);
+
+        indices.orElse(Arrays.stream(indicesInResponse).toList()).forEach(index -> {
+            final var settings = indexToSettingsMap.get(index);
+            if(settings != null) {
+                final Settings blockSettings = settings.getByPrefix(BLOCK_SETTINGS_PREFIX);
+
+                if (!blockSettings.isEmpty()) {
+                    final Set<String> blockSettingsNames = blockSettings.names();
+                    final Set<String> blockSettingsSetToTrue = blockSettingsNames.stream()
+                            .filter(s -> blockSettings.getAsBoolean(s, false))
+                            .map(s -> BLOCK_SETTINGS_PREFIX + s)
+                            .collect(Collectors.toSet());
+                    if (!blockSettingsSetToTrue.isEmpty()) {
+                        result.addIndexBlocks(index, blockSettingsSetToTrue);
+                    }
                 }
             }
-        }
+        });
 
         return result;
     }

diff --git a/...-storage-opensearch2/src/main/java/org/graylog/storage/opensearch2/IndicesAdapterOS2.java b/...-storage-opensearch2/src/main/java/org/graylog/storage/opensearch2/IndicesAdapterOS2.java
@@ -109,6 +109,11 @@ public class IndicesAdapterOS2 implements IndicesAdapter {
     private final ClusterStateApi clusterStateApi;
     private final IndexTemplateAdapter indexTemplateAdapter;
 
+    // this is the maximum amount of bytes that the index list is supposed to fill in a request,
+    // it assumes that these don't need url encoding. If we exceed the maximum, we request settings for all indices
+    // and filter after wards
+    private final int MAX_INDICES_URL_LENGTH = 3000;
+
     @Inject
     public IndicesAdapterOS2(OpenSearchClient client,
                              StatsApi statsApi,
@@ -399,19 +404,23 @@ public List<ShardsInfo> getShardsInfo(String indexName) {
         return catApi.getShardsInfo(indexName);
     }
 
+
     @Override
     public IndicesBlockStatus getIndicesBlocksStatus(final List<String> indices) {
         if (indices == null || indices.isEmpty()) {
             throw new IllegalArgumentException("Expecting list of indices with at least one index present.");
         }
-        final GetSettingsRequest getSettingsRequest = new GetSettingsRequest()
-                .indices(indices.toArray(new String[]{}))
+
+        final GetSettingsRequest request = new GetSettingsRequest()
                 .indicesOptions(IndicesOptions.fromOptions(false, true, true, true))
-                .names(new String[]{});
+                .names("index.blocks.read", "index.blocks.write", "index.blocks.metadata", "index.blocks.read_only", "index.blocks.read_only_allow_delete");
+
+        final var maxLengthExceeded = String.join(",", indices).length() > MAX_INDICES_URL_LENGTH;
+        final GetSettingsRequest getSettingsRequest = maxLengthExceeded ? request : request.indices(indices.toArray(new String[]{}));
 
         return client.execute((c, requestOptions) -> {
             final GetSettingsResponse settingsResponse = c.indices().getSettings(getSettingsRequest, requestOptions);
-            return BlockSettingsParser.parseBlockSettings(settingsResponse);
+            return BlockSettingsParser.parseBlockSettings(settingsResponse, maxLengthExceeded ? Optional.of(indices) : Optional.empty());
         });
     }
 

diff --git a/...opensearch2/src/main/java/org/graylog/storage/opensearch2/blocks/BlockSettingsParser.java b/...opensearch2/src/main/java/org/graylog/storage/opensearch2/blocks/BlockSettingsParser.java
@@ -20,6 +20,8 @@
 import org.graylog.shaded.opensearch2.org.opensearch.action.admin.indices.settings.get.GetSettingsResponse;
 import org.graylog.shaded.opensearch2.org.opensearch.common.settings.Settings;
 
+import java.util.List;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -28,23 +30,30 @@ public class BlockSettingsParser {
     static final String BLOCK_SETTINGS_PREFIX = "index.blocks.";
 
     public static IndicesBlockStatus parseBlockSettings(final GetSettingsResponse settingsResponse) {
-        IndicesBlockStatus result = new IndicesBlockStatus();
+        return parseBlockSettings(settingsResponse, Optional.empty());
+    }
+
+    public static IndicesBlockStatus parseBlockSettings(final GetSettingsResponse settingsResponse, final Optional<List<String>> indices) {
+        final IndicesBlockStatus result = new IndicesBlockStatus();
         final var indexToSettingsMap = settingsResponse.getIndexToSettings();
-        final String[] indicesInResponse = indexToSettingsMap.keySet().toArray(new String[0]);
-        for (String index : indicesInResponse) {
-            final Settings blockSettings = indexToSettingsMap.get(index).getByPrefix(BLOCK_SETTINGS_PREFIX);
-
-            if (!blockSettings.isEmpty()) {
-                final Set<String> blockSettingsNames = blockSettings.names();
-                final Set<String> blockSettingsSetToTrue = blockSettingsNames.stream()
-                        .filter(s -> blockSettings.getAsBoolean(s, false))
-                        .map(s -> BLOCK_SETTINGS_PREFIX + s)
-                        .collect(Collectors.toSet());
-                if (!blockSettingsSetToTrue.isEmpty()) {
-                    result.addIndexBlocks(index, blockSettingsSetToTrue);
+
+        indices.orElse(indexToSettingsMap.keySet().stream().toList()).forEach(index -> {
+            final var settings = indexToSettingsMap.get(index);
+            if(settings != null) {
+                final Settings blockSettings = settings.getByPrefix(BLOCK_SETTINGS_PREFIX);
+
+                if (!blockSettings.isEmpty()) {
+                    final Set<String> blockSettingsNames = blockSettings.names();
+                    final Set<String> blockSettingsSetToTrue = blockSettingsNames.stream()
+                            .filter(s -> blockSettings.getAsBoolean(s, false))
+                            .map(s -> BLOCK_SETTINGS_PREFIX + s)
+                            .collect(Collectors.toSet());
+                    if (!blockSettingsSetToTrue.isEmpty()) {
+                        result.addIndexBlocks(index, blockSettingsSetToTrue);
+                    }
                 }
             }
-        }
+        });
 
         return result;
     }

diff --git a/...search2/src/test/java/org/graylog/storage/opensearch2/blocks/BlockSettingsParserTest.java b/...search2/src/test/java/org/graylog/storage/opensearch2/blocks/BlockSettingsParserTest.java
@@ -23,6 +23,7 @@
 
 import java.util.Collection;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 
 import static org.junit.Assert.assertEquals;
@@ -35,7 +36,7 @@ public class BlockSettingsParserTest {
     @Test
     public void noBlockedIndicesIdentifiedIfEmptyResponseParsed() {
         GetSettingsResponse emptyResponse = new GetSettingsResponse(Map.of(), Map.of());
-        final IndicesBlockStatus indicesBlockStatus = BlockSettingsParser.parseBlockSettings(emptyResponse);
+        final IndicesBlockStatus indicesBlockStatus = BlockSettingsParser.parseBlockSettings(emptyResponse, Optional.empty());
         assertNotNull(indicesBlockStatus);
         assertEquals(0, indicesBlockStatus.countBlockedIndices());
     }
@@ -44,7 +45,7 @@ public void noBlockedIndicesIdentifiedIfEmptyResponseParsed() {
     public void noBlockedIndicesIdentifiedIfEmptySettingsPresent() {
         var settingsBuilder = Map.of("index_0", Settings.builder().build());
         GetSettingsResponse emptySettingsResponse = new GetSettingsResponse(settingsBuilder, Map.of());
-        final IndicesBlockStatus indicesBlockStatus = BlockSettingsParser.parseBlockSettings(emptySettingsResponse);
+        final IndicesBlockStatus indicesBlockStatus = BlockSettingsParser.parseBlockSettings(emptySettingsResponse, Optional.empty());
         assertNotNull(indicesBlockStatus);
         assertEquals(0, indicesBlockStatus.countBlockedIndices());
     }
@@ -64,7 +65,7 @@ public void parserProperlyResponseWithMultipleIndicesWithDifferentBlockSettings(
                         .put("index.blocks.read_only_allow_delete", true)
                         .build());
         GetSettingsResponse settingsResponse = new GetSettingsResponse(settingsBuilder, Map.of());
-        final IndicesBlockStatus indicesBlockStatus = BlockSettingsParser.parseBlockSettings(settingsResponse);
+        final IndicesBlockStatus indicesBlockStatus = BlockSettingsParser.parseBlockSettings(settingsResponse, Optional.empty());
         assertNotNull(indicesBlockStatus);
         assertEquals(3, indicesBlockStatus.countBlockedIndices());
         final Set<String> blockedIndices = indicesBlockStatus.getBlockedIndices();