feat: handle pg_net persistence mismatch for PG14 upgrade

[cfunkhouser]

Execute ALTER TABLE net.http_request_queue SET LOGGED on PG14 databases before upgrade.

There is a state mismatch in which net.http_request_queue_id_seq sequence is logged, but the table net.http_request_queue is not. This mismatch prevents pg_upgrade from proceeding. This change forces that alteration to occur at upgrade initiation time, before any of the other upgrade process has occurred.

[Crispy1975]

LGTM; we have a ConfigCat allowPg14LaunchOrgs flag which allows an org slug to be added. This mean we can spin up a PG14 project and then do surgery to mimic the conditions we're gonna fix.

[steve-chavez]

Q: Since the http_request_queue could be big (in some rare cases), would setting it to LOGGED have a perf impact?

IIUC, LOGGED involves a table rewrite + I/O + WAL generation.

[steve-chavez]

If we patch the extension as mentioned below perhaps we could avoid this step?

[cfunkhouser]

Yeah, I would definitely expect a perf impact for this. The changes here are scoped only to upgrade time, to get us out of the breaking situation where the persistence mismatch is not accepted during the upgrade, so no database should be online and serving traffic normally with net.http_request_queue logged.

[steve-chavez]

Could you clarify why is pg_upgrade a problem?

If the table is UNLOGGED + sequence LOGGED, this should run fine on pg 15.

Or is the problem restoring from pg 15, where table + sequence are UNLOGGED, to pg 14 (which doesn't supported UNLOGGED tables)?

[steve-chavez]

Also how do we decide if an upgrade script goes into ansible or in the Nix build itself?

For example we have some upgrade testing inside the Nix expression here:

postgres/nix/ext/tests/postgis.nix

Lines 99 to 100 in e157099

	with subtest("Check upgrade path with postgresql 15"):
	test.check_upgrade_path("15")

And in some cases we patch the extensions like on:

postgres/nix/ext/pg_cron/pg_cron-1.3.1-pg15.patch

Line 16 in e157099

disable_timeout(STATEMENT_TIMEOUT, false);

IMO it looks it'd be better to handle this pg_net case inside the Nix expression itself since we already have a testing framework in place (and this can be ran locally).

cc @samrose

[cfunkhouser]

After some... prolonged internal discussion, I'm going to close this pull request. The TL;DR is, we're not actually sure how to reproduce the issue, and therefor cannot test that this fix does anything more than complicate these scripts. Will resurrect if we are able to repro.

[cfunkhouser]

@steve-chavez to not leave you hanging on your question: this fix was for an issue affecting migrations from 14 to something more recent. 14 predates Nix, so for this particular concern, ansible is the only lever we have to pull. It's a good question to consider for other cases, once we get past 14 deprecation.

[samrose]

Also how do we decide if an upgrade script goes into ansible or in the Nix build itself?

For example we have some upgrade testing inside the Nix expression here:

postgres/nix/ext/tests/postgis.nix

Lines 99 to 100 in e157099

	with subtest("Check upgrade path with postgresql 15"):
	test.check_upgrade_path("15")

And in some cases we patch the extensions like on:

postgres/nix/ext/pg_cron/pg_cron-1.3.1-pg15.patch

Line 16 in e157099

disable_timeout(STATEMENT_TIMEOUT, false);

IMO it looks it'd be better to handle this pg_net case inside the Nix expression itself since we already have a testing framework in place (and this can be ran locally).

cc @samrose

agree with handle in pg_net packaging in nix if it otherwise doesn't make sense generically in the extension itself and seems unique to an end user case (like supabase)