Introduce QueryPhaseSearcher extension point (SearchPlugin) #1931

reta · 2022-01-18T20:15:53Z

Signed-off-by: Andriy Redko andriy.redko@aiven.io

Description

In scope of #1500 implementation, provides an extension point to allow plugging in own search implementation to be used at the QueryPhase time. With that, the concurrent search could be implemented using sandbox plugin, without touching the core.

Issues Resolved

[List any issues this PR will resolve]

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

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.

opensearch-ci-bot · 2022-01-18T20:16:04Z

Can one of the admins verify this patch?

opensearch-ci-bot · 2022-01-18T21:03:47Z

✅ Gradle Check success 88a4aa49f99d69279067a1c4b4432490f5bd67a9
Log 1973

Reports 1973

opensearch-ci-bot · 2022-01-18T21:41:16Z

❌ Gradle Check failure 0a8505b67781ea2965972a9b2ece6a654ef9ddde
Log 1977

Reports 1977

opensearch-ci-bot · 2022-01-19T18:38:08Z

✅ Gradle Check success b27d0af9da908f5732f47264bb5d9739153d0037
Log 1987

Reports 1987

opensearch-ci-bot · 2022-01-19T21:45:34Z

❌ Gradle Check failure 3c366a7a0d9649b9e99f2552334ebbe4e0aaede2
Log 1991

Reports 1991

opensearch-ci-bot · 2022-01-19T22:57:39Z

✅ Gradle Check success f786b579fccd732f8c78639abe8e4d449e23abcb
Log 1993

Reports 1993

opensearch-ci-bot · 2022-01-20T16:07:38Z

❌ Gradle Check failure 3ce92cd1ef437398c0bcddfe42a71a445582582b
Log 2003

Reports 2003

opensearch-ci-bot · 2022-01-20T16:21:44Z

❌ Gradle Check failure d031fbb0ea173d60cf22a3cdd7eb1a4ead3dd427
Log 2004

Reports 2004

reta · 2022-01-20T16:23:20Z

server/src/main/java/org/opensearch/plugins/SearchPlugin.java

@@ -178,6 +184,36 @@
        return emptyList();
    }

+    /**


@nknize @andrross guys would really appreciate the feedback if I am on the right track here, moving towards sandboxing concurrent search implementation (#1500) .

Basically, the idea is to provide the extension points to allow plugging in different searcher implementation(s) at QueryPhase time (the default would be the current sequential scan). In this case the core would know nothing about concurrent / non-concurrent nature of the search over Apache Lucene segments. But the sandbox plugin, if installed, could tweak that (even on per index basis) and substitute the searcher with concurrent one.

Thank you.

Long-term, is it possible for concurrent search to become the only implementation, even if there are some cases where it is configured to run with a single thread?

I think so, yeah, there are two flows to address (aggregations and forced early termination) but I don't see the reasons why it couldn't become the only implementation in future.

My initial thought is that it would be good to get towards a single implementation for something like this and so it might not be worth it to make it pluggable at this point. Would love to get Nick's @nknize opinion on this as well.

Yeah, the initial implementation (#1500) is doing exactly that, but @nknize had the concerns and suggested to sandbox the changes.

lol, then let's definitely get Nick's input on this!

reta · 2022-01-20T16:25:29Z

x Gradle Check failure d031fbb Log 2004

Reports 2004

Infra issues accessing Gradle plugin, response (HTTP/502):

 Execution failed for task ':spotlessInternalRegisterDependencies'.
> Could not resolve all files for configuration ':detachedConfiguration3'.
   > Could not resolve com.diffplug.spotless:spotless-eclipse-base:3.3.0.
     Required by:
         project :
      > Skipped due to earlier error
   > Could not resolve org.eclipse.platform:org.eclipse.core.runtime:3.18.0.
     Required by:
         project :
      > Skipped due to earlier error

opensearch-ci-bot · 2022-01-20T20:38:50Z

✅ Gradle Check success 50957f87d2d80a2f4b1d3962128382fda9fba576
Log 2006

Reports 2006

dblock · 2022-02-01T20:47:36Z

Let's move this across the finish line? Rebase? @andrross any other concerns?

andrross

The code looks good to me so I've approved. Just want to confirm with @nknize that this is the approach we want to take based on the above conversion in this PR.

opensearch-ci-bot · 2022-02-02T01:07:45Z

✅ Gradle Check success a675281caf4d796164e784b39f04f1d51a0681a3
Log 2162

Reports 2162

nknize

nit: one minor change on the executeInternal method.

Sorry for the delay on this; the lucene 9 changes and types removal have been taking priority. Let me dig in a little deeper on the API changes to ensure minimal impact to other plugins. We may need to backport this to the 2x and 2.0 branches once they're cut so it makes 2.0 GA but that's not a big issue.

server/src/main/java/org/opensearch/search/query/QueryPhase.java

nknize · 2022-03-21T19:05:49Z

mind re-basing @reta ? Thx!

opensearch-ci-bot · 2022-03-21T19:53:54Z

❌ Gradle Check failure 90b027159482a361c87b92a028d6fbc7480469b0
Log 3608

Reports 3608

reta · 2022-03-21T20:04:06Z

x Gradle Check failure 90b0271 Log 3608

Reports 3608

#1703 :(

andrross · 2022-03-21T20:19:20Z

start gradle check

opensearch-ci-bot · 2022-03-21T20:39:30Z

❌ Gradle Check failure ba6db21f7979246ad4c543cfe711cea75cfc886d
Log 3618

Reports 3618

dblock · 2022-03-21T20:50:57Z


REPRODUCE WITH: ./gradlew ':server:internalClusterTest' --tests "org.opensearch.cluster.allocation.ClusterRerouteIT.testDelayWithALargeAmountOfShards" -Dtests.seed=F608059C10BBC365 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=ar-AE -Dtests.timezone=America/Havana -Druntime.java=17

org.opensearch.cluster.allocation.ClusterRerouteIT > testDelayWithALargeAmountOfShards FAILED
    java.lang.AssertionError: AcknowledgedResponse failed - not acked
    Expected: <true>
         but: was <false>
        at __randomizedtesting.SeedInfo.seed([F608059C10BBC365:DA790F7D01A5E987]:0)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked(OpenSearchAssertions.java:127)
        at org.opensearch.test.hamcrest.OpenSearchAssertions.assertAcked(OpenSearchAssertions.java:131)
        at org.opensearch.test.TestCluster.wipeIndices(TestCluster.java:167)
        at org.opensearch.test.TestCluster.wipe(TestCluster.java:90)
        at org.opensearch.test.OpenSearchIntegTestCase.afterInternal(OpenSearchIntegTestCase.java:601)
        at org.opensearch.test.OpenSearchIntegTestCase.cleanUpCluster(OpenSearchIntegTestCase.java:2235)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at com.carrotsearch.randomizedtesting.RandomizedRunner.invoke(RandomizedRunner.java:1750)
        at com.carrotsearch.randomizedtesting.RandomizedRunner$10.evaluate(RandomizedRunner.java:996)
        at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
        at org.junit.rules.RunRules.evaluate(RunRules.java:20)
        at org.apache.lucene.tests.util.TestRuleSetupTeardownChained$1.evaluate(TestRuleSetupTeardownChained.java:44)
        at org.apache.lucene.tests.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:43)
        at org.apache.lucene.tests.util.TestRuleThreadAndTestName$1.evaluate(TestRuleThreadAndTestName.java:45)
        at org.apache.lucene.tests.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:60)
        at org.apache.lucene.tests.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:44)
        at org.junit.rules.RunRules.evaluate(RunRules.java:20)
        at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
        at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:368)
        at com.carrotsearch.randomizedtesting.ThreadLeakControl.forkTimeoutingTask(ThreadLeakControl.java:817)
        at com.carrotsearch.randomizedtesting.ThreadLeakControl$3.evaluate(ThreadLeakControl.java:468)
        at com.carrotsearch.randomizedtesting.RandomizedRunner.runSingleTest(RandomizedRunner.java:947)
        at com.carrotsearch.randomizedtesting.RandomizedRunner$5.evaluate(RandomizedRunner.java:832)
        at com.carrotsearch.randomizedtesting.RandomizedRunner$6.evaluate(RandomizedRunner.java:883)
        at com.carrotsearch.randomizedtesting.RandomizedRunner$7.evaluate(RandomizedRunner.java:894)
        at org.apache.lucene.tests.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:43)
        at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
        at org.apache.lucene.tests.util.TestRuleStoreClassName$1.evaluate(TestRuleStoreClassName.java:38)
        at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
        at com.carrotsearch.randomizedtesting.rules.NoShadowingOrOverridesOnMethodsRule$1.evaluate(NoShadowingOrOverridesOnMethodsRule.java:40)
        at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
        at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
        at org.apache.lucene.tests.util.TestRuleAssertionsRequired$1.evaluate(TestRuleAssertionsRequired.java:53)
        at org.apache.lucene.tests.util.AbstractBeforeAfterRule$1.evaluate(AbstractBeforeAfterRule.java:43)
        at org.apache.lucene.tests.util.TestRuleMarkFailure$1.evaluate(TestRuleMarkFailure.java:44)
        at org.apache.lucene.tests.util.TestRuleIgnoreAfterMaxFailures$1.evaluate(TestRuleIgnoreAfterMaxFailures.java:60)
        at org.apache.lucene.tests.util.TestRuleIgnoreTestSuites$1.evaluate(TestRuleIgnoreTestSuites.java:47)
        at org.junit.rules.RunRules.evaluate(RunRules.java:20)
        at com.carrotsearch.randomizedtesting.rules.StatementAdapter.evaluate(StatementAdapter.java:36)
        at com.carrotsearch.randomizedtesting.ThreadLeakControl$StatementRunner.run(ThreadLeakControl.java:368)
        at java.base/java.lang.Thread.run(Thread.java:833)

#1561

dblock · 2022-03-21T20:51:26Z

start gradle check

opensearch-ci-bot · 2022-03-21T20:59:43Z

✅ Gradle Check success ba6db21f7979246ad4c543cfe711cea75cfc886d
Log 3623

Reports 3623

opensearch-ci-bot · 2022-03-21T21:20:59Z

❌ Gradle Check failure ba6db21f7979246ad4c543cfe711cea75cfc886d
Log 3625

Reports 3625

dblock · 2022-03-21T21:25:36Z


org.opensearch.cluster.ClusterHealthIT > testHealthOnMasterFailover FAILED
    java.util.concurrent.ExecutionException: MasterNotDiscoveredException[NodeDisconnectedException[[node_s0][127.0.0.1:40775][cluster:monitor/health] disconnected]]; nested: NodeDisconnectedException[[node_s0][127.0.0.1:40775][cluster:monitor/health] disconnected];
        at __randomizedtesting.SeedInfo.seed([1E83A408EA7A1585:806C8B25E9900963]:0)
        at org.opensearch.common.util.concurrent.BaseFuture$Sync.getValue(BaseFuture.java:281)
        at org.opensearch.common.util.concurrent.BaseFuture$Sync.get(BaseFuture.java:268)
        at org.opensearch.common.util.concurrent.BaseFuture.get(BaseFuture.java:99)
        at org.opensearch.cluster.ClusterHealthIT.testHealthOnMasterFailover(ClusterHealthIT.java:393)

        Caused by:
        MasterNotDiscoveredException[NodeDisconnectedException[[node_s0][127.0.0.1:40775][cluster:monitor/health] disconnected]]; nested: NodeDisconnectedException[[node_s0][127.0.0.1:40775][cluster:monitor/health] disconnected];
            at app//org.opensearch.action.support.master.TransportMasterNodeAction$AsyncSingleAction$2.onTimeout(TransportMasterNodeAction.java:275)
            at app//org.opensearch.cluster.ClusterStateObserver$ContextPreservingListener.onTimeout(ClusterStateObserver.java:369)
            at app//org.opensearch.cluster.ClusterStateObserver.waitForNextChange(ClusterStateObserver.java:174)
            at app//org.opensearch.cluster.ClusterStateObserver.waitForNextChange(ClusterStateObserver.java:142)
            at app//org.opensearch.action.support.master.TransportMasterNodeAction$AsyncSingleAction.retry(TransportMasterNodeAction.java:258)
            at app//org.opensearch.action.support.master.TransportMasterNodeAction$AsyncSingleAction.retryOnMasterChange(TransportMasterNodeAction.java:239)
            at app//org.opensearch.action.support.master.TransportMasterNodeAction$AsyncSingleAction$1.handleException(TransportMasterNodeAction.java:224)
            at app//org.opensearch.transport.TransportService$6.handleException(TransportService.java:735)
            at app//org.opensearch.transport.TransportService$ContextRestoreResponseHandler.handleException(TransportService.java:1350)
            at app//org.opensearch.transport.TransportService$9.run(TransportService.java:1207)
            at app//org.opensearch.common.util.concurrent.ThreadContext$ContextPreservingRunnable.run(ThreadContext.java:733)
            at java.base@17.0.2/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136)
            at java.base@17.0.2/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635)
            at java.base@17.0.2/java.lang.Thread.run(Thread.java:833)

            Caused by:
            NodeDisconnectedException[[node_s0][127.0.0.1:40775][cluster:monitor/health] disconnected]

dblock · 2022-03-21T21:25:55Z

start gradle check

opensearch-ci-bot · 2022-03-21T22:03:23Z

❌ Gradle Check failure ba6db21f7979246ad4c543cfe711cea75cfc886d
Log 3631

Reports 3631

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

opensearch-ci-bot · 2022-03-21T23:16:34Z

✅ Gradle Check success 4f492fb
Log 3640

Reports 3640

nknize

This LGTM! Thx for adding this extension. Nice to have concurrent searching in sandbox!

reta · 2022-03-22T12:05:48Z

@nknize @dblock do we want to backport it to 1.x?

dblock · 2022-03-22T14:16:20Z

I marked it to backport 1.x, but since we're branching 2.0 maybe it's time to stop piling things onto that?

Signed-off-by: Andriy Redko <andriy.redko@aiven.io> (cherry picked from commit 82fb7ab)

reta force-pushed the issue-1286.search.plugin branch from 88a4aa4 to 0a8505b Compare January 18, 2022 21:09

reta force-pushed the issue-1286.search.plugin branch from 0a8505b to b27d0af Compare January 19, 2022 17:54

reta force-pushed the issue-1286.search.plugin branch from b27d0af to 3c366a7 Compare January 19, 2022 21:17

reta force-pushed the issue-1286.search.plugin branch from 3c366a7 to f786b57 Compare January 19, 2022 22:13

reta force-pushed the issue-1286.search.plugin branch from f786b57 to 3ce92cd Compare January 20, 2022 16:01

reta force-pushed the issue-1286.search.plugin branch from 3ce92cd to d031fbb Compare January 20, 2022 16:08

reta commented Jan 20, 2022

View reviewed changes

reta marked this pull request as ready for review January 20, 2022 16:24

reta requested a review from a team as a code owner January 20, 2022 16:24

reta force-pushed the issue-1286.search.plugin branch from d031fbb to 50957f8 Compare January 20, 2022 19:57

reta force-pushed the issue-1286.search.plugin branch from 50957f8 to a675281 Compare February 2, 2022 00:24

andrross approved these changes Feb 2, 2022

View reviewed changes

nknize requested changes Mar 21, 2022

View reviewed changes

server/src/main/java/org/opensearch/search/query/QueryPhase.java Show resolved Hide resolved

reta force-pushed the issue-1286.search.plugin branch from a675281 to 90b0271 Compare March 21, 2022 19:23

reta changed the title ~~[WIP] Introduce QueryPhaseSearcher extension point (SearchPlugin)~~ Introduce QueryPhaseSearcher extension point (SearchPlugin) Mar 21, 2022

reta force-pushed the issue-1286.search.plugin branch from 90b0271 to ba6db21 Compare March 21, 2022 20:04

reta mentioned this pull request Mar 21, 2022

Concurrent Searching (Experimental) #1500

Merged

14 tasks

dblock approved these changes Mar 21, 2022

View reviewed changes

Introduce QueryPhaseSearcher extension point (SearchPlugin)

4f492fb

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

reta force-pushed the issue-1286.search.plugin branch from ba6db21 to 4f492fb Compare March 21, 2022 22:30

nknize self-requested a review March 22, 2022 04:08

nknize approved these changes Mar 22, 2022

View reviewed changes

nknize merged commit 82fb7ab into opensearch-project:main Mar 22, 2022

dblock added the backport 1.x label Mar 22, 2022

opensearch-trigger-bot bot mentioned this pull request Mar 22, 2022

[Backport 1.x] Introduce QueryPhaseSearcher extension point (SearchPlugin) #2559

Closed

opensearch-trigger-bot bot pushed a commit that referenced this pull request Mar 22, 2022

Introduce QueryPhaseSearcher extension point (SearchPlugin) (#1931)

4980ae2

Signed-off-by: Andriy Redko <andriy.redko@aiven.io> (cherry picked from commit 82fb7ab)

reta mentioned this pull request Mar 23, 2022

[Backport] [1.x] Introduce QueryPhaseSearcher extension point (SearchPlugin) #2569

Merged

5 tasks

Introduce QueryPhaseSearcher extension point (SearchPlugin) #1931

Introduce QueryPhaseSearcher extension point (SearchPlugin) #1931

Conversation

reta commented Jan 18, 2022 • edited Loading

Description

Issues Resolved

Check List

opensearch-ci-bot commented Jan 18, 2022

opensearch-ci-bot commented Jan 18, 2022

opensearch-ci-bot commented Jan 18, 2022

opensearch-ci-bot commented Jan 19, 2022

opensearch-ci-bot commented Jan 19, 2022

opensearch-ci-bot commented Jan 19, 2022

opensearch-ci-bot commented Jan 20, 2022

opensearch-ci-bot commented Jan 20, 2022

reta Jan 20, 2022

Choose a reason for hiding this comment

andrross Jan 20, 2022

Choose a reason for hiding this comment

reta Jan 20, 2022

Choose a reason for hiding this comment

andrross Jan 20, 2022

Choose a reason for hiding this comment

reta Jan 20, 2022

Choose a reason for hiding this comment

andrross Jan 20, 2022

Choose a reason for hiding this comment

reta commented Jan 20, 2022 • edited Loading

opensearch-ci-bot commented Jan 20, 2022

dblock commented Feb 1, 2022

andrross left a comment

Choose a reason for hiding this comment

opensearch-ci-bot commented Feb 2, 2022

nknize left a comment

Choose a reason for hiding this comment

nknize commented Mar 21, 2022

opensearch-ci-bot commented Mar 21, 2022

reta commented Mar 21, 2022

andrross commented Mar 21, 2022

opensearch-ci-bot commented Mar 21, 2022

dblock commented Mar 21, 2022 • edited Loading

dblock commented Mar 21, 2022

opensearch-ci-bot commented Mar 21, 2022

opensearch-ci-bot commented Mar 21, 2022

dblock commented Mar 21, 2022

dblock commented Mar 21, 2022

opensearch-ci-bot commented Mar 21, 2022

opensearch-ci-bot commented Mar 21, 2022

nknize left a comment

Choose a reason for hiding this comment

reta commented Mar 22, 2022

dblock commented Mar 22, 2022

reta commented Jan 18, 2022 •

edited

Loading

reta commented Jan 20, 2022 •

edited

Loading

dblock commented Mar 21, 2022 •

edited

Loading