Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix to load indices status in batches so that the URL used to call OpenSearch does not exceed maximum length (6.0) #21269

Merged
merged 1 commit into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog/unreleased/pr-21208.toml
Original file line number Diff line number Diff line change
@@ -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"]
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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());
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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());
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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());
}
Expand All @@ -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());
}
Expand All @@ -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();
Expand Down
Loading