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

Add request parameter 'cluster_manager_timeout' and deprecate 'master_timeout' - in Snapshot APIs #2680

Merged
merged 12 commits into from
Apr 12, 2022

Conversation

tlfeng
Copy link
Collaborator

@tlfeng tlfeng commented Mar 31, 2022

Description

  • Deprecate the request parameter master_timeout that used in Snapshot APIs which have got the parameter.
  • Add alternative new request parameter cluster_manager_timeout.
  • Add unit tests.

List of the Snapshot APIs that have got request parameter master_timeout:
Put snapshot repository API PUT /_snapshot/<repository>, POST /_snapshot/<repository>
Get snapshot repository API
Delete snapshot repository API
Clean up snapshot repository API POST /_snapshot/<repository>/_cleanup
Verify snapshot repository API POST /_snapshot/<repository>/_verify
Create snapshot API
Get snapshot API
Delete snapshot API
Restore snapshot API POST /_snapshot/<repository>/<snapshot>/_restore
Clone snapshot API
Get snapshot status API GET _snapshot/<repository>/<snapshot>/_status

Issues Resolved

A part of issue #2511

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.

@tlfeng tlfeng added enhancement Enhancement or improvement to existing feature or request v2.0.0 Version 2.0.0 backport 2.x Backport to 2.x branch backport 2.0 Backport to 2.0 branch labels Mar 31, 2022
@tlfeng tlfeng force-pushed the manager-timeout-snapshot-api branch from 138eafd to 6142b93 Compare March 31, 2022 05:05
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success c998932
Log 3958

Reports 3958

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 138eafddb434351306909d14b6539fe185ce8f1f
Log 3959

Reports 3959

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 6142b93
Log 3962

Reports 3962

@tlfeng tlfeng added the WIP Work in progress label Mar 31, 2022
@tlfeng tlfeng force-pushed the manager-timeout-snapshot-api branch from 2e293ca to 6142b93 Compare April 1, 2022 08:08
Signed-off-by: Tianli Feng <[email protected]>

# Conflicts:
#	server/src/main/java/org/opensearch/rest/BaseRestHandler.java
@tlfeng tlfeng force-pushed the manager-timeout-snapshot-api branch from 7e6e9dd to 2a289e4 Compare April 1, 2022 08:17
@tlfeng tlfeng marked this pull request as ready for review April 1, 2022 08:34
@tlfeng tlfeng requested a review from a team as a code owner April 1, 2022 08:34
@tlfeng tlfeng removed the WIP Work in progress label Apr 1, 2022
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 2e293caaed8ed8c230fc1d4786b90a1f4456e85b
Log 4022

Reports 4022

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 6142b93
Log 4023

Reports 4023

@tlfeng
Copy link
Collaborator Author

tlfeng commented Apr 1, 2022

In log 4023:

REPRODUCE WITH: ./gradlew ':server:internalClusterTest' --tests "org.opensearch.search.SearchCancellationIT.testMSearchChildRequestCancellationWithClusterLevelTimeout" -Dtests.seed=60DB4EDF578765F3 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=sq -Dtests.timezone=Asia/Beirut -Druntime.java=17

org.opensearch.search.SearchCancellationIT > testMSearchChildRequestCancellationWithClusterLevelTimeout FAILED
    java.lang.AssertionError: Actual child request with cancellation failure is different that expected expected:<[0, 1]> but was:<[]>
        at __randomizedtesting.SeedInfo.seed([60DB4EDF578765F3:DE66853AC6918584]:0)
        at org.junit.Assert.fail(Assert.java:89)
        at org.junit.Assert.failNotEquals(Assert.java:835)
        at org.junit.Assert.assertEquals(Assert.java:120)
        at org.opensearch.search.SearchCancellationIT.ensureMSearchWasCancelled(SearchCancellationIT.java:194)
        at org.opensearch.search.SearchCancellationIT.testMSearchChildRequestCancellationWithClusterLevelTimeout(SearchCancellationIT.java:537)
> Task :client:rest-high-level:asyncIntegTest

REPRODUCE WITH: ./gradlew ':client:rest-high-level:asyncIntegTest' --tests "org.opensearch.client.SnapshotIT.testSnapshotsStatus" -Dtests.seed=60DB4EDF578765F3 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=en -Dtests.timezone=Mexico/BajaSur -Druntime.java=17

org.opensearch.client.SnapshotIT > testSnapshotsStatus FAILED
    org.opensearch.client.WarningFailureException: method [PUT], host [http://[::1]:39105], URI [/_snapshot/test?master_timeout=30s&timeout=30s], status line [HTTP/1.1 200 OK]
    Warnings: [Deprecated parameter [master_timeout] used. To promote inclusive language, please use [cluster_manager_timeout] instead. It will be unsupported in a future major version.]
    {"acknowledged":true}
        at __randomizedtesting.SeedInfo.seed([60DB4EDF578765F3:9713EF5C5009D61B]:0)
        at app//org.opensearch.client.RestClient.convertResponse(RestClient.java:346)
        at app//org.opensearch.client.RestClient$1.completed(RestClient.java:400)
        at app//org.opensearch.client.RestClient$1.completed(RestClient.java:396)
        at app//org.apache.http.concurrent.BasicFuture.completed(BasicFuture.java:122)
        at app//org.apache.http.impl.nio.client.DefaultClientExchangeHandlerImpl.responseCompleted(DefaultClientExchangeHandlerImpl.java:181)

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 2a289e4
Log 4024

Reports 4024

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure d04e4bd
Log 4025

Reports 4025

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 336e7c7
Log 4027

Reports 4027

Signed-off-by: Tianli Feng <[email protected]>

# Conflicts:
#	server/src/test/java/org/opensearch/action/RenamedTimeoutRequestParameterTests.java
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 697712a
Log 4061

Reports 4061

@tlfeng
Copy link
Collaborator Author

tlfeng commented Apr 3, 2022

In log 4061:


REPRODUCE WITH: ./gradlew ':qa:remote-clusters:integTest' --tests "org.opensearch.cluster.remote.test.RemoteClustersIT.testHAProxyModeConnectionWorks" -Dtests.seed=2743972FDE83A85C -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=ar-BH -Dtests.timezone=Asia/Irkutsk -Druntime.java=17

org.opensearch.cluster.remote.test.RemoteClustersIT > testHAProxyModeConnectionWorks FAILED
    java.lang.AssertionError
        at __randomizedtesting.SeedInfo.seed([2743972FDE83A85C:203542E4E505DE01]:0)
        at org.junit.Assert.fail(Assert.java:87)
        at org.junit.Assert.assertTrue(Assert.java:42)
        at org.junit.Assert.assertTrue(Assert.java:53)
        at org.opensearch.cluster.remote.test.RemoteClustersIT.testHAProxyModeConnectionWorks(RemoteClustersIT.java:125)

It's reported in issue #1703

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success bd1f0a3
Log 4129

Reports 4129

Signed-off-by: Tianli Feng <[email protected]>

# Conflicts:
#	server/src/test/java/org/opensearch/action/RenamedTimeoutRequestParameterTests.java
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success b4e9c8b
Log 4155

Reports 4155

Copy link
Member

@dreamer-89 dreamer-89 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! LGTM

Signed-off-by: Tianli Feng <[email protected]>

# Conflicts:
#	server/src/test/java/org/opensearch/action/RenamedTimeoutRequestParameterTests.java
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 0d53a86
Log 4209

Reports 4209

@tlfeng
Copy link
Collaborator Author

tlfeng commented Apr 5, 2022

In log 4209:

> Task :sandbox:plugins:concurrent-search:test

REPRODUCE WITH: ./gradlew ':sandbox:plugins:concurrent-search:test' --tests "org.opensearch.search.query.QueryProfilePhaseTests" -Dtests.method="testTerminateAfterEarlyTermination {p0=5 p1=org.opensearch.search.query.ConcurrentQueryPhaseSearcher@2c4eae94}" -Dtests.seed=CDA03188B38F0C0D -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=ar-TN -Dtests.timezone=America/Noronha -Druntime.java=17

org.opensearch.search.query.QueryProfilePhaseTests > testTerminateAfterEarlyTermination {p0=5 p1=org.opensearch.search.query.ConcurrentQueryPhaseSearcher@2c4eae94} FAILED
    java.lang.AssertionError: 
    Expected: a value greater than <0L>
         but: <0L> was equal to <0L>
        at __randomizedtesting.SeedInfo.seed([CDA03188B38F0C0D:5B04881FBABD7064]:0)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.junit.Assert.assertThat(Assert.java:964)
        at org.junit.Assert.assertThat(Assert.java:930)
        at org.opensearch.search.query.QueryProfilePhaseTests.lambda$testTerminateAfterEarlyTermination$26(QueryProfilePhaseTests.java:553)
        at org.opensearch.search.query.QueryProfilePhaseTests.lambda$assertProfileData$55(QueryProfilePhaseTests.java:1072)
        at org.opensearch.search.query.QueryProfilePhaseTests.assertProfileData(QueryProfilePhaseTests.java:1123)
        at org.opensearch.search.query.QueryProfilePhaseTests.assertProfileData(QueryProfilePhaseTests.java:1078)
        at org.opensearch.search.query.QueryProfilePhaseTests.assertProfileData(QueryProfilePhaseTests.java:1069)
        at org.opensearch.search.query.QueryProfilePhaseTests.testTerminateAfterEarlyTermination(QueryProfilePhaseTests.java:534)
REPRODUCE WITH: ./gradlew ':sandbox:plugins:concurrent-search:test' --tests "org.opensearch.search.query.QueryProfilePhaseTests" -Dtests.method="testTerminateAfterEarlyTermination {p0=0 p1=org.opensearch.search.query.QueryPhase$DefaultQueryPhaseSearcher@6ad198c9}" -Dtests.seed=CDA03188B38F0C0D -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=ar-TN -Dtests.timezone=America/Noronha -Druntime.java=17

org.opensearch.search.query.QueryProfilePhaseTests > testTerminateAfterEarlyTermination {p0=0 p1=org.opensearch.search.query.QueryPhase$DefaultQueryPhaseSearcher@6ad198c9} FAILED
    java.lang.AssertionError: 
    Expected: a value greater than <0L>
         but: <0L> was equal to <0L>
... (same as above)
* What went wrong:
Execution failed for task ':sandbox:plugins:concurrent-search:test'.
> org.gradle.test-retry was unable to retry the following test methods, which is unexpected. Please file a bug report at https://github.com/gradle/test-retry-gradle-plugin/issues
     org.opensearch.search.query.QueryProfilePhaseTests#testTerminateAfterEarlyTermination {p0=5 p1=org.opensearch.search.query.ConcurrentQueryPhaseSearcher@2c4eae94}
     org.opensearch.search.query.QueryProfilePhaseTests#testTerminateAfterEarlyTermination {p0=0 p1=org.opensearch.search.query.QueryPhase$DefaultQueryPhaseSearcher@6ad198c9}

It's reproducible in main branch: ./gradlew ':sandbox:plugins:concurrent-search:test' --tests "org.opensearch.search.query.QueryProfilePhaseTests" -Dtests.method="testTerminateAfterEarlyTermination" -Dtests.seed=CDA03188B38F0C0D. I created issue #2775

@tlfeng
Copy link
Collaborator Author

tlfeng commented Apr 5, 2022

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 0d53a86
Log 4213

Reports 4213

Tianli Feng added 2 commits April 11, 2022 17:35
# Conflicts:
#	server/src/test/java/org/opensearch/action/RenamedTimeoutRequestParameterTests.java
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 712952e
Log 4392

Reports 4392

@tlfeng tlfeng merged commit a89b7e6 into opensearch-project:main Apr 12, 2022
@tlfeng tlfeng deleted the manager-timeout-snapshot-api branch April 12, 2022 03:43
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 12, 2022
…_timeout' - in Snapshot APIs (#2680)

* Deprecate the request parameter `master_timeout` that used in Snapshot APIs which have got the parameter.
* Add alternative new request parameter `cluster_manager_timeout`.
* Add unit tests.

Signed-off-by: Tianli Feng <[email protected]>
(cherry picked from commit a89b7e6)
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 12, 2022
…_timeout' - in Snapshot APIs (#2680)

* Deprecate the request parameter `master_timeout` that used in Snapshot APIs which have got the parameter.
* Add alternative new request parameter `cluster_manager_timeout`.
* Add unit tests.

Signed-off-by: Tianli Feng <[email protected]>
(cherry picked from commit a89b7e6)
tlfeng pushed a commit that referenced this pull request Apr 12, 2022
…_timeout' - in Snapshot APIs (#2680) (#2870)

* Deprecate the request parameter `master_timeout` that used in Snapshot APIs which have got the parameter.
* Add alternative new request parameter `cluster_manager_timeout`.
* Add unit tests.

Signed-off-by: Tianli Feng <[email protected]>
(cherry picked from commit a89b7e6)
tlfeng pushed a commit that referenced this pull request Apr 12, 2022
…_timeout' - in Snapshot APIs (#2680) (#2871)

* Deprecate the request parameter `master_timeout` that used in Snapshot APIs which have got the parameter.
* Add alternative new request parameter `cluster_manager_timeout`.
* Add unit tests.

Signed-off-by: Tianli Feng <[email protected]>
(cherry picked from commit a89b7e6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x Backport to 2.x branch backport 2.0 Backport to 2.0 branch enhancement Enhancement or improvement to existing feature or request v2.0.0 Version 2.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants