Skip to content

registry-change-notifier can drop events or emit them out of order #3046

@mschuwalow

Description

@mschuwalow

There are a couple of correctness issues with the current change notifier.

General Issues

The notify function is not called reliably. Use site code writes the transaction to the db and then calls notify, resulting in the later being easily skipped in the case of errors or request / future drops. Use site example.

This interface is fundamentally unsound and should either be dropped or treated as just a hint for sqlite to get the latest values from the db.
The db should always be authoritative when it comes to determining which events to emit.

Postgres issues

The code assume that event_id is a monotonically increasing number. This is not true for the postgres implementation as the event_id is a normal bigserial key which will be incremented as a side effect inside the transaction (which also means that postgres does not guarantee that the event ids are contiguous).
The actual pg_notify's are emitted in commit order, so there is a mismatch there.

Proposed fix (postgres)

Postgres should not rely on notify at all (already done here)

Using the autoincremented key is fundamentally the wrong primitive to use as a cursor for the change notifications. A better fit would be the drop the event_id + listen/notify mechanism entirely and rely on logical replication https://www.postgresql.org/docs/current/logical-replication.html to get the commit log. That is the same approach products like debezium use to implement change data capture for postgres.

Another option (with a serious performance hit), would be to always insert the events as the last step in a transaction and acquire an advisory lock before inserting the event, holding it until the transaction commits. That would effectively force the event_ids to be monotonically increasing, but I would not advise doing this.

Proposed fix (sqlite)

(I believe in sqlite the event_ids will be monotonically increasing due to the write lock, but that should be confirmed)
For sqlite the main problem is that notify is fundamentally unreliable. Instead of treating it as authorative, it can be left and used as a hint that the db needs to be polled for new events. Both on notify and on a timer (in case notify is not called), we need to poll the db and emit all new events based on the cursor.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions