Skip to content

Commit

Permalink
Internal: TransportSearchTypeAction shouldn't extend TransportAction
Browse files Browse the repository at this point in the history
All the actions that extend TransportSearchTypeAction are subactions of the main TransportSearchAction. The main one is and should be a transport action, register request handlers, support request and response filtering etc. but the subactions shouldn't as that becomes just double work. At the moment each search request goes through validation and filters twice, one as part of the main action, and the second one as part of the subaction execution. The subactions don't need to extend TransportAction, but can be simple support classes, as they are always executed on the same node as their main action.

This commit modifies TransportSearchTypeAction to not extend TransportAction but simply AbstractComponent.

Closes elastic#11710
  • Loading branch information
javanna committed Feb 17, 2016
1 parent 3290cfb commit d493375
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 24 deletions.
Expand Up @@ -24,7 +24,6 @@
import org.elasticsearch.action.search.ReduceSearchPhaseException;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.cluster.ClusterService;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.node.DiscoveryNode;
Expand Down Expand Up @@ -52,12 +51,12 @@ public class TransportSearchDfsQueryAndFetchAction extends TransportSearchTypeAc
@Inject
public TransportSearchDfsQueryAndFetchAction(Settings settings, ThreadPool threadPool, ClusterService clusterService,
SearchServiceTransportAction searchService, SearchPhaseController searchPhaseController,
ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver) {
super(settings, threadPool, clusterService, searchService, searchPhaseController, actionFilters, indexNameExpressionResolver);
IndexNameExpressionResolver indexNameExpressionResolver) {
super(settings, threadPool, clusterService, searchService, searchPhaseController, indexNameExpressionResolver);
}

@Override
protected void doExecute(SearchRequest searchRequest, ActionListener<SearchResponse> listener) {
public void execute(SearchRequest searchRequest, ActionListener<SearchResponse> listener) {
new AsyncAction(searchRequest, listener).start();
}

Expand Down
Expand Up @@ -27,7 +27,6 @@
import org.elasticsearch.action.search.SearchPhaseExecutionException;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.cluster.ClusterService;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.node.DiscoveryNode;
Expand Down Expand Up @@ -58,12 +57,12 @@ public class TransportSearchDfsQueryThenFetchAction extends TransportSearchTypeA
@Inject
public TransportSearchDfsQueryThenFetchAction(Settings settings, ThreadPool threadPool, ClusterService clusterService,
SearchServiceTransportAction searchService, SearchPhaseController searchPhaseController,
ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver) {
super(settings, threadPool, clusterService, searchService, searchPhaseController, actionFilters, indexNameExpressionResolver);
IndexNameExpressionResolver indexNameExpressionResolver) {
super(settings, threadPool, clusterService, searchService, searchPhaseController, indexNameExpressionResolver);
}

@Override
protected void doExecute(SearchRequest searchRequest, ActionListener<SearchResponse> listener) {
public void execute(SearchRequest searchRequest, ActionListener<SearchResponse> listener) {
new AsyncAction(searchRequest, listener).start();
}

Expand Down
Expand Up @@ -24,7 +24,6 @@
import org.elasticsearch.action.search.ReduceSearchPhaseException;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.cluster.ClusterService;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.node.DiscoveryNode;
Expand All @@ -49,12 +48,12 @@ public class TransportSearchQueryAndFetchAction extends TransportSearchTypeActio
@Inject
public TransportSearchQueryAndFetchAction(Settings settings, ThreadPool threadPool, ClusterService clusterService,
SearchServiceTransportAction searchService, SearchPhaseController searchPhaseController,
ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver) {
super(settings, threadPool, clusterService, searchService, searchPhaseController, actionFilters, indexNameExpressionResolver);
IndexNameExpressionResolver indexNameExpressionResolver) {
super(settings, threadPool, clusterService, searchService, searchPhaseController, indexNameExpressionResolver);
}

@Override
protected void doExecute(SearchRequest searchRequest, ActionListener<SearchResponse> listener) {
public void execute(SearchRequest searchRequest, ActionListener<SearchResponse> listener) {
new AsyncAction(searchRequest, listener).start();
}

Expand Down
Expand Up @@ -26,7 +26,6 @@
import org.elasticsearch.action.search.ReduceSearchPhaseException;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.cluster.ClusterService;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.cluster.node.DiscoveryNode;
Expand Down Expand Up @@ -54,12 +53,12 @@ public class TransportSearchQueryThenFetchAction extends TransportSearchTypeActi
@Inject
public TransportSearchQueryThenFetchAction(Settings settings, ThreadPool threadPool, ClusterService clusterService,
SearchServiceTransportAction searchService, SearchPhaseController searchPhaseController,
ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver) {
super(settings, threadPool, clusterService, searchService, searchPhaseController, actionFilters, indexNameExpressionResolver);
IndexNameExpressionResolver indexNameExpressionResolver) {
super(settings, threadPool, clusterService, searchService, searchPhaseController, indexNameExpressionResolver);
}

@Override
protected void doExecute(SearchRequest searchRequest, ActionListener<SearchResponse> listener) {
public void execute(SearchRequest searchRequest, ActionListener<SearchResponse> listener) {
new AsyncAction(searchRequest, listener).start();
}

Expand Down
Expand Up @@ -25,13 +25,10 @@
import org.elasticsearch.action.ActionListener;
import org.elasticsearch.action.NoShardAvailableActionException;
import org.elasticsearch.action.search.ReduceSearchPhaseException;
import org.elasticsearch.action.search.SearchAction;
import org.elasticsearch.action.search.SearchPhaseExecutionException;
import org.elasticsearch.action.search.SearchRequest;
import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.search.ShardSearchFailure;
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.action.support.TransportAction;
import org.elasticsearch.action.support.TransportActions;
import org.elasticsearch.cluster.ClusterService;
import org.elasticsearch.cluster.ClusterState;
Expand All @@ -43,6 +40,7 @@
import org.elasticsearch.cluster.routing.ShardIterator;
import org.elasticsearch.cluster.routing.ShardRouting;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.component.AbstractComponent;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.util.concurrent.AtomicArray;
import org.elasticsearch.search.SearchPhaseResult;
Expand All @@ -54,9 +52,7 @@
import org.elasticsearch.search.internal.ShardSearchTransportRequest;
import org.elasticsearch.search.query.QuerySearchResult;
import org.elasticsearch.search.query.QuerySearchResultProvider;
import org.elasticsearch.tasks.TaskManager;
import org.elasticsearch.threadpool.ThreadPool;
import org.elasticsearch.transport.TransportService;

import java.util.List;
import java.util.Map;
Expand All @@ -68,23 +64,32 @@
/**
*
*/
public abstract class TransportSearchTypeAction extends TransportAction<SearchRequest, SearchResponse> {
public abstract class TransportSearchTypeAction extends AbstractComponent {

protected final ThreadPool threadPool;

protected final ClusterService clusterService;

protected final SearchServiceTransportAction searchService;

protected final SearchPhaseController searchPhaseController;

protected final IndexNameExpressionResolver indexNameExpressionResolver;

public TransportSearchTypeAction(Settings settings, ThreadPool threadPool, ClusterService clusterService,
SearchServiceTransportAction searchService, SearchPhaseController searchPhaseController,
ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver) {
super(settings, SearchAction.NAME, threadPool, actionFilters, indexNameExpressionResolver, clusterService.getTaskManager());
IndexNameExpressionResolver indexNameExpressionResolver) {

super(settings);
this.threadPool = threadPool;
this.clusterService = clusterService;
this.searchService = searchService;
this.searchPhaseController = searchPhaseController;
this.indexNameExpressionResolver = indexNameExpressionResolver;
}

public abstract void execute(SearchRequest searchRequest, ActionListener<SearchResponse> listener);

protected abstract class BaseAsyncAction<FirstResult extends SearchPhaseResult> extends AbstractAsyncAction {

protected final ActionListener<SearchResponse> listener;
Expand Down

0 comments on commit d493375

Please sign in to comment.