Don't ping when Connection Reset = false

[bgrainger]

When Connection Reset = true, the connection is reset (with a COM_RESET packet) when it's retrieved from the pool. This has the side-effect of checking connection liveness.

When Connection Reset = false, no reset occurs, but a COM_PING packet is still sent to check liveness (thus incurring a server roundtrip). Clients who wish to opt out of this can set Connection Idle Ping Time = <large value> but this is documented as an experimental setting.

There's no ADO.NET requirement that the server be pinged; it was just done to improve the odds that MySqlConnection.Open always returns a valid connection (instead of one that is in the pool but has been closed by the server). We could allow Connection Reset = false to be a "power user" / performance setting that also disables any server roundtrip (both reset and ping) and simply returns a connection from the pool if one is present. (This supersedes the more complicated options proposed in #967.)

The documentation would be updated to call out the effects of this change, and to note that MySqlConnection.Open might actually return an invalid connection that throws as soon as it's used. (Technically, this is always possible, because a network partition can happen at any time. Thus, code that's already written to retry wouldn't be affected.) Users could always call MySqlConnection.Ping(Async) if they still want the old behaviour; there's no reason it needs to be built into the library.

MySqlConnectionStringBuilder.ConnectionIdlePingTime would be marked [Obsolete], stop having any effect, and eventually be removed.

[danielgindi]

What you are saying is that MySqlConnection.Open will possibly return a pooled connection that's closed?
If I look at SqlConnection.Open docs, it says that it "draws an open connection from the connection pool if one is available".
A standard user will interpret that as providing a valid/usable connection.

Of course the base DbConnection.Open only says "When overridden in a derived class, opens a database connection with the settings specified by the ConnectionString.", but shouldn't we have a ConnectionString option to open a valid connection like we do with other connectors?

[bgrainger]

will possibly return a pooled connection that's closed

Yes, that's possibly true. But it's also possibly true that between the time Open returns (with a valid connection) and your code calls MySqlCommand.ExecuteNonQuery, a network partition has occurred and the server is no longer reachable. (Not trying to downplay the issue; just saying that all "correct" network I/O code needs to retry on failure.)

A standard user will interpret that as providing a valid/usable connection. ... shouldn't we have a ConnectionString option to open a valid connection like we do with other connectors?

What you describe is the default behaviour of MySqlConnector; it doesn't need a connection string option to enable it. (But you can set Connection Reset = True; if you want to be explicit, albeit redundant.)

Connection Reset = false is an advanced option that has several important ramifications: server state is not cleared, temporary tables still exist, user variables are not reset, a pooled connection is not tested upon retrieval. Anyone who opts in to that setting (it's not the default) needs to program their client accordingly.

[danielgindi]

Oh for some reason I understood that setting it to true is now also changed to not pinging the server.

It's true that network errors can occur, but that is sometimes something that you want to error out and log an error. But a connection from the pool being in the wrong state - that's something that you expect to be handled when you call Open() :)

[bgrainger]

Yes; that was my rationale for making Connection Reset = false still send a COM_PING packet in the first place. There are reasons I think that may be unnecessary caution: MySql.Data never did it, and there don't seem to be any user complaints, and Connection Idle Timeout = 180 (3 minutes) by default, so connections never get much of a chance to "go stale" while sitting in the pool. I believe the risk of it actually happening in practice is incredibly low, but this change can always be reverted if it turns out to be a significant issue.

[danielgindi]

MySql.Data never did it, and there don't seem to be any user complaints

One of the main reasons I switched was this exactly. Random timeouts on Open(), in a very unpredictable and undebuggable manner.

but this change can always be reverted if it turns out to be a significant issue.

As long as a true values still sends a ping packet, I think we're good. If anyone is interested in getting a dirty connection, they're going to execute queries for the preparation of the connection anyway.

[bgrainger]

One of the main reasons I switched was this exactly. Random timeouts on Open(), in a very unpredictable and undebuggable manner.

That's helpful to know.

If anyone is interested in getting a dirty connection, they're going to execute queries for the preparation of the connection anyway.

That was my assumption. One can also explicitly call PingAsync to force the old pinging behaviour.