Skip to content

Clean up FsHealthService after MDP removal#73136

Merged
DaveCTurner merged 3 commits intoelastic:masterfrom
DaveCTurner:2021-05-17-fshealthservice-mdp-cleanup
May 17, 2021
Merged

Clean up FsHealthService after MDP removal#73136
DaveCTurner merged 3 commits intoelastic:masterfrom
DaveCTurner:2021-05-17-fshealthservice-mdp-cleanup

Conversation

@DaveCTurner
Copy link
Copy Markdown
Member

Following #72432 we now no longer need a Set of unhealthy paths, we
can just track the individual path directly.

Following elastic#72432 we now no longer need a `Set` of unhealthy paths, we
can just track the individual path directly.
@DaveCTurner DaveCTurner added >refactoring :Core/Infra/Resiliency Keep running when everything is ok. Die quickly if things go horribly wrong. v8.0.0 labels May 17, 2021
@DaveCTurner DaveCTurner requested a review from rjernst May 17, 2021 07:23
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label May 17, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

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


private static final Logger logger = LogManager.getLogger(FsHealthService.class);
private final ThreadPool threadPool;
private volatile StatusInfo statusInfo = new StatusInfo(HEALTHY, "not started");
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Updating the status by writing to those two volatile fields was not quite correct: you could in theory observe a healthy state when transitioning from broken to unhealthy. It looks to be fixable by ordering the reads and writes carefully but the reasoning is quite fragile and it's much simpler just to do this.

Copy link
Copy Markdown
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM

@DaveCTurner DaveCTurner merged commit 4268c76 into elastic:master May 17, 2021
@DaveCTurner DaveCTurner deleted the 2021-05-17-fshealthservice-mdp-cleanup branch May 17, 2021 12:57
rjernst added a commit to rjernst/elasticsearch that referenced this pull request Oct 7, 2021
This reverts commit 6a7298e.

The revert was not clean. There were two adjustements necessary.
First, this effectively must revert elastic#73136. Second, some tests
were written after MDP removal so needed to be made multi path aware.

relates elastic#78525
relates elastic#71205
rjernst added a commit that referenced this pull request Oct 11, 2021
This reverts commit 6a7298e.

The revert was not clean. There were two adjustements necessary.
First, this effectively must revert #73136. Second, some tests
were written after MDP removal so needed to be made multi path aware.

relates #78525
relates #71205
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Resiliency Keep running when everything is ok. Die quickly if things go horribly wrong. >refactoring Team:Core/Infra Meta label for core/infra team v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants