-
Notifications
You must be signed in to change notification settings - Fork 19
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
Remove my postgres connection pools #149
Conversation
self.sql_conn_pool = DatabaseCycleConnectionPool( | ||
n_conn, max_conn, self.dbnames, self.conn_info) | ||
self.sql_conn_pool = DBAffinityConnectionsNoLimit( | ||
self.dbnames, self.conn_info) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This section around line 160 used to read:
self.sql_conn_pool = DBAffinityConnectionsNoLimit(
self.dbnames, n_conn, self.conn_info)
Where self.n_conn = n_conn
was part of the function arguments.
Similarly on 170 we used to say:
sql_conns, conn_info = self.sql_conn_pool.get_conns()
instead of proposed:
sql_conns = self.sql_conn_pool.get_conns(self.n_conn)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that was fairly messy, so this is a cleanup of that.
import ujson | ||
|
||
|
||
class DBAffinityConnectionsNoLimit(object): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking back at history I was expecting to see 3x as much content here, mostly based off this SHA:
And then bringing some of the hstore / ujson changes in from:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We didn't use the other database pools, so I removed them.
The ujson stuff is there. The globally=True thing I wanted to leave off in case it caused the connection problem.
@@ -533,7 +533,7 @@ def tilequeue_process(cfg, peripherals): | |||
io_pool = ThreadPool(n_io_workers) | |||
|
|||
feature_fetcher = DataFetcher(cfg.postgresql_conn_info, all_layer_data, | |||
io_pool, n_layers, n_total_needed_query) | |||
io_pool, n_layers) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@zerebubuth for more substantive comments. |
@iandees Can you cleanup the merge conflict, please? |
🤔 I did, but the commit's not showing up ... I re-pushed it just now and I see it showed up. |
I'm going to go ahead and merge this per the tiles catchup meeting today. |
Backs out the connection pool bits of #145 (because they didn't work in production), puts back the existing
DBAffinityConnectionsNoLimit
implementation and adds the one-liner to useujson.loads
forregister_json()
.