Skip to content

Commit

Permalink
Rethrow OpenSearch exception in concurrent search (opensearch-project…
Browse files Browse the repository at this point in the history
…#9177)

Signed-off-by: Neetika Singhal <neetiks@amazon.com>
Signed-off-by: Kiran Reddy <kkreddy@amazon.com>
  • Loading branch information
neetikasinghal authored and kkmr committed Aug 28, 2023
1 parent d17b3b9 commit fe7000d
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- [Refactor] Task foundation classes to core library - pt 1 ([#9082](https://github.com/opensearch-project/OpenSearch/pull/9082))
- Add base class for parameterizing the search based tests #9083 ([#9083](https://github.com/opensearch-project/OpenSearch/pull/9083))
- Add support for wrapping CollectorManager with profiling during concurrent execution ([#9129](https://github.com/opensearch-project/OpenSearch/pull/9129))
- Rethrow OpenSearch exception for non-concurrent path while using concurrent search ([#9177](https://github.com/opensearch-project/OpenSearch/pull/9177))

### Deprecated

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

package org.opensearch.indices.memory.breaker;

import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
import org.opensearch.ExceptionsHelper;
import org.opensearch.action.admin.cluster.node.stats.NodeStats;
import org.opensearch.action.admin.cluster.node.stats.NodesStatsResponse;
Expand All @@ -46,6 +47,7 @@
import org.opensearch.client.Requests;
import org.opensearch.cluster.metadata.IndexMetadata;
import org.opensearch.cluster.routing.allocation.decider.EnableAllocationDecider;
import org.opensearch.common.util.FeatureFlags;
import org.opensearch.core.common.breaker.NoopCircuitBreaker;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.unit.TimeValue;
Expand All @@ -57,14 +59,17 @@
import org.opensearch.core.xcontent.MediaTypeRegistry;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.indices.breaker.HierarchyCircuitBreakerService;
import org.opensearch.search.SearchService;
import org.opensearch.search.sort.SortOrder;
import org.opensearch.test.OpenSearchIntegTestCase;
import org.opensearch.test.OpenSearchIntegTestCase.ClusterScope;

import org.junit.After;
import org.junit.Before;
import org.opensearch.test.ParameterizedOpenSearchIntegTestCase;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.TimeUnit;
Expand All @@ -73,6 +78,7 @@
import static org.opensearch.action.admin.cluster.node.stats.NodesStatsRequest.Metric.BREAKER;
import static org.opensearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS;
import static org.opensearch.index.query.QueryBuilders.matchAllQuery;
import static org.opensearch.search.SearchService.CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING;
import static org.opensearch.search.aggregations.AggregationBuilders.cardinality;
import static org.opensearch.search.aggregations.AggregationBuilders.terms;
import static org.opensearch.test.OpenSearchIntegTestCase.Scope.TEST;
Expand All @@ -89,7 +95,32 @@
* Integration tests for InternalCircuitBreakerService
*/
@ClusterScope(scope = TEST, numClientNodes = 0, maxNumDataNodes = 1)
public class CircuitBreakerServiceIT extends OpenSearchIntegTestCase {
public class CircuitBreakerServiceIT extends ParameterizedOpenSearchIntegTestCase {
public CircuitBreakerServiceIT(Settings dynamicSettings) {
super(dynamicSettings);
}

@ParametersFactory
public static Collection<Object[]> parameters() {
return Arrays.asList(
new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), false).build() },
new Object[] { Settings.builder().put(CLUSTER_CONCURRENT_SEGMENT_SEARCH_SETTING.getKey(), true).build() }
);
}

@Override
protected Settings featureFlagSettings() {
return Settings.builder().put(super.featureFlagSettings()).put(FeatureFlags.CONCURRENT_SEGMENT_SEARCH, "true").build();
}

@Override
protected Settings nodeSettings(int nodeOrdinal) {
return Settings.builder()
.put(super.nodeSettings(nodeOrdinal))
.put(SearchService.CONCURRENT_SEGMENT_SEARCH_TARGET_MAX_SLICE_COUNT_KEY, randomIntBetween(1, 2))
.build();
}

/** Reset all breaker settings back to their defaults */
private void reset() {
logger.info("--> resetting breaker settings");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import org.apache.lucene.search.Collector;
import org.apache.lucene.search.CollectorManager;
import org.apache.lucene.search.Query;
import org.opensearch.OpenSearchException;
import org.opensearch.search.aggregations.AggregationProcessor;
import org.opensearch.search.aggregations.ConcurrentAggregationProcessor;
import org.opensearch.search.internal.ContextIndexSearcher;
Expand Down Expand Up @@ -103,8 +104,8 @@ public AggregationProcessor aggregationProcessor(SearchContext searchContext) {
}

private static <T extends Exception> void rethrowCauseIfPossible(RuntimeException re, SearchContext searchContext) throws T {
// Rethrow exception if cause is null
if (re.getCause() == null) {
// Rethrow exception if cause is null or if it's an instance of OpenSearchException
if (re.getCause() == null || re instanceof OpenSearchException) {
throw re;
}

Expand Down

0 comments on commit fe7000d

Please sign in to comment.