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

Fix problem with reminder notifications if the checkable is flapping #6292

Merged
merged 1 commit into from
May 17, 2018

Conversation

dnsmichi
Copy link
Contributor

@dnsmichi dnsmichi commented May 9, 2018

No description provided.

@dnsmichi dnsmichi added this to the 2.9.0 milestone May 9, 2018
@dnsmichi dnsmichi added bug Something isn't working area/notifications Notification events labels May 9, 2018
@dnsmichi
Copy link
Contributor Author

dnsmichi commented May 9, 2018

@Crunsher as we've talked already before your vacation, this needs proper tests and a protocol with the results in here.

@Crunsher
Copy link
Contributor

I tested this quite extensively already by getting a Host to start flapping with their last hard state being down.
Before the fix there were down reminder notifications, none after. I had a branch for this lying around, so to my knowledge this can be merged

@dnsmichi dnsmichi requested a review from N-o-X May 16, 2018 08:41
@dnsmichi
Copy link
Contributor Author

@N-o-X please test this one as well prior to merging.

@N-o-X
Copy link
Contributor

N-o-X commented May 17, 2018

Could you describe the expected behavior? I don't really know how those reminder notifications should act while a host/service is flapping.

@Crunsher
Copy link
Contributor

They should not be sent, the problem only occurs in the following circumstances:
Service/Host is hard down and goes flapping with the same statechange. Expected and patched behavior is for only a flapping notification and no reminder notifications.

To reproduce this: Have a Host with a low flapping threshold and a host check that switches state with each result. To monitor notifications either have them written somewhere you can easily read them or activate the debug log and grep for NotificationComponent. I could also show you and explain next week

Copy link
Contributor

@N-o-X N-o-X left a comment

Choose a reason for hiding this comment

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

Thanks @Crunsher, that helped 👍
I've tested the fix and it now works like expected.

@N-o-X N-o-X merged commit a5a2194 into master May 17, 2018
@N-o-X N-o-X deleted the fix/flapping-reminder-notification branch May 18, 2018 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/notifications Notification events bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants