refactor: Provide API to stop listener programatically

[mkleczek]

DISCLAIMER:
This commit was authored entirely by a human without the assistance of LLMs.

This change introduces possibility to stop listener thread programatically. It is a prerequisite for HSpec tests starting the listener as they have to be able to stop it when cleaning up.

[wolfgangwalther]

I'm not a fan of exposing internal APIs just for tests - that means we're shipping code that we don't actually need.

Is it a requirement to test this from hspec? Can't we observe the same thing in an IO test?

[mkleczek]

I'm not a fan of exposing internal APIs just for tests - that means we're shipping code that we don't actually need.

Me neither but in this particular case, I'd say there is more to it. The direct reason for this change is to be able to test from hspec but, to be honest, the way how we handle closing the listener at the moment begs for improvement - today we just kill the thread not letting it do the cleanup.
This change makes it possible to gracefully trigger listener shutdown (it is not implemented in this PR though).

Is it a requirement to test this from hspec? Can't we observe the same thing in an IO test?

This is a longer discussion started here: #4668 (comment)
Based on it we merged #4732 and #4671 to facilitate implementing more tests in Haskell. Continuing in this direction we have #4836 pending in the queue.

Testing in Haskell has several advantages but the most important thing is typed observations monitoring (contrary to io tests where we parse log messages which is extremely brittle).

Answering directly: no, there is no requirement to test from Hspec but it is much more convenient. There are more listener related Hspec tests in the queue in #4673 (eg. https://github.com/mkleczek/postgrest/blob/b8ec90af233ff3d377426e1b23b7e1db2c989b29/test/observability/Observation/ToxiSpec.hs#L66) - waiting for #4836 to be merged.

[steve-chavez]

I'm not a fan of exposing internal APIs just for tests - that means we're shipping code that we don't actually need.

Note: we're already doing this for the PGRST_INTERNAL_* sleeps.

postgrest/src/PostgREST/SchemaCache.hs

Lines 171 to 173 in 127490e

	_ <-
	let sleepCall = SQL.Statement "select pg_sleep($1 / 1000.0)" (param HE.int4) HD.noResult True in
	for_ configInternalSCQuerySleep (`SQL.statement` sleepCall) -- only used for testing

I've been thinking, couldn't we do these points of failure/testing based on Observations? These are essentially callbacks that can execute IO (), they can be kinda like PostgreSQL injection points.

That would allow us to not ship the testing code in the binaries, we would only inject the sleeps (and other things) during testing via a callback.

[wolfgangwalther]

This change introduces possibility to stop listener thread programatically. It is a prerequisite for HSpec tests starting the listener as they have to be able to stop it when cleaning up.

I looked at this from a perspective of "what would I do from non-hspec tests, for example pytest, to start/stop the listener?". It seems like db-channel-enabled is reloadable, so I should be able to change the config file and reload to do just that.

[mkleczek]

It seems like db-channel-enabled is reloadable

I believe it is not - there is no code that would stop/start listener thread upon config reload. If it is marked as such in the docs it is a documentation issue.

[wolfgangwalther]

It seems like db-channel-enabled is reloadable

I believe it is not - there is no code that would stop/start listener thread upon config reload. If it is marked as such in the docs it is a documentation issue.

Interesting, I was going by the docs only. Maybe we can fix the code instead of the documentation then, to make it reloadable?