[Feature/extensions] Renaming tests to be consistent with main branch, changing latches, and removing NamedWriteableRegistry from extensions by ryanbogan · Pull Request #5571 · opensearch-project/OpenSearch

ryanbogan · 2022-12-14T20:57:18Z

Signed-off-by: Ryan Bogan [email protected]

Description

Renaming tests to be consistent with main branch

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.

Signed-off-by: Ryan Bogan <[email protected]>

github-actions · 2022-12-14T21:20:19Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/8106/
CommitID: 2d4f7be
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?

github-actions · 2022-12-14T21:29:49Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/8107/
CommitID: 0eeddf2
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?

github-actions · 2022-12-14T21:56:56Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/8110/
CommitID: 7108f84
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?

ryanbogan · 2022-12-14T22:18:40Z

The test that is failing is also broken on main - I will rebase once the fix is pushed there.

dreamer-89 · 2022-12-14T22:42:51Z

Last few gradle check failures happening due to version bump after 2.4.1 release, #5560. Fixed is merged into main branch. @ryanbogan : Can you please rebase your branch against latest main branch ?

* What went wrong:
Execution failed for task ':distribution:bwc:bugfix:buildBwcLinuxTar'.
> Building 2.4.1 didn't generate expected file /var/jenkins/workspace/gradle-check/search/distribution/bwc/bugfix/build/bwc/checkout-2.4/distribution/archives/linux-tar/build/distributions/opensearch-min-2.4.1-SNAPSHOT-linux-x64.tar.gz

github-actions · 2022-12-14T23:03:00Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.client.PitIT.testDeleteAllAndListAllPits

URL: https://build.ci.opensearch.org/job/gradle-check/8115/
CommitID: 07865c7
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

codecov-commenter · 2022-12-14T23:04:50Z

Codecov Report

Merging #5571 (a973607) into feature/extensions (c7c908e) will increase coverage by 0.05%.
The diff coverage is 16.66%.

@@                   Coverage Diff                    @@
##             feature/extensions    opensearch-project/OpenSearch#5571      +/-   ##
========================================================
+ Coverage                 70.92%   70.97%   +0.05%     
- Complexity                58504    58547      +43     
========================================================
  Files                      4767     4762       -5     
  Lines                    279189   279038     -151     
  Branches                  40316    40301      -15     
========================================================
+ Hits                     198001   198034      +33     
+ Misses                    65058    64874     -184     
  Partials                  16130    16130

Impacted Files	Coverage Δ
...rch/extensions/rest/RestSendToExtensionAction.java	`28.37% <0.00%> (ø)`
...a/org/opensearch/extensions/ExtensionsManager.java	`58.98% <100.00%> (-2.28%)`	⬇️
...port/ResponseHandlerFailureTransportException.java	`0.00% <0.00%> (-60.00%)`	⬇️
...r/src/main/java/org/opensearch/http/HttpUtils.java	`33.33% <0.00%> (-33.34%)`	⬇️
...nsearch/index/shard/IndexShardClosedException.java	`66.66% <0.00%> (-33.34%)`	⬇️
...a/org/opensearch/index/mapper/MapperException.java	`75.00% <0.00%> (-25.00%)`	⬇️
...ensearch/client/indices/DetailAnalyzeResponse.java	`20.54% <0.00%> (-24.66%)`	⬇️
...a/org/opensearch/extensions/OpenSearchRequest.java	`0.00% <0.00%> (-23.53%)`	⬇️
...pensearch/index/mapper/MapperParsingException.java	`77.77% <0.00%> (-22.23%)`	⬇️
...n/decider/SnapshotInProgressAllocationDecider.java	`52.17% <0.00%> (-21.74%)`	⬇️
... and 460 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

dbwiddis · 2022-12-15T16:51:32Z

server/src/main/java/org/opensearch/extensions/rest/RestSendToExtensionAction.java

            );
            try {
-                inProgressLatch.await(ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS);
+                inProgressFuture.get(ExtensionsManager.EXTENSION_REQUEST_WAIT_TIMEOUT, TimeUnit.SECONDS);


The .get() call blocks so this gives you no improvement on the synchronous nature of the countdown latch.

Recommend you use an .orTimeout() to implement the timeout functionality. That will also change the handling of timeout below from an InterruptedException to an isCompletedExceptionally() test. (Probably include an isCancelled() as well.). Or you could call .join() next which would throw a CompletionException to keep the same logic.

@dbwiddis I wanted this to still be synchronous for now. I think we should either raise a PR on feature/extensions that changes all the latches and .get() calls now or we should wait and do a similar PR once everything is merged to main. Either way, I think this approach is better than changing them one by one as we merge everything to main. Thoughts?

Fair, raise an issue, add a TODO comment and we can merge this.

@ryanbogan can you link the issue here?

https://github.com/opensearch-project/OpenSearch/issues/5595

Signed-off-by: Ryan Bogan <[email protected]>

github-actions · 2022-12-15T23:21:33Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.indices.stats.IndexStatsIT.testFilterCacheStats

URL: https://build.ci.opensearch.org/job/gradle-check/8169/
CommitID: a973607
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: Ryan Bogan <[email protected]>

github-actions · 2022-12-16T18:20:24Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/8197/
CommitID: bc35f74

ryanbogan added 2 commits December 14, 2022 20:56

Renaming tests to be consistent with main branch

2d4f7be

Signed-off-by: Ryan Bogan <[email protected]>

Update CHANGELOG

0eeddf2

Signed-off-by: Ryan Bogan <[email protected]>

Merge branch 'feature/extensions' into rename_tests

7108f84

ryanbogan marked this pull request as ready for review December 14, 2022 22:18

ryanbogan requested review from Bukhtawar, CEHENKLE, Rishikesh1159, VachaShah, adnapibar, anasalkouz, andrross, dblock, dreamer-89, kartg, kotwanikunal, mch2, nknize, owaiskazi19, reta, saratvemulapalli, setiah, shwetathareja, tlfeng and xuezhou25 as code owners December 14, 2022 22:18

Merge branch 'feature/extensions' into rename_tests

07865c7

dbwiddis requested changes Dec 15, 2022

View reviewed changes

Removed NamedWriteableRegistry from extensions and added TODO comments

a973607

Signed-off-by: Ryan Bogan <[email protected]>

ryanbogan changed the title ~~[Feature/extensions] Renaming tests to be consistent with main branch~~ [Feature/extensions] Renaming tests to be consistent with main branch, changing latches, and removing NamedWriteableRegistry from extensions Dec 15, 2022

Rename ExtensionBooleanResponse to AcknowledgedResponse

bc35f74

Signed-off-by: Ryan Bogan <[email protected]>

owaiskazi19 approved these changes Dec 16, 2022

View reviewed changes

ryanbogan merged commit ea057bd into opensearch-project:feature/extensions Dec 19, 2022

ryanbogan deleted the rename_tests branch December 19, 2022 17:52

Conversation

ryanbogan commented Dec 14, 2022

Description

Check List

Uh oh!

github-actions bot commented Dec 14, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Dec 14, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Dec 14, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

ryanbogan commented Dec 14, 2022

Uh oh!

dreamer-89 commented Dec 14, 2022

Uh oh!

github-actions bot commented Dec 14, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

codecov-commenter commented Dec 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dbwiddis Dec 15, 2022

Choose a reason for hiding this comment

Uh oh!

ryanbogan Dec 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dbwiddis Dec 15, 2022

Choose a reason for hiding this comment

Uh oh!

owaiskazi19 Dec 16, 2022

Choose a reason for hiding this comment

Uh oh!

ryanbogan Dec 16, 2022

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 15, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

github-actions bot commented Dec 16, 2022

Gradle Check (Jenkins) Run Completed with:

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov-commenter commented Dec 14, 2022 •

edited

Loading

ryanbogan Dec 15, 2022 •

edited

Loading