Skip to content

Commit 3fce77d

Browse files
vonzshikvonzshik
authored
Fix unpooled connection return with multiple hosts (#5784)
Fixes #5783
1 parent 28d41bf commit 3fce77d

6 files changed

Lines changed: 17 additions & 25 deletions

File tree

‎src/Npgsql/Internal/NpgsqlConnector.cs‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2016,9 +2016,6 @@ internal void Close()
20162016
LogMessages.ClosedPhysicalConnection(ConnectionLogger, Host, Port, Database, UserFacingConnectionString, Id);
20172017
}
20182018

2019-
internal bool TryRemovePendingEnlistedConnector(Transaction transaction)
2020-
=> DataSource.TryRemovePendingEnlistedConnector(this, transaction);
2021-
20222019
internal void Return() => DataSource.Return(this);
20232020

20242021
/// <summary>

‎src/Npgsql/MultiplexingDataSource.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,8 @@ sealed class MultiplexingDataSource : PoolingDataSource
3333

3434
internal MultiplexingDataSource(
3535
NpgsqlConnectionStringBuilder settings,
36-
NpgsqlDataSourceConfiguration dataSourceConfig,
37-
NpgsqlMultiHostDataSource? parentPool = null)
38-
: base(settings, dataSourceConfig, parentPool)
36+
NpgsqlDataSourceConfiguration dataSourceConfig)
37+
: base(settings, dataSourceConfig)
3938
{
4039
Debug.Assert(Settings.Multiplexing);
4140

‎src/Npgsql/NpgsqlMultiHostDataSource.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ internal NpgsqlMultiHostDataSource(NpgsqlConnectionStringBuilder settings, Npgsq
4949
poolSettings.Host = host.ToString();
5050

5151
_pools[i] = settings.Pooling
52-
? new PoolingDataSource(poolSettings, dataSourceConfig, this)
52+
? new PoolingDataSource(poolSettings, dataSourceConfig)
5353
: new UnpooledDataSource(poolSettings, dataSourceConfig);
5454
}
5555

‎src/Npgsql/PoolingDataSource.cs‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,6 @@ class PoolingDataSource : NpgsqlDataSource
3131
/// </summary>
3232
private protected readonly NpgsqlConnector?[] Connectors;
3333

34-
readonly NpgsqlMultiHostDataSource? _parentPool;
35-
3634
/// <summary>
3735
/// Reader side for the idle connector channel. Contains nulls in order to release waiting attempts after
3836
/// a connector has been physically closed/broken.
@@ -76,15 +74,12 @@ internal sealed override (int Total, int Idle, int Busy) Statistics
7674

7775
internal PoolingDataSource(
7876
NpgsqlConnectionStringBuilder settings,
79-
NpgsqlDataSourceConfiguration dataSourceConfig,
80-
NpgsqlMultiHostDataSource? parentPool = null)
77+
NpgsqlDataSourceConfiguration dataSourceConfig)
8178
: base(settings, dataSourceConfig)
8279
{
8380
if (settings.MaxPoolSize < settings.MinPoolSize)
8481
throw new ArgumentException($"Connection can't have 'Max Pool Size' {settings.MaxPoolSize} under 'Min Pool Size' {settings.MinPoolSize}");
8582

86-
_parentPool = parentPool;
87-
8883
// We enforce Max Pool Size, so no need to to create a bounded channel (which is less efficient)
8984
// On the consuming side, we have the multiplexing write loop but also non-multiplexing Rents
9085
// On the producing side, we have connections being released back into the pool (both multiplexing and not)
@@ -400,11 +395,6 @@ void CloseConnector(NpgsqlConnector connector)
400395
UpdatePruningTimer();
401396
}
402397

403-
internal override bool TryRemovePendingEnlistedConnector(NpgsqlConnector connector, Transaction transaction)
404-
=> _parentPool is null
405-
? base.TryRemovePendingEnlistedConnector(connector, transaction)
406-
: _parentPool.TryRemovePendingEnlistedConnector(connector, transaction);
407-
408398
#region Pruning
409399

410400
void UpdatePruningTimer()

‎src/Npgsql/VolatileResourceManager.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ namespace Npgsql;
1717
sealed class VolatileResourceManager : ISinglePhaseNotification
1818
{
1919
NpgsqlConnector _connector;
20+
NpgsqlDataSource _dataSource;
2021
Transaction _transaction;
2122
readonly string _txId;
2223
NpgsqlTransaction _localTx = null!;
@@ -31,6 +32,7 @@ sealed class VolatileResourceManager : ISinglePhaseNotification
3132
internal VolatileResourceManager(NpgsqlConnection connection, Transaction transaction)
3233
{
3334
_connector = connection.Connector!;
35+
_dataSource = connection.NpgsqlDataSource;
3436
_transaction = transaction;
3537
// _tx gets disposed by System.Transactions at some point, but we want to be able to log its local ID
3638
_txId = transaction.TransactionInformation.LocalIdentifier;
@@ -277,8 +279,10 @@ void Dispose()
277279
{
278280
// We're here for connections which were closed before their TransactionScope completes.
279281
// These need to be closed now.
280-
// We should return the connector to the pool only if we've successfully removed it from the pending list
281-
if (_connector.TryRemovePendingEnlistedConnector(_transaction))
282+
// We should return the connector to the pool only if we've successfully removed it from the pending list.
283+
// Note that we remove it from the NpgsqlDataSource bound to connection and not to connector
284+
// because of NpgsqlMultiHostDataSource which has its own list to which connection adds connectors.
285+
if (_dataSource.TryRemovePendingEnlistedConnector(_connector, _transaction))
282286
_connector.Return();
283287
}
284288

‎test/Npgsql.Tests/SystemTransactionTests.cs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -310,13 +310,15 @@ public void Single_unpooled_connection()
310310
scope.Complete();
311311
}
312312

313-
[Test, IssueLink("https://github.com/npgsql/npgsql/issues/4963")]
314-
public void Single_unpooled_closed_connection()
313+
[Test]
314+
[IssueLink("https://github.com/npgsql/npgsql/issues/4963"), IssueLink("https://github.com/npgsql/npgsql/issues/5783")]
315+
public void Single_closed_connection_in_transaction_scope([Values] bool pooling, [Values] bool multipleHosts)
315316
{
316317
using var dataSource = CreateDataSource(csb =>
317318
{
318-
csb.Pooling = false;
319+
csb.Pooling = pooling;
319320
csb.Enlist = true;
321+
csb.Host = multipleHosts ? "localhost,127.0.0.1" : csb.Host;
320322
});
321323

322324
using (var scope = new TransactionScope())
@@ -325,11 +327,11 @@ public void Single_unpooled_closed_connection()
325327
{
326328
cmd.ExecuteNonQuery();
327329
conn.Close();
328-
Assert.That(dataSource.Statistics.Total, Is.EqualTo(1));
330+
Assert.That(pooling ? dataSource.Statistics.Busy : dataSource.Statistics.Total, Is.EqualTo(1));
329331
scope.Complete();
330332
}
331333

332-
Assert.That(dataSource.Statistics.Total, Is.EqualTo(0));
334+
Assert.That(pooling ? dataSource.Statistics.Busy : dataSource.Statistics.Total, Is.EqualTo(0));
333335
}
334336

335337
[Test]

0 commit comments

Comments
 (0)