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

[TEST] Fix snapshot restore exit condition #90696

Merged

Conversation

grcevski
Copy link
Contributor

@grcevski grcevski commented Oct 5, 2022

This is yet another issue with the exit condition of the snapshots restore file settings test. The code as I had written it is wrong. I used to set-up two different latches, one that waited on cluster state version getting to 0 and then another that was waiting on non-zero.

However, there's no guarantees that a cluster state update will not unlock the second latch first, because we weren't checking to see if we have achieved the non-zero condition first.

The solution is to have a state machine in the same cluster change listener that first waits for 0 version (snapshot restored) and then waits to see a non-zero in a subsequent update. After both have been achieved we can continue with the test.

Closes #90692

@grcevski grcevski added :Core/Infra/Core Core issues without another label >test-failure Triaged test failures from CI Team:Core/Infra Meta label for core/infra team auto-backport-and-merge Automatically create backport pull requests and merge when ready v8.5.1 v8.6.0 labels Oct 5, 2022
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

Copy link
Contributor

@ChrisHegarty ChrisHegarty left a comment

Choose a reason for hiding this comment

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

LGTM

@grcevski grcevski merged commit 2326bcf into elastic:main Oct 6, 2022
@grcevski
Copy link
Contributor Author

grcevski commented Oct 6, 2022

Thanks Chris!

@grcevski grcevski deleted the fix/another_snapshots_file_settings branch October 6, 2022 15:09
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.5

grcevski added a commit to grcevski/elasticsearch that referenced this pull request Oct 6, 2022
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Oct 10, 2022
* main: (150 commits)
  Remove ToXContent interface from ChunkedToXContent (elastic#90409)
  Remove extra SearchService constructor (elastic#90733)
  Update min version for the diagnosis yaml test (elastic#90731)
  Use the AggTestConfig object in testCase (elastic#90699)
  [DOCS] Add links to clear trained model deployment cache API (elastic#90727)
  Assert wildcards are not expanded as specified by request options  (elastic#90641)
  [TEST] Fix exit snapshot restore exit condition (elastic#90696)
  [TEST] Change to atomic file contents save (elastic#90695)
  Update forbiddenapis to 3.4 (elastic#90624)
  [Tests] Don't use concurrent search in scripted field type tests (elastic#90712)
  [ML] Move scaling is possible check for starting trained model (elastic#90706)
  Add new base test case for chunked xcontent types  (elastic#90707)
  Fix testRedNoBlockedIndicesAndRedAllRoleNodes (elastic#90671)
  Fix nullpointer in docs test setup (elastic#90660)
  Don't produce build logs artifact when in a composite build
  Fixing a race condition in EnrichCoordinatorProxyAction that can leave an item stuck in its queue (elastic#90688)
  docs: update fleet/agent pipeline docs (elastic#90659)
  [HealthAPI] Use plural consistently in resource types (elastic#90682)
  [Testing] Enable bwc and fix sorting for 500_date_range (elastic#90681)
  Add profiling and documentation for dfs phase (elastic#90536)
  ...

# Conflicts:
#	x-pack/plugin/mapper-aggregate-metric/src/test/java/org/elasticsearch/xpack/aggregatemetric/mapper/AggregateDoubleMetricFieldMapperTests.java
@csoulios csoulios added v8.5.0 and removed v8.5.1 labels Nov 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport-and-merge Automatically create backport pull requests and merge when ready :Core/Infra/Core Core issues without another label Team:Core/Infra Meta label for core/infra team >test-failure Triaged test failures from CI v8.5.0 v8.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] SnaphotsAndFileSettingsIT testRestoreWithPersistedFileSettings failing
4 participants