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

bump lib/pq ftom 1.2 to 1.10 #715

Closed
wants to merge 1 commit into from

Conversation

roylee17
Copy link

I'm seeing "broken pipe" errors when working with CRDB using sqlx.
The issue seems to be the db driver conns become stale when tcp connections were disconnected.

It happens more often when the DB is behind a proxy. In our cases, the pods were proxied by the envoy sidecar.

There were other instances on the community reporting similar issues, and took different workaround by sending periodic dummy queries in app as keepalive, prolonging proxy idle timeout, or shortening the lifetime of db conns.

This has been reported and fixed in the lib/pq in v1.9+

lib/pq#1013

Other reference:

lib/pq#723
lib/pq#897
lib/pq#870

grafana/grafana#29957

    I'm seeing "broken pipe" errors when working with CRDB using sqlx.
    The issue seemed to be the tcp connections were diconnected while the conns
    in db driver (pq) still has stale connection.

    It happens more often when the DB is behind a proxy.
    In our cases, the pods were proxied by the envoy sidecar.

    There were other instances on the community reporting similar issues,
    and took different workaround by sebding perodic dummy queries in app
    mimicing keepalive, enlenghthen proxy idle timeout, or shortening the
    lifetime of db conn.

    This has been reported and fixed by the lib/pq upstream in v1.9+

      lib/pq#1013

      lib/pq#723
      lib/pq#897
      lib/pq#870

      grafana/grafana#29957
@coveralls
Copy link

coveralls commented Mar 21, 2021

Pull Request Test Coverage Report for Build 212

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 73.71%

Totals Coverage Status
Change from base Build 194: 0.0%
Covered Lines: 1200
Relevant Lines: 1628

💛 - Coveralls

@roylee17
Copy link
Author

roylee17 commented Aug 4, 2021

ping @jmoiron

@ehasnain
Copy link

We are also experiencing the same broken_pipe issue. After a lot of research landed up here.
Commenting for an upvote here.
And thanks @roylee17 for the PR. :)

@panadeluxe
Copy link

Would also really appreciate this. It is a huge annoyance(for how simple the fix is) and leads to inaccessible apps.

@mka-2016
Copy link

@jmoiron I would also be very grateful if this small but important fix could be merged.

ehasnain pushed a commit to ehasnain/sqlx that referenced this pull request May 19, 2022
@JRaspass
Copy link

So I'm fairly sure that sqlx only uses the mysql, pq, and sqlite drivers for tests. So which version of pq you use in your application is down to you.

I have these in my go.mod:

require (
	...
	github.com/jmoiron/sqlx v1.3.5
	github.com/lib/pq v1.10.7
	...
)

Which becomes these in my go.sum:

github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
github.com/lib/pq v1.10.7 h1:p7ZhMD+KsSRozJr34udlUrhboJwWAgCg34+/ZZNvZZw=
github.com/lib/pq v1.10.7/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
...
github.com/jmoiron/sqlx v1.3.5 h1:vFFPA71p1o5gAeqtEAwLU4dnX2napprKtHr7PYIcN3g=
github.com/jmoiron/sqlx v1.3.5/go.mod h1:nRVWtLre0KfCLJvgxzCsLVMogSvQ1zNJtpYr2Ccp0mQ=

So you can see it has fetched the old pq but it doesn't end up in the binary:

$ go version -m my-compiled-app-binary | grep pq
	dep	github.com/lib/pq	v1.10.7	h1:p7ZhMD+KsSRozJr34udlUrhboJwWAgCg34+/ZZNvZZw=

@BenKnigge
Copy link

I've created a fork with updated current drivers https://github.com/BenKnigge/sqly

@dlsniper
Copy link
Collaborator

dlsniper commented Feb 1, 2024

Hello, @ardan-bkennedy, and I recently stepped in to help maintain this project.
We are sorting the opened issues and will close this PR in favor of #909.
Thank you for your contribution.

@dlsniper dlsniper closed this Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants