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

Replace non-inclusive terminology 'master' in code comments and internal variable/method/class names #2150

Closed
wants to merge 28 commits into from

Conversation

tlfeng
Copy link
Collaborator

@tlfeng tlfeng commented Feb 17, 2022

Description

  • Replace the non-inclusive terminology "master" with "cluster manager" in code comments, internal variable/method/class names, where the backwards compatibility is not impacted.

Replacement rules:

  • master -> cluster_manager or clusterManager (variable name) or cluster manager (code comment)
    e.g. master node -> cluster_manager node
  • Master -> ClusterManager

Issues Resolved

Resolve #1548

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.

Tianli Feng and others added 16 commits February 14, 2022 16:36
@opensearch-ci-bot
Copy link
Collaborator

Can one of the admins verify this patch?

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

❌   Gradle Check failure 9026aed
Log 2483

Reports 2483

@tlfeng
Copy link
Collaborator Author

tlfeng commented Feb 17, 2022

In log 2483:

> Task :qa:mixed-cluster:v1.3.0#mixedClusterTest

REPRODUCE WITH: ./gradlew ':qa:mixed-cluster:v1.3.0#mixedClusterTest' --tests "org.opensearch.backwards.MixedClusterClientYamlTestSuiteIT"

I will check the cause for this failure.

@@ -47,7 +47,7 @@
public static final TimeValue DEFAULT_MASTER_NODE_TIMEOUT = TimeValue.timeValueSeconds(30);
Copy link
Collaborator

Choose a reason for hiding this comment

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

You might also want to update this variable name.

Copy link
Collaborator Author

@tlfeng tlfeng Feb 17, 2022

Choose a reason for hiding this comment

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

Thank you for the review!
I think I'm not going to rename this variable name. It's a Java API which may break the compatibility to other software that uses server package: https://opensearch.org/javadocs/1.2.4/OpenSearch/server/build/docs/javadoc/org/opensearch/action/support/master/MasterNodeRequest.html#DEFAULT_MASTER_NODE_TIMEOUT
The current plan is to replace all those names in Java API directly in version 3.0.0 .

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah okay! Thank you :)

@VachaShah
Copy link
Collaborator

MixedClusterClientYamlTestSuiteIT

This is related to #2143.

@tlfeng
Copy link
Collaborator Author

tlfeng commented Feb 17, 2022

MixedClusterClientYamlTestSuiteIT

This is related to #2143.

Thanks a lot for this pointer 👍😄

Tianli Feng added 2 commits February 17, 2022 14:52
…rnal

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

# Conflicts:
#	test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 4f1211c
Log 3429

Reports 3429

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 717bc11
Log 3430

Reports 3430

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 36a5bbe
Log 3485

Reports 3485

@tlfeng tlfeng force-pushed the replace-master-internal branch from 463eade to f20c029 Compare March 19, 2022 07:16
@tlfeng tlfeng added blocked Identifies issues that are blocked labels Mar 19, 2022
@tlfeng tlfeng force-pushed the replace-master-internal branch from f20c029 to 6ad42d9 Compare March 19, 2022 07:25
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 463eade6817ee7d0c8953345607b439fdde148ac
Log 3537

Reports 3537

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure f20c02998f38fdea96166c32c471be879316cb18
Log 3538

Reports 3538

@tlfeng tlfeng force-pushed the replace-master-internal branch from 6ad42d9 to 5bd150a Compare March 19, 2022 07:37
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 6ad42d973e5910dfd5ce6c4df0ffa13c07e90d41
Log 3539

Reports 3539

@tlfeng
Copy link
Collaborator Author

tlfeng commented Mar 19, 2022

In log 3539:

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

REPRODUCE WITH: ./gradlew ':client:rest-high-level:asyncIntegTest' --tests "org.opensearch.client.ClusterClientIT.testClusterHealthYellowIndicesLevel" -Dtests.seed=CC36987E39D8B3EA -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=ca-ES -Dtests.timezone=America/Mazatlan -Druntime.java=17

org.opensearch.client.ClusterClientIT > testClusterHealthYellowIndicesLevel FAILED
    UncategorizedExecutionException[Failed execution]; nested: ExecutionException[java.io.IOException: Unable to parse response body for Response{requestLine=GET /_cluster/health/index,index2?master_timeout=5s&level=indices&timeout=5s HTTP/1.1, host=http://127.0.0.1:46587, response=HTTP/1.1 200 OK}]; nested: IOException[Unable to parse response body for Response{requestLine=GET /_cluster/health/index,index2?master_timeout=5s&level=indices&timeout=5s HTTP/1.1, host=http://127.0.0.1:46587, response=HTTP/1.1 200 OK}]; nested: IllegalArgumentException[Required [discovered_cluster_manager]];
        at __randomizedtesting.SeedInfo.seed([CC36987E39D8B3EA:5ABFCEA0918E637F]:0)
        at app//org.opensearch.common.util.concurrent.FutureUtils.rethrowExecutionException(FutureUtils.java:104)
        at app//org.opensearch.common.util.concurrent.FutureUtils.get(FutureUtils.java:74)
        at app//org.opensearch.action.support.AdapterActionFuture.actionGet(AdapterActionFuture.java:50)
        at app//org.opensearch.client.OpenSearchRestHighLevelClientTestCase.execute(OpenSearchRestHighLevelClientTestCase.java:134)
        at app//org.opensearch.client.OpenSearchRestHighLevelClientTestCase.execute(OpenSearchRestHighLevelClientTestCase.java:117)
        at app//org.opensearch.client.ClusterClientIT.testClusterHealthYellowIndicesLevel(ClusterClientIT.java:248)
> Task :rest-api-spec:yamlRestTest

REPRODUCE WITH: ./gradlew ':rest-api-spec:yamlRestTest' --tests "org.opensearch.test.rest.ClientYamlTestSuiteIT" -Dtests.method="test {p0=cluster.health/10_basic/Get cluster health has same value for discovered_master and discovered_cluster_manager}" -Dtests.seed=CC36987E39D8B3EA -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=ja -Dtests.timezone=America/Noronha -Druntime.java=17

org.opensearch.test.rest.ClientYamlTestSuiteIT > test {p0=cluster.health/10_basic/Get cluster health has same value for discovered_master and discovered_cluster_manager} FAILED
    java.lang.AssertionError: Failure at [cluster.health/10_basic:290]: field [discovered_cluster_manager] is null
        at __randomizedtesting.SeedInfo.seed([CC36987E39D8B3EA:4462A7A49724DE12]:0)
        at org.opensearch.test.rest.yaml.OpenSearchClientYamlSuiteTestCase.executeSection(OpenSearchClientYamlSuiteTestCase.java:454)
        at org.opensearch.test.rest.yaml.OpenSearchClientYamlSuiteTestCase.test(OpenSearchClientYamlSuiteTestCase.java:427)

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

# Conflicts:
#	buildSrc/src/main/groovy/org/opensearch/gradle/test/ClusterFormationTasks.groovy
#	client/rest/src/main/java/org/opensearch/client/Node.java
#	server/src/main/java/org/opensearch/action/admin/cluster/health/ClusterHealthResponse.java
#	server/src/main/java/org/opensearch/cluster/coordination/ClusterBootstrapService.java
#	server/src/main/java/org/opensearch/cluster/coordination/NoMasterBlockService.java
#	server/src/test/java/org/opensearch/cluster/coordination/NoMasterBlockServiceTests.java
#	server/src/test/java/org/opensearch/env/NodeRepurposeCommandTests.java
#	test/framework/src/main/java/org/opensearch/test/InternalTestCluster.java
@tlfeng tlfeng force-pushed the replace-master-internal branch from 5bd150a to 0817155 Compare March 19, 2022 08:18
@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 5bd150aaa6669306b73c1e5117a2327fb679543d
Log 3542

Reports 3542

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 0817155
Log 3546

Reports 3546

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 3c204eb
Log 3549

Reports 3549

@tlfeng
Copy link
Collaborator Author

tlfeng commented Mar 19, 2022

In log 3549:

REPRODUCE WITH: ./gradlew ':qa:remote-clusters:integTest' --tests "org.opensearch.cluster.remote.test.RemoteClustersIT.testHAProxyModeConnectionWorks" -Dtests.seed=AD318DBD0523C5F5 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=nn-NO -Dtests.timezone=Africa/Sao_Tome -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([AD318DBD0523C5F5:AA4758763EA5B3A8]: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.
start gradle check

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 3c204eb
Log 3557

Reports 3557

@tlfeng
Copy link
Collaborator Author

tlfeng commented Mar 19, 2022

The PR will be split into smaller ones, based on the changes in different directories. See issue #1548 for the split PRs. (Such as PR #2519 #2520 #2521)

@tlfeng tlfeng closed this Mar 19, 2022
@tlfeng tlfeng removed the blocked Identifies issues that are blocked label Mar 19, 2022
@tlfeng tlfeng deleted the replace-master-internal branch April 26, 2022 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.

Replace "master" terminology in code comments, and internal variable, method and class names
3 participants