Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Event triggers don't work on read replicas #3414

Open
steve-chavez opened this issue Apr 16, 2024 · 6 comments · May be fixed by #3462
Open

Event triggers don't work on read replicas #3414

steve-chavez opened this issue Apr 16, 2024 · 6 comments · May be fixed by #3462
Labels
enhancement a feature, ready for implementation

Comments

@steve-chavez
Copy link
Member

steve-chavez commented Apr 16, 2024

Problem

See https://stackoverflow.com/questions/29931136/postgresql-replica-where-i-can-create-triggers/29932655#29932655.

This means that PostgREST NOTIFY reloading won’t work in read replicas.

Previously, I thought this was only a problem with just LISTEN/NOTIFY on read replicas (#2781), but it's worse.

Solution

Enable a configurable connection string for the LISTEN channel.

db-channel-uri = "postgres://authenticator:mysecretpassword@localhost:5433/postgres?parameters=val"

Similar to the https://postgrest.org/en/v12/references/configuration.html#db-uri. By default it will use the same value as the db-uri.

This way the LISTEN channel can point to the primary instead of the replica and notify reloading can work normally.


@steve-chavez steve-chavez added idea Needs of discussion to become an enhancement, not ready for implementation enhancement a feature, ready for implementation difficulty: beginner Pure Haskell task and removed idea Needs of discussion to become an enhancement, not ready for implementation difficulty: beginner Pure Haskell task labels Apr 16, 2024
@wolfgangwalther
Copy link
Member

Previously, I thought this was only a problem with just LISTEN/NOTIFY on read replicas (#2781), but it's worse.

In which way is it worse than what you thought before? It's exactly that, no?

Enable a configurable connection string for the LISTEN channel.

Does this really make sense as long as we don't have support for read replica connections to route reads to, in the first place? How does this come up in practice without?

@steve-chavez
Copy link
Member Author

steve-chavez commented Apr 16, 2024

Does this really make sense as long as we don't have support for read replica connections to route reads to, in the first place? How does this come up in practice without?

It makes sense in a setup where you have a PostgREST on the primary and with a replica that has its own PostgREST too.

Yeah ideally it would be a single PostgREST with #2429 but that's not coming anytime soon. This would be a much easier to implement alternative in the meantime.

It doesn't add much complexity too. Once we have read replica support I guess we could drop the config as well.

@wolfgangwalther
Copy link
Member

How do both db-uri and db-channel-uri and the PG_ environment variables interact?

IMHO, this will quickly become very annoying to configure. For example, in a kubernetes setup, it's much more practical to pass all the database config options via separate environment variables - including the host name, which could already be available in a secret to consume, provided by your favorite pg operator. But now I have to pass those in a db-uri format. Meh.

How about using PostgreSQL's "Multiple Hosts" feature in libpq?

We could do the following:

  • Tell the user to provide the hosts as a comma separated list, i.e. read-only.host,primary.host. This should also work with PG_HOST, which the k8s pg operator could then provide as a list, too, depending on how many read-only nodes there are.
  • Use target_session_attrs=read-write (or primary, not sure) for the listener connection.

This should make postgrest pick the first connection, which is read-only, for all regular connections, but use the primary node for the listener.

Advantages:

  • A more "postgres native" approach.
  • Gives us a head start to implement read only replicas via the same mechanism - i.e. flag some of our pool connections as read-only or read-write and pass this parameter to target_session_attrs, then put the respective requests primarily on those connections.
  • Most importantly: Should give some kind of connection error for the listener connection, when the user only provides read only nodes, but still tries to enable the listener. Much better default than right now, where it just silently doesn't work.

@steve-chavez
Copy link
Member Author

steve-chavez commented Apr 25, 2024

@wolfgangwalther Not sure I get your idea. So I've just tried it multiple hosts and target_session_attrs and they work like this:

# having only one pg listening on 5432 this will succeed, since libpq will try the second host
psql postgresql://user:pass@localhost:5433,localhost:5432/postgres

# this will now fail since our pg is read-write and we tell libpq to only use a read-only session
psql postgresql://user:pass@localhost:5433,localhost:5432/postgres?target_session_attrs=read-only

connection to server at "localhost" (::1), port 5432 failed: session is not read-only

target_session_attrs is always applied to all hosts.

Use target_session_attrs=read-write (or primary, not sure) for the listener connection.
This should make postgrest pick the first connection, which is read-only, for all regular connections, but use the primary node for the listener.

Is the above true? So on the instance having a read replica + postgrest, we'd have:

PG_HOST="postgresql://user:[email protected],readwrite.host/postgres?target_session_attrs=read-write"

Then all connections will be routed to the readwrite.host, not just the listener connection. Is it possible to apply the target_session_attrs for just one host? It doesn't seem so.

I'm not seeing how is possible to solve this without 2 db-uris, one for the pool and another one for the listener.

@steve-chavez
Copy link
Member Author

steve-chavez commented Apr 25, 2024

Use target_session_attrs=read-write (or primary, not sure) for the listener connection.

Oh, perhaps you meant to dynamically inject that target_session_attrs=read-write to the internal listener db-uri? Like what we do for the fallback_application_name?

Yeah, that could work! Great idea! 💡

(PG_HOST/db-uri would not contain target_session_attrs, but if it's specified.. I guess we would need to force read-write)

@wolfgangwalther
Copy link
Member

Oh, perhaps you meant to dynamically inject that target_session_attrs=read-write to the internal listener db-uri? Like what we do for the fallback_application_name?

That's exactly what I meant to say :)

@steve-chavez steve-chavez linked a pull request Apr 30, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement a feature, ready for implementation
2 participants