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

Adding support for dynamically updating Leader/follower checker timeouts #10528

Merged
merged 21 commits into from
Nov 12, 2023

Conversation

niyatiagg
Copy link
Contributor

@niyatiagg niyatiagg commented Oct 10, 2023

Description

Adding support for dynamically updating Leader/follower checker timeouts

Related Issues

Resolves #10377

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)
  • Public documentation issue/PR created

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.

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>
Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>
Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>
Copy link
Contributor

github-actions bot commented Nov 8, 2023

Gradle Check (Jenkins) Run Completed with:

@niyatiagg
Copy link
Contributor Author

niyatiagg commented Nov 8, 2023

@niyatiagg are you still working on this?

Yes, I am still on it. Have made the necessary changes and resolved conflicts!

Copy link
Contributor

github-actions bot commented Nov 8, 2023

Gradle Check (Jenkins) Run Completed with:

@owaiskazi19
Copy link
Member

Looks like unrelated change. Let me rerun the check.

REPRODUCE WITH: ./gradlew ':server:test' --tests "org.opensearch.cluster.coordination.FollowersCheckerTests.testFailureCounterResetsOnSuccess" -Dtests.seed=9F1E3EE301F62571 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=ar-DZ -Dtests.timezone=America/Inuvik -Druntime.java=21

org.opensearch.cluster.coordination.FollowersCheckerTests > testFailureCounterResetsOnSuccess FAILED
    java.lang.IllegalArgumentException: failed to parse value [84005ms] for setting [cluster.fault_detection.follower_check.timeout], must be <= [60000ms]
        at __randomizedtesting.SeedInfo.seed([9F1E3EE301F62571:ADD04656C38CBEB1]:0)
        at org.opensearch.common.settings.Setting.lambda$minMaxTimeValueParser$46(Setting.java:2721)
        at org.opensearch.common.settings.Setting$MinMaxTimeValueParser.apply(Setting.java:2652)
        at org.opensearch.common.settings.Setting$MinMaxTimeValueParser.apply(Setting.java:2606)
        at org.opensearch.common.settings.Setting.get(Setting.java:483)
        at org.opensearch.common.settings.Setting.get(Setting.java:477)
        at org.opensearch.cluster.coordination.FollowersChecker.<init>(FollowersChecker.java:146)
        at org.opensearch.cluster.coordination.FollowersCheckerTests.testBehaviourOfFailingNode(FollowersCheckerTests.java:395)
        at org.opensearch.cluster.coordination.FollowersCheckerTests.testFailureCounterResetsOnSuccess(FollowersCheckerTests.java:232)

Copy link
Contributor

github-actions bot commented Nov 9, 2023

Gradle Check (Jenkins) Run Completed with:

@jainankitk
Copy link
Collaborator

@niyatiagg - It seems that changing from OpenSearchTestCase to SingleNodeTestCase has regressed unrelated tests. We already have initiative to reduce to flaky tests and don't want to increase their count. Hence, it might be better to revert to OpenSearchTestCase and add these new settings tests to separate class extending from SingleNodeTestCase.

@niyatiagg
Copy link
Contributor Author

niyatiagg commented Nov 9, 2023

@niyatiagg - It seems that changing from OpenSearchTestCase to SingleNodeTestCase has regressed unrelated tests. We already have initiative to reduce to flaky tests and don't want to increase their count. Hence, it might be better to revert to OpenSearchTestCase and add these new settings tests to separate class extending from SingleNodeTestCase.

Makes sense. Will make the changes

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>
Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>
Copy link
Contributor

github-actions bot commented Nov 9, 2023

Gradle Check (Jenkins) Run Completed with:

Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@niyatiagg
Copy link
Contributor Author

@owaiskazi19 - The test failures seem unrelated to my changes. Can you please take a look?

Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@owaiskazi19 owaiskazi19 added the backport 2.x Backport to 2.x branch label Nov 12, 2023
@owaiskazi19 owaiskazi19 merged commit 0452d14 into opensearch-project:main Nov 12, 2023
32 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Nov 12, 2023
…uts (#10528)

* making leader check timeout dynamic

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* making follower check timeout dynamic

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* fixing existing unit tests

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* fixing checkstyle violations

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* adding tests for leader/follower check timeout

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* setting maximum and minimum timeout value for leader/follower checker

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* adding tests for checking boundary cases

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Fixing checkstyle violations

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* changed the log file and added other suggested changes

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* fixing checkstyle violations

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Addressing review comments

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* addressing proposed changes

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Applying checkstyle fixes

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Fixing flakiness for existing tests

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Applying checkstyle fixes

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Fixing the timeout value limits for randomSettings

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

---------

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>
(cherry picked from commit 0452d14)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
owaiskazi19 pushed a commit that referenced this pull request Nov 14, 2023
…uts (#10528) (#11166)

* making leader check timeout dynamic



* making follower check timeout dynamic



* fixing existing unit tests



* fixing checkstyle violations



* adding tests for leader/follower check timeout



* setting maximum and minimum timeout value for leader/follower checker



* adding tests for checking boundary cases



* Fixing checkstyle violations



* changed the log file and added other suggested changes



* fixing checkstyle violations



* Addressing review comments



* addressing proposed changes



* Applying checkstyle fixes



* Fixing flakiness for existing tests



* Applying checkstyle fixes



* Fixing the timeout value limits for randomSettings



---------


(cherry picked from commit 0452d14)

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
@jainankitk
Copy link
Collaborator

@niyatiagg - Can you also create tracking issue for updating the documentation? You can refer to one of the issue here: https://github.com/orgs/opensearch-project/projects/22

@niyatiagg
Copy link
Contributor Author

@niyatiagg - Can you also create tracking issue for updating the documentation? You can refer to one of the issue here: https://github.com/orgs/opensearch-project/projects/22

Done!
opensearch-project/documentation-website#5603

fahadshamiinsta pushed a commit to fahadshamiinsta/OpenSearch270 that referenced this pull request Dec 4, 2023
…uts (opensearch-project#10528)

* making leader check timeout dynamic

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* making follower check timeout dynamic

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* fixing existing unit tests

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* fixing checkstyle violations

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* adding tests for leader/follower check timeout

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* setting maximum and minimum timeout value for leader/follower checker

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* adding tests for checking boundary cases

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Fixing checkstyle violations

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* changed the log file and added other suggested changes

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* fixing checkstyle violations

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Addressing review comments

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* addressing proposed changes

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Applying checkstyle fixes

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Fixing flakiness for existing tests

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Applying checkstyle fixes

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Fixing the timeout value limits for randomSettings

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

---------

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>
rayshrey pushed a commit to rayshrey/OpenSearch that referenced this pull request Mar 18, 2024
…uts (opensearch-project#10528)

* making leader check timeout dynamic

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* making follower check timeout dynamic

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* fixing existing unit tests

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* fixing checkstyle violations

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* adding tests for leader/follower check timeout

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* setting maximum and minimum timeout value for leader/follower checker

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* adding tests for checking boundary cases

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Fixing checkstyle violations

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* changed the log file and added other suggested changes

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* fixing checkstyle violations

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Addressing review comments

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* addressing proposed changes

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Applying checkstyle fixes

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Fixing flakiness for existing tests

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Applying checkstyle fixes

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Fixing the timeout value limits for randomSettings

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

---------

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>
shiv0408 pushed a commit to Gaurav614/OpenSearch that referenced this pull request Apr 25, 2024
…uts (opensearch-project#10528)

* making leader check timeout dynamic

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* making follower check timeout dynamic

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* fixing existing unit tests

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* fixing checkstyle violations

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* adding tests for leader/follower check timeout

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* setting maximum and minimum timeout value for leader/follower checker

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* adding tests for checking boundary cases

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Fixing checkstyle violations

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* changed the log file and added other suggested changes

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* fixing checkstyle violations

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Addressing review comments

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* addressing proposed changes

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Applying checkstyle fixes

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Fixing flakiness for existing tests

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Applying checkstyle fixes

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

* Fixing the timeout value limits for randomSettings

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>

---------

Signed-off-by: Niyati Aggarwal <niyatiagg4641@gmail.com>
Signed-off-by: Shivansh Arora <hishiv@amazon.com>
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 Cluster Manager enhancement Enhancement or improvement to existing feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Leader/follower checker timeouts cannot be dynamically set
4 participants