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.

Sign up for GitHub

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

Jump to bottom

[2.x] Load the deprecated master role in a dedocated method instead of in setAdditionalRoles() #4653

Merged
merged 3 commits into from
Oct 10, 2022
Merged

[2.x] Load the deprecated master role in a dedocated method instead of in setAdditionalRoles() #4653

merged 3 commits into from
Oct 10, 2022

Conversation

tlfeng
Copy link
Collaborator

@tlfeng tlfeng commented Sep 30, 2022 •
edited
Loading

Description

Backport PR #4582 / commit ff2d5be to 2.x branch

The original code is introduced by commit ea31483 / PR #2424, where node role 'master' is deprecated, and the alternative role 'cluster_manager' is added.
The location for loading the deprecated master role in the workaround is not suitable.

Changes:

  • Change the location to load the definition of deprecated master role from the method setAdditionalRoles() (which used to load roles defined by plugins) to a new dedicated method setDeprecatedMasterRole().
  • Change the unit test accordingly.
  • Restore the method setAdditionalRoles() by removing my recent change.

Issues Resolved

#4579

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
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

Load the deprecated master role in a dedocated method instead of in s…
3030e1f 
…etAdditionalRoles()

Signed-off-by: Tianli Feng <ftianli@amazon.com>
@tlfeng tlfeng requested review from a team and reta as code owners September 30, 2022 17:29
@tlfeng tlfeng changed the title Load the deprecated master role in a dedocated method instead of in setAdditionalRoles() [2.x] Load the deprecated master role in a dedocated method instead of in setAdditionalRoles() Sep 30, 2022
@tlfeng tlfeng added enhancement Enhancement or improvement to existing feature or request backport PRs or issues specific to backporting features or enhancments v2.4.0 'Issues and PRs related to version v2.4.0' labels Sep 30, 2022
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@tlfeng
Copy link
Collaborator Author

tlfeng commented Sep 30, 2022 •
edited
Loading

In build 3641:
The test failure is reported in #3650 #4162

REPRODUCE WITH: ./gradlew ':server:internalClusterTest' --tests "org.opensearch.cluster.routing.allocation.decider.DiskThresholdDeciderIT.testRestoreSnapshotAllocationDoesNotExceedWatermark" -Dtests.seed=9E613588316B0B9C -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=es-VE -Dtests.timezone=America/Argentina/Tucuman -Druntime.java=17
  2> java.lang.AssertionError: 
    Expected: an empty collection
         but: <[[tripibrwuz][0], node[1kzq28Y4TZav3IRqbjLUWQ], [P], s[STARTED], a[id=hdt0hQPWR0qLYqToiCoDxQ], [tripibrwuz][4], node[1kzq28Y4TZav3IRqbjLUWQ], [P], s[STARTED], a[id=aadGAHXGSF6ifyRGEmwbuA]]>
        at __randomizedtesting.SeedInfo.seed([9E613588316B0B9C:8CD993E95AD21424]:0)
        at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
        at org.junit.Assert.assertThat(Assert.java:964)
        at org.junit.Assert.assertThat(Assert.java:930)
        at org.opensearch.cluster.routing.allocation.decider.DiskThresholdDeciderIT.lambda$testRestoreSnapshotAllocationDoesNotExceedWatermark$2(DiskThresholdDeciderIT.java:260)
        at org.opensearch.test.OpenSearchTestCase.assertBusy(OpenSearchTestCase.java:1049)
        at org.opensearch.test.OpenSearchTestCase.assertBusy(OpenSearchTestCase.java:1022)
        at org.opensearch.cluster.routing.allocation.decider.DiskThresholdDeciderIT.testRestoreSnapshotAllocationDoesNotExceedWatermark(DiskThresholdDeciderIT.java:260)

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

xuezhou25
xuezhou25 approved these changes Oct 6, 2022
View reviewed changes
Copy link
Contributor

@xuezhou25 xuezhou25 left a comment

Choose a reason for hiding this comment

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

LGTM

Merge remote-tracking branch 'upstream/2.x' into 2.x-master-not-addit…
27ba8ba 
…ionalrole

Signed-off-by: Tianli Feng <ftianli@amazon.com>

# Conflicts:
#	CHANGELOG.md
@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2022

Gradle Check (Jenkins) Run Completed with:

@tlfeng
Copy link
Collaborator Author

tlfeng commented Oct 7, 2022

The test failure In build 4158

REPRODUCE WITH: ./gradlew ':server:test' --tests "org.opensearch.index.fielddata.SortedSetDVStringFieldDataTests.testSortMissingLastReverse" -Dtests.seed=993E0C93626592D3 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=es-SV -Dtests.timezone=America/North_Dakota/Beulah -Druntime.java=17

org.opensearch.index.fielddata.SortedSetDVStringFieldDataTests > testSortMissingLastReverse FAILED
    java.lang.AssertionError: expected:<1096> but was:<1046>
        at __randomizedtesting.SeedInfo.seed([993E0C93626592D3:560A3D20DD518DFE]:0)
        at org.junit.Assert.fail(Assert.java:89)
        at org.junit.Assert.failNotEquals(Assert.java:835)
        at org.junit.Assert.assertEquals(Assert.java:647)
        at org.junit.Assert.assertEquals(Assert.java:633)
        at org.opensearch.index.fielddata.AbstractStringFieldDataTestCase.testSortMissing(AbstractStringFieldDataTestCase.java:343)
        at org.opensearch.index.fielddata.AbstractStringFieldDataTestCase.testSortMissingLastReverse(AbstractStringFieldDataTestCase.java:313)

@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2022

Gradle Check (Jenkins) Run Completed with:

Merge remote-tracking branch 'upstream/2.x' into 2.x-master-not-addit…
02ab2fc 
…ionalrole

Signed-off-by: Tianli Feng <ftianli@amazon.com>
@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2022

Gradle Check (Jenkins) Run Completed with:

@tlfeng
Copy link
Collaborator Author

tlfeng commented Oct 10, 2022 •
edited
Loading

test failure in 4198

REPRODUCE WITH: ./gradlew ':server:test' --tests "org.opensearch.index.fielddata.SortedSetDVStringFieldDataTests.testSortMissingLast" -Dtests.seed=513A44EA1726382 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=tr-TR -Dtests.timezone=America/Anchorage -Druntime.java=17

org.opensearch.index.fielddata.SortedSetDVStringFieldDataTests > testSortMissingLast FAILED
    java.lang.AssertionError: expected:<1727> but was:<1440>
        at __randomizedtesting.SeedInfo.seed([513A44EA1726382:ED6C2BC717D33683]:0)
        at org.junit.Assert.fail(Assert.java:89)
        at org.junit.Assert.failNotEquals(Assert.java:835)
        at org.junit.Assert.assertEquals(Assert.java:647)
        at org.junit.Assert.assertEquals(Assert.java:633)
        at org.opensearch.index.fielddata.AbstractStringFieldDataTestCase.testSortMissing(AbstractStringFieldDataTestCase.java:343)
        at org.opensearch.index.fielddata.AbstractStringFieldDataTestCase.testSortMissingLast(AbstractStringFieldDataTestCase.java:309)

The test failure is tracked in issue #4238, but the fix is not backported to 2.x branch after Lucene upgraded to 9.4.0 in 2.x branch.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@tlfeng tlfeng merged commit 1637590 into opensearch-project:2.x Oct 10, 2022
@tlfeng tlfeng deleted the 2.x-master-not-additionalrole branch October 10, 2022 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xuezhou25 xuezhou25 xuezhou25 approved these changes

@reta reta Awaiting requested review from reta reta is a code owner

Assignees
No one assigned
Labels
backport PRs or issues specific to backporting features or enhancments enhancement Enhancement or improvement to existing feature or request v2.4.0 'Issues and PRs related to version v2.4.0'
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@tlfeng @xuezhou25