Allow schema cache reloading with NOTIFY

[steve-chavez]

Fixes #1512

Helps on environments where you can't send unix signals(Google Cloud managed
containers, Windows). Also provides better UX for schema reloads - NOTIFY
can be sent from pg clients(psql, pgadmin).

By default, NOTIFY pgrst - with no payload - should be done to reload
the schema cache. Notifications with a payload will be ignored.

There's limitation for now, if the LISTEN connection is lost the
connection won't be retried.

Edits:

• The LISTEN thread uses a dedicated pg connection. This connection is recovered if it fails. A debounce of 1ms is done in case too many NOTIFYs arrive.

• The channel can be enabled with db-channel-enabled(false by default) and its name can be configured with db-channel("pgrst" by default).

[steve-chavez]

NOTIFY pgrst - with no payload - should be done to reload the schema cache.

Made this restriction because it might be useful to later add NOTIFY pgrst, 'SIGUSR2'(related to #1289). NOTIFY pgrst, 'SIGUSR1' would have the same effect as no payload.

[dwagin]

How to disable this feature?

[steve-chavez]

Hey @dwagin. Right now I'm including it by default with no option to disable it.
But I could add a db-channel-enable(default true) config for disabling it, would that work for you?

[steve-chavez]

@dwagin I've added the db-channel-enabled config option! db-channel-enabled=false can be done for no LISTEN.

[dwagin]

@dwagin I've added the db-channel-enabled config option! db-channel-enabled=false can be done for no LISTEN.

This is very good, because LISTEN/NOTIFY doesn't work with PgBouncer and additional extra connection is not needed in this situation.

[jsommr]

If this is something that'll potentially cause trouble, like with PgBouncer, wouldn't it make sense to have db-channel-enabled=false as default?

[steve-chavez]

@dwagin Could you share how are you using PgBouncer?

On SQL feature map for pooling modes, they mention that LISTEN/NOTIFY does work with Session pooling(not transaction pooling). And #1011 left me with the impression that transaction pooling is not working with PostgREST now(only session pooling works), or did you managed to make that work?

[steve-chavez]

@nerfpops It was tempting to enable it by default and just tell users to run NOTIFY pgrst, but with the ideas in #1512 (comment) I think we can instead have it as a recommended config in the docs(the EVENT TRIGGER would be an extra step anyway). So yes, I can disable it by default to be on the safe side.

[dwagin]

@steve-chavez Session pooling doesn't make sense, we use transaction pooling (with small hasql patch).

patch-hasql-1.4.2_library_Hasql_Private_IO.hs

[steve-chavez]

An update on the implementation. The LISTEN connection is now retried if lost.
Also, I've added a debounce of 1 second on the NOTIFY reloading. This is necessary for migrations as mentioned in #1512 (comment).

Without the debounce, doing this in bash:

for i in {1..20}; do psql postgrest_test -c "NOTIFY pgrst"; done

Results in 20 schema reloads. The debounce reduces that to one.

Weirdly, I'm not able to reproduce the bad 20 schema reloads with just psql:

DO $$ BEGIN FOR counter IN 1..20 LOOP NOTIFY pgrst; END LOOP; END$$;

That always results in one NOTIFY sent, not sure what could be wrong there. But to be on the safe sade, I'll add the integration tests in bash.

[steve-chavez]

Adding the bash integration tests to CI is proving to be a lot of work. The with_tmp_db shell script would have to be modified and we'd have to interact with pg_ctl directly(stop/start). So I've just manual tested that the reloading is good and I've ensured the SIGUSR1 is working as usual.

On the introduction of SIGHUP(#869) we also did manual bash tests. I have a general idea on how to automate this, but I'll open another issue for that.

So I think this is good to merge now 🚀

[wolfgangwalther]

Weirdly, I'm not able to reproduce the bad 20 schema reloads with just psql:

DO $$ BEGIN FOR counter IN 1..20 LOOP NOTIFY pgrst; END LOOP; END$$;

That always results in one NOTIFY sent, not sure what could be wrong there. But to be on the safe sade, I'll add the integration tests in bash.

I'm pretty sure this is because all those NOTIFYs are executed in one transaction - there will be only one notification with the same payload sent per transaction. Citing the pg docs:

If the same channel name is signaled multiple times from the same transaction with identical payload strings, the database server can decide to deliver a single notification only.

If this was guaranteed (the wording doesn't seem like it is), we wouldn't need the debounce, because a schema migration could just be run in a single transaction. I guess in most cases it would be, anyways.

[steve-chavez]

@wolfgangwalther Nice catch! I missed that one.

(For reference: https://www.postgresql.org/docs/current/sql-notify.html. ctrl +f If the same channel name is signaled multiple times)

Since the debounce was easy to set up - it didn't require an extra lib and just a few lines of code - I guess we can keep it to be on the safe side. But the pg doc reference could be noted as a comment on the code.

[laurenceisla]

@dwagin @nerfpops Just a heads up: database-channel-enabled setting is now true by default since 214a92f, and needs to be explicitly set to false when using PgBouncer.