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.

Sign up for GitHub

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

Jump to bottom

Consider switch to jackc/pgx from lib/pq #1775

Closed
daa opened this issue Jul 28, 2021 · 3 comments · Fixed by #4913
Closed

Consider switch to jackc/pgx from lib/pq #1775

daa opened this issue Jul 28, 2021 · 3 comments · Fixed by #4913
Labels
enhancement New feature or request

Comments

@daa
Copy link

daa commented Jul 28, 2021 •
edited
Loading

Is your feature request related to a problem? Please describe.
I have a HA PostgreSQL cluster consisting of 1 master and 2 replicas with quorum commit and I'd like to use client-side failover like libpq has (https://www.postgresql.org/docs/13/libpq-connect.html, 31.3.3). But currently I can't because Go's lib/pq module is in maintenance mode (https://github.com/lib/pq#status) and lacks required features (lib/pq#683).

Describe the solution you'd like
I think that the best and easiest way would be to use pgx as a PostgreSQL driver instead of pq because it's actively maintained, has necessary features and is compatible with sqlx used in SQL plugins.

Describe alternatives you've considered
Alternative solution may be to deploy and use proxy to handle failover and load balancing however this makes scheme more complex which I'd like to avoid.

Additional context
Authors of lib/pq recommend using pgx on package front page.
@daa daa added the enhancement New feature or request label Jul 28, 2021
@daa daa changed the title Consider switch to jacks/pgx from lib/pq Consider switch to jackc/pgx from lib/pq Jul 30, 2021
@wilsoniya
Copy link

I'd also like to see support for multiple hosts for HA Postgres.

Absent this, is there documentation for alternative ways to use multi-host (i.e., HA) postgres with temporal?

@yiminc
Copy link
Member

yiminc commented Oct 14, 2022

We don't have resource to work on this at the moment. Contribution on this would be awesome.

@sync-by-unito sync-by-unito bot closed this as completed Mar 3, 2023
@yiminc yiminc reopened this Mar 3, 2023
@wilsoniya
Copy link

wilsoniya commented Jun 15, 2023 •
edited
Loading

A bug in lib/pq appears to be a direct cause of this #4184

@EHDoPfuH EHDoPfuH mentioned this issue Jul 14, 2023
Closed
@ndtretyak ndtretyak mentioned this issue Oct 2, 2023
Merged
rodrigozhou pushed a commit that referenced this issue Oct 9, 2023
@ndtretyak
pgx support (#4913)
0612ebf 
<!-- Describe what has changed in this PR -->
**What changed?**
`Driver` interface (with two implementations) was added to the
postgresql persistence package in order to support other drivers than
`lib/pq`. Fixes #1775

Essentially, this PR does
#4914 in a
backward-compatible and extendable way

<!-- Tell your future self why have you made these changes -->
**Why?**
Since `lib/pq` does not support multi-node clusters


<!-- How have you verified this change? Tested locally? Added a unit
test? Checked in staging env? -->
**How did you test it?**
Existing tests


<!-- Assuming the worst case, what can be broken when deploying this
change to production? -->
**Potential risks**
No, since you should explicitly change the driver in your config file.
However, `connectAttributes` depend on the driver (e.g.,
`binary_parameters` is supported only by `lib/pq` and is not a part of
postresql standard), so one should be careful changing drivers in
production.


<!-- Is this PR a hotfix candidate or require that a notification be
sent to the broader community? (Yes/No) -->
**Is hotfix candidate?**
No
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

pgx support ndtretyak/temporal
3 participants
@daa @wilsoniya @yiminc