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 Ingest APIs and Script APIs #2682

Merged

Conversation

tlfeng
Copy link
Collaborator

@tlfeng tlfeng commented Mar 31, 2022

Description

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

List of the Ingest APIs and Script APIs that have got request parameter master_timeout:

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
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 89444cba51759853a3d1f83753799d832d952596
Log 3963

Reports 3963

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success b5563d3
Log 3964

Reports 3964

@tlfeng tlfeng added the WIP Work in progress label Mar 31, 2022
@tlfeng
Copy link
Collaborator Author

tlfeng commented Apr 1, 2022

In log 3964:

REPRODUCE WITH: ./gradlew ':client:rest-high-level:asyncIntegTest' --tests "org.opensearch.client.StoredScriptsIT.testDeleteStoredScript" -Dtests.seed=555C66C872D25111 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=es -Dtests.timezone=Israel -Druntime.java=17

org.opensearch.client.StoredScriptsIT > testDeleteStoredScript FAILED
    org.opensearch.client.WarningFailureException: method [DELETE], host [http://[::1]:36429], URI [/_scripts/calculate-score?master_timeout=50s&timeout=50s], 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([555C66C872D25111:D1CB1B2F4ED7FB98]: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)

The Client test failure will be resolved in #2702

Tianli Feng added 3 commits April 1, 2022 17:41
Signed-off-by: Tianli Feng <[email protected]>

# Conflicts:
#	server/src/main/java/org/opensearch/rest/BaseRestHandler.java
Signed-off-by: Tianli Feng <[email protected]>
@tlfeng tlfeng removed the WIP Work in progress label Apr 2, 2022
@tlfeng tlfeng marked this pull request as ready for review April 2, 2022 01:19
@tlfeng tlfeng requested a review from a team as a code owner April 2, 2022 01:19
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 59260e9
Log 4063

Reports 4063

@tlfeng
Copy link
Collaborator Author

tlfeng commented Apr 2, 2022

In log 4063:

> Task :client:rest-high-level:asyncIntegTest

REPRODUCE WITH: ./gradlew ':client:rest-high-level:asyncIntegTest' --tests "org.opensearch.client.BulkProcessorIT.testGlobalParametersAndBulkProcessor" -Dtests.seed=F88723257C886B84 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=es-ES -Dtests.timezone=Asia/Pyongyang -Druntime.java=17

org.opensearch.client.BulkProcessorIT > testGlobalParametersAndBulkProcessor FAILED
    org.opensearch.client.WarningFailureException: method [PUT], host [http://127.0.0.1:38971], URI [/_ingest/pipeline/pipeline_id?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}

The Client test failure will be resolved in #2702

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!

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure a606dd1
Log 4128

Reports 4128

@tlfeng
Copy link
Collaborator Author

tlfeng commented Apr 4, 2022

In log 4128:

REPRODUCE WITH: ./gradlew ':qa:mixed-cluster:v2.0.0#mixedClusterTest' --tests "org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT" -Dtests.method="test {p0=tasks.list/10_basic/tasks_list test}" -Dtests.seed=E160957F1503B0FE -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=ar-MA -Dtests.timezone=Europe/Ulyanovsk -Druntime.java=17

org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT > test {p0=tasks.list/10_basic/tasks_list test} FAILED
    java.lang.AssertionError: Failure at [tasks.list/10_basic:15]: field [nodes.$node_id.roles] doesn't have a true value
    Expected: not null
         but: was null
        at __randomizedtesting.SeedInfo.seed([E160957F1503B0FE:6934AAA5BBFFDD06]:0)
        at org.opensearch.test.rest.yaml.OpenSearchClientYamlSuiteTestCase.executeSection(OpenSearchClientYamlSuiteTestCase.java:454)
        at org.opensearch.test.rest.yaml.OpenSearchClientYamlSuiteTestCase.test(OpenSearchClientYamlSuiteTestCase.java:427)
failure [{
  1>   "stash" : {
  1>     "body" : {
  1>       "node_failures" : [
  1>         {
  1>           "type" : "failed_node_exception",
  1>           "reason" : "Failed node [cjwC_XsARUu3ON71UEUyWA]",
  1>           "node_id" : "cjwC_XsARUu3ON71UEUyWA",
  1>           "caused_by" : {
  1>             "type" : "transport_serialization_exception",
  1>             "reason" : "Failed to deserialize response from handler [org.opensearch.transport.TransportService$ContextRestoreResponseHandler/org.opensearch.transport.TransportService$6/[cluster:monitor/tasks/lists[n]]:org.opensearch.action.support.tasks.TransportTasksAction$AsyncAction$1@659ce5e4]",
  1>             "caused_by" : {
  1>               "type" : "illegal_state_exception",
  1>               "reason" : "Message not fully read (response) for requestId [112832], handler [org.opensearch.transport.TransportService$ContextRestoreResponseHandler/org.opensearch.transport.TransportService$6/[cluster:monitor/tasks/lists[n]]:org.opensearch.action.support.tasks.TransportTasksAction$AsyncAction$1@659ce5e4], error [false]; resetting"
  1>             }
  1>           }
  1>         },
  1>         {
  1>           "type" : "failed_node_exception",
  1>           "reason" : "Failed node [0cVfsr_NS4mt76zLdOGoBQ]",
  1>           "node_id" : "0cVfsr_NS4mt76zLdOGoBQ",
  1>           "caused_by" : {
  1>             "type" : "transport_serialization_exception",
  1>             "reason" : "Failed to deserialize response from handler [org.opensearch.transport.TransportService$ContextRestoreResponseHandler/org.opensearch.transport.TransportService$6/[cluster:monitor/tasks/lists[n]]:org.opensearch.action.support.tasks.TransportTasksAction$AsyncAction$1@7f66c0b6]",
  1>             "caused_by" : {
  1>               "type" : "illegal_state_exception",
  1>               "reason" : "Message not fully read (response) for requestId [112835], handler [org.opensearch.transport.TransportService$ContextRestoreResponseHandler/org.opensearch.transport.TransportService$6/[cluster:monitor/tasks/lists[n]]:org.opensearch.action.support.tasks.TransportTasksAction$AsyncAction$1@7f66c0b6], error [false]; resetting"
  1>             }
  1>           }
  1>         }
  1>       ],

It's not reproducible locally. re-run: start gradle check
Update: It can be reproduced sometime by running multiple times, and it's tracked in issue #2757

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success a606dd1
Log 4137

Reports 4137

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 64dc9f3
Log 4156

Reports 4156

@tlfeng
Copy link
Collaborator Author

tlfeng commented Apr 4, 2022

In log 4156:

> Task :server:test

REPRODUCE WITH: ./gradlew ':server:test' --tests "org.opensearch.cluster.coordination.JoinTaskExecutorTests.testPreventJoinClusterWithUnsupportedNodeVersions" -Dtests.seed=4F91E1AE4511AA5E -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=mt -Dtests.timezone=Europe/Belfast -Druntime.java=17

org.opensearch.cluster.coordination.JoinTaskExecutorTests > testPreventJoinClusterWithUnsupportedNodeVersions FAILED
    junit.framework.AssertionFailedError: Expected exception IllegalStateException but no exception was thrown
        at __randomizedtesting.SeedInfo.seed([4F91E1AE4511AA5E:CCEE46D497D23A12]:0)
        at org.apache.lucene.tests.util.LuceneTestCase.expectThrows(LuceneTestCase.java:2898)
        at org.apache.lucene.tests.util.LuceneTestCase.expectThrows(LuceneTestCase.java:2884)
        at org.opensearch.cluster.coordination.JoinTaskExecutorTests.testPreventJoinClusterWithUnsupportedNodeVersions(JoinTaskExecutorTests.java:122)

Also seen in #2752

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 1699174
Log 4161

Reports 4161

@tlfeng
Copy link
Collaborator Author

tlfeng commented Apr 4, 2022

In log 4161:

REPRODUCE WITH: ./gradlew ':qa:remote-clusters:integTest' --tests "org.opensearch.cluster.remote.test.RemoteClustersIT.testHAProxyModeConnectionWorks" -Dtests.seed=C9D4052D39E3B73 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=ru -Dtests.timezone=America/Cancun -Druntime.java=17

org.opensearch.cluster.remote.test.RemoteClustersIT > testHAProxyModeConnectionWorks FAILED
    java.lang.AssertionError
        at __randomizedtesting.SeedInfo.seed([C9D4052D39E3B73:BEB9599E8184D2E]: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 1699174
Log 4165

Reports 4165

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 cafd2cc
Log 4220

Reports 4220

"description":"Specify timeout for connection to master",
"deprecated":{
"version":"2.0.0",
"description":"To promote inclusive language, use 'cluster_manager_timeout' instead."
Copy link
Member

Choose a reason for hiding this comment

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

It feels to me promote is the not the right work. may be to support inclusive language. or to be more inclusive, use ..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for your opinion! I will change the word promote to support.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed in commit 7d0de09

@tlfeng tlfeng force-pushed the manager-timeout-ingest-script-api branch from 545f1f1 to 7d0de09 Compare April 11, 2022 21:38
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 7d0de09
Log 4376

Reports 4376

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 545f1f1e921b11030a7f5dd0671ab9fa183ec0d1
Log 4379

Reports 4379

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 7d0de09
Log 4381

Reports 4381

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 3214cc1
Log 4394

Reports 4394

…est-script-api

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 a679634
Log 4399

Reports 4399

@tlfeng tlfeng merged commit 08e4a35 into opensearch-project:main Apr 13, 2022
@tlfeng tlfeng deleted the manager-timeout-ingest-script-api branch April 13, 2022 18:13
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 13, 2022
…_timeout' - in Ingest APIs and Script APIs (#2682)

- Deprecate the request parameter `master_timeout` that used in Ingest APIs and Script 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 08e4a35)
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 13, 2022
…_timeout' - in Ingest APIs and Script APIs (#2682)

- Deprecate the request parameter `master_timeout` that used in Ingest APIs and Script 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 08e4a35)
tlfeng pushed a commit that referenced this pull request Apr 13, 2022
…_timeout' - in Ingest APIs and Script APIs (#2682) (#2890)

- Deprecate the request parameter `master_timeout` that used in Ingest APIs and Script 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 08e4a35)
tlfeng pushed a commit that referenced this pull request Apr 13, 2022
…_timeout' - in Ingest APIs and Script APIs (#2682) (#2891)

- Deprecate the request parameter `master_timeout` that used in Ingest APIs and Script 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 08e4a35)
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.

5 participants