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

[dev.icinga.com #5103] The way notification_id is implemented is broken #952

Closed
icinga-migration opened this issue Nov 19, 2013 · 9 comments

Comments

Projects
None yet
1 participant
@icinga-migration
Copy link
Member

commented Nov 19, 2013

This issue has been migrated from Redmine: https://dev.icinga.com/issues/5103

Created by gbeutner on 2013-11-19 15:39:55 +00:00

Assignee: mfriedrich
Status: Resolved (closed on 2014-01-29 16:46:21 +00:00)
Target Version: 0.0.7
Last Update: 2014-09-16 09:19:32 +00:00 (in Redmine)

Icinga Version: 0.0.4

a) Multiple notifications can run in parallel.

b) The NotificationSentToAllUsers event is sent before all notifications for a service are sent (which is a bug in itself). However, the real problem is that it's possible that the event is send after some of the notifications are sent - in which case the INSERT for icinga_notifications hasn't been pushed to the DbConnection objects yet.

Changesets

2014-01-27 16:22:48 +00:00 by (unknown) d883926

DB IDO: Refactor notification signal handling (WIP).

Refs #5103

2014-01-28 14:53:12 +00:00 by (unknown) f30eca5

DB IDO: Refactor notification signal handling.

Refs #5103
Fixes #5265

2014-01-28 16:53:40 +00:00 by (unknown) a3097ff

DB IDO: Fix the way notification_id is handled.

Fixes #5103
Fixes #5265

2014-01-29 12:34:43 +00:00 by (unknown) 5c5d94b

DB IDO: Implement notification object insert id cache (WIP).

Refs #5103

2014-01-29 16:38:02 +00:00 by (unknown) d31ca31

DB IDO: Implement notification object insert id cache.

Refs #5103

Relations:

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Jan 27, 2014

Updated by mfriedrich on 2014-01-27 08:33:49 +00:00

  • Status changed from New to Assigned
  • Assigned to set to mfriedrich
  • Target Version set to 0.0.7
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Jan 27, 2014

Updated by mfriedrich on 2014-01-27 08:48:24 +00:00

  • Priority changed from Normal to High
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Jan 28, 2014

Updated by mfriedrich on 2014-01-28 15:02:11 +00:00

The wrong id and therefore unique constraint violation is handled in #5265 changing the notification signal order.

The most obvious problem is still that db_ido doesn't have any notification db object (not config, nor status) for historical insert ids. By changing the signal handler to pass the current notification object ptr it would be reasonable to create an additional HistoryInsertID cache based on the current notification object. That id could then be used for the notification_id referenced by the contactnotifications table.

Problem: If a new notification is triggered, it will overwrite the last notification id, even if there are pending contact notification entries.

That would require making the notification component's process blocking (rather: send notifications to users in sequentional order) which is not intended.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Jan 28, 2014

Updated by mfriedrich on 2014-01-28 16:52:29 +00:00

Workaround for sequential inserts

  • re-order the notification user filter checks in its own function (happens before getting async)
  • collect all to-be-notified users in a seperate std::set
  • fire one event for notifications and contactnotifications for db ido with notified users as argument
  • leave the asynchronous event alone - that one is used for the compatlogger (and requires the command name which is only available over there!)
[2014-01-28 17:48:31 +0100]  information/icinga: Sending notifications for service 'localhost-foo!processes'
[2014-01-28 17:48:31 +0100]  debug/icinga: Service 'localhost-foo!processes' has 1 notification(s).
[2014-01-28 17:48:31 +0100]  debug/icinga: FType=8, TypeFilter=511
[2014-01-28 17:48:31 +0100]  debug/icinga: Sending notification for user 'manager'
[2014-01-28 17:48:31 +0100]  debug/base: Running command '/etc/icinga2/scripts/mail-notification.sh'.
[2014-01-28 17:48:31 +0100]  debug/icinga: Sending notification for user 'icingaadmin'
[2014-01-28 17:48:31 +0100]  debug/db_ido: add notification history for 'localhost-foo!processes'
[2014-01-28 17:48:31 +0100]  debug/db_ido_mysql: Query: INSERT INTO icinga_notifications (contacts_notified, end_time, end_time_usec, escalated, instance_id, long_output, notification_reason, notification_type, object_id, output, start_time, start_time_usec, state) VALUES ('2', FROM_UNIXTIME(1390927711), '202081', '0', 1, '', '8', '1', 50, 'PROCS WARNING: 279 processes ', FROM_UNIXTIME(1390927711), '202081', '1')
[2014-01-28 17:48:31 +0100]  debug/base: Running command '/etc/icinga2/scripts/mail-notification.sh'.
[2014-01-28 17:48:31 +0100]  debug/db_ido: add contact notification history for service 'localhost-foo!processes' and user 'manager'.
[2014-01-28 17:48:31 +0100]  debug/db_ido: add contact notification history for service 'localhost-foo!processes' and user 'icingaadmin'.
[2014-01-28 17:48:31 +0100]  debug/db_ido: saving contactnotification notification_id=14191
[2014-01-28 17:48:31 +0100]  debug/db_ido_mysql: Query: INSERT INTO icinga_contactnotifications (contact_object_id, end_time, end_time_usec, instance_id, notification_id, start_time, start_time_usec) VALUES (5176, FROM_UNIXTIME(1390927711), '202081', 1, 14191, FROM_UNIXTIME(1390927711), '202081')
[2014-01-28 17:48:31 +0100]  debug/db_ido_mysql: Query: INSERT INTO icinga_contactnotifications (contact_object_id, end_time, end_time_usec, instance_id, notification_id, start_time, start_time_usec) VALUES (35, FROM_UNIXTIME(1390927711), '202081', 1, 14191, FROM_UNIXTIME(1390927711), '202081')
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Jan 28, 2014

Updated by Anonymous on 2014-01-28 16:57:47 +00:00

  • Status changed from Assigned to Resolved
  • Done % changed from 0 to 100

Applied in changeset i2:a3097ff3c63abd7995bcee1d0e49aac7c655b74d.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Jan 29, 2014

Updated by mfriedrich on 2014-01-29 08:23:31 +00:00

  • Status changed from Resolved to Assigned
  • Done % changed from 100 to 0
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Jan 29, 2014

Updated by mfriedrich on 2014-01-29 16:46:21 +00:00

  • Status changed from Assigned to Resolved
  • Done % changed from 0 to 100

The cache itself is working for both MySQL and PostgreSQL. Though, it's ugly and introduced another object ptr inside the query object which should be refactored in #5579 - closing this issue as the main functionality is restored.

@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Sep 16, 2014

Updated by mfriedrich on 2014-09-16 09:19:33 +00:00

  • Project changed from 32 to Icinga 2
  • Category set to DB IDO
@icinga-migration

This comment has been minimized.

Copy link
Member Author

commented Mar 21, 2016

Updated by mfriedrich on 2016-03-21 09:56:56 +00:00

  • Relates set to 11387
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.