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

Significantly reduce number of db connection in flex output #2134

Merged
merged 3 commits into from
Feb 8, 2024

Conversation

joto
Copy link
Collaborator

@joto joto commented Feb 4, 2024

Flex outputs uses a lot of connections, basically O(number of threads * number of tables). They are mostly idle but need resources on the server. And the PostgreSQL default max number of connections is often not large enough.

With these changes the number of connections is much smaller, connections are not open for such long a time.

  • Use the same connection for all tables when creating the tables and various other setup tasks
  • We have one extra connection for the COPYs
  • Clustering, index creation is done in one connection per thread in the thread pool

There is probably more we can do here, closing some connections earlier or so. But this is a big step solving most of the problem.

joto added 3 commits February 4, 2024 10:25
This way prepared statements are different and we can have several of
them in the same connection. We'll use that in the future.
Each table of the flex output kept its own database connection in the
table_connection_t class. With this change, the connection is kept
in a vector in the flex output instead.

This is a refactoring step towards using fewer database connections. As
long as each table had its own connection, we can not reduce them, but
if we keep them outside, we will be able to re-use the same connection
for different tables.
Most operations are not run in parallel anyway, they only need a single
connection. For the parts running in parallel, we have one connection
per thread. We don't have connections for each table any more.
@rustprooflabs
Copy link
Contributor

Wonderful! I'll try to test this out soon. This might remove my motivation behind trying to get pgbouncer to work. 😆

@rustprooflabs
Copy link
Contributor

This works well in the initial load. Before the patch I get 41 idle connections on my default load, this change drops it to 3 idle. 👍

I ran into an error when using osm2pgsql-replication. I haven't dug any further than this error message yet. All I changed in my project was cloning a different repo/branch. I can get the exact commands involved if needed for testing on your end.

2024-02-04 14:35:22  ERROR: --database: At Most 1 required but received 2
Traceback (most recent call last):
File "/usr/local/bin/osm2pgsql-replication", line 724, in <module>
sys.exit(main())
File "/usr/local/bin/osm2pgsql-replication", line 716, in main
return args.handler(props, args)
File "/usr/local/bin/osm2pgsql-replication", line 552, in update

@joto
Copy link
Collaborator Author

joto commented Feb 5, 2024

The error you are getting is probably not related to this change but to an earlier change (PR #2115). Can you run with debugging so that we can see what command osm2pgsql-replication is calling that leads to this error?

@rustprooflabs
Copy link
Contributor

It looks like the error was in my code running osm2pgsql-replication. This command had been working prior to these changes:

osm2pgsql-replication update -d $PGOSM_CONN \
    -- \
    --output=flex --style=./run.lua \
    --slim \
    -d $PGOSM_CONN

It works if I take off the second -d $PGOSM_CONN. I don't know why I had been defining the -d option twice but there it is! I have to set this aside for the rest of today, but will come back and double check the drop in connection usage during this stage too. Replication updates were where it hit really hard with 200+ connections.

@joto
Copy link
Collaborator Author

joto commented Feb 6, 2024

@rustprooflabs As I had suspected, that is due to #2115. We are a bit more picky in checking the command line options. We'll put a warning in the release notes.

@lonvia lonvia merged commit e6a3096 into osm2pgsql-dev:master Feb 8, 2024
26 checks passed
@joto joto deleted the fewer-db-conns branch February 12, 2024 08:45
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.

3 participants