Reset session when returning to pool

[MrSmoke]

I currently have a use case were I get far better performance setting ConnectionReset to false but I still want the connection reset for safety purposes.

I was thinking that when ReturnToPool is called the connection could be reset there (if the enabled) instead of when it's pulled out of the pool, leaving the reset to be done in the background (before being added back into the pool).

It would however mean that the pool is easier to deplete.

[bgrainger]

It seems like this should work; I've created #179 to investigate it.

In my current PR, the connection is reset synchronously when MySqlConnection.Dispose is called; since the caller is still blocked, it's possible that this is not enough to increase performance.

The final solution may need to queue the work of resetting the connection (or disposing it) to a threadpool thread so that the client is freed up immediately when closing the connection. Presumably in that implementation, any exceptions would simply be swallowed and just cause the connection to be dropped from the pool. The downside would be that any errors would be silently ignored and only manifest themselves as a performance problem (because the pool would never contain live connections).

This would also be a behavioural change from the official connector, although I'm not sure it's one that would affect anyone. However, I did have to turn ConnectionReset off for a test because it was causing a session variable to be reset immediately, rather than having the desired side-effect.

@caleblloyd Any thoughts on the suggestion or proposed implementation?

[caleblloyd]

@MrSmoke what is the Round-Trip time from your client to your server (ping your MySql Host from wherever your client code resides)

[MrSmoke]

Around 1-2ms.

[caleblloyd]

How big is the speedup when you turn off connection reset? 2-4ms or is it higher? I'm wondering if what you're seeing is environment specific.

[bgrainger]

Another benefit of resetting the connection (in the background) as soon as it's returned to the pool is that it would immediately free up any session-specific server variables (e.g., temp tables). Right now, I'm assuming that those persist in memory on the server until the connection is pulled from the pool and reset.

[caleblloyd]

Right now, I'm assuming that those persist in memory on the server until the connection is pulled from the pool and reset.

Or until the connection is reaped

I can see the benefits, my main concern is forcing the reset commands to run over Sync I/O for everyone. The concurrency tests have shown that even 1ms of synchronous execution can cause a huge difference in throughput.

What about adapting Close and CloseAsync to reset the session if they are called by the user. Dispose would still not reset the connection. This would give high latency clients the option to add a manual reset at the end of a connection, and it would even support async via CloseAsync

[bgrainger]

What about adapting Close and CloseAsync to reset the session if they are called by the user. Dispose would still not reset the connection. This would give high latency clients the option to add a manual reset at the end of a connection, and it would even support async via CloseAsync

I think one of the major benefits we can provide by doing it asynchronously is that Dispose can return immediately (while the connection is reset in the background before being returned to the pool). This allows application code to resume immediately (without waiting for the latency of resetting). If concurrent connection usage is less than maximum pool size, then opening a new connection doesn't have to block either, because there are sufficient idle (and reset) connections in the pool. So you might be able to have 90% connections in use, 5% idle and waiting, and 5% returned but being reset in the background. If the idle pool can be replenished from background-reset connections faster than it's being depleted, the client would get resetting for "free".

[caleblloyd]

I hadn't considered doing it this way, I like your approach!

[bgrainger]

Alternatively, we could follow npgsql's approach and prepend the "connection reset" packet to the first query that's sent on a connection retrieved from the pool; see npgsql/npgsql#552.

[athoscouto]

#179 was closed in favor of #264, but the later was also closed :(

Are there any other approaches we can try to prevent the reset overhead when retrieving connections from the pool?

In my case I can't afford to set ConnectionReset=false and the time I spent getting the connection >7ms is way higher than the my average connection lifetime ~1ms.

I've implemented a PoC of a pool of Tasks to get new open connections. Every time I need a connection I await a Task from the pool and put a new Task on it. With it I've managed to reduce my average time to open connections from >7ms to ~1.5ms.

I wouldn't push this pool overlay into production, but it shows that there may be room for improvement on the ConnectionPool logic.

[bgrainger]

#179 was closed in favor of #264, but the later was also closed :(

Those were both implementations that weren't viable. This issue remains open to track the problem.

I assume it's important to you that the connection reset not happen at all on a "user" thread? I.e., it wouldn't be OK to reset the connection during DisposeAsync (so that returning the connection to the pool doesn't complete before the connection has been reset)?

[athoscouto]

I assume it's important to you that the connection reset not happen at all on a "user" thread?

I don't think this would be a problem for me. I have a wrapper around MySqlConnection to add some monitoring. When I dispose my wrapper I could just call the connection's DisposeAsync and let it run in background, without blocking the "user" thread.

I don't know if this would make sense for the library as a whole, though.

[athoscouto]

An approach that I think would be equivalent to mine:

• Starting the clean up when disposing the connection.
• Adding it to the pool without awaiting for the session clean up.
• awaiting the clean up when getting it connections from the pool.

What is the downside of doing it like this?

Sorry if I am missing something obvious, I'm not very knowledgable about async/Task internals and their implications when developing general-purpose libraries.

[bgrainger]

One benefit of the current approach is that resetting the connection (on retrieval from the pool) checks the liveness of the connection. (An idle connection can be closed by the server due to wait_timeout, and failure won't be noticed until we try to use it.)

Pinging the connection would be a way to detect the problem, but would reintroduce the latency of a roundtrip to the connection. #821 proposes a different solution, but one that has its own drawbacks. Or we could not test the connection at all, and put the burden of knowing that new MySqlConnection().Open() may return a connection in an invalid state on the user. (This last option seems like it would be quite unexpected and probably a breaking change for most users.)

[athoscouto]

Your considerations are spot on. What do you think about letting the user configure how they want to handle it? Would it introduce too much complexity?

In my use case I have a proxy that is supposed to be pretty fast, and one of its bottlenecks is opening connections to the data base. An average DB operation takes about 1ms, but the average time to open a connection is at least 3ms.

Any suggestions to avoid this overhead?
If you think there is no way forward with this, I think I'll go back to the second layer pool of opened connections proof of concept and try to "productionize" it.

Anyway, thanks for the attention and the remarks :)

[bgrainger]

What do you think about letting the user configure how they want to handle it?

If the current behaviour were changed, it would probably be exposed as an opt-in setting, as it would likely require changes to user code (e.g., retrying OpenAsync because it might return a closed connection, preferring DisposeAsync, etc.)

In my use case I have a proxy that is supposed to be pretty fast, and one of its bottlenecks is opening connections to the data base.

This is what the current connection pooling is designed to solve (unless your proxy also makes resetting a connection slow).

If you think there is no way forward with this

It's not that I don't think there's no way forward with this; I just haven't worked out the ideal solution (and it may be very complex to implement).

Just had a thought: if MySqlConnection exposed ResetConnectionAsync, would that help you manage the eager resetting of connections? You could call OpenAsync to get a connection from the pool, use it, transfer it to a background thread to call ResetConnectionAsync then put it back in the pool (and set ConnectionReset=false in your connection string because you're handling it manually). That would avoid the overhead of resetting when retrieving a connection from the pool, save you from having to maintain your own connection pool, and not block your "main" thread with reset connection requests (because you'd handle it on your own background thread).

[athoscouto]

This would be perfect for me.

[bgrainger]

Opened #831 to track that suggestion.

[bgrainger]

Another attempt at solving this problem: https://github.com/mysql-net/MySqlConnector/tree/reset-connection-in-background

The new solution is conceptually very simple: closing/disposing a connection starts resetting it (asynchronously) in the background, then returns immediately. A background thread awaits the reset, then returns the connection to the pool. This may cause a few extra connections to be used while the reset happens.

[bgrainger]

Benchmarks from the new code:

Open/OpenAsync = default connection string options (SslMode=None)
OpenNoReset/OpenNoResetAsync = ConnectionReset=false -- no connection reset, but still pings on Open
OpenNoPing/OpenNoPingAsync = ConnectionReset=false;ConnectionIdlePingTime=1000 -- no reset, no ping on Open
OpenDefer/OpenDeferAsync (new code only) = DeferConnectionReset=true -- should be the same as Open in the "Before" code

Local Docker Container

Before

Method	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
Open	1,128.7 µs	15.9 µs	14.1 µs	-	-	-	649 B
OpenAsync	1,141.4 µs	14.6 µs	13.0 µs	-	-	-	3498 B
OpenNoReset	548.5 µs	5.0 µs	4.4 µs	-	-	-	648 B
OpenNoResetAsync	640.5 µs	12.2 µs	12.6 µs	-	-	-	2377 B
OpenNoPing	0.8 µs	0 µs	0 µs	0.0772	-	-	648 B
OpenNoPingAsync	0.8 µs	0 µs	0 µs	0.0429	-	-	360 B

After

Method	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
OpenDefer	1,127.2 µs	8.3 µs	7.4 µs	-	-	-	649 B
OpenDeferAsync	1,141.1 µs	8.4 µs	7.0 µs	-	-	-	3497 B
Open	649.6 µs	9.8 µs	9.1 µs	-	-	-	3374 B
OpenAsync	891.2 µs	15.6 µs	27.7 µs	-	-	-	5103 B
OpenNoReset	547.3 µs	3.7 µs	3.1 µs	-	-	-	648 B
OpenNoResetAsync	668.2 µs	6.2 µs	5.5 µs	-	-	-	2376 B
OpenNoPing	0.8 µs	0 µs	0 µs	0.0772	-	-	648 B
OpenNoPingAsync	0.7 µs	0 µs	0 µs	0.0429	-	-	360 B

Remote MySQL (~16ms ping)

Before

Method	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
Open	38.17 ms	0.758 ms	1.308 ms	-	-	-	669 B
OpenAsync	38.98 ms	0.778 ms	2.076 ms	-	-	-	3518 B
OpenNoReset	18.82 ms	0.375 ms	0.488 ms	-	-	-	657 B
OpenNoResetAsync	18.84 ms	0.370 ms	0.564 ms	-	-	-	2395 B

Method	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
OpenDefer	37.88 ms	0.755 ms	1.262 ms	-	-	-	670 B
OpenDeferAsync	38.71 ms	0.744 ms	1.569 ms	-	-	-	3518 B
Open	23.36 ms	0.414 ms	0.443 ms	-	-	-	3396 B
OpenAsync	23.49 ms	0.450 ms	0.518 ms	-	-	-	5110 B
OpenNoReset	19.49 ms	0.387 ms	0.833 ms	-	-	-	657 B
OpenNoResetAsync	19.05 ms	0.374 ms	0.537 ms	-	-	-	2385 B

Summary

In the new code, Open is almost as quick as OpenNoReset in the old code, but brings the benefit that the connection is still reset and in a known good state when it's retrieved from the pool. For ultimate speed, you can disable pinging (with the caveat that a broken connection may be returned and you need retry logic around any database operation, but you should generally have that anyway).

BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19042
Intel Core i7-10875H CPU 2.30GHz, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=5.0.200-preview.20601.7
[Host] : .NET Core 5.0.1 (CoreCLR 5.0.120.57516, CoreFX 5.0.120.57516), X64 RyuJIT
Job-NRYEMX : .NET Core 5.0.1 (CoreCLR 5.0.120.57516, CoreFX 5.0.120.57516), X64 RyuJIT

Platform=X64 Runtime=.NET Core 5.0

[bgrainger]

This is available for testing in 1.3.0-beta.1.

[bgrainger]

Added in 1.3.0.

[penguinawesome]

Hi @bgrainger with the new updates, since the connectionreset is being done in the background, would there be any PING at the Open?

[bgrainger]

Yes, by default there is still a PING during Open. This is to verify that a valid connection is being handed back; otherwise a broken connection might be returned from the pool.

Discussion about handling/changing that is at #461 (comment) (and the following comments); probably needs to be split out to a separate new issue.

[penguinawesome]

@bgrainger with the new implementation of OpenAsync in 1.30+, how come the OpenNoResetAsync is much faster than OpenAsync? If the reset of connection happens in the background then there would be no waiting time for OpenAsync?

Also, could you share some specific example query or any statement that is dangerous when using ConnectionReset=false in the connection string? We are also looking at setting the ConnectionReset to false since we want to achieve the best performance as possible.

[bgrainger]

how come the OpenNoResetAsync is much faster than OpenAsync?

I'm not completely sure. (And it's not, for a local Docker container.)

Also, could you share some specific example query or any statement that is dangerous when using ConnectionReset=false in the connection string?

Anything that uses temporary tables or sets variables on the server. create temporary table might fail because it already exists, for example. And those server-side resources won't get cleaned up, so they will keep consuming memory on the MySQL server.

[penguinawesome]

@bgrainger thank you, It looks like we are not using create temporary table and we dont set variables locally or globally like
SET @name = 43;
SET @total_tax = (SELECT SUM(tax) FROM taxable_transactions);

We will try to use the ConnectionReset=false;ConnectionIdlePingTime=10;

[bgrainger]

probably needs to be split out to a separate new issue

See #967.

[bgrainger]

This was turned off by default in 1.3.10 and removed in 2.0: #1013; reopening this issue.