Support URL Config for Pool

[skeggse]

Unless I missed something, the pool's configuration cannot be a url if you want to override the default pool-specific configuration options, including waitForConnections, connectionLimit, and queueLimit.

[dougwilson]

I'm not really sure if the connection string (which is meant to be for the connection) should be intermingled with the pool (which is application-dependent, really). How about we change it to accept all these:

mysql.createPool({
  // all options; current behavior
})

mysql.createPool(connStr, {
  // pool options
})

mysql.createPool(connOpts, {
  // pool options
})

This would allow the app to control the pool options and let it pass-through whatever the connection options are.

[dougwilson]

Example here: master...feature;pool-config-conn-string

[skeggse]

I can see how this syntax could be useful, but we need to be able to configure the pool options in the same place as the connection options. In some applications the pool options can certainly be hardcoded, but in our case that doesn't work.

[dougwilson]

Can you not just store the configuration as a JSON string and then mysql.createPool(JSON.parse(configurationString))? The connection string format is not really supposed to have anything referencing the pool from it, as the pool is a separate abstraction from actually how to connect to the MySQL database, which is why I'm hesitant.

[skeggse]

We are basically storing the mysql connection and pool information as a JSON object right now, but it just seems like allowing the following would be useful:

mysql.createPool('mysql://user:pass@localhost:5672/database?connectionLimit=100');

That being said, pools and connections are certainly different abstractions. Tangentially, mongoose allows the url strings to contain information about replicas, which seem like a different abstraction from the mongodb connection itself. I don't know that it's the right connection, but it doesn't seem like the connectionLimit options would ever conflict with connection options.

[dougwilson]

but it doesn't seem like the connectionLimit options would ever conflict with connection options.

Right, I don't think they would, either.

The other reason why I would rather the pool configuration be separate would be to make it easy to force pool configurations in the app-level, because think if you want to use the same connection string for an entire pool of app servers; some may have different pool configurations that others, but they would all share the same connection string. This is how our enterprise environments function.

[skeggse]

some may have different pool configurations that others, but they would all share the same connection string.

Which is why I indicated I liked your approach.

It definitely makes sense to allow individual applications to have control over their pooling configuration, but what if you want a central pool config as part of the URL? How about this: let the pool config options exist in the URL, but if specific in a second argument they'll be overridden.

mysql.createPool('...?connectionLimit=100', {
  // overrides the 100 in the URL
  connectionLimit: 50,
  queueLimit: 1000
});

[dougwilson]

How would you feel about making the pool config in the URL just a single parameter?

mysql://user:pass@localhost:5672/database?poolConfig=%7B%22connectionLimit%22%3A100%7D

[skeggse]

That could work. It's less readable which is part of the drive for URL-based configurations IMO.

Personally, I like having connectionLimit as its own parameter. What's the drive to have all pool configuration in a single parameter?

[dougwilson]

What's the drive to have all pool configuration in a single parameter?

Just so there would be a clear separation between pool configuration and connection configuration. We could also do something like:

mysql://user:pass@localhost:5672/database?poolConfig[connectionLimit]=100&poolConfig[waitForConnections]=false

or

mysql://user:pass@localhost:5672/database?poolConnectionLimit=100&poolWaitForConnections=false

We will likely add a debug to the pool config that would be separate from the connection config, among other things. Some current things are already combined together, but I'm trying to keep more new things from also getting combined for these kind of future things.

[skeggse]

Okay, that makes sense. Those two solutions seem perfectly fine to me, though I prefer the former--poolConfig[connectionLimit]=100 is nice and readable to me.

[dougwilson]

I may just end up adding your original suggestion. Let me mull it over a bit more ;)

[skeggse]

Of course!

[dougwilson]

Of course!

haha, I'm sorry if I do for dragging you around :) At least I added some actual tests for PoolConfig now (they would have caught the typo in your initial PR).

[skeggse]

Tests are always a good thing 😄