Fix test race condition in file settings monitoring #137965
Conversation
When file settings tests look for changes in cluster state to assert application of changes they do so by adding a cluster state listener. But if there are no more cluster state updates after the listener is added the test will never see the initial state of file settings. This commit ensures the current cluster state is seen in that case. closes elastic#36454
rjernst
added
>test
elasticsearchmachine
added
v9.3.0
Team:Core/Infra
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
prdoyle
left a comment
prdoyle
left a comment
There was a problem hiding this comment.
One more race condition down.
| if (reservedState != null && reservedState.version() == fileSettingsVersion) { | ||
| clusterService.removeListener(this); | ||
| metadataVersion.set(event.state().metadata().version()); | ||
| metadataVersion.set(clusterState.metadata().version()); |
There was a problem hiding this comment.
Do I understand this correctly? If the reserved state version is the one we're expecting, we infer that the metadata version must also be the one we're expecting, on the assumption that the latest metadata update was the one that updated the reserved state. This seems ok in these particular tests where there are very few metadata updates, even though it wouldn't be correct in general.
Am I close?
Did you manage to induce this logic path with sleeps in the tests that call this method?
There was a problem hiding this comment.
We don't assert anything about the metadata version, this captures the metadata version once we see the file settings version we expect.
And yes I reproduced with a sleep just before creating the state listener.
4 participants
When file settings tests look for changes in cluster state to assert application of changes they do so by adding a cluster state listener. But if there are no more cluster state updates after the listener is added the test will never see the initial state of file settings. This commit ensures the current cluster state is seen in that case.
closes #36454