-
Notifications
You must be signed in to change notification settings - Fork 2.3k
[GRPC] SearchService and Search GRPC endpoint v1 #17830
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
Conversation
❌ Gradle check result for 11b9615: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for ddee102: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
4110529
to
d950579
Compare
❌ Gradle check result for d950579: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
c9a59dd
to
9d23b96
Compare
❌ Gradle check result for 9d23b96: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
7a4bf87
to
5ee1000
Compare
❕ Gradle check result for 5ee1000: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
❌ Gradle check result for c8015b9: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
❌ Gradle check result for 67bed87: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
@karenyrx Can you rebase in the latest test fixes from main and fix the CHANGELOG conflict? |
server/src/main/java/org/opensearch/action/support/IndicesOptions.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/opensearch/plugin/transport/grpc/listeners/SearchRequestActionListener.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Karen Xu <[email protected]>
Signed-off-by: Karen Xu <[email protected]>
Signed-off-by: Karen Xu <[email protected]>
Signed-off-by: Karen Xu <[email protected]>
Signed-off-by: Karen Xu <[email protected]>
❌ Gradle check result for ada3465: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Signed-off-by: Karen Xu <[email protected]>
❕ Gradle check result for 5bb04e1: UNSTABLE Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure. |
Signed-off-by: Andrew Ross <[email protected]>
} | ||
|
||
if (fetchSource != null || sourceIncludes != null || sourceExcludes != null) { | ||
return new FetchSourceContext(fetchSource == null ? true : fetchSource, sourceIncludes, sourceExcludes); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If fetchSource is explicitly set to false here fetchSource != null
is true. Should we default fetchSource to false and change the condition to if (fetchSource || ...
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the equivalent REST-side implementation: https://github.com/opensearch-project/OpenSearch/blob/main/server/src/main/java/org/opensearch/search/fetch/subphase/FetchSourceContext.java#L135 which also does the same check. Following this, I think this GRPC code is accurate?
...main/java/org/opensearch/plugin/transport/grpc/proto/request/common/ObjectMapProtoUtils.java
Show resolved
Hide resolved
...rc/main/java/org/opensearch/plugin/transport/grpc/proto/request/common/ScriptProtoUtils.java
Show resolved
Hide resolved
...rc/main/java/org/opensearch/plugin/transport/grpc/proto/request/common/ScriptProtoUtils.java
Outdated
Show resolved
Hide resolved
...ava/org/opensearch/plugin/transport/grpc/proto/request/search/CollapseBuilderProtoUtils.java
Outdated
Show resolved
Hide resolved
.../exceptions/shardoperationfailedexception/ReplicationResponseShardInfoFailureProtoUtils.java
Outdated
Show resolved
Hide resolved
.../exceptions/shardoperationfailedexception/ReplicationResponseShardInfoFailureProtoUtils.java
Outdated
Show resolved
Hide resolved
.../proto/response/exceptions/shardoperationfailedexception/SnapshotShardFailureProtoUtils.java
Show resolved
Hide resolved
...ain/java/org/opensearch/plugin/transport/grpc/proto/response/search/SearchHitProtoUtils.java
Outdated
Show resolved
Hide resolved
...st/java/org/opensearch/plugin/transport/grpc/listeners/SearchRequestActionListenerTests.java
Outdated
Show resolved
Hide resolved
❌ Gradle check result for 148f712: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
164c00b
to
6be7f0c
Compare
…ity, add more javadocs Signed-off-by: Karen Xu <[email protected]>
…17830) Signed-off-by: Karen Xu <[email protected]> Signed-off-by: Andrew Ross <[email protected]> Co-authored-by: Andrew Ross <[email protected]> Signed-off-by: Sriram Ganesh <[email protected]>
…17830) Signed-off-by: Karen Xu <[email protected]> Signed-off-by: Andrew Ross <[email protected]> Co-authored-by: Andrew Ross <[email protected]> Signed-off-by: Harsh Kothari <[email protected]>
…17830) Signed-off-by: Karen Xu <[email protected]> Signed-off-by: Andrew Ross <[email protected]> Co-authored-by: Andrew Ross <[email protected]> Signed-off-by: Harsh Kothari <[email protected]>
Description
Test Plan
a) Send a MatchAll query with "size" parameter in both the URL parameters as well as request body.
SearchRequest (same for both GRPC and HTTP)
b) TermQuery
SearchRequest (same for both GRPC and HTTP)
c) MatchNone with source_includes field (compare POJOs not the response)
SearchRequest (same for both GRPC and HTTP)
The documents ingested 3 of the above tests were:
Related Issues
#16783
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.