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

Add pgBouncer to funnel connections #243

Closed
wants to merge 12 commits into from
Closed

Add pgBouncer to funnel connections #243

wants to merge 12 commits into from

Conversation

rustprooflabs
Copy link
Owner

Addresses #236.

@rustprooflabs rustprooflabs added enhancement New feature or request Docker Docker image and/or related script(s) labels Mar 31, 2022
@rustprooflabs rustprooflabs added this to the 0.4.7 milestone Mar 31, 2022
@rustprooflabs rustprooflabs removed this from the 0.4.7 milestone Apr 10, 2022
@rustprooflabs
Copy link
Owner Author

Closing this effort. See #236 (comment)

@rustprooflabs
Copy link
Owner Author

Rebased

@rustprooflabs
Copy link
Owner Author

Current version appears to be working when running manually. Fails make though. I went back and forth trying to find an obvious fix, didn't find anything. Current error from make:

# process DC file, pretending it's a region instead of subregion
docker exec -it \
	-e POSTGRES_PASSWORD=mysecretpassword \
	-e POSTGRES_USER=postgres \
	-u 1000:1000 \
	pgosm python3 docker/pgosm_flex.py  \
	--layerset=minimal \
	--ram=2 \
	--region=region-dc \
	--skip-qgis-style --skip-nested # Make this test run faster
2023-10-18 20:57:05,951:INFO:pgosm-flex:pgosm_flex:PgOSM Flex starting...
Setting up pgBouncer configuration files
Running pgbouncer as postgres user
Traceback (most recent call last):
  File "/app/docker/pgosm_flex.py", line 625, in <module>
    run_pgosm_flex()
  File "/usr/local/lib/python3.9/dist-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
  File "/usr/local/lib/python3.9/dist-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
  File "/usr/local/lib/python3.9/dist-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/local/lib/python3.9/dist-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
  File "/app/docker/pgosm_flex.py", line 103, in run_pgosm_flex
    pgbouncer.run()
  File "/app/docker/pgbouncer.py", line 58, in run
    subprocess.run(
  File "/usr/lib/python3.9/subprocess.py", line 505, in run
    with Popen(*popenargs, **kwargs) as process:
  File "/usr/lib/python3.9/subprocess.py", line 951, in __init__
    self._execute_child(args, executable, preexec_fn, close_fds,
  File "/usr/lib/python3.9/subprocess.py", line 1823, in _execute_child
    raise child_exception_type(errno_num, err_msg, err_filename)
PermissionError: [Errno 1] Operation not permitted
make: *** [Makefile:174: docker-exec-region] Error 1

docker/pgbouncer.py Fixed Show fixed Hide fixed
docker/pgbouncer.py Fixed Show fixed Hide fixed
docker/pgbouncer.py Fixed Show fixed Hide fixed
@rustprooflabs rustprooflabs added this to the 0.10.3 milestone Oct 22, 2023

LOGGER.warning('Saving password in plain text within the container for pgBouncer.')
with open(PGBOUNCER_USER_LIST_PATH, "w") as user_file:
user_file.write(user_list)

Check failure

Code scanning / CodeQL

Clear-text storage of sensitive information

This expression stores [sensitive data (password)](1) as clear text.
)

with open(PGBOUNCER_INI_PATH, "w") as ini_file:
ini_file.write(pgbouncer_ini)

Check failure

Code scanning / CodeQL

Clear-text storage of sensitive information

This expression stores [sensitive data (password)](1) as clear text.
@rustprooflabs
Copy link
Owner Author

It appears to be working nicely. Haven't tested with external connections yet, or with append mode. It does appear to be working as expected on internal Postgres so hopefully it just works from here...

docker stop pgosm && docker build -t rustprooflabs/pgosm-flex . 
docker run --name pgosm -d --rm \
    -v ~/pgosm-data:/app/output \
    -v /etc/localtime:/etc/localtime:ro \
    -e POSTGRES_PASSWORD=$POSTGRES_PASSWORD \
    -p 5433:6432 \ # Port for pgBouncer instead of base Postgres
    -d rustprooflabs/pgosm-flex

time docker exec -it \
    pgosm python3 docker/pgosm_flex.py \
        --ram=8 \
        --region=north-america/us \
        --subregion=district-of-columbia \
        --pgbouncer-pool-size=10

New output to logs at beginning. Running docker exec multiple times respects the --pgbouncer-pool-size setting. If it's enabled it'll always restart pgbouncer to ensure config is proper. If it's disabled (-1) it will attempt to stop the pgbouncer service.

2023-10-22 12:08:40,321:INFO:pgosm-flex:pgosm_flex:PgOSM Flex starting...
2023-10-22 12:08:40,322:WARNING:pgosm-flex:pgbouncer:Saving password in plain text within the container for pgBouncer.
2023-10-22 12:08:40,322:INFO:pgosm-flex:pgbouncer:Setting up pgBouncer configuration files with pool size 10
2023-10-22 12:08:40,325:INFO:pgosm-flex:pgbouncer:pgBouncer started
2023-10-22 12:08:40,325:INFO:pgosm-flex:db:Checking for Postgres service to be available

@rustprooflabs rustprooflabs marked this pull request as ready for review October 22, 2023 18:11
@rustprooflabs rustprooflabs changed the base branch from main to dev October 22, 2023 18:11
@rustprooflabs rustprooflabs changed the base branch from dev to main October 22, 2023 18:11
@rustprooflabs rustprooflabs changed the base branch from main to dev October 22, 2023 18:12
@rustprooflabs
Copy link
Owner Author

Crud, this actually wasn't working as I had expected. After making the osm2pgsql connections go through pgBouncer I'm getting failures.

2023-10-23 17:01:15,341:INFO:pgosm-flex:helpers:2023-10-23 17:01:15  Storing properties to table '"public"."osm2pgsql_properties"'.
2023-10-23 17:01:15,353:INFO:pgosm-flex:helpers:2023-10-23 17:01:15  ERROR: SQL command failed: EXECUTE set_property(attributes,false)
2023-10-23 17:01:15,354:INFO:pgosm-flex:helpers:2023-10-23 17:01:15  ERROR: Database error: FATAL:  prepared statement did not exist
2023-10-23 17:01:15,354:INFO:pgosm-flex:helpers:server closed the connection unexpectedly
2023-10-23 17:01:15,354:INFO:pgosm-flex:helpers:This probably means the server terminated abnormally
2023-10-23 17:01:15,354:INFO:pgosm-flex:helpers:before or while processing the request.
2023-10-23 17:01:15,354:INFO:pgosm-flex:helpers:(7)
2023-10-23 17:01:16,361:ERROR:pgosm-flex:pgosm_flex:Failed to run osm2pgsql. Return code: 1
Failed to run osm2pgsql. Return code: 1 - Check the log output for details

Looking deeper and reading closer I found on this page:

"Note: This tracking and rewriting of prepared statement commands does not work for SQL-level prepared statement commands such as PREPARE, EXECUTE, DEALLOCATE, DEALLOCATE ALL and DISCARD ALL. "

Searching in osm2pgsql I found this in properties.cpp

    db_connection.exec(
        "PREPARE set_property(text, text) AS"
        " INSERT INTO {} (property, value) VALUES ($1, $2)"
        " ON CONFLICT (property) DO UPDATE SET value = EXCLUDED.value",
        table_name());
    db_connection.exec_prepared("set_property", inserted.first,
                                inserted.second);

I'm unsure how to proceed on this ATM. I'm going to have to do some deeper digging and testing to figure out how this should/could work.

@rustprooflabs rustprooflabs removed this from the 0.10.3 milestone Oct 23, 2023
@rustprooflabs
Copy link
Owner Author

rustprooflabs commented Dec 8, 2023

Maybe refactoring prepared queries to use PQprepare would enable pgbouncer? With the way PQexecPrepared is used it seems PQprepare might be an option too.

@rustprooflabs
Copy link
Owner Author

rustprooflabs commented Dec 27, 2023

Started a discussion on osm2pgsql to see if the approach with PGprepare might pan out. osm2pgsql-dev/osm2pgsql#2118

@rustprooflabs
Copy link
Owner Author

Rebased, setup osm2pgsql repo URL to be dynamic for planned testing.

@rustprooflabs
Copy link
Owner Author

Recent work in osm2pgsql to reduce idle connections has removed my reasoning to work further on this. See #376 for details. The initial import w/ default layerset dropped from 43 connections down to 10. Replication updates dropped from 207 connections down to 17.

@rustprooflabs rustprooflabs deleted the pgbouncer branch February 18, 2024 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docker Docker image and/or related script(s) enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant