Use a different connection pool for read vs. write operations

[amotl]

About

At #72 (comment), there is an item about using two different connection pools for read vs. write operations. In this way, excessive/overuse of one communication channel will not impact the other, and different usage characteristics can be tuned individually.

[seut]

Is this by mistake? looks like this is not needed and breaks the laziness?

[amotl]

Thanks. I think the comment got misplaced, so it is indeed confusing. I just fixed it, and added more details at #81 (review) about it. Please let me know if you still think I am missing something.

[seut]

Is this really testing our code and not pgx one's as our code will generate this connection string by given setting maxConnections?
Afaik TestPoolsWithMaxConnections is the related test, what value does this test add over TestPoolsWithMaxConnections?

[amotl]

I agree that both functions test similar conditions, but still there are differences.

• TestConfigurePoolWithPoolSizeFromConnectionString directly uses pgxpool.ParseConfig() for creating a pgxpool.Config object from a DSN-style connection string (which is using the pool_max_conns parameter to configure the maximum pool size), and then uses the createPool() application primitive to create a pgxpool.Pool object. That's a bare-bones test case.
• TestPoolsWithMaxConnections on the other hand creates an application configuration object, in this case the "builtin" exemplar, and adjusts the MaxConnections setting manually. Then, it uses newCrateEndpoint() to create an Endpoint object, and invokes the endpoint.createPools() method on it, in order to run the full setup of creating both read+write pools. At the end, it verifies that both pool sizes derive their values from the MaxConnections configuration setting. That's a bit of a more integrated test case than the other one.

In order to add a few more thoughts here, which may help to understand my nitpickyness on test cases here: I am planning to abandon the configuration style of suppyling individual settings for Host, Port, etc. completely, and just use the connecting-string-style way, similar to pgx and libpq.

There are so many details in the parsing routines there, like recently added capabilities to describe and parse multiple host names and such, there is no way to emulate all of it in any different way. So, the best path forward, also reducing maintenance a lot, is to give users full power by just using a single connection string value as configuration setting.

While working on this, I will be happy to have different kinds of test cases around, which offer different perspectives to the parameter juggling and its upcoming changes.

[amotl]

Thanks for the review. I found another two spots to improve, and responded to your questions.

[amotl]

I agree that both functions test similar conditions, but still there are differences.

• TestConfigurePoolWithPoolSizeFromConnectionString directly uses pgxpool.ParseConfig() for creating a pgxpool.Config object from a DSN-style connection string (which is using the pool_max_conns parameter to configure the maximum pool size), and then uses the createPool() application primitive to create a pgxpool.Pool object. That's a bare-bones test case.
• TestPoolsWithMaxConnections on the other hand creates an application configuration object, in this case the "builtin" exemplar, and adjusts the MaxConnections setting manually. Then, it uses newCrateEndpoint() to create an Endpoint object, and invokes the endpoint.createPools() method on it, in order to run the full setup of creating both read+write pools. At the end, it verifies that both pool sizes derive their values from the MaxConnections configuration setting. That's a bit of a more integrated test case than the other one.

In order to add a few more thoughts here, which may help to understand my nitpickyness on test cases here: I am planning to abandon the configuration style of suppyling individual settings for Host, Port, etc. completely, and just use the connecting-string-style way, similar to pgx and libpq.

There are so many details in the parsing routines there, like recently added capabilities to describe and parse multiple host names and such, there is no way to emulate all of it in any different way. So, the best path forward, also reducing maintenance a lot, is to give users full power by just using a single connection string value as configuration setting.

While working on this, I will be happy to have different kinds of test cases around, which offer different perspectives to the parameter juggling and its upcoming changes.

[amotl]

Added the comment about lazy initialization back here, to the spot where it was coming from. The delayed intialization seems to be implemented by returning a function closure here, that detail did not change.

The pool creation has just been refactored into the createPools() method, and this one is still called from within the function closure.