[DilwoarH]

This creates a wal2json replication slot to allow us to listen to changes to the notifications table.

[klssmith]

Can we test this on a dev env? It seems like we're going to run into the problem of output_plugin_libraries now needing to include wal2json. This PR added that to the Concourse test image, but I'd expect us to need the same change in our environments

[DilwoarH]

Can we test this on a dev env?

Good point, Will run another test.

It seems like we're going to run into the problem of output_plugin_libraries now needing to include wal2json. This PR added that to the Concourse test image, but I'd expect us to need the same change in our environments

I noticed that I missed the default ones, it should have been:

pgoutput, test_decoding, wal2json

I'll have a look at adding this in.

[risicle]

Dude before we consider enabling this for production we need to choose and set a max_slot_wal_keep_size. I think it's currently set to "infinite", which means this would just start eating up our disk space forever if nothing were consuming the slot.

[DilwoarH]

Dude before we consider enabling this for production we need to choose and set a max_slot_wal_keep_size. I think it's currently set to "infinite", which means this would just start eating up our disk space forever if nothing were consuming the slot.

Yeah - I'm avoiding merging this in before the processing code (#4930).

[DilwoarH]

Can we test this on a dev env?

Good point, Will run another test.

It seems like we're going to run into the problem of output_plugin_libraries now needing to include wal2json. This PR added that to the Concourse test image, but I'd expect us to need the same change in our environments

I noticed that I missed the default ones, it should have been:

pgoutput, test_decoding, wal2json

I'll have a look at adding this in.

A PR was merged to fix the concourse test dbs - #4955

[DilwoarH]

Can we test this on a dev env? It seems like we're going to run into the problem of output_plugin_libraries now needing to include wal2json. This PR added that to the Concourse test image, but I'd expect us to need the same change in our environments

@klssmith additional test ran on dev-d - worked fine.

[DilwoarH]

Todo before merging this in:

• Set max size limit
• Set output_plugin_libraries

[DilwoarH]

@risicle - looks like there is a default limit set of 2GB already:

https://docs.aws.amazon.com/AmazonRDS/latest/UserGuide/Appendix.PostgreSQL.CommonDBATasks.Parameters.html#:~:text=max_wal_size,SHOW%20max_wal_size%3B

(from dev d)

[klssmith]

I realised this will work in our environments now because we're using Postgres 15.17, and the restriction on output_plugin_libraries was only introduced in 15.19. But we will presumably have issues when we do upgrade Postgres - it feels like it would be best just to update output_plugin_libraries now before we're using wal2json and in case we don't remember we need to do that before upgrading Postgres. That wouldn't block this PR but might be something to discuss

[risicle]

@risicle - looks like there is a default limit set of 2GB already:

That's not the same config setting.

[DilwoarH]

@risicle - looks like there is a default limit set of 2GB already:

That's not the same config setting.

PR added: https://github.com/alphagov/notifications-aws/pull/3094

[DilwoarH]

#3094 has been merged in but requires reboot to apply changes

[DilwoarH]

#3094 has been merged in but requires reboot to apply changes

Restart not required: https://github.com/alphagov/notifications-aws/pull/3096

[DilwoarH]

@risicle @klssmith - are you both happy with this now?

[klssmith]

It doesn't block this, but I still think we need a definite plan or PR for when we'll change output_plugin_libraries to include wal2json. Otherwise it will get forgotten about and cause an incident when the Postgres version is upgraded and our code no longer works

[DilwoarH]

It doesn't block this, but I still think we need a definite plan or PR for when we'll change output_plugin_libraries to include wal2json. Otherwise it will get forgotten about and cause an incident when the Postgres version is upgraded and our code no longer works

PR added to support this:

https://github.com/alphagov/notifications-aws/pull/3100

[risicle]

So ... are we happy we have max_slot_wal_keep_size set now? Are we ready to merge this?

[risicle]

You'll need to rebase though.

[risicle]

Not crazy about this name. I'd call it dbstreamer_replication_slot because dbstreamer is what we probably want to call the daemon that we eventually run to consume this.

[DilwoarH]

I think it makes it clear what it is - dbstreamer sounds very generic

[risicle]

That's the point - we're probably going to use this for a lot more than the dashboard.

[DilwoarH]

Then a better name is probably notify_replication_slot

[risicle]

But we also use replication for our read-replicas, and when we do our postgres upgrade we're going to be using it for logical replication to sync the dbs before we switch.

[risicle]

Just noticed the migration name misspells replication.

[risicle]

Also notify_replication_slot isn't really a helpful name because we use replication in like 3 different ways. I want to call the app consuming this the "db streamer" because it's a term that isn't already overloaded.

[DilwoarH]

Also notify_replication_slot isn't really a helpful name because we use replication in like 3 different ways. I want to call the app consuming this the "db streamer" because it's a term that isn't already overloaded.

Renamed it to notify_wal2json_slot. This will avoid any confusion with any other terms used as well as specifically refer back to the plugin used to initialise the slot.

I initially changed it to notify_db_streamer_slot but decided against it because it is not obvious what it is it for. Happy to chat about this offline to come to an agreement if there are any further suggestions.