Skip to content

Conversation

@dechdev
Copy link

@dechdev dechdev commented Jan 6, 2022

Helps solve ERROR SQL query failed: pq: sorry, too many clients already error seen in hyperledger/firefly#380

Signed-off-by: David Echelberger <david.echelberger@kaleido.io>
PostgreSQL: &CommonDBConfig{
URL: getPostgresURL(member),
URL: getPostgresURL(member),
MaxConns: 50,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this take the value from the config, but with a default of 50 set?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, ok - one sec ... maybe this code uses config different. Let me take a look around the change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh - sorry. Ok, completely my bad. I assumed I was looking at firefly (not firefly-cli).

I think this code change should be in FireFly (not the CLI).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment here: https://github.com/hyperledger/firefly/blob/4f4390336ee0b6c9f68b5ed2b79ffc3fbbf293d1/internal/database/sqlcommon/config.go#L44

... I think we've learned PostgreSQL should be in the group that sets a default

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Example of how SQLite does the default to 1, which you could copy to PSQL to set the default to 50:
https://github.com/hyperledger/firefly/blob/4f4390336ee0b6c9f68b5ed2b79ffc3fbbf293d1/internal/database/sqlite3/config.go#L27-L34

@dechdev
Copy link
Author

dechdev commented Jan 6, 2022

hyperledger/firefly#385 Made a PR in firefly for this. Closing this

@dechdev dechdev closed this Jan 6, 2022
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.

2 participants