Skip to content
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

Mute the query profile IT with concurrent execution #9840

Merged

Conversation

ticheng-aws
Copy link
Contributor

@ticheng-aws ticheng-aws commented Sep 6, 2023

Description

Disabling the query profile tests with concurrent execution for now and track to enable it as part of the rewrite fix.

Related Issues

Resolves #9815 #9787

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

@reta
Copy link
Collaborator

reta commented Sep 6, 2023

@ticheng-aws thanks for looking, I am worried about the NPE we are having

aused by: OpenSearchException[Cannot invoke "java.lang.Long.longValue()" because the return value of "java.util.Map.get(Object)" is null]; nested: NullPointerException[Cannot invoke "java.lang.Long.longValue()" because the return value of "java.util.Map.get(Object)" is null];
	at app//org.opensearch.OpenSearchException.guessRootCauses(OpenSearchException.java:708)
	at app//org.opensearch.action.search.AbstractSearchAsyncAction.executeNextPhase(AbstractSearchAsyncAction.java:371)
	... 28 more

This could clearly impact the core search flows

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

Gradle Check (Jenkins) Run Completed with:

@ticheng-aws
Copy link
Contributor Author

@ticheng-aws thanks for looking, I am worried about the NPE we are having

aused by: OpenSearchException[Cannot invoke "java.lang.Long.longValue()" because the return value of "java.util.Map.get(Object)" is null]; nested: NullPointerException[Cannot invoke "java.lang.Long.longValue()" because the return value of "java.util.Map.get(Object)" is null];
	at app//org.opensearch.OpenSearchException.guessRootCauses(OpenSearchException.java:708)
	at app//org.opensearch.action.search.AbstractSearchAsyncAction.executeNextPhase(AbstractSearchAsyncAction.java:371)
	... 28 more

This could clearly impact the core search flows

Hey @reta, the issue only happened on concurrent search queries with the profile set to true. The core search flows won't be impact when the profile is false. We plan to disable the query profile tests with concurrent execution for now, and track to enable it as part of the upcoming query profile rewrite field fix.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

Compatibility status:

Checks if related components are compatible with change ecb8d93

Incompatible components

Incompatible components: [https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/neural-search.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git]

@reta
Copy link
Collaborator

reta commented Sep 6, 2023

Hey @reta, the issue only happened on concurrent search queries with the profile set to true.

Thanks @ticheng-aws , this is what I mean by "core search flow": people will try the feature but run into NPE (this is not the mainstream search flow but variation of it). I think we should fix it or consider reverting - it is not good to deliver the experimental feature that NPEs (my opinion surely).

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

Compatibility status:

Checks if related components are compatible with change 6d38f55

Incompatible components

Incompatible components: [https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/cross-cluster-replication.git]

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/reporting.git]

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

Gradle Check (Jenkins) Run Completed with:

@sohami
Copy link
Collaborator

sohami commented Sep 6, 2023

Hey @reta, the issue only happened on concurrent search queries with the profile set to true.

Thanks @ticheng-aws , this is what I mean by "core search flow": people will try the feature but run into NPE (this is not the mainstream search flow but variation of it). I think we should fix it or consider reverting - it is not good to deliver the experimental feature that NPEs (my opinion surely).

@reta The NPE issue is not due to the new changes to the profile flow. It already exists in the core for concurrent search path. It is showing up now because the QueryProfilerIT test got enabled now with concurrent segment search. So the new changes should still be fine to be merged and we will work on the fix as a follow-up.

@reta
Copy link
Collaborator

reta commented Sep 6, 2023

So the new changes should still be fine to be merged and we will work on the fix as a follow-up.

Thanks @sohami , I think with #9835 (comment) we are on the same page, no objection to mute tests to reduce the noise for the builds

@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

Gradle Check (Jenkins) Run Completed with:

@sohami
Copy link
Collaborator

sohami commented Sep 7, 2023

Gradle Check (Jenkins) Run Completed with:

Failed for below:

Execution failed for task ':distribution:bwc:minor:buildBwcLinuxTar'.

@ticheng-aws ticheng-aws force-pushed the mute-test-profile-query branch from 6d38f55 to 91d8eb8 Compare September 8, 2023 00:00
@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2023

Compatibility status:

Checks if related components are compatible with change 91d8eb8

Incompatible components

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/reporting.git]

@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2023

Gradle Check (Jenkins) Run Completed with:

@ticheng-aws ticheng-aws force-pushed the mute-test-profile-query branch from 91d8eb8 to 037a36e Compare September 8, 2023 01:48
@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2023

Compatibility status:

Checks if related components are compatible with change 037a36e

Incompatible components

Skipped components

Compatible components

Compatible components: [https://github.com/opensearch-project/security.git, https://github.com/opensearch-project/alerting.git, https://github.com/opensearch-project/index-management.git, https://github.com/opensearch-project/anomaly-detection.git, https://github.com/opensearch-project/sql.git, https://github.com/opensearch-project/job-scheduler.git, https://github.com/opensearch-project/asynchronous-search.git, https://github.com/opensearch-project/observability.git, https://github.com/opensearch-project/common-utils.git, https://github.com/opensearch-project/k-nn.git, https://github.com/opensearch-project/reporting.git, https://github.com/opensearch-project/cross-cluster-replication.git, https://github.com/opensearch-project/geospatial.git, https://github.com/opensearch-project/performance-analyzer.git, https://github.com/opensearch-project/notifications.git, https://github.com/opensearch-project/ml-commons.git, https://github.com/opensearch-project/performance-analyzer-rca.git, https://github.com/opensearch-project/neural-search.git, https://github.com/opensearch-project/security-analytics.git, https://github.com/opensearch-project/opensearch-oci-object-storage.git]

@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2023

Gradle Check (Jenkins) Run Completed with:

@codecov
Copy link

codecov bot commented Sep 8, 2023

Codecov Report

Merging #9840 (037a36e) into main (4ccecd6) will decrease coverage by 0.18%.
Report is 2 commits behind head on main.
The diff coverage is 25.60%.

@@             Coverage Diff              @@
##               main    #9840      +/-   ##
============================================
- Coverage     71.16%   70.99%   -0.18%     
+ Complexity    58115    58056      -59     
============================================
  Files          4831     4831              
  Lines        273999   274058      +59     
  Branches      39920    39922       +2     
============================================
- Hits         195005   194574     -431     
- Misses        62604    63187     +583     
+ Partials      16390    16297      -93     
Files Changed Coverage Δ
...arch/index/recovery/RemoteStoreRestoreService.java 12.03% <0.00%> (ø)
server/src/main/java/org/opensearch/node/Node.java 85.54% <0.00%> (ø)
...arch/gateway/remote/RemoteClusterStateService.java 65.37% <23.37%> (-13.05%) ⬇️
...search/gateway/remote/ClusterMetadataManifest.java 97.46% <100.00%> (+0.53%) ⬆️

... and 465 files with indirect coverage changes

📢 Have feedback on the report? Share it here.

@sohami sohami merged commit c99ba63 into opensearch-project:main Sep 8, 2023
kaushalmahi12 pushed a commit to kaushalmahi12/OpenSearch that referenced this pull request Sep 12, 2023
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/OpenSearch that referenced this pull request Sep 20, 2023
brusic pushed a commit to brusic/OpenSearch that referenced this pull request Sep 25, 2023
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
3 participants