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

Change the "Master" nomenclature #463

Closed
1 task done
Tracked by #2589 ...
tlfeng opened this issue Mar 4, 2022 · 10 comments
Closed
1 task done
Tracked by #2589 ...

Change the "Master" nomenclature #463

tlfeng opened this issue Mar 4, 2022 · 10 comments
Assignees
Labels
enhancement New feature or request v2.1.0 v2.3.0 'Issues and PRs related to version v2.3.0'

Comments

@tlfeng
Copy link

tlfeng commented Mar 4, 2022

Is your feature request related to a problem? Please describe.
OpenSearch repository is going to replace the terminology "master"with "cluster manager".
issue: opensearch-project/OpenSearch#472, with the plan for its terminology replacement.

Although the existing usages with "master" will be supported in OpenSearch version 2.x to keep the backwards compatibility, please prepare for the nomenclature change in advance, and replace all the usages with "master" terminology in the code base.
All the OpenSearch REST APIs and settings that contain "master" terminology will be deprecated in 2.0, and alternative usages will be added.

Describe the solution you'd like
Replace the terminology "master" with "cluster manager".

When being compatible with OpenSearch 2.0:

  • Deprecate REST API (request parameter) that contains "master" and provide alternative inclusive usage:
    deprecate parameter "master_timeout" and add new parameter "cluster_manager_timeout" in RestQuerySettingsAction (updated after the below comment)

When being compatible with OpenSearch 3.0:

  • Replace "master" in the reference to OpenSearch Java API
    • clusterUpdateSettingsRequest.masterNodeTimeout -> clusterUpdateSettingsRequest.clusterManagerNodeTimeout (new API is available in version 2.2)

Describe alternatives you've considered
None.

Additional context

@dblock
Copy link
Member

dblock commented Apr 19, 2022

I am still seeing

clusterUpdateSettingsRequest.masterNodeTimeout(request.paramAsTime(

reopening

@dblock dblock reopened this Apr 19, 2022
@dai-chen
Copy link
Collaborator

I am still seeing

clusterUpdateSettingsRequest.masterNodeTimeout(request.paramAsTime(

reopening

I didn't see the new API for this. Do we have a doc for this?

@tlfeng
Copy link
Author

tlfeng commented Apr 19, 2022

@dai-chen Thanks for taking care of the issue.
Looks like it's a REST API of SQL plugin to update setting, which using a request parameter master_timeout which contains "master" word. Sorry I didn't understand this usage in SQL plugin correctly and didn't mention correctly in above.

I suggest to deprecate the request parameter master_timeout, and add a new request parameter cluster_manager_timeout. Then remove the support of parameter master_timeout in the future.

How OpenSearch deal with the API request parameter master_timeout:
In OpenSearch, the parameter master_timeout is deprecated and will be replaced by parameter cluster_manager_timeout.
The timeline for deprecating the parameter master_timeout is special in OpenSearch (core):

  1. In version 2.0: new parameter cluster_manager_timeout is added, while no warning for using master_timeout.
  2. In version 3.0: deprecation warning will be emitted if using the parameter master_timeout.
  3. In version 4.0: the support of master_timeout will be removed.

Technically it is fine to keep using the parameter master_timeout when being compatible with OpenSearch 2.0, but deprecation warning will be shown in 3.0 .

I described all the deprecated non-inclusive REST API / Settings and the alternatives in issue opensearch-project/documentation-website#450.
The code change was tracked in the issue opensearch-project/OpenSearch#2511
You could take them as reference.

Note that the method masterNodeTimeout() will be replaced in OpenSearch 3.0, so there is no action for being compatible with version 2.0 . The work to replace "master" in OpenSearch public Java API is tracked in the issue opensearch-project/OpenSearch#1684, and the work hasn't been started yet.

@dai-chen
Copy link
Collaborator

@tlfeng Thanks for the very detailed info! Will refresh OpenSearch dependency and check the latest API. Thanks

@tlfeng
Copy link
Author

tlfeng commented Apr 20, 2022

To clarify, there are 2 usages with "master" word in this line of code:

clusterUpdateSettingsRequest.masterNodeTimeout(request.paramAsTime("master_timeout", clusterUpdateSettingsRequest.masterNodeTimeout()));
  1. ClusterUpdateSettingsRequest.masterNodeTimeout()
    It's an OpenSearch Java API, and need to wait for the OpenSearch (core) to make the name change before version 3.0.
    It can only changed when being compatible with OpenSearch 3.0 .
  2. "master_timeout"
    It's a REST API request parameter. In this use case, the parameter is not only involved in the REST API of SQL plugin, but also transmits to OpenSearch REST API call.
    It's recommended to use cluster_manager_timeout instead in version 2.0, but please consider the backwards compatibility.
    It has to be changed when being compatible with OpenSearch 3.0, otherwise deprecation warning and log with be generated due to the use of master_timeout in OpenSearch REST API call.

There is no impact for not making changes on the 2 "master" usages in OpenSearch 2.0, so I'm fine to close this issue.
But I recommend to deprecate the use of "master_timeout" properly in version 2.x .

Here is an example to deprecate parameter master_timeout and add support for the alternative cluster_manager_timeout in "RestAction" class:
https://github.com/opensearch-project/OpenSearch/blob/ba8657aca5d8918c5393308d248a711a75bd4905/server/src/main/java/org/opensearch/rest/action/admin/indices/RestGetMappingAction.java#L100-L108

@dai-chen
Copy link
Collaborator

As @tlfeng clarified above, closing this for now. Feel free to comment and reopen if concern.

@dblock dblock added v2.1.0 and removed v2.0.0 labels May 3, 2022
@dblock
Copy link
Member

dblock commented May 3, 2022

I think this shouldn't have been closed, we're just saying "stop using master_timeout in 2.1", I'll reopen.

clusterUpdateSettingsRequest.masterNodeTimeout(request.paramAsTime(

@dblock dblock reopened this May 3, 2022
@dai-chen
Copy link
Collaborator

dai-chen commented May 3, 2022

I think this shouldn't have been closed, we're just saying "stop using master_timeout in 2.1", I'll reopen.

clusterUpdateSettingsRequest.masterNodeTimeout(request.paramAsTime(

Sure. Once OpenSearch API change complete, this can be fixed in SQL accordingly.

@anasalkouz anasalkouz added the v2.3.0 'Issues and PRs related to version v2.3.0' label Aug 25, 2022
@anasalkouz
Copy link
Member

OpenSearch Java APIs are completed and released on 2.2.
@dai-chen @anirudha Could you please proceed and complete the remaining part from your end. Let's target to finish and close this on 2.3.

@dai-chen
Copy link
Collaborator

OpenSearch Java APIs are completed and released on 2.2. @dai-chen @anirudha Could you please proceed and complete the remaining part from your end. Let's target to finish and close this on 2.3.

Sure, will work on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request v2.1.0 v2.3.0 'Issues and PRs related to version v2.3.0'
Projects
None yet
Development

No branches or pull requests

5 participants