Skip to content

Commit

Permalink
Merge branch 'main' into 13531-range-agg
Browse files Browse the repository at this point in the history
  • Loading branch information
bowenlan-amzn committed Jun 2, 2024
2 parents c10c775 + 771f4ec commit 783b14a
Show file tree
Hide file tree
Showing 37 changed files with 406 additions and 82 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/assemble.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ jobs:
- name: Setup docker (missing on MacOS)
if: runner.os == 'macos'
uses: douglascamata/setup-docker-macos-action@main
with:
upgrade-qemu: true
- name: Run Gradle (assemble)
run: |
./gradlew assemble --parallel --no-build-cache -PDISABLE_BUILD_CACHE
1 change: 0 additions & 1 deletion CHANGELOG-3.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Add task completion count in search backpressure stats API ([#10028](https://github.com/opensearch-project/OpenSearch/pull/10028/))
- Deprecate CamelCase `PathHierarchy` tokenizer name in favor to lowercase `path_hierarchy` ([#10894](https://github.com/opensearch-project/OpenSearch/pull/10894))
- Breaking change: Do not request "search_pipelines" metrics by default in NodesInfoRequest ([#12497](https://github.com/opensearch-project/OpenSearch/pull/12497))
- Remove deprecated constant META_FIELDS_BEFORE_7DOT8 ([#13860](https://github.com/opensearch-project/OpenSearch/pull/13860))

### Deprecated

Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- [Remote Store] Upload translog checkpoint as object metadata to translog.tlog([#13637](https://github.com/opensearch-project/OpenSearch/pull/13637))
- Add getMetadataFields to MapperService ([#13819](https://github.com/opensearch-project/OpenSearch/pull/13819))
- [Remote State] Add async remote state deletion task running on an interval, configurable by a setting ([#13131](https://github.com/opensearch-project/OpenSearch/pull/13131))
- Allow setting query parameters on requests ([#13776](https://github.com/opensearch-project/OpenSearch/issues/13776))
- Apply the date histogram rewrite optimization to range aggregation ([#13865](https://github.com/opensearch-project/OpenSearch/pull/13865))

### Dependencies
Expand Down Expand Up @@ -46,6 +47,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Fix get field mapping API returns 404 error in mixed cluster with multiple versions ([#13624](https://github.com/opensearch-project/OpenSearch/pull/13624))
- Allow clearing `remote_store.compatibility_mode` setting ([#13646](https://github.com/opensearch-project/OpenSearch/pull/13646))
- Fix ReplicaShardBatchAllocator to batch shards without duplicates ([#13710](https://github.com/opensearch-project/OpenSearch/pull/13710))
- Don't return negative scores from `multi_match` query with `cross_fields` type ([#13829](https://github.com/opensearch-project/OpenSearch/pull/13829))
- Pass parent filter to inner hit query ([#13903](https://github.com/opensearch-project/OpenSearch/pull/13903))
- Fix NPE on restore searchable snapshot ([#13911](https://github.com/opensearch-project/OpenSearch/pull/13911))

### Security

Expand Down
5 changes: 5 additions & 0 deletions DEVELOPER_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
- [LineLint](#linelint)
- [Lucene Snapshots](#lucene-snapshots)
- [Flaky Tests](#flaky-tests)
- [Gradle Check Metrics Dashboard](#gradle-check-metrics-dashboard)

# Developer Guide

Expand Down Expand Up @@ -660,3 +661,7 @@ If you encounter a build/test failure in CI that is unrelated to the change in y
4. If an existing issue is found, paste a link to the known issue in a comment to your PR.
5. If no existing issue is found, open one.
6. Retry CI via the GitHub UX or by pushing an update to your PR.

### Gradle Check Metrics Dashboard

To get the comprehensive insights and analysis of the Gradle Check test failures, visit the [OpenSearch Gradle Check Metrics Dashboard](https://metrics.opensearch.org/_dashboards/app/dashboards#/view/e5e64d40-ed31-11ee-be99-69d1dbc75083). This dashboard is part of the [OpenSearch Metrics Project](https://github.com/opensearch-project/opensearch-metrics) initiative. The dashboard contains multiple data points that can help investigate and resolve flaky failures. Additionally, this dashboard can be used to drill down, slice, and dice the data using multiple supported filters, which further aids in troubleshooting and resolving issues.
8 changes: 7 additions & 1 deletion client/rest/src/main/java/org/opensearch/client/Request.java
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,13 @@ public void addParameters(Map<String, String> paramSource) {
* will change it.
*/
public Map<String, String> getParameters() {
return unmodifiableMap(parameters);
if (options.getParameters().isEmpty()) {
return unmodifiableMap(parameters);
} else {
Map<String, String> combinedParameters = new HashMap<>(parameters);
combinedParameters.putAll(options.getParameters());
return unmodifiableMap(combinedParameters);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,11 @@

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;

/**
* The portion of an HTTP request to OpenSearch that can be
Expand All @@ -53,18 +56,21 @@ public final class RequestOptions {
*/
public static final RequestOptions DEFAULT = new Builder(
Collections.emptyList(),
Collections.emptyMap(),
HeapBufferedResponseConsumerFactory.DEFAULT,
null,
null
).build();

private final List<Header> headers;
private final Map<String, String> parameters;
private final HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory;
private final WarningsHandler warningsHandler;
private final RequestConfig requestConfig;

private RequestOptions(Builder builder) {
this.headers = Collections.unmodifiableList(new ArrayList<>(builder.headers));
this.parameters = Collections.unmodifiableMap(new HashMap<>(builder.parameters));
this.httpAsyncResponseConsumerFactory = builder.httpAsyncResponseConsumerFactory;
this.warningsHandler = builder.warningsHandler;
this.requestConfig = builder.requestConfig;
Expand All @@ -74,7 +80,7 @@ private RequestOptions(Builder builder) {
* Create a builder that contains these options but can be modified.
*/
public Builder toBuilder() {
return new Builder(headers, httpAsyncResponseConsumerFactory, warningsHandler, requestConfig);
return new Builder(headers, parameters, httpAsyncResponseConsumerFactory, warningsHandler, requestConfig);
}

/**
Expand All @@ -84,6 +90,14 @@ public List<Header> getHeaders() {
return headers;
}

/**
* Query parameters to attach to the request. Any parameters present here
* will override matching parameters in the {@link Request}, if they exist.
*/
public Map<String, String> getParameters() {
return parameters;
}

/**
* The {@link HttpAsyncResponseConsumerFactory} used to create one
* {@link AsyncResponseConsumer} callback per retry. Controls how the
Expand Down Expand Up @@ -142,6 +156,12 @@ public String toString() {
b.append(headers.get(h).toString());
}
}
if (parameters.size() > 0) {
if (comma) b.append(", ");
comma = true;
b.append("parameters=");
b.append(parameters.entrySet().stream().map(e -> e.getKey() + "=" + e.getValue()).collect(Collectors.joining(",")));
}
if (httpAsyncResponseConsumerFactory != HttpAsyncResponseConsumerFactory.DEFAULT) {
if (comma) b.append(", ");
comma = true;
Expand Down Expand Up @@ -170,6 +190,7 @@ public boolean equals(Object obj) {

RequestOptions other = (RequestOptions) obj;
return headers.equals(other.headers)
&& parameters.equals(other.parameters)
&& httpAsyncResponseConsumerFactory.equals(other.httpAsyncResponseConsumerFactory)
&& Objects.equals(warningsHandler, other.warningsHandler);
}
Expand All @@ -179,7 +200,7 @@ public boolean equals(Object obj) {
*/
@Override
public int hashCode() {
return Objects.hash(headers, httpAsyncResponseConsumerFactory, warningsHandler);
return Objects.hash(headers, parameters, httpAsyncResponseConsumerFactory, warningsHandler);
}

/**
Expand All @@ -189,17 +210,20 @@ public int hashCode() {
*/
public static class Builder {
private final List<Header> headers;
private final Map<String, String> parameters;
private HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory;
private WarningsHandler warningsHandler;
private RequestConfig requestConfig;

private Builder(
List<Header> headers,
Map<String, String> parameters,
HttpAsyncResponseConsumerFactory httpAsyncResponseConsumerFactory,
WarningsHandler warningsHandler,
RequestConfig requestConfig
) {
this.headers = new ArrayList<>(headers);
this.parameters = new HashMap<>(parameters);
this.httpAsyncResponseConsumerFactory = httpAsyncResponseConsumerFactory;
this.warningsHandler = warningsHandler;
this.requestConfig = requestConfig;
Expand All @@ -226,6 +250,21 @@ public Builder addHeader(String name, String value) {
return this;
}

/**
* Add the provided query parameter to the request. Any parameters added here
* will override matching parameters in the {@link Request}, if they exist.
*
* @param name the query parameter name
* @param value the query parameter value
* @throws NullPointerException if {@code name} or {@code value} is null.
*/
public Builder addParameter(String name, String value) {
Objects.requireNonNull(name, "query parameter name cannot be null");
Objects.requireNonNull(value, "query parameter value cannot be null");
this.parameters.put(name, value);
return this;
}

/**
* Set the {@link HttpAsyncResponseConsumerFactory} used to create one
* {@link AsyncResponseConsumer} callback per retry. Controls how the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,15 @@

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.fail;
import static org.mockito.Mockito.mock;

Expand Down Expand Up @@ -90,6 +93,39 @@ public void testAddHeader() {
}
}

public void testAddParameter() {
assertThrows(
"query parameter name cannot be null",
NullPointerException.class,
() -> randomBuilder().addParameter(null, randomAsciiLettersOfLengthBetween(3, 10))
);

assertThrows(
"query parameter value cannot be null",
NullPointerException.class,
() -> randomBuilder().addParameter(randomAsciiLettersOfLengthBetween(3, 10), null)
);

RequestOptions.Builder builder = RequestOptions.DEFAULT.toBuilder();
int numParameters = between(0, 5);
Map<String, String> parameters = new HashMap<>();
for (int i = 0; i < numParameters; i++) {
String name = randomAsciiAlphanumOfLengthBetween(5, 10);
String value = randomAsciiAlphanumOfLength(3);
parameters.put(name, value);
builder.addParameter(name, value);
}
RequestOptions options = builder.build();
assertEquals(parameters, options.getParameters());

try {
options.getParameters().put(randomAsciiAlphanumOfLengthBetween(5, 10), randomAsciiAlphanumOfLength(3));
fail("expected failure");
} catch (UnsupportedOperationException e) {
assertNull(e.getMessage());
}
}

public void testSetHttpAsyncResponseConsumerFactory() {
try {
RequestOptions.DEFAULT.toBuilder().setHttpAsyncResponseConsumerFactory(null);
Expand Down Expand Up @@ -145,6 +181,13 @@ static RequestOptions.Builder randomBuilder() {
}
}

if (randomBoolean()) {
int queryParamCount = between(1, 5);
for (int i = 0; i < queryParamCount; i++) {
builder.addParameter(randomAsciiAlphanumOfLength(3), randomAsciiAlphanumOfLength(3));
}
}

if (randomBoolean()) {
builder.setHttpAsyncResponseConsumerFactory(new HeapBufferedResponseConsumerFactory(1));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.StreamSupport;

import fixture.s3.S3HttpHandler;
Expand Down Expand Up @@ -206,7 +205,12 @@ public void testRequestStats() throws Exception {
} catch (RepositoryMissingException e) {
return null;
}
}).filter(Objects::nonNull).map(Repository::stats).reduce(RepositoryStats::merge).get();
}).filter(b -> {
if (b instanceof BlobStoreRepository) {
return ((BlobStoreRepository) b).blobStore() != null;
}
return false;
}).map(Repository::stats).reduce(RepositoryStats::merge).get();

Map<BlobStore.Metric, Map<String, Long>> extendedStats = repositoryStats.extendedStats;
Map<String, Long> aggregatedStats = new HashMap<>();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
"Cross fields do not return negative scores":
- skip:
version: " - 2.99.99"
reason: "This fix is in 2.15. Until we do the BWC dance, we need to skip all pre-3.0, though."
- do:
index:
index: test
id: 1
body: { "color" : "orange red yellow" }
- do:
index:
index: test
id: 2
body: { "color": "orange red purple", "shape": "red square" }
- do:
index:
index: test
id: 3
body: { "color" : "orange red yellow purple" }
- do:
indices.refresh: { }
- do:
search:
index: test
body:
query:
multi_match:
query: "red"
type: "cross_fields"
fields: [ "color", "shape^100"]
tie_breaker: 0.1
explain: true
- match: { hits.total.value: 3 }
- match: { hits.hits.0._id: "2" }
- gt: { hits.hits.2._score: 0.0 }
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import org.opensearch.node.Node;
import org.opensearch.repositories.fs.FsRepository;
import org.hamcrest.MatcherAssert;
import org.junit.After;

import java.io.IOException;
import java.nio.file.Files;
Expand All @@ -62,6 +63,10 @@

import static org.opensearch.action.admin.cluster.node.stats.NodesStatsRequest.Metric.FS;
import static org.opensearch.core.common.util.CollectionUtils.iterableAsArrayList;
import static org.opensearch.index.store.remote.filecache.FileCacheSettings.DATA_TO_FILE_CACHE_SIZE_RATIO_SETTING;
import static org.opensearch.test.NodeRoles.clusterManagerOnlyNode;
import static org.opensearch.test.NodeRoles.dataNode;
import static org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
Expand Down Expand Up @@ -939,6 +944,52 @@ public void testRelocateSearchableSnapshotIndex() throws Exception {
assertSearchableSnapshotIndexDirectoryExistence(searchNode2, index, false);
}

public void testCreateSearchableSnapshotWithSpecifiedRemoteDataRatio() throws Exception {
final String snapshotName = "test-snap";
final String repoName = "test-repo";
final String indexName1 = "test-idx-1";
final String restoredIndexName1 = indexName1 + "-copy";
final String indexName2 = "test-idx-2";
final String restoredIndexName2 = indexName2 + "-copy";
final int numReplicasIndex1 = 1;
final int numReplicasIndex2 = 1;

Settings clusterManagerNodeSettings = clusterManagerOnlyNode();
internalCluster().startNodes(2, clusterManagerNodeSettings);
Settings dateNodeSettings = dataNode();
internalCluster().startNodes(2, dateNodeSettings);
createIndexWithDocsAndEnsureGreen(numReplicasIndex1, 100, indexName1);
createIndexWithDocsAndEnsureGreen(numReplicasIndex2, 100, indexName2);

final Client client = client();
assertAcked(
client.admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(Settings.builder().put(DATA_TO_FILE_CACHE_SIZE_RATIO_SETTING.getKey(), 5))
);

createRepositoryWithSettings(null, repoName);
takeSnapshot(client, snapshotName, repoName, indexName1, indexName2);

internalCluster().ensureAtLeastNumSearchNodes(Math.max(numReplicasIndex1, numReplicasIndex2) + 1);
restoreSnapshotAndEnsureGreen(client, snapshotName, repoName);

assertDocCount(restoredIndexName1, 100L);
assertDocCount(restoredIndexName2, 100L);
assertIndexDirectoryDoesNotExist(restoredIndexName1, restoredIndexName2);
}

@After
public void cleanup() throws Exception {
assertAcked(
client().admin()
.cluster()
.prepareUpdateSettings()
.setTransientSettings(Settings.builder().putNull(DATA_TO_FILE_CACHE_SIZE_RATIO_SETTING.getKey()))
);
}

private void assertSearchableSnapshotIndexDirectoryExistence(String nodeName, Index index, boolean exists) throws Exception {
final Node node = internalCluster().getInstance(Node.class, nodeName);
final ShardId shardId = new ShardId(index, 0);
Expand Down
Loading

0 comments on commit 783b14a

Please sign in to comment.