Skip to content

Commit dcbb5c2

Browse files
NinoFlorisNinoFloris
authored
Move all reloadable state into one reference (#6303)
1 parent 53030fb commit dcbb5c2

File tree

9 files changed

+70
-63
lines changed

9 files changed

+70
-63
lines changed

‎src/Npgsql/Internal/NpgsqlConnector.cs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -117,12 +117,13 @@ internal string InferredUserName
117117
/// </summary>
118118
internal int Id => BackendProcessId;
119119

120-
internal PgSerializerOptions SerializerOptions { get; set; } = default!;
120+
internal NpgsqlDataSource.ReloadableState ReloadableState = null!;
121121

122122
/// <summary>
123123
/// Information about PostgreSQL and PostgreSQL-like databases (e.g. type definitions, capabilities...).
124124
/// </summary>
125-
public NpgsqlDatabaseInfo DatabaseInfo { get; internal set; } = default!;
125+
public NpgsqlDatabaseInfo DatabaseInfo => ReloadableState.DatabaseInfo;
126+
internal PgSerializerOptions SerializerOptions => ReloadableState.SerializerOptions;
126127

127128
/// <summary>
128129
/// The current transaction status for this connector.
@@ -507,10 +508,9 @@ internal async Task Open(NpgsqlTimeout timeout, bool async, CancellationToken ca
507508

508509
await DataSource.Bootstrap(this, timeout, forceReload: false, async, cancellationToken).ConfigureAwait(false);
509510

510-
Debug.Assert(DataSource.SerializerOptions is not null);
511-
Debug.Assert(DataSource.DatabaseInfo is not null);
512-
SerializerOptions = DataSource.SerializerOptions;
513-
DatabaseInfo = DataSource.DatabaseInfo;
511+
// The connector directly references the current reloadable state reference, to protect it against changes by a concurrent
512+
// ReloadTypes. We update them here before returning the connector from the pool.
513+
ReloadableState = DataSource.CurrentReloadableState;
514514

515515
if (Settings.Pooling && Settings is { Multiplexing: false, NoResetOnClose: false } && DatabaseInfo.SupportsDiscard)
516516
{

‎src/Npgsql/NpgsqlCommand.cs‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -671,7 +671,7 @@ Task Prepare(bool async, CancellationToken cancellationToken = default)
671671
{
672672
foreach (var batchCommand in InternalBatchCommands)
673673
{
674-
batchCommand._parameters?.ProcessParameters(connector.SerializerOptions, validateValues: false, batchCommand.CommandType);
674+
batchCommand._parameters?.ProcessParameters(connector.ReloadableState, validateValues: false, batchCommand.CommandType);
675675
ProcessRawQuery(connector.SqlQueryParser, connector.UseConformingStrings, batchCommand);
676676

677677
needToPrepare = batchCommand.ExplicitPrepare(connector) || needToPrepare;
@@ -689,7 +689,7 @@ IEnumerable<string> CommandTexts()
689689
}
690690
else
691691
{
692-
_parameters?.ProcessParameters(connector.SerializerOptions, validateValues: false, CommandType);
692+
_parameters?.ProcessParameters(connector.ReloadableState, validateValues: false, CommandType);
693693
ProcessRawQuery(connector.SqlQueryParser, connector.UseConformingStrings, batchCommand: null);
694694

695695
foreach (var batchCommand in InternalBatchCommands)
@@ -1410,6 +1410,7 @@ internal virtual async ValueTask<NpgsqlDataReader> ExecuteReader(bool async, Com
14101410
if (connector is not null)
14111411
{
14121412
var logger = connector.CommandLogger;
1413+
var reloadableState = connector.ReloadableState;
14131414

14141415
cancellationToken.ThrowIfCancellationRequested();
14151416
// We cannot pass a token here, as we'll cancel a non-send query
@@ -1440,7 +1441,7 @@ internal virtual async ValueTask<NpgsqlDataReader> ExecuteReader(bool async, Com
14401441
goto case false;
14411442
}
14421443

1443-
batchCommand._parameters?.ProcessParameters(connector.SerializerOptions, validateParameterValues, batchCommand.CommandType);
1444+
batchCommand._parameters?.ProcessParameters(reloadableState, validateParameterValues, batchCommand.CommandType);
14441445
}
14451446
}
14461447
else
@@ -1453,7 +1454,7 @@ internal virtual async ValueTask<NpgsqlDataReader> ExecuteReader(bool async, Com
14531454
ResetPreparation();
14541455
goto case false;
14551456
}
1456-
_parameters?.ProcessParameters(connector.SerializerOptions, validateParameterValues, CommandType);
1457+
_parameters?.ProcessParameters(reloadableState, validateParameterValues, CommandType);
14571458
}
14581459

14591460
NpgsqlEventSource.Log.CommandStartPrepared();
@@ -1469,7 +1470,7 @@ internal virtual async ValueTask<NpgsqlDataReader> ExecuteReader(bool async, Com
14691470
{
14701471
var batchCommand = InternalBatchCommands[i];
14711472

1472-
batchCommand._parameters?.ProcessParameters(connector.SerializerOptions, validateParameterValues, batchCommand.CommandType);
1473+
batchCommand._parameters?.ProcessParameters(reloadableState, validateParameterValues, batchCommand.CommandType);
14731474
ProcessRawQuery(connector.SqlQueryParser, connector.UseConformingStrings, batchCommand);
14741475

14751476
if (connector.Settings.MaxAutoPrepare > 0 && batchCommand.TryAutoPrepare(connector))
@@ -1481,7 +1482,7 @@ internal virtual async ValueTask<NpgsqlDataReader> ExecuteReader(bool async, Com
14811482
}
14821483
else
14831484
{
1484-
_parameters?.ProcessParameters(connector.SerializerOptions, validateParameterValues, CommandType);
1485+
_parameters?.ProcessParameters(reloadableState, validateParameterValues, CommandType);
14851486
ProcessRawQuery(connector.SqlQueryParser, connector.UseConformingStrings, batchCommand: null);
14861487

14871488
if (connector.Settings.MaxAutoPrepare > 0)
@@ -1565,6 +1566,7 @@ internal virtual async ValueTask<NpgsqlDataReader> ExecuteReader(bool async, Com
15651566

15661567
// The connection isn't bound to a connector - it's multiplexing time.
15671568
var dataSource = (MultiplexingDataSource)conn.NpgsqlDataSource;
1569+
var reloadableState = dataSource.CurrentReloadableState;
15681570

15691571
if (!async)
15701572
{
@@ -1577,13 +1579,13 @@ internal virtual async ValueTask<NpgsqlDataReader> ExecuteReader(bool async, Com
15771579
{
15781580
foreach (var batchCommand in InternalBatchCommands)
15791581
{
1580-
batchCommand._parameters?.ProcessParameters(dataSource.SerializerOptions, validateValues: true, batchCommand.CommandType);
1582+
batchCommand._parameters?.ProcessParameters(reloadableState, validateValues: true, batchCommand.CommandType);
15811583
ProcessRawQuery(null, standardConformingStrings: true, batchCommand);
15821584
}
15831585
}
15841586
else
15851587
{
1586-
_parameters?.ProcessParameters(dataSource.SerializerOptions, validateValues: true, CommandType);
1588+
_parameters?.ProcessParameters(reloadableState, validateValues: true, CommandType);
15871589
ProcessRawQuery(null, standardConformingStrings: true, batchCommand: null);
15881590
}
15891591

‎src/Npgsql/NpgsqlDataSource.cs‎

Lines changed: 28 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,19 @@ public abstract class NpgsqlDataSource : DbDataSource
3131
internal NpgsqlLoggingConfiguration LoggingConfiguration { get; }
3232

3333
readonly PgTypeInfoResolverChain _resolverChain;
34-
internal PgSerializerOptions SerializerOptions { get; private set; } = null!; // Initialized at bootstrapping
3534

36-
/// <summary>
37-
/// Information about PostgreSQL and PostgreSQL-like databases (e.g. type definitions, capabilities...).
38-
/// </summary>
39-
internal NpgsqlDatabaseInfo DatabaseInfo { get; private set; } = null!; // Initialized at bootstrapping
35+
internal ReloadableState CurrentReloadableState = null!; // Initialized during bootstrapping.
36+
37+
// Initialized at bootstrapping
38+
internal sealed class ReloadableState(NpgsqlDatabaseInfo databaseInfo, PgSerializerOptions serializerOptions)
39+
{
40+
/// <summary>
41+
/// Information about PostgreSQL and PostgreSQL-like databases (e.g. type definitions, capabilities...).
42+
/// </summary>
43+
public NpgsqlDatabaseInfo DatabaseInfo { get; } = databaseInfo;
44+
45+
public PgSerializerOptions SerializerOptions { get; } = serializerOptions;
46+
}
4047

4148
internal TransportSecurityHandler TransportSecurityHandler { get; }
4249

@@ -105,7 +112,7 @@ internal NpgsqlDataSource(
105112
_periodicPasswordProvider,
106113
_periodicPasswordSuccessRefreshInterval,
107114
_periodicPasswordFailureRefreshInterval,
108-
var resolverChain,
115+
_resolverChain,
109116
_defaultNameTranslator,
110117
ConnectionInitializer,
111118
ConnectionInitializerAsync,
@@ -115,7 +122,6 @@ internal NpgsqlDataSource(
115122

116123
Debug.Assert(_passwordProvider is null || _passwordProviderAsync is not null);
117124

118-
_resolverChain = resolverChain;
119125
_password = settings.Password;
120126

121127
if (_periodicPasswordSuccessRefreshInterval != default)
@@ -272,27 +278,30 @@ internal async Task Bootstrap(
272278

273279
// The type loading below will need to send queries to the database, and that depends on a type mapper being set up (even if its
274280
// empty). So we set up a minimal version here, and then later inject the actual DatabaseInfo.
275-
connector.SerializerOptions =
276-
new(PostgresMinimalDatabaseInfo.DefaultTypeCatalog)
281+
connector.ReloadableState = new(
282+
databaseInfo: PostgresMinimalDatabaseInfo.DefaultTypeCatalog,
283+
serializerOptions: new(PostgresMinimalDatabaseInfo.DefaultTypeCatalog)
277284
{
278285
TextEncoding = connector.TextEncoding,
279286
TypeInfoResolver = AdoTypeInfoResolverFactory.Instance.CreateResolver(),
280-
};
287+
});
281288

282289
NpgsqlDatabaseInfo databaseInfo;
283290

284291
using (connector.StartUserAction(ConnectorState.Executing, cancellationToken))
285292
databaseInfo = await NpgsqlDatabaseInfo.Load(connector, timeout, async).ConfigureAwait(false);
286293

287-
connector.DatabaseInfo = DatabaseInfo = databaseInfo;
288-
connector.SerializerOptions = SerializerOptions =
289-
new(databaseInfo, _resolverChain, CreateTimeZoneProvider(connector.Timezone))
290-
{
291-
ArrayNullabilityMode = Settings.ArrayNullabilityMode,
292-
EnableDateTimeInfinityConversions = !Statics.DisableDateTimeInfinityConversions,
293-
TextEncoding = connector.TextEncoding,
294-
DefaultNameTranslator = _defaultNameTranslator
295-
};
294+
var serializerOptions = new PgSerializerOptions(databaseInfo, _resolverChain, CreateTimeZoneProvider(connector.Timezone))
295+
{
296+
ArrayNullabilityMode = Settings.ArrayNullabilityMode,
297+
EnableDateTimeInfinityConversions = !Statics.DisableDateTimeInfinityConversions,
298+
TextEncoding = connector.TextEncoding,
299+
DefaultNameTranslator = _defaultNameTranslator
300+
};
301+
302+
connector.ReloadableState = CurrentReloadableState = new ReloadableState(
303+
databaseInfo: databaseInfo,
304+
serializerOptions: serializerOptions);
296305

297306
IsBootstrapped = true;
298307
}

‎src/Npgsql/NpgsqlParameterCollection.cs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
using System.Data.Common;
66
using System.Diagnostics;
77
using System.Diagnostics.CodeAnalysis;
8-
using Npgsql.Internal;
98
using NpgsqlTypes;
109

1110
namespace Npgsql;
@@ -668,7 +667,7 @@ internal void CloneTo(NpgsqlParameterCollection other)
668667
}
669668
}
670669

671-
internal void ProcessParameters(PgSerializerOptions options, bool validateValues, CommandType commandType)
670+
internal void ProcessParameters(NpgsqlDataSource.ReloadableState reloadableState, bool validateValues, CommandType commandType)
672671
{
673672
HasOutputParameters = false;
674673
PlaceholderType = PlaceholderType.NoParameters;
@@ -725,7 +724,7 @@ internal void ProcessParameters(PgSerializerOptions options, bool validateValues
725724
break;
726725
}
727726

728-
p.ResolveTypeInfo(options);
727+
p.ResolveTypeInfo(reloadableState.SerializerOptions);
729728

730729
if (validateValues)
731730
{

‎src/Npgsql/PoolingDataSource.cs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
using System.Threading;
66
using System.Threading.Channels;
77
using System.Threading.Tasks;
8-
using System.Transactions;
98
using Microsoft.Extensions.Logging;
109
using Npgsql.Internal;
1110
using Npgsql.Util;
@@ -239,12 +238,9 @@ bool CheckIdleConnector([NotNullWhen(true)] NpgsqlConnector? connector)
239238
return false;
240239
}
241240

242-
// The connector directly references the data source type mapper into the connector, to protect it against changes by a concurrent
241+
// The connector directly references the current reloadable state reference, to protect it against changes by a concurrent
243242
// ReloadTypes. We update them here before returning the connector from the pool.
244-
Debug.Assert(SerializerOptions is not null);
245-
Debug.Assert(DatabaseInfo is not null);
246-
connector.SerializerOptions = SerializerOptions;
247-
connector.DatabaseInfo = DatabaseInfo;
243+
connector.ReloadableState = CurrentReloadableState;
248244

249245
Debug.Assert(connector.State == ConnectorState.Ready,
250246
$"Got idle connector but {nameof(connector.State)} is {connector.State}");

‎test/Npgsql.Benchmarks/ResolveHandler.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public void Setup()
2222
if (NumPlugins > 1)
2323
dataSourceBuilder.UseNetTopologySuite();
2424
_dataSource = dataSourceBuilder.Build();
25-
_serializerOptions = _dataSource.SerializerOptions;
25+
_serializerOptions = _dataSource.CurrentReloadableState.SerializerOptions;
2626
}
2727

2828
[GlobalCleanup]

‎test/Npgsql.Tests/ConnectionTests.cs‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ public async Task Timezone_connection_param()
431431
public async Task Application_name_env_var()
432432
{
433433
const string testAppName = "MyTestApp";
434-
434+
435435
// Note that the pool is unaware of the environment variable, so if a connection is
436436
// returned from the pool it may contain the wrong application name
437437
using var _ = SetEnvironmentVariable("PGAPPNAME", testAppName);
@@ -444,7 +444,7 @@ public async Task Application_name_env_var()
444444
public async Task Application_name_connection_param()
445445
{
446446
const string testAppName = "MyTestApp2";
447-
447+
448448
await using var dataSource = CreateDataSource(csb => csb.ApplicationName = testAppName);
449449
await using var conn = await dataSource.OpenConnectionAsync();
450450
Assert.That(conn.PostgresParameters["application_name"], Is.EqualTo(testAppName));
@@ -456,7 +456,7 @@ public async Task Application_name_connection_param_overrides_env_var()
456456
{
457457
const string envAppName = "EnvApp";
458458
const string connAppName = "ConnApp";
459-
459+
460460
using var _ = SetEnvironmentVariable("PGAPPNAME", envAppName);
461461
await using var dataSource = CreateDataSource(csb => csb.ApplicationName = connAppName);
462462
await using var conn = await dataSource.OpenConnectionAsync();
@@ -774,25 +774,26 @@ public async Task Set_Schemas_And_Load_Relevant_Types(string testSchema, string
774774
});
775775
});
776776
using var conn = await dataSource.OpenConnectionAsync();
777+
var databaseInfo = dataSource.CurrentReloadableState.DatabaseInfo;
777778
if (enabled)
778779
{
779-
Assert.That(dataSource.DatabaseInfo.CompositeTypes.Any(x => x.Name == "test_type_1"));
780+
Assert.That(databaseInfo.CompositeTypes.Any(x => x.Name == "test_type_1"));
780781
if (testSchema == "public" || otherSchema == "public")
781782
{
782-
Assert.That(dataSource.DatabaseInfo.CompositeTypes.Any(x => x.Name == "test_type_2"));
783-
Assert.That(dataSource.DatabaseInfo.CompositeTypes.Any(x => x.Name == "test_type_3"));
783+
Assert.That(databaseInfo.CompositeTypes.Any(x => x.Name == "test_type_2"));
784+
Assert.That(databaseInfo.CompositeTypes.Any(x => x.Name == "test_type_3"));
784785
}
785786
else
786787
{
787-
Assert.That(dataSource.DatabaseInfo.CompositeTypes.Any(x => x.Name == "test_type_2"));
788-
Assert.That(dataSource.DatabaseInfo.CompositeTypes.Any(x => x.Name == "test_type_3"), Is.False);
788+
Assert.That(databaseInfo.CompositeTypes.Any(x => x.Name == "test_type_2"));
789+
Assert.That(databaseInfo.CompositeTypes.Any(x => x.Name == "test_type_3"), Is.False);
789790
}
790791
}
791792
else
792793
{
793-
Assert.That(dataSource.DatabaseInfo.CompositeTypes.Any(x => x.Name == "test_type_1"));
794-
Assert.That(dataSource.DatabaseInfo.CompositeTypes.Any(x => x.Name == "test_type_2"));
795-
Assert.That(dataSource.DatabaseInfo.CompositeTypes.Any(x => x.Name == "test_type_3"));
794+
Assert.That(databaseInfo.CompositeTypes.Any(x => x.Name == "test_type_1"));
795+
Assert.That(databaseInfo.CompositeTypes.Any(x => x.Name == "test_type_2"));
796+
Assert.That(databaseInfo.CompositeTypes.Any(x => x.Name == "test_type_3"));
796797
}
797798
}
798799
finally

0 commit comments

Comments
 (0)