feat(postgres): Add sane defaults for the connection #213
Conversation
| statement_timeout: 7_200_000, // 2 hours in milliseconds | ||
| lock_timeout: 30_000, // 30 seconds in milliseconds | ||
| idle_in_transaction_session_timeout: 300_000, // 5 minutes in milliseconds | ||
| application_name: "etl".to_string(), |
There was a problem hiding this comment.
What does Postgres to Postgres replication use for the timeout settings? Ok to use etl as application name for now, but we should later on use separate names for table sync and apply workers, probably same or similar as slot names.
There was a problem hiding this comment.
Definitely, I was considering using different names but yeah, let's do in it in a follow up, since it might add a lot of lines.
There was a problem hiding this comment.
I have checked the code and online, it seems like that the timeouts are set on a per subscription basis where the subscription contains the connection string. This means that no timeout is used by default.
There was a problem hiding this comment.
Maybe we should enable users to configure it in the future and keep it disabled for now, this way we can avoid possible problems related to timeouts.
There was a problem hiding this comment.
Let's disable statement_timeout and can keep the other timeouts at their current values in the PR.
There was a problem hiding this comment.
By disable I meant that statement_timeout is set to 0. If we omit it it will take the default value set for the role we use to connect which may or may not be 0.
iambriccardo
deleted the
riccardobusetti/etl-129-set-sane-options-when-connecting-to-postgres
branch
This PR implements sane defaults for the Postgres connection to avoid having the database misconfigured.