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

[Search pipelines] Pass "adhocness" flag to processor factories #8390

Closed
wants to merge 5 commits into from

Conversation

mingshl
Copy link
Contributor

@mingshl mingshl commented Jun 30, 2023

Description

resolve merge conflict of #8164

Related Issues

Resolves #8163

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.

msfroh and others added 5 commits June 29, 2023 00:31
A named search pipeline may be created with a PUT request, while
an "anonymous" or "ad hoc" search pipeline can be defined in the
search request source. In the latter case, we don't want to create any
"resource-heavy" processors, since they're potentially increasing the
cost of every search request, whereas names pipeline processors get
reused.

This change passes a configuration flag to a processor factory if it's
being called as part of an ad hoc pipeline. The factory can use that
information to avoid allocating expensive resources (maybe by throwing
an exception instead).

Signed-off-by: Michael Froh <froh@amazon.com>
Thanks to @dblock for the suggestion to pass the pipeline creation
source in a way that accounts for possible future pipeline sources (and
lets us distinguish between actual named pipeline creation and the
validation create() that executes before we write a pipeline definition
to cluster state).

Signed-off-by: Michael Froh <froh@amazon.com>
Signed-off-by: Michael Froh <froh@amazon.com>
# Conflicts:
#	CHANGELOG.md
#	server/src/main/java/org/opensearch/plugins/SearchPipelinePlugin.java
#	server/src/main/java/org/opensearch/search/pipeline/PipelineWithMetrics.java
#	server/src/test/java/org/opensearch/search/pipeline/SearchPipelineServiceTests.java
Signed-off-by: Mingshi Liu <mingshl@amazon.com>
@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      1 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testNodeDropWithOngoingReplication

@codecov
Copy link

codecov bot commented Jul 1, 2023

Codecov Report

Merging #8390 (188799a) into main (a68733a) will increase coverage by 0.03%.
The diff coverage is 79.06%.

❗ Current head 188799a differs from pull request most recent head d136a8f. Consider uploading reports for the commit d136a8f to get more accurate results

@@             Coverage Diff              @@
##               main    #8390      +/-   ##
============================================
+ Coverage     70.82%   70.86%   +0.03%     
+ Complexity    56922    56885      -37     
============================================
  Files          4757     4757              
  Lines        269125   269148      +23     
  Branches      39403    39402       -1     
============================================
+ Hits         190609   190727     +118     
+ Misses        62479    62330     -149     
- Partials      16037    16091      +54     
Impacted Files Coverage Δ
.../pipeline/common/RenameFieldResponseProcessor.java 94.59% <ø> (ø)
...search/pipeline/common/ScriptRequestProcessor.java 30.00% <0.00%> (-5.30%) ⬇️
...eline/common/SearchPipelineCommonModulePlugin.java 0.00% <ø> (ø)
...pensearch/search/pipeline/PipelineWithMetrics.java 90.65% <88.88%> (-0.44%) ⬇️
...h/pipeline/common/FilterQueryRequestProcessor.java 96.00% <100.00%> (+10.70%) ⬆️
...a/org/opensearch/plugins/SearchPipelinePlugin.java 80.00% <100.00%> (+80.00%) ⬆️
...java/org/opensearch/search/pipeline/Processor.java 40.00% <100.00%> (-60.00%) ⬇️
...nsearch/search/pipeline/SearchPipelineService.java 84.54% <100.00%> (ø)

... and 446 files with indirect coverage changes

@mingshl mingshl requested a review from sachinpkale as a code owner July 1, 2023 00:03
@mingshl
Copy link
Contributor Author

mingshl commented Jul 1, 2023

@dblock created this PR to resolve merge conflict for 8164 , can you please take a review? There are other PR #8373 that will follow this one as well.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2023

Gradle Check (Jenkins) Run Completed with:

  • RESULT: UNSTABLE ❕
  • TEST FAILURES:
      2 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testNodeDropWithOngoingReplication
      1 org.opensearch.remotestore.SegmentReplicationUsingRemoteStoreIT.testDropPrimaryDuringReplication
      1 org.opensearch.action.admin.cluster.node.tasks.ResourceAwareTasksTests.testBasicTaskResourceTracking

@msfroh
Copy link
Collaborator

msfroh commented Jul 2, 2023

@mingshl -- I've got this. Sorry, I was traveling for a couple of days and didn't have stable internet access.

I've updated #8164

@mingshl mingshl closed this Jul 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[Search Pipelines] Let a processor factory know if it's being called from an ad hoc / per-request pipeline
2 participants