Skip to content

Commit

Permalink
Remove unnecessary bwc from get-aliases API
Browse files Browse the repository at this point in the history 
Dropping support for pre-8.12 requests from remote nodes, and also
cleaning up some unnecessary abstraction in the request builder
hierarchy.

Relates elastic#107984 (drops some unnecessary trappy timeouts)
  • Loading branch information
@DaveCTurner
DaveCTurner committed Sep 12, 2024
1 parent 6d18607 commit 7428245
Show file tree
Hide file tree
Showing 11 changed files with 61 additions and 142 deletions.
2 changes: 1 addition & 1 deletion ...data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ public void testResolvabilityOfDataStreamsInAPIs() throws Exception {
verifyResolvability(dataStreamName, indicesAdmin().prepareForceMerge(dataStreamName), false);
verifyResolvability(dataStreamName, indicesAdmin().prepareValidateQuery(dataStreamName), false);
verifyResolvability(dataStreamName, indicesAdmin().prepareRecoveries(dataStreamName), false);
verifyResolvability(dataStreamName, indicesAdmin().prepareGetAliases("dummy").addIndices(dataStreamName), false);
verifyResolvability(dataStreamName, indicesAdmin().prepareGetAliases("dummy").setIndices(dataStreamName), false);
verifyResolvability(dataStreamName, indicesAdmin().prepareGetFieldMappings(dataStreamName), false);
verifyResolvability(dataStreamName, indicesAdmin().preparePutMapping(dataStreamName).setSource("""
{"_doc":{"properties": {"my_field":{"type":"keyword"}}}}""", XContentType.JSON), false);
Expand Down
38 changes: 19 additions & 19 deletions server/src/internalClusterTest/java/org/elasticsearch/aliases/IndexAliasesIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -868,7 +868,7 @@ public void testIndicesGetAliases() throws Exception {
);

logger.info("--> getting bar and baz for index bazbar");
getResponse = indicesAdmin().prepareGetAliases("bar", "bac").addIndices("bazbar").get();
getResponse = indicesAdmin().prepareGetAliases("bar", "bac").setIndices("bazbar").get();
assertThat(getResponse, notNullValue());
assertThat(getResponse.getAliases().size(), equalTo(1));
assertThat(getResponse.getAliases().get("bazbar").size(), equalTo(2));
Expand All @@ -886,11 +886,11 @@ public void testIndicesGetAliases() throws Exception {
assertThat(getResponse.getAliases().get("bazbar").get(1).getSearchRouting(), nullValue());
assertFalse(indicesAdmin().prepareGetAliases("bar").get().getAliases().isEmpty());
assertFalse(indicesAdmin().prepareGetAliases("bac").get().getAliases().isEmpty());
assertFalse(indicesAdmin().prepareGetAliases("bar").addIndices("bazbar").get().getAliases().isEmpty());
assertFalse(indicesAdmin().prepareGetAliases("bac").addIndices("bazbar").get().getAliases().isEmpty());
assertFalse(indicesAdmin().prepareGetAliases("bar").setIndices("bazbar").get().getAliases().isEmpty());
assertFalse(indicesAdmin().prepareGetAliases("bac").setIndices("bazbar").get().getAliases().isEmpty());

logger.info("--> getting *b* for index baz*");
getResponse = indicesAdmin().prepareGetAliases("*b*").addIndices("baz*").get();
getResponse = indicesAdmin().prepareGetAliases("*b*").setIndices("baz*").get();
assertThat(getResponse, notNullValue());
assertThat(getResponse.getAliases().size(), equalTo(1));
assertThat(getResponse.getAliases().get("bazbar").size(), equalTo(2));
Expand All @@ -906,10 +906,10 @@ public void testIndicesGetAliases() throws Exception {
assertThat(getResponse.getAliases().get("bazbar").get(1).getFilter(), nullValue());
assertThat(getResponse.getAliases().get("bazbar").get(1).getIndexRouting(), nullValue());
assertThat(getResponse.getAliases().get("bazbar").get(1).getSearchRouting(), nullValue());
assertFalse(indicesAdmin().prepareGetAliases("*b*").addIndices("baz*").get().getAliases().isEmpty());
assertFalse(indicesAdmin().prepareGetAliases("*b*").setIndices("baz*").get().getAliases().isEmpty());

logger.info("--> getting *b* for index *bar");
getResponse = indicesAdmin().prepareGetAliases("b*").addIndices("*bar").get();
getResponse = indicesAdmin().prepareGetAliases("b*").setIndices("*bar").get();
assertThat(getResponse, notNullValue());
assertThat(getResponse.getAliases().size(), equalTo(2));
assertThat(getResponse.getAliases().get("bazbar").size(), equalTo(2));
Expand All @@ -930,22 +930,22 @@ public void testIndicesGetAliases() throws Exception {
assertThat(getResponse.getAliases().get("foobar").get(0).getFilter(), nullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).getIndexRouting(), equalTo("bla"));
assertThat(getResponse.getAliases().get("foobar").get(0).getSearchRouting(), equalTo("bla"));
assertFalse(indicesAdmin().prepareGetAliases("b*").addIndices("*bar").get().getAliases().isEmpty());
assertFalse(indicesAdmin().prepareGetAliases("b*").setIndices("*bar").get().getAliases().isEmpty());

logger.info("--> getting f* for index *bar");
getResponse = indicesAdmin().prepareGetAliases("f*").addIndices("*bar").get();
getResponse = indicesAdmin().prepareGetAliases("f*").setIndices("*bar").get();
assertThat(getResponse, notNullValue());
assertThat(getResponse.getAliases().size(), equalTo(1));
assertThat(getResponse.getAliases().get("foobar").get(0), notNullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).alias(), equalTo("foo"));
assertThat(getResponse.getAliases().get("foobar").get(0).getFilter(), nullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).getIndexRouting(), nullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).getSearchRouting(), nullValue());
assertFalse(indicesAdmin().prepareGetAliases("f*").addIndices("*bar").get().getAliases().isEmpty());
assertFalse(indicesAdmin().prepareGetAliases("f*").setIndices("*bar").get().getAliases().isEmpty());

// alias at work
logger.info("--> getting f* for index *bac");
getResponse = indicesAdmin().prepareGetAliases("foo").addIndices("*bac").get();
getResponse = indicesAdmin().prepareGetAliases("foo").setIndices("*bac").get();
assertThat(getResponse, notNullValue());
assertThat(getResponse.getAliases().size(), equalTo(1));
assertThat(getResponse.getAliases().get("foobar").size(), equalTo(1));
Expand All @@ -954,24 +954,24 @@ public void testIndicesGetAliases() throws Exception {
assertThat(getResponse.getAliases().get("foobar").get(0).getFilter(), nullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).getIndexRouting(), nullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).getSearchRouting(), nullValue());
assertFalse(indicesAdmin().prepareGetAliases("foo").addIndices("*bac").get().getAliases().isEmpty());
assertFalse(indicesAdmin().prepareGetAliases("foo").setIndices("*bac").get().getAliases().isEmpty());

logger.info("--> getting foo for index foobar");
getResponse = indicesAdmin().prepareGetAliases("foo").addIndices("foobar").get();
getResponse = indicesAdmin().prepareGetAliases("foo").setIndices("foobar").get();
assertThat(getResponse, notNullValue());
assertThat(getResponse.getAliases().size(), equalTo(1));
assertThat(getResponse.getAliases().get("foobar").get(0), notNullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).alias(), equalTo("foo"));
assertThat(getResponse.getAliases().get("foobar").get(0).getFilter(), nullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).getIndexRouting(), nullValue());
assertThat(getResponse.getAliases().get("foobar").get(0).getSearchRouting(), nullValue());
assertFalse(indicesAdmin().prepareGetAliases("foo").addIndices("foobar").get().getAliases().isEmpty());
assertFalse(indicesAdmin().prepareGetAliases("foo").setIndices("foobar").get().getAliases().isEmpty());

for (String aliasName : new String[] { null, "_all", "*" }) {
logger.info("--> getting {} alias for index foobar", aliasName);
getResponse = aliasName != null
? indicesAdmin().prepareGetAliases(aliasName).addIndices("foobar").get()
: indicesAdmin().prepareGetAliases().addIndices("foobar").get();
? indicesAdmin().prepareGetAliases(aliasName).setIndices("foobar").get()
: indicesAdmin().prepareGetAliases().setIndices("foobar").get();
assertThat(getResponse, notNullValue());
assertThat(getResponse.getAliases().size(), equalTo(1));
assertThat(getResponse.getAliases().get("foobar").size(), equalTo(4));
Expand All @@ -983,20 +983,20 @@ public void testIndicesGetAliases() throws Exception {

// alias at work again
logger.info("--> getting * for index *bac");
getResponse = indicesAdmin().prepareGetAliases("*").addIndices("*bac").get();
getResponse = indicesAdmin().prepareGetAliases("*").setIndices("*bac").get();
assertThat(getResponse, notNullValue());
assertThat(getResponse.getAliases().size(), equalTo(2));
assertThat(getResponse.getAliases().get("foobar").size(), equalTo(4));
assertThat(getResponse.getAliases().get("bazbar").size(), equalTo(2));
assertFalse(indicesAdmin().prepareGetAliases("*").addIndices("*bac").get().getAliases().isEmpty());
assertFalse(indicesAdmin().prepareGetAliases("*").setIndices("*bac").get().getAliases().isEmpty());

assertAcked(indicesAdmin().prepareAliases().removeAlias("foobar", "foo"));

getResponse = indicesAdmin().prepareGetAliases("foo").addIndices("foobar").get();
getResponse = indicesAdmin().prepareGetAliases("foo").setIndices("foobar").get();
for (final Map.Entry<String, List<AliasMetadata>> entry : getResponse.getAliases().entrySet()) {
assertTrue(entry.getValue().isEmpty());
}
assertTrue(indicesAdmin().prepareGetAliases("foo").addIndices("foobar").get().getAliases().isEmpty());
assertTrue(indicesAdmin().prepareGetAliases("foo").setIndices("foobar").get().getAliases().isEmpty());
}

public void testGetAllAliasesWorks() {
Expand Down
2 changes: 1 addition & 1 deletion ...r/src/internalClusterTest/java/org/elasticsearch/indices/IndicesOptionsIntegrationIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ static ValidateQueryRequestBuilder validateQuery(String... indices) {
}

static GetAliasesRequestBuilder getAliases(String... indices) {
return indicesAdmin().prepareGetAliases("dummy").addIndices(indices);
return indicesAdmin().prepareGetAliases("dummy").setIndices(indices);
}

static GetFieldMappingsRequestBuilder getFieldMapping(String... indices) {
Expand Down
2 changes: 1 addition & 1 deletion ...rc/internalClusterTest/java/org/elasticsearch/indices/template/SimpleIndexTemplateIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,7 @@ public void testMultipleAliasesPrecedence() throws Exception {

ensureGreen();

GetAliasesResponse getAliasesResponse = indicesAdmin().prepareGetAliases().addIndices("test").get();
GetAliasesResponse getAliasesResponse = indicesAdmin().prepareGetAliases().setIndices("test").get();
assertThat(getAliasesResponse.getAliases().get("test").size(), equalTo(4));

for (AliasMetadata aliasMetadata : getAliasesResponse.getAliases().get("test")) {
Expand Down
64 changes: 0 additions & 64 deletions ...main/java/org/elasticsearch/action/admin/indices/alias/get/BaseAliasesRequestBuilder.java
View file

This file was deleted.

28 changes: 5 additions & 23 deletions server/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,54 +7,36 @@
*/
package org.elasticsearch.action.admin.indices.alias.get;

import org.elasticsearch.action.ActionRequest;
import org.elasticsearch.action.ActionRequestValidationException;
import org.elasticsearch.action.AliasesRequest;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.action.support.TransportAction;
import org.elasticsearch.action.support.master.MasterNodeReadRequest;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.core.UpdateForV9;
import org.elasticsearch.tasks.CancellableTask;
import org.elasticsearch.tasks.Task;
import org.elasticsearch.tasks.TaskId;

import java.io.IOException;
import java.util.Map;

@UpdateForV9 // make this class a regular ActionRequest rather than a MasterNodeReadRequest
public class GetAliasesRequest extends MasterNodeReadRequest<GetAliasesRequest> implements AliasesRequest {
public class GetAliasesRequest extends ActionRequest implements AliasesRequest {

public static final IndicesOptions DEFAULT_INDICES_OPTIONS = IndicesOptions.strictExpandHidden();

private String[] aliases;
private String[] originalAliases;
private String[] indices = Strings.EMPTY_ARRAY;
private String[] aliases = Strings.EMPTY_ARRAY;
private IndicesOptions indicesOptions = DEFAULT_INDICES_OPTIONS;
private String[] originalAliases = Strings.EMPTY_ARRAY;

public GetAliasesRequest(String... aliases) {
super(TRAPPY_IMPLICIT_DEFAULT_MASTER_NODE_TIMEOUT);
this.aliases = aliases;
this.originalAliases = aliases;
}

public GetAliasesRequest() {
super(TRAPPY_IMPLICIT_DEFAULT_MASTER_NODE_TIMEOUT);
}

/**
* NB prior to 8.12 get-aliases was a TransportMasterNodeReadAction so for BwC we must remain able to read these requests until we no
* longer need to support calling this action remotely. Once we remove this we can also make this class a regular ActionRequest instead
* of a MasterNodeReadRequest.
*/
@UpdateForV9 // remove this constructor
public GetAliasesRequest(StreamInput in) throws IOException {
super(in);
indices = in.readStringArray();
aliases = in.readStringArray();
indicesOptions = IndicesOptions.readIndicesOptions(in);
originalAliases = in.readStringArray();
this(Strings.EMPTY_ARRAY);
}

@Override
Expand Down
22 changes: 19 additions & 3 deletions .../main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesRequestBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,27 @@

package org.elasticsearch.action.admin.indices.alias.get;

import org.elasticsearch.action.ActionRequestBuilder;
import org.elasticsearch.action.support.IndicesOptions;
import org.elasticsearch.client.internal.ElasticsearchClient;

public class GetAliasesRequestBuilder extends BaseAliasesRequestBuilder<GetAliasesResponse, GetAliasesRequestBuilder> {

public class GetAliasesRequestBuilder extends ActionRequestBuilder<GetAliasesRequest, GetAliasesResponse> {
public GetAliasesRequestBuilder(ElasticsearchClient client, String... aliases) {
super(client, GetAliasesAction.INSTANCE, aliases);
super(client, GetAliasesAction.INSTANCE, new GetAliasesRequest(aliases));
}

public GetAliasesRequestBuilder setAliases(String... aliases) {
request.aliases(aliases);
return this;
}

public GetAliasesRequestBuilder setIndices(String... indices) {
request.indices(indices);
return this;
}

public GetAliasesRequestBuilder setIndicesOptions(IndicesOptions options) {
request.indicesOptions(options);
return this;
}
}
10 changes: 2 additions & 8 deletions ...er/src/main/java/org/elasticsearch/action/admin/indices/alias/get/GetAliasesResponse.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
package org.elasticsearch.action.admin.indices.alias.get;

import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.action.support.TransportAction;
import org.elasticsearch.cluster.metadata.AliasMetadata;
import org.elasticsearch.cluster.metadata.DataStreamAlias;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.core.UpdateForV9;

import java.io.IOException;
import java.util.List;
Expand All @@ -37,15 +37,9 @@ public Map<String, List<DataStreamAlias>> getDataStreamAliases() {
return dataStreamAliases;
}

/**
* NB prior to 8.12 get-aliases was a TransportMasterNodeReadAction so for BwC we must remain able to write these responses until we no
* longer need to support calling this action remotely.
*/
@UpdateForV9 // replace this implementation with TransportAction.localOnly()
@Override
public void writeTo(StreamOutput out) throws IOException {
out.writeMap(aliases, StreamOutput::writeCollection);
out.writeMap(dataStreamAliases, StreamOutput::writeCollection);
TransportAction.localOnly();
}

@Override
Expand Down
11 changes: 2 additions & 9 deletions ...main/java/org/elasticsearch/action/admin/indices/alias/get/TransportGetAliasesAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.core.Predicates;
import org.elasticsearch.core.UpdateForV9;
import org.elasticsearch.indices.SystemIndices;
import org.elasticsearch.indices.SystemIndices.SystemIndexAccessLevel;
import org.elasticsearch.injection.guice.Inject;
Expand All @@ -40,11 +39,6 @@
import java.util.Map;
import java.util.function.Predicate;

/**
* NB prior to 8.12 this was a TransportMasterNodeReadAction so for BwC it must be registered with the TransportService (i.e. a
* HandledTransportAction) until we no longer need to support calling this action remotely.
*/
@UpdateForV9 // remove the HandledTransportAction superclass, this action need not be registered with the TransportService
public class TransportGetAliasesAction extends TransportLocalClusterStateAction<GetAliasesRequest, GetAliasesResponse> {
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(TransportGetAliasesAction.class);

Expand All @@ -62,10 +56,9 @@ public TransportGetAliasesAction(
) {
super(
GetAliasesAction.NAME,
clusterService,
transportService,
actionFilters,
GetAliasesRequest::new,
transportService.getTaskManager(),
clusterService,
clusterService.threadPool().executor(ThreadPool.Names.MANAGEMENT)
);
this.indexNameExpressionResolver = indexNameExpressionResolver;
Expand Down
Loading

0 comments on commit 7428245

Please sign in to comment.