Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Instrument with DiagnosticSource #1910

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 3 additions & 10 deletions src/Npgsql/Npgsql.csproj
@@ -1,5 +1,4 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<Description>Npgsql is the open source .NET data provider for PostgreSQL.</Description>
<Authors>Shay Rojansky;Emil Lenngren;Francisco Figueiredo Jr.;Kenji Uno</Authors>
Expand All @@ -24,39 +23,33 @@
<!-- This is somehow important for Microsoft.CodeQuality.Analyzers -->
<Features>IOperation</Features>
</PropertyGroup>

<ItemGroup>
<EmbeddedResource Include="NpgsqlMetaData.xml" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="System.Diagnostics.DiagnosticSource" Version="4.4.1" />
<PackageReference Include="System.Runtime.CompilerServices.Unsafe" Version="4.4.0" />
<PackageReference Include="System.ValueTuple" Version="4.4.0" />
<PackageReference Include="System.Threading.Tasks.Extensions" Version="4.4.0" />
<!-- Causes issues in Appveyor and Travis
<!-- Causes issues in Appveyor and Travis
<PackageReference Include="Microsoft.CodeQuality.Analyzers" Version="2.6.0-beta2" PrivateAssets="All" />
-->
</ItemGroup>

<ItemGroup Condition=" '$(TargetFramework)' == 'net45' ">
<Reference Include="System.Transactions" Pack="false" />
<Reference Include="System.DirectoryServices" Pack="false" />
</ItemGroup>

<ItemGroup Condition=" '$(TargetFramework)' == 'net451' ">
<Reference Include="System.Transactions" Pack="false" />
<Reference Include="System.DirectoryServices" Pack="false" />
</ItemGroup>

<PropertyGroup Condition=" '$(Configuration)' == 'ReleaseOptimizedCryptography' ">
<DefineConstants>$(DefineConstants);RELEASE;TRACE;OPTIMIZED_CRYPTOGRAPHY</DefineConstants>
<Optimize>true</Optimize>
</PropertyGroup>

<PropertyGroup Condition=" '$(Configuration)' == 'DebugOptimizedCryptography' ">
<DefineConstants>$(DefineConstants);DEBUG;TRACE;OPTIMIZED_CRYPTOGRAPHY</DefineConstants>
<DebugSymbols>true</DebugSymbols>
<Optimize>false</Optimize>
</PropertyGroup>

</Project>
</Project>
20 changes: 16 additions & 4 deletions src/Npgsql/NpgsqlCommand.cs
Expand Up @@ -84,6 +84,8 @@ public sealed class NpgsqlCommand : DbCommand, ICloneable
static readonly SingleThreadSynchronizationContext SingleThreadSynchronizationContext = new SingleThreadSynchronizationContext("NpgsqlRemainingAsyncSendWorker");

static readonly NpgsqlLogger Log = NpgsqlLogManager.GetCurrentClassLogger();

static readonly DiagnosticListener NpgsqlDiagnosticListener = new DiagnosticListener(NpgsqlDiagnosticListenerExtensions.CommandDiagnosticListenerName);

#endregion Fields

Expand Down Expand Up @@ -1043,7 +1045,7 @@ async Task<int> ExecuteNonQuery(bool async, CancellationToken cancellationToken)
{
using (var reader = await ExecuteDbDataReader(CommandBehavior.Default, async, cancellationToken))
{
while (async ? await reader.NextResultAsync(cancellationToken) : reader.NextResult()) {}
while (async ? await reader.NextResultAsync(cancellationToken) : reader.NextResult()){ }
reader.Close();
return reader.RecordsAffected;
}
Expand Down Expand Up @@ -1083,6 +1085,7 @@ async ValueTask<object> ExecuteScalar(bool async, CancellationToken cancellation
var behavior = CommandBehavior.SingleRow;
if (!Parameters.HasOutputParameters)
behavior |= CommandBehavior.SequentialAccess;

using (var reader = await ExecuteDbDataReader(behavior, async, cancellationToken))
return reader.Read() && reader.FieldCount != 0 ? reader.GetValue(0) : null;
}
Expand All @@ -1099,7 +1102,10 @@ async ValueTask<object> ExecuteScalar(bool async, CancellationToken cancellation
/// DataReader.
/// </remarks>
/// <returns>A DbDataReader object.</returns>
public new NpgsqlDataReader ExecuteReader() => (NpgsqlDataReader) base.ExecuteReader();
public new NpgsqlDataReader ExecuteReader()
{
return (NpgsqlDataReader)base.ExecuteReader();
}

/// <summary>
/// Executes the CommandText against the Connection, and returns an DbDataReader using one
Expand All @@ -1110,7 +1116,10 @@ async ValueTask<object> ExecuteScalar(bool async, CancellationToken cancellation
/// DataReader.
/// </remarks>
/// <returns>A DbDataReader object.</returns>
public new NpgsqlDataReader ExecuteReader(CommandBehavior behavior) => (NpgsqlDataReader) base.ExecuteReader(behavior);
public new NpgsqlDataReader ExecuteReader(CommandBehavior behavior)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, you're repeating the same piece of code over and over, it really should be written only once in ExecuteDbDataReader().

{
return (NpgsqlDataReader)base.ExecuteReader(behavior);
}

/// <summary>
/// Executes the command text against the connection.
Expand All @@ -1137,6 +1146,7 @@ async ValueTask<DbDataReader> ExecuteDbDataReader(CommandBehavior behavior, bool
connector.StartUserAction(this);
try
{
NpgsqlDiagnosticListener.ExecuteCommandStart(this);
using (cancellationToken.Register(cmd => ((NpgsqlCommand)cmd).Cancel(), this))
{
ValidateParameters();
Expand Down Expand Up @@ -1219,11 +1229,13 @@ async ValueTask<DbDataReader> ExecuteDbDataReader(CommandBehavior behavior, bool
await reader.NextResultAsync(cancellationToken);
else
reader.NextResult();
NpgsqlDiagnosticListener.ExecuteCommandStop(this);
return reader;
}
}
catch
catch(Exception exception)
{
NpgsqlDiagnosticListener.ExecuteCommandError(this, exception);
State = CommandState.Idle;
Connection.Connector?.EndUserAction();

Expand Down
121 changes: 73 additions & 48 deletions src/Npgsql/NpgsqlConnection.cs
Expand Up @@ -83,6 +83,8 @@ public sealed class NpgsqlConnection : DbConnection, ICloneable

static readonly NpgsqlConnectionStringBuilder DefaultSettings = new NpgsqlConnectionStringBuilder();

static readonly DiagnosticListener NpgsqlDiagnosticListener = new DiagnosticListener(NpgsqlDiagnosticListenerExtensions.ConnectionDiagnosticListenerName);

[CanBeNull]
ConnectorPool _pool;

Expand Down Expand Up @@ -222,29 +224,39 @@ Task Open(bool async, CancellationToken cancellationToken)
{
// This is an optimized path for when a connection can be taken from the pool
// with no waiting or I/O
try
{
NpgsqlDiagnosticListener.OpenConnectionStart(this);

CheckConnectionClosed();

CheckConnectionClosed();

Log.Trace("Opening connection...");

if (_pool == null || Settings.Enlist || !_pool.TryAllocateFast(this, out Connector))
return OpenLong();
Log.Trace("Opening connection...");

_userFacingConnectionString = _pool.UserFacingConnectionString;
if (_pool == null || Settings.Enlist || !_pool.TryAllocateFast(this, out Connector))
return OpenLong();

Counters.SoftConnectsPerSecond.Increment();
_userFacingConnectionString = _pool.UserFacingConnectionString;

// Since this pooled connector was opened, global mappings may have
// changed. Bring this up to date if needed.
var mapper = Connector.TypeMapper;
if (mapper.ChangeCounter != TypeMapping.GlobalTypeMapper.Instance.ChangeCounter)
mapper.Reset();
Counters.SoftConnectsPerSecond.Increment();

Debug.Assert(Connector.Connection != null, "Open done but connector not set on Connection");
Log.Debug("Connection opened", Connector.Id);
OnStateChange(new StateChangeEventArgs(ConnectionState.Closed, ConnectionState.Open));
return PGUtil.CompletedTask;
// Since this pooled connector was opened, global mappings may have
// changed. Bring this up to date if needed.
var mapper = Connector.TypeMapper;
if (mapper.ChangeCounter != TypeMapping.GlobalTypeMapper.Instance.ChangeCounter)
mapper.Reset();

Debug.Assert(Connector.Connection != null, "Open done but connector not set on Connection");
Log.Debug("Connection opened", Connector.Id);
OnStateChange(new StateChangeEventArgs(ConnectionState.Closed, ConnectionState.Open));
NpgsqlDiagnosticListener.OpenConnectionStop(this);
return PGUtil.CompletedTask;
}
catch (Exception ex)
{
NpgsqlDiagnosticListener.WriteConnectionOpenError(this, ex);
throw;
}

async Task OpenLong()
{
CheckConnectionClosed();
Expand Down Expand Up @@ -298,12 +310,12 @@ async Task OpenLong()
Connector = await _pool.AllocateLong(this, timeout, async, cancellationToken);
}
}
else // No enlist
else // No enlist
Connector = await _pool.AllocateLong(this, timeout, async, cancellationToken);

// Since this pooled connector was opened, global mappings may have
// changed. Bring this up to date if needed.
mapper = Connector.TypeMapper;
var mapper = Connector.TypeMapper;
if (mapper.ChangeCounter != TypeMapping.GlobalTypeMapper.Instance.ChangeCounter)
mapper.Reset();
}
Expand All @@ -317,6 +329,7 @@ async Task OpenLong()
Connector = null;
throw;
}

Debug.Assert(Connector.Connection != null, "Open done but connector not set on Connection");
Log.Debug("Connection opened", Connector.Id);
OnStateChange(ClosedToOpenEventArgs);
Expand Down Expand Up @@ -606,44 +619,56 @@ public override void EnlistTransaction(Transaction transaction)

internal void Close(bool wasBroken)
{
if (Connector == null)
return;
var connectorId = Connector.Id;
Log.Trace("Closing connection...", connectorId);
_wasBroken = wasBroken;

try
{
if (Connector == null)
return;
NpgsqlDiagnosticListener.CloseConnectionStart(this);
var connectorId = Connector.Id;
Log.Trace("Closing connection...", connectorId);
_wasBroken = wasBroken;

Connector.CloseOngoingOperations();
Connector.CloseOngoingOperations();

if (Settings.Pooling)
{
if (EnlistedTransaction == null)
_pool.Release(Connector);
else
if (Settings.Pooling)
{
if (EnlistedTransaction == null)
_pool.Release(Connector);
else
{
// A System.Transactions transaction is still in progress, we need to wait for it to complete.
// Close the connection and disconnect it from the resource manager but leave the connector
// in a enlisted pending list in the pool.
_pool.AddPendingEnlistedConnector(Connector, EnlistedTransaction);
Connector.Connection = null;
EnlistedTransaction = null;
}
}
else // Non-pooled connection
{
// A System.Transactions transaction is still in progress, we need to wait for it to complete.
// Close the connection and disconnect it from the resource manager but leave the connector
// in a enlisted pending list in the pool.
_pool.AddPendingEnlistedConnector(Connector, EnlistedTransaction);
if (EnlistedTransaction == null)
Connector.Close();
// If a non-pooled connection is being closed but is enlisted in an ongoing
// TransactionScope, simply detach the connector from the connection and leave
// it open. It will be closed when the TransactionScope is disposed.
Connector.Connection = null;
EnlistedTransaction = null;
}
}
else // Non-pooled connection
{
if (EnlistedTransaction == null)
Connector.Close();
// If a non-pooled connection is being closed but is enlisted in an ongoing
// TransactionScope, simply detach the connector from the connection and leave
// it open. It will be closed when the TransactionScope is disposed.
Connector.Connection = null;
EnlistedTransaction = null;
}

Log.Debug("Connection closed", connectorId);
Log.Debug("Connection closed", connectorId);

Connector = null;
Connector = null;

OnStateChange(OpenToClosedEventArgs);
OnStateChange(OpenToClosedEventArgs);

NpgsqlDiagnosticListener.CloseConnectionStop(this);
}
catch (Exception exception)
{
NpgsqlDiagnosticListener.CloseConnectionError(this, exception);
throw;
}
}

/// <summary>
Expand Down