Incremental config update limitations

[sukhwinder33445]

Event rule

• Removing an event rule escalation causes the changed_at flag to be updated for all other escalations.
• Adding/removing/updating a recipient causes the escalation and all its recipients to have a new changed_at.

The changed_at column

• It should be added at a central point (Database.php) to avoid future errors.
• It should be precise to milliseconds in order to avoid a data race (daemon side), e.g. if two users update the data at the same time.
• (fixed with Let On_ASSEMBLE_INSERT / ON_ASSEMBLE_UPDATE events handle the changed_at column #227)

[nilmerg]

It should be precise to milliseconds in order to avoid a data race (daemon side), e.g. if two users update the data at the same time.

If two users update the same configuration at the same time, one should get a conflict and need to re-submit. This is no reason to increase precision on the timestamp values.

[nilmerg]

It should be precise to milliseconds in order to avoid a data race (daemon side), e.g. if two users update the data at the same time.

If two users update the same configuration at the same time, one should get a conflict and need to re-submit. This is no reason to increase precision on the timestamp values.

Though, in case it's not possible to differentiate between new and updated entries (which is the case), it is...

[Al2Klimov]

a data race (daemon side), e.g. if two users update the data at the same time

Indeed!

• The daemon simply retrieves everything since the last known changed_at: https://github.com/Icinga/icinga-notifications/blob/114c1be12dbe6ff8d3b9280fb67e02812100d101/internal/config/incremental_sync.go#L66
• Web just inserts changed_at=now:

  icinga-notifications-web/application/forms/ChannelForm.php

  Line 203 in 41595b9

  $channel['changed_at'] = time() * 1000;

Now, if an insertion is slow enough and/or clocks are just a bit out of sync (e.g CockroachDB allows +-7ms), two config updates which differ only by 1-2 ms could be inserted out of order, so that one gets lost:

1. A get inserted with changed_at=123
2. Daemon consumes A
3. B get inserted with changed_at=122
4. Daemon does nothing, as it thinks it has already consumed everything 🤷‍♂

Actually time() knows only seconds, so race conditions even apply to two config updates during the same second. #306

Suggestion

1. Do all DML ops affecting changed_at columns inside serializable transactions
2. In case of Serialization Failure (error 1213/40001), repeat the transaction (loop)
3. Insert not just ms_time_from_php(), but GREATEST(ms_time_from_php(), MAX(changed_at)+1)

This way, rows updated later will always have a greater changed_at.