Backwards compatibility tests for async search #95

bharath-techie · 2022-02-22T06:28:04Z

Description

Description of changes:

Added Async search backwards compatibility tests for mixed cluster, rolling upgrade and full restart scenarios
Updates DEVELOPER_GUIDE to add instructions for running bwc tests

Issues Resolved

[List any issues this PR will resolve]
#33

Check List

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.

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

codecov-commenter · 2022-02-22T06:34:37Z

Codecov Report

❗ No coverage uploaded for pull request base (main@3e2e50c). Click here to learn what that means.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main      #95   +/-   ##
=======================================
  Coverage        ?   83.38%           
  Complexity      ?      554           
=======================================
  Files           ?       61           
  Lines           ?     2058           
  Branches        ?      146           
=======================================
  Hits            ?     1716           
  Misses          ?      236           
  Partials        ?      106

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e2e50c...ab0bdb1. Read the comment docs.

eirsep · 2022-02-22T10:26:10Z

README.md

@@ -73,7 +73,10 @@ The project in this package uses the [Gradle](https://docs.gradle.org/current/us
 5. `./gradlew integTest -PnumNodes=3` launches a multi-node cluster with the asynchronous search plugin installed and runs all integ tests.
 6. `./gradlew integTest -Dtests.class=*AsynchronousSearchRestIT` runs a single integ class
 7.  `./gradlew integTest -Dtests.class=*AsynchronousSearchRestIT -Dtests.method="testSubmitWithRetainedResponse"` runs a single integ test method (remember to quote the test method name if it contains spaces)
-
+8. `./gradlew asynSearchCluster#mixedClusterTask -Dtests.security.manager=false` launches a cluster of three nodes of bwc version of OpenSearch with async search plugin and tests backwards compatibility by performing rolling upgrade of each node with the current version of OpenSearch with async search plugin.


8. mixedClusterTask and 9. rollingUpgradeClusterTask have the same description?

eirsep · 2022-02-22T10:28:30Z

build.gradle

 buildscript {
    ext {
        distribution = 'oss-zip'
        opensearch_group = "org.opensearch"
        isSnapshot = "true" == System.getProperty("build.snapshot", "true")
        opensearch_version = System.getProperty("opensearch.version", "1.3.0-SNAPSHOT")
        // 1.0.0 -> 1.0.0.0, and 1.0.0-SNAPSHOT -> 1.0.0.0-SNAPSHOT
+        os_as_bwc_version = System.getProperty("bwc.version", "1.13.0.1")


os_as_bwc_version sounds confusing. Is this just the opendistro_version

eirsep · 2022-02-22T10:34:33Z

src/test/java/org/opensearch/search/asynchronous/restIT/AsynchronousSearchRestTestCase.java

@@ -86,15 +86,29 @@ public void indexDocuments() throws IOException {
        client().performRequest(new Request(HttpPost.METHOD_NAME, "/_refresh"));
    }

-    AsynchronousSearchResponse executeGetAsynchronousSearch(GetAsynchronousSearchRequest getAsynchronousSearchRequest) throws IOException {


Can we revert this if it's an unintended indentation?

no the code has changed a bit as i've overloaded this method

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

bharath-techie · 2022-03-08T17:02:52Z

@dblock @ohltyler can any of you please review this PR ?

VachaShah · 2022-03-09T19:54:33Z

build.gradle

+        }
+        systemProperty 'tests.rest.bwcsuite_cluster', 'old_cluster'
+        systemProperty 'tests.rest.bwcsuite_round', 'old'
+        systemProperty 'tests.plugin_bwc_version', opendistro_plugin_version


I don't see this system property being used anywhere. Do you still need this?

VachaShah · 2022-03-09T20:07:24Z

src/test/java/org/opensearch/search/asynchronous/bwc/AsyncSearchBackwardsCompatibilityIT.java

+            List<Map<String, Object>> plugins = (List<Map<String, Object>>) response.get("plugins");
+            Set<Object> pluginNames =
+                    plugins.stream().map(map -> map.get("name")).collect(Collectors.toSet());
+            switch (CLUSTER_TYPE) {


Do we need this switch case since we are calling testAsyncSearchAndSettingsApi(true) for all of them?

i made changes to call with false for upgraded plugin.

VachaShah · 2022-03-09T20:07:46Z

src/test/java/org/opensearch/search/asynchronous/bwc/AsyncSearchBackwardsCompatibilityIT.java

+                case OLD:
+                case MIXED:
+                case UPGRADED:
+                    testAsyncSearchAndSettingsApi(true);


Is shouldUseLegacyApi true even for an upgraded cluster?

I initially wrote with a different case where legacy will be used only for old and mixed. But changed it to check if legacy APIs will work for all 3 cases.

Is there any recommendation for upgraded cluster ? should we use the current APIs for upgraded instead of legacy ?

VachaShah · 2022-03-09T20:09:10Z

src/test/java/org/opensearch/search/asynchronous/bwc/AsyncSearchBackwardsCompatibilityIT.java

+                (Map<String, Map<String, Object>>) getAsMap(uri).get("nodes");
+        for (Map<String, Object> response : responseMap.values()) {
+            List<Map<String, Object>> plugins = (List<Map<String, Object>>) response.get("plugins");
+            Set<Object> pluginNames =


Can we verify the plugins installed for the nodes?

VachaShah · 2022-03-09T20:13:41Z

src/test/java/org/opensearch/search/asynchronous/utils/RestTestUtils.java

@@ -67,20 +79,42 @@ public static Request buildHttpRequest(SubmitAsynchronousSearchRequest submitAsy
        return request;
    }

+


Nit: you can remove these empty lines.

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

VachaShah

LGTM, thank you for adding this @bharath-techie!

bharath-techie added 2 commits February 22, 2022 11:34

Async search backwards compatibility tests

cf63d39

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

Async search backwards compatibility tests

117bfd1

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

bharath-techie force-pushed the bctnew branch 2 times, most recently from b9d346b to 0cb3348 Compare February 22, 2022 10:00

Merge remote-tracking branch 'origin/bctnew' into bctnew

e8151ca

bharath-techie force-pushed the bctnew branch from 0cb3348 to e8151ca Compare February 22, 2022 10:03

bharath-techie marked this pull request as ready for review February 22, 2022 10:20

bharath-techie requested a review from a team as a code owner February 22, 2022 10:20

eirsep reviewed Feb 22, 2022

View reviewed changes

addressing review comments

31c1f0f

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

saratvemulapalli requested a review from VachaShah March 8, 2022 17:06

VachaShah reviewed Mar 9, 2022

View reviewed changes

bharath-techie force-pushed the bctnew branch from 60dbd2a to c8b8cb9 Compare March 10, 2022 16:00

Addressing review comments

4fa7880

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

bharath-techie force-pushed the bctnew branch from c8b8cb9 to 4fa7880 Compare March 10, 2022 16:03

bharath-techie added 2 commits March 10, 2022 21:34

Addressing review comments

61f0a93

Signed-off-by: Bharathwaj G <bharath78910@gmail.com>

Merge remote-tracking branch 'origin/bctnew' into bctnew

dd474ba

VachaShah approved these changes Mar 10, 2022

View reviewed changes

eirsep approved these changes Mar 11, 2022

View reviewed changes

eirsep merged commit 229a84a into opensearch-project:main Mar 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Backwards compatibility tests for async search #95

Backwards compatibility tests for async search #95

bharath-techie commented Feb 22, 2022 •

edited

Loading

codecov-commenter commented Feb 22, 2022

eirsep Feb 22, 2022

eirsep Feb 22, 2022

eirsep Feb 22, 2022

bharath-techie Feb 22, 2022

bharath-techie commented Mar 8, 2022

VachaShah Mar 9, 2022

VachaShah Mar 9, 2022

bharath-techie Mar 10, 2022

VachaShah Mar 9, 2022

bharath-techie Mar 10, 2022

VachaShah Mar 9, 2022

bharath-techie Mar 10, 2022

VachaShah Mar 9, 2022

VachaShah left a comment

		@@ -67,20 +79,42 @@ public static Request buildHttpRequest(SubmitAsynchronousSearchRequest submitAsy
		return request;
		}

Backwards compatibility tests for async search #95

Backwards compatibility tests for async search #95

Conversation

bharath-techie commented Feb 22, 2022 • edited Loading

Description

Issues Resolved

Check List

codecov-commenter commented Feb 22, 2022

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bharath-techie commented Mar 8, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VachaShah left a comment

Choose a reason for hiding this comment

bharath-techie commented Feb 22, 2022 •

edited

Loading