Replace parameter 'master_timeout' with 'cluster_manager_tiemout' in RequestConverters of High-Level-Rest-Client

[tlfeng]

Description

This PR can only be merged after all the other PRs listed in the description of issue #2511 (except #2744) are merged.
[^ Prerequisite achieved]

In server directory, the REST API parameter master_timeout is deprecated in version 3.0 (and will be removed in 4.0) to support inclusive naming, and alternative parameter cluster_manager_timeout is introduced (in version 2.0, in issue #2511).
Correspondingly, the new parameter should be used in the High-Level-Rest-Client, which will be done by this PR.

Please note the change will break the compatibility for High-Level-Rest-Client version 3.0 with server 1.x (and below), which means High-Level-Rest-Client version 3.0 will only support server 2.x and 3.x (4.x not exist yet).

• Use parameter cluster_manager_timeout instead of master_timeout in High-Level-Rest-Client RequestConverters class for building REST requests (Otherwise :client:rest-high-level:asyncIntegTest tests will be broken caused by using master_timeout generating deprecation warning) (in commit 2dfb976)
• Modify corresponding unit tests
• Change lots of "master timeout" in internal method and class names. (in commit a2bcb0c)

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.

[opensearch-ci-bot]

❌   Gradle Check failure d15b979dd227f527d665ef823145908ec479eaae
Log 3967

Reports 3967

[opensearch-ci-bot]

❌   Gradle Check failure 2dfb976
Log 3968

Reports 3968

[opensearch-ci-bot]

✅   Gradle Check success cda5e04
Log 4447

Reports 4447

[opensearch-ci-bot]

✅   Gradle Check success 0962c11
Log 4454

Reports 4454

[tlfeng]

Note: this is the core change for this PR, using parameter "cluster_manager_timeout" instead of "master_timeout" to build REST requests can avoid the deprecation warning.

[opensearch-ci-bot]

✅   Gradle Check success a2bcb0c
Log 4455

Reports 4455

[kotwanikunal]

Looking at the files, it looks like a direct replacement of the terms, even in the case of tests even though the parameter is just deprecated. Should we add in duplicate tests around the deprecated term to ensure that the change is non breaking? We can test for the warning instead.

[tlfeng]

Looking at the files, it looks like a direct replacement of the terms, even in the case of tests even though the parameter is just deprecated. Should we add in duplicate tests around the deprecated term to ensure that the change is non breaking?

Hi @kotwanikunal Thanks for reviewing!
Hmm, actually it's not necessary to deprecate the old method Params withMasterTimeout(TimeValue masterTimeout) 😂, because the whole RequestConverter class is an internal class (which isn't exposed to the public, seeing from the class access modifier)
So I don't think adding test for the deprecated "master_timeout" parameter in the REST Client is necessary, because nobody from outside can directly call the method. Do you think it's better for me to remove the old method Params withMasterTimeout(TimeValue masterTimeout) instead of deprecation?

[tlfeng]

@kotwanikunal Thanks for the reminding. I didn't realize that this will be a breaking change for the High Level Rest Client, which makes it can not communicate with OpenSearch 1.x.

The problem is:
Currently the High Level Rest Client will add the optional parameter master_timeout for every 60 API that accepts this parameter when building the REST API requests.
Keep using master_timeout will cause client test failures due to a deprecation warning in HTTP response header.
Change to use cluster_manager_timeout will break the compatibility for the client with server version 1.x .

There are 3 options to deal with the parameter in High Level Rest Client:

1. Break the compatibility with OpenSearch 1.x sever to build REST API requests with new parameter cluster_manager_timeout in High-Level-Rest-Client version 2.x. (In the meantime, I think 2.x server has broken the compatibility with 1.x client due to the mapping type removal)
2. Use deprecated parameter master_timeout to build REST API requests, make 2.x client keep the compatibility with 1.x server. This will result a deprecation warning in the response header for the 60 APIs.
3. Create an additional method for each of the 60 APIs to allow build REST API requests with either old or new parameter. This seems requires lots of code change. It will make 2.x client keep the compatibility with 1.x server.

In the documentation, it's said 1.x client works with 1.x server, so it's reasonable for the 2.x client to only support 2.x server.
What do you think?

[tlfeng]

A follow up, I will find out the exact impact to users about the deprecation warning in the response header. If the impact is subtle, I will keep using "master_timeout" in the RestClient to build requests and add "expectWarnings()" into tests.

[tlfeng]

Replaced by PR #2912 to keep the backwards compatibility of Rest Client 2.x with server 1.x .

[tlfeng]

The change can exist in version >= 3.0

[opensearch-ci-bot]

✅   Gradle Check success a2bcb0c
Log 4527

Reports 4527