[Bug] MySQL Outbox - All pooled connections are in use
Describe the bug
When doing DepositPostAsync and then clear messages explicitly by calling ClearOutboxAsync the connection pool becomes full very quick. It looks like the connections are not properly disposed. With each call to ClearOutboxAsync there is +1 connection in a connection pool.
This bug was already fixed in the past: https://github.com/BrighterCommand/Brighter/issues/2077 but probably reintroduced in version 10.
I tried to extract the Outbox code and called
connection.Dispose();
after closing the connection but it didn't help.
It seems that in v9 we were able to configure the lifetime of an Outbox, but in v10 it's strictly Singleton. @iancooper @preardon Please advise if there could be a workaround for this issue?
Exceptions (if any)
MySqlConnector.MySqlException (0x80004005): Connect Timeout expired. All pooled connections are in use.
at MySqlConnector.MySqlConnection.CreateSessionAsync(ConnectionPool pool, Int64 startingTimestamp, Activity activity, Nullable`1 ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlConnection.cs:line 946
at MySqlConnector.MySqlConnection.OpenAsync(Nullable`1 ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlConnection.cs:line 419
Further technical details
- Brighter version: 1.0.0-preview.3
- The OS: Windows
Thanks @romtur we will take a look and see why it regressed from https://github.com/BrighterCommand/Brighter/issues/2077 @preardon
I think I found where the issue is. I was concentrating on the ReadFromStore method, but it happens in the WriteToStore method when we are marking the message as dispatched.
We need to close the connection there in the way it's done in MsSqlOutbox:
if (transactionProvider != null) transactionProvider.Close(); else connection.Close();
I'll have a look, I thought this was done in the relationalOutbox Abstraction
I'll have a look, I thought this was done in the relationalOutbox Abstraction
It is possible that I broke something when I changed some of this for V10.
PR Raised, @iancooper let me know what you think @romtur sorry it took me so long to get to this and thanks for the traige
the Following PR furhter fixes this #3186
@iancooper after getting these out it could be good for us to get out the next preview release of v10
@preardon Yeah. I would like to get the sample changes done, in the next few days, so that we can at least see some of the new tracing stuff there, even if it won't flow into all the transports and databases yet.