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

Avoid double notifying on node termination. #37

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

kjnilsson
Copy link
Contributor

When a node was terminated aten would sometimes send two notifications to registered listeners. One when it detected the node as not responding and one next interval when it detected it as gone.

This adds a check for gone case to see if the node has already been detected as down and only sends a down notification in this scenario if the prior state is not down.

Also return ignore if trying to monitor the current node.

When a node was terminated aten would sometimes send two notifications
to registered listeners. One when it detected the node as not responding
and one next interval when it detected it as gone.

This adds a check for gone case to see if the node has already been
detected as down and only sends a down notification in this scenario
if the prior state is not down.

Also return `ignore` if trying to monitor the current node.
@kjnilsson kjnilsson changed the title Avoid double notifying on node termiation. Avoid double notifying on node termination. Dec 19, 2023
@Zabrane
Copy link

Zabrane commented Dec 19, 2023

When a node was terminated aten would sometimes send two notifications to registered listeners. One when it detected the node as not responding and one next interval when it detected it as gone.

This adds a check for gone case to see if the node has already been detected as down and only sends a down notification in this scenario if the prior state is not down.

Also return ignore if trying to monitor the current node.

@kjnilsson The fix works like a charm. I'm only getting 1 down message now and I can't register myself anymore.
Many thanks for your support.

@lukebakken
Copy link
Contributor

@Zabrane thank you for testing this PR - it helps our team a lot.

@kjnilsson kjnilsson merged commit 123cd57 into main Dec 20, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants