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' as the alternative for 'master_timeout', and deprecate 'master_timeout' - in CAT APIs #2557

Merged
merged 14 commits into from
Apr 1, 2022

Conversation

tlfeng
Copy link
Collaborator

@tlfeng tlfeng commented Mar 22, 2022

Description

Apply the change of CAT Nodes API in PR #2435 to other applicable CAT APIs.

  • Deprecate the request parameter master_timeout that used in many CAT APIs.
  • Add alternative new request parameter cluster_manager_timeout.
  • Add unit tests.

List of the CAT APIs that have got request parameter master_timeout:
CAT Allocation API https://opensearch.org/docs/opensearch/rest-api/cat/cat-allocation/
CAT Indices API https://opensearch.org/docs/opensearch/rest-api/cat/cat-indices/
CAT Master API https://opensearch.org/docs/opensearch/rest-api/cat/cat-master/
CAT Master API https://opensearch.org/docs/opensearch/rest-api/cat/cat-nodeattrs/
CAT Nodes API https://opensearch.org/docs/latest/opensearch/rest-api/cat/cat-nodes/
CAT Nodeattrs API https://opensearch.org/docs/opensearch/rest-api/cat/cat-nodes/
CAT Pending tasks API https://opensearch.org/docs/opensearch/rest-api/cat/cat-pending-tasks/
CAT Plugins API https://opensearch.org/docs/opensearch/rest-api/cat/cat-plugins/
CAT Repositories API https://opensearch.org/docs/opensearch/rest-api/cat/cat-repositories/
CAT Shards API https://opensearch.org/docs/opensearch/rest-api/cat/cat-shards/
CAT Snapshots API https://opensearch.org/docs/opensearch/rest-api/cat/cat-snapshots/
CAT Tamplates API https://opensearch.org/docs/opensearch/rest-api/cat/cat-templates/
CAT Thread pool API https://opensearch.org/docs/opensearch/rest-api/cat/cat-thread-pool/

Issues Resolved

A part of #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 Severity-Blocker v2.0.0 Version 2.0.0 WIP Work in progress labels Mar 22, 2022
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 006da61
Log 3658

Reports 3658

Tianli Feng added 2 commits March 22, 2022 01:11
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure fbec303
Log 3659

Reports 3659

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 7274938
Log 3660

Reports 3660

@tlfeng tlfeng added backport 2.x Backport to 2.x branch backport 2.0 Backport to 2.0 branch and removed WIP Work in progress labels Mar 28, 2022
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure ef81890
Log 3835

Reports 3835

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 365f0bd
Log 3836

Reports 3836

@tlfeng
Copy link
Collaborator Author

tlfeng commented Mar 28, 2022

In log 3836:

REPRODUCE WITH: ./gradlew ':qa:remote-clusters:integTest' --tests "org.opensearch.cluster.remote.test.RemoteClustersIT.testHAProxyModeConnectionWorks" -Dtests.seed=5479B92A16DF1BD9 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=tr -Dtests.timezone=Etc/UCT -Druntime.java=17
WARNING: A terminally deprecated method in java.lang.System has been called
WARNING: System::setSecurityManager has been called by org.gradle.api.internal.tasks.testing.worker.TestWorker (file:/home/ubuntu/.gradle/wrapper/dists/gradle-7.3.3-all/4295vidhdd9hd3gbjyw1xqxpo/gradle-7.3.3/lib/plugins/gradle-testing-base-7.3.3.jar)
WARNING: Please consider reporting this to the maintainers of org.gradle.api.internal.tasks.testing.worker.TestWorker
WARNING: System::setSecurityManager will be removed in a future release

org.opensearch.cluster.remote.test.RemoteClustersIT > testHAProxyModeConnectionWorks FAILED
    java.lang.AssertionError
        at __randomizedtesting.SeedInfo.seed([5479B92A16DF1BD9:530F6CE12D596D84]: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

indices,
indicesOptions,
local,
clusterManagerNodeTimeout,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Note: the massive change around these lines is only caused by the indentation change, which is made by variable name changed from masterNodeTimeout to clusterManagerNodeTimeout.

@tlfeng tlfeng marked this pull request as ready for review March 29, 2022 17:28
@tlfeng tlfeng requested a review from a team as a code owner March 29, 2022 17:28
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 99e2b7b
Log 3863

Reports 3863

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success e1a4be0
Log 3886

Reports 3886

…r class to reduce duplication

Signed-off-by: Tianli Feng <[email protected]>
@tlfeng tlfeng force-pushed the cluster-manager-timeout-cat branch from 80d99fd to f97d74b Compare March 30, 2022 18:25
@tlfeng tlfeng added the WIP Work in progress label Mar 30, 2022
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 80d99fd6ace86da3c8182889ad6d4d11fbe1ab0a
Log 3905

Reports 3905

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success f97d74b
Log 3906

Reports 3906

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

# Conflicts:
#	server/src/main/java/org/opensearch/rest/BaseRestHandler.java
#	server/src/main/java/org/opensearch/rest/action/cat/RestNodesAction.java
#	server/src/test/java/org/opensearch/action/RenamedTimeoutRequestParameterTests.java
@tlfeng tlfeng force-pushed the cluster-manager-timeout-cat branch from 4d0eb19 to dc30f42 Compare April 1, 2022 02:40
@tlfeng tlfeng removed the WIP Work in progress label Apr 1, 2022
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 4d0eb19a2184e87a8f8612dfe329c41229faca95
Log 4004

Reports 4004

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success dc30f42
Log 4005

Reports 4005

Copy link
Member

@mch2 mch2 left a comment

Choose a reason for hiding this comment

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

LGTM

@tlfeng tlfeng merged commit 78465b4 into opensearch-project:main Apr 1, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 1, 2022
…r 'master_timeout', and deprecate 'master_timeout' - in CAT APIs (#2557)

Apply the change of CAT Nodes API in PR #2435 to other applicable CAT APIs.

- Deprecate the request parameter `master_timeout` that used in many CAT APIs.
- Add alternative new request parameter `cluster_manager_timeout`.
- Add unit tests.

Signed-off-by: Tianli Feng <[email protected]>
(cherry picked from commit 78465b4)
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 1, 2022
…r 'master_timeout', and deprecate 'master_timeout' - in CAT APIs (#2557)

Apply the change of CAT Nodes API in PR #2435 to other applicable CAT APIs.

- Deprecate the request parameter `master_timeout` that used in many CAT APIs.
- Add alternative new request parameter `cluster_manager_timeout`.
- Add unit tests.

Signed-off-by: Tianli Feng <[email protected]>
(cherry picked from commit 78465b4)
@tlfeng tlfeng deleted the cluster-manager-timeout-cat branch April 11, 2022 22:47
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.

3 participants