fix: empty options not accepted by Supavisor

[dossy]

Supavisor will close the connection and log a :bad_startup_payload
if an empty options connection parameter is sent.

Fixes #1259

[dossy]

@arp242 FYI - I've almost got a clean run of the tests against Supavisor - found a number of places where connections weren't being released, etc., which would cause the session mode connection pool to run out of connections.

[arp242]

I can't get it to work on my machine:

% docker compose up supavisor-create-tenant
Attaching to supavisor-create-tenant-1
Container pqgo-supavisor-db-1 Waiting
Container pqgo-supavisor-db-1 Healthy
Container pqgo-supavisor-1 Waiting
Container pqgo-supavisor-1 Error dependency supavisor failed to start
dependency failed to start: container pqgo-supavisor-1 exited (1)

% docker compose up supavisor
Attaching to supavisor-1
Container pqgo-supavisor-db-1 Waiting
Container pqgo-supavisor-db-1 Healthy
supavisor-1  | Setting RLIMIT_NOFILE to 100000
supavisor-1  | /app/limits.sh: line 6: ulimit: open files: cannot modify limit: Operation not permitted
supavisor-1 exited with code 1

So, hm, not sure why that fails? 🤔

---

Does it need to have supavisor-db PostgreSQL instance? Can't it use the pg18?

Ideally it should use the same connection parameters as everything else, except the port being different. This is also how it can "detect" if it's running pgbouncer or supaviser or anything else, which is a bit hacky but straight-forward and works.

It uses the following users for tests:

pqgo             password=pqgo
pqgomd5          password=wordpass  (md5 auth only)
pqgopassword     password=wordpass  (password auth only)
pqgoscram        password=wordpass  (scram-sha-256 auth only)

pqgossl          password=pqgo  (require SSL auth)
pqgosslcert      password=pqgo  (require SSL client certificate auth)

Only the first one is strictly needed, for the rest the tests can be skipped if it's not supported by supaviser or too hard to set up. e.g. for pgbouncer it skips those SSL tests as I couldn't get it to work. Some tests is better than no tests.

It also needs to be added to GitHub actions as a final step.

[arp242]

Don't mind doing all of the testing/CI stuff in a separate PR by the way, and just merging the fix now.

[dossy]

I can't get it to work on my machine:

% docker compose up supavisor-create-tenant
...

You need to docker compose up -d supavisor-create-tenant so that the supavisor-db and supavisor services continue to run in the background afterwards. However, this is puzzling:

supavisor-1  | /app/limits.sh: line 6: ulimit: open files: cannot modify limit: Operation not permitted

The supavisor container must run as root. Are you running Docker in rootless mode, or have an override file that's running the container as a non-root user?

Does it need to have supavisor-db PostgreSQL instance? Can't it use the pg18?

It may be able to use pg18 but I'm just following the self-hosted Supavisor guidance in their docker-compose.yml which uses their own supabase/postgres:15.1.0.148 image. I don't think they have modified Postgres itself in any way, but presumably the image is configured to work with Supavisor, as opposed to having to adapt the official Postgres Docker image with any configuration or modifications necessary. Seemed like the path of least resistance, so I took it.

Since Supavisor stores its own data for its own operation in whatever Postgres database it sits in front of, reusing that database for other tests could unintentionally impact Supavisor's operation, so it's best to keep it isolated for the purpose of testing.

Officially, I believe Supavisor supports up to Postgres 17 at the moment, but I suspect it'll work with Postgres 18 just fine, but I don't want to find out that it doesn't and chase ghosts trying to get tests passing only to find out the issue is caused by some change in v18 that requires a change to Supavisor ...

Don't mind doing all of the testing/CI stuff in a separate PR by the way, and just merging the fix now.

That works for me. I'll submit the testing changes in a separate PR.

[arp242]

Okay, I backed out the compose.yaml changes and merged just the fix.

---

Are you running Docker in rootless mode, or have an override file that's running the container as a non-root user?

No, just the standard Void Linux setup. I spent 0 time looking in to it so may be something simple.

None of the tests ever write to the database (all uses create temp table, which is how they can run in parallel also), so that shouldn't be an issue. It can use pg17 instead of pg18 (or even pg15).

[dossy]

None of the tests ever write to the database (all uses create temp table, which is how they can run in parallel also), so that shouldn't be an issue. It can use pg17 instead of pg18 (or even pg15).

Right, but Supavisor does write to the database to store its metadata (tenant configuration, etc.) which, however unlikely, could interfere with tests as the tests are implemented assuming an empty database, but it won't be if the database is shared/being used by Supavisor.