Skip to content

Commit 2563b95

Browse files
ckadlubackadluba
committed
Refactoring and performance optimizations
Refactoring * Removed DataTableCreator from class MsSqlServerSink. Pushed it down to SqlBulkBatchWriter. This also made it possible to remove reference to System.Data in MsSqlServerSink. * Removed DataTableCreator from SinkDependencies. * Removed IBulkBatchWriter interface from SqlInsertStatementWriter. MSSqlServerSink class now has separate instances of ISqlBulkBatchWriter and ISqlLogEventWriter and chooses which one to use based on sink option `UseSqlBulkCopy`. * Added new class SqlCommandFactory with interface ISqlCommandFactory Performance optimizations in batched sink * Generate schema and table string only once and do not use string.Format(). * Do not cast on each column of each log event. Performance optimizations in audit sink * Render INSERT string only once and not for each log event since it will not change between log events. * Do not create a separate SqlCommand for each log event. Reuse the same and set only new SqlConnection, CommandText and parameters for each log event. Added new tests and adapted existing ones.
1 parent e9869c6 commit 2563b95

28 files changed

+494
-323
lines changed

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Dependencies/SinkDependencies.cs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ namespace Serilog.Sinks.MSSqlServer.Dependencies
44
{
55
internal class SinkDependencies
66
{
7-
public IDataTableCreator DataTableCreator { get; set; }
87
public ISqlCommandExecutor SqlDatabaseCreator { get; set; }
98
public ISqlCommandExecutor SqlTableCreator { get; set; }
109
public ISqlBulkBatchWriter SqlBulkBatchWriter { get; set; }

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Dependencies/SinkDependenciesFactory.cs

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ internal static SinkDependencies Create(
2323
var sqlConnectionStringBuilderWrapper = new SqlConnectionStringBuilderWrapper(
2424
connectionString, sinkOptions.EnlistInTransaction);
2525
var sqlConnectionFactory = new SqlConnectionFactory(sqlConnectionStringBuilderWrapper);
26+
var sqlCommandFactory = new SqlCommandFactory();
2627
var dataTableCreator = new DataTableCreator(sinkOptions.TableName, columnOptions);
2728
var sqlCreateTableWriter = new SqlCreateTableWriter(sinkOptions.SchemaName,
2829
sinkOptions.TableName, columnOptions, dataTableCreator);
@@ -47,21 +48,16 @@ internal static SinkDependencies Create(
4748

4849
var sinkDependencies = new SinkDependencies
4950
{
50-
DataTableCreator = dataTableCreator,
5151
SqlDatabaseCreator = new SqlDatabaseCreator(
52-
sqlCreateDatabaseWriter, sqlConnectionFactoryNoDb),
52+
sqlCreateDatabaseWriter, sqlConnectionFactoryNoDb, sqlCommandFactory),
5353
SqlTableCreator = new SqlTableCreator(
54-
sqlCreateTableWriter, sqlConnectionFactory),
55-
SqlBulkBatchWriter = sinkOptions.UseSqlBulkCopy
56-
? (ISqlBulkBatchWriter)new SqlBulkBatchWriter(
57-
sinkOptions.TableName, sinkOptions.SchemaName, columnOptions.DisableTriggers,
58-
sqlConnectionFactory, logEventDataGenerator)
59-
: (ISqlBulkBatchWriter)new SqlInsertStatementWriter(
60-
sinkOptions.TableName, sinkOptions.SchemaName,
61-
sqlConnectionFactory, logEventDataGenerator),
54+
sqlCreateTableWriter, sqlConnectionFactory, sqlCommandFactory),
55+
SqlBulkBatchWriter = new SqlBulkBatchWriter(
56+
sinkOptions.TableName, sinkOptions.SchemaName, columnOptions.DisableTriggers,
57+
dataTableCreator, sqlConnectionFactory, logEventDataGenerator),
6258
SqlLogEventWriter = new SqlInsertStatementWriter(
6359
sinkOptions.TableName, sinkOptions.SchemaName,
64-
sqlConnectionFactory, logEventDataGenerator)
60+
sqlConnectionFactory, sqlCommandFactory, logEventDataGenerator)
6561
};
6662

6763
return sinkDependencies;

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/MSSqlServerAuditSink.cs

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
// Copyright 2024 Serilog Contributors
2-
//
1+
// Copyright 2024 Serilog Contributors
2+
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
55
// You may obtain a copy of the License at
6-
//
6+
//
77
// http://www.apache.org/licenses/LICENSE-2.0
8-
//
8+
//
99
// Unless required by applicable law or agreed to in writing, software
1010
// distributed under the License is distributed on an "AS IS" BASIS,
1111
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -30,11 +30,13 @@ public class MSSqlServerAuditSink : ILogEventSink, IDisposable
3030
{
3131
private readonly ISqlLogEventWriter _sqlLogEventWriter;
3232

33+
private bool _disposedValue;
34+
3335
/// <summary>
3436
/// Construct a sink posting to the specified database.
3537
///
3638
/// Note: this is the legacy version of the extension method. Please use the new one using MSSqlServerSinkOptions instead.
37-
///
39+
///
3840
/// </summary>
3941
/// <param name="connectionString">Connection string to access the database.</param>
4042
/// <param name="tableName">Name of the table to store the data in.</param>
@@ -113,7 +115,15 @@ public void Dispose()
113115
/// <param name="disposing">True to release both managed and unmanaged resources; false to release only unmanaged resources.</param>
114116
protected virtual void Dispose(bool disposing)
115117
{
116-
// This class needn't to dispose anything. This is just here for sink interface compatibility.
118+
if (!_disposedValue)
119+
{
120+
if (disposing)
121+
{
122+
_sqlLogEventWriter.Dispose();
123+
}
124+
125+
_disposedValue = true;
126+
}
117127
}
118128

119129
private static void ValidateParameters(MSSqlServerSinkOptions sinkOptions, ColumnOptions columnOptions)
@@ -134,11 +144,6 @@ private static void CheckSinkDependencies(SinkDependencies sinkDependencies)
134144
throw new ArgumentNullException(nameof(sinkDependencies));
135145
}
136146

137-
if (sinkDependencies.DataTableCreator == null)
138-
{
139-
throw new InvalidOperationException("DataTableCreator is not initialized!");
140-
}
141-
142147
if (sinkDependencies.SqlTableCreator == null)
143148
{
144149
throw new InvalidOperationException("SqlTableCreator is not initialized!");

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/MSSqlServerSink.cs

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
// Copyright 2024 Serilog Contributors
2-
//
1+
// Copyright 2024 Serilog Contributors
2+
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
55
// You may obtain a copy of the License at
6-
//
6+
//
77
// http://www.apache.org/licenses/LICENSE-2.0
8-
//
8+
//
99
// Unless required by applicable law or agreed to in writing, software
1010
// distributed under the License is distributed on an "AS IS" BASIS,
1111
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
@@ -14,7 +14,6 @@
1414

1515
using System;
1616
using System.Collections.Generic;
17-
using System.Data;
1817
using System.Threading.Tasks;
1918
using Serilog.Events;
2019
using Serilog.Formatting;
@@ -29,8 +28,9 @@ namespace Serilog.Sinks.MSSqlServer
2928
/// </summary>
3029
public class MSSqlServerSink : IBatchedLogEventSink, IDisposable
3130
{
31+
private readonly MSSqlServerSinkOptions _sinkOptions;
3232
private readonly ISqlBulkBatchWriter _sqlBulkBatchWriter;
33-
private readonly DataTable _eventTable;
33+
private readonly ISqlLogEventWriter _sqlLogEventWriter; // Used if sink option UseSqlBulkCopy is set to false
3434

3535
/// <summary>
3636
/// The default database schema name.
@@ -53,7 +53,7 @@ public class MSSqlServerSink : IBatchedLogEventSink, IDisposable
5353
/// Construct a sink posting to the specified database.
5454
///
5555
/// Note: this is the legacy version of the extension method. Please use the new one using MSSqlServerSinkOptions instead.
56-
///
56+
///
5757
/// </summary>
5858
/// <param name="connectionString">Connection string to access the database.</param>
5959
/// <param name="tableName">Name of the table to store the data in.</param>
@@ -108,8 +108,9 @@ internal MSSqlServerSink(
108108
ValidateParameters(sinkOptions);
109109
CheckSinkDependencies(sinkDependencies);
110110

111+
_sinkOptions = sinkOptions;
111112
_sqlBulkBatchWriter = sinkDependencies.SqlBulkBatchWriter;
112-
_eventTable = sinkDependencies.DataTableCreator.CreateDataTable();
113+
_sqlLogEventWriter = sinkDependencies.SqlLogEventWriter;
113114

114115
CreateDatabaseAndTable(sinkOptions, sinkDependencies);
115116
}
@@ -119,7 +120,9 @@ internal MSSqlServerSink(
119120
/// </summary>
120121
/// <param name="batch">The events to emit.</param>
121122
public Task EmitBatchAsync(IReadOnlyCollection<LogEvent> batch) =>
122-
_sqlBulkBatchWriter.WriteBatch(batch, _eventTable);
123+
_sinkOptions.UseSqlBulkCopy
124+
? _sqlBulkBatchWriter.WriteBatch(batch)
125+
: _sqlLogEventWriter.WriteEvents(batch);
123126

124127
/// <summary>
125128
/// Called upon batchperiod if no data is in batch. Not used by this sink.
@@ -148,7 +151,7 @@ protected virtual void Dispose(bool disposing)
148151
{
149152
if (disposing)
150153
{
151-
_eventTable.Dispose();
154+
_sqlLogEventWriter.Dispose();
152155
}
153156

154157
_disposedValue = true;
@@ -170,11 +173,6 @@ private static void CheckSinkDependencies(SinkDependencies sinkDependencies)
170173
throw new ArgumentNullException(nameof(sinkDependencies));
171174
}
172175

173-
if (sinkDependencies.DataTableCreator == null)
174-
{
175-
throw new InvalidOperationException("DataTableCreator is not initialized!");
176-
}
177-
178176
if (sinkDependencies.SqlTableCreator == null)
179177
{
180178
throw new InvalidOperationException("SqlTableCreator is not initialized!");
@@ -184,6 +182,11 @@ private static void CheckSinkDependencies(SinkDependencies sinkDependencies)
184182
{
185183
throw new InvalidOperationException("SqlBulkBatchWriter is not initialized!");
186184
}
185+
186+
if (sinkDependencies.SqlLogEventWriter == null)
187+
{
188+
throw new InvalidOperationException("SqlLogEventWriter is not initialized!");
189+
}
187190
}
188191

189192
private void CreateDatabaseAndTable(MSSqlServerSinkOptions sinkOptions, SinkDependencies sinkDependencies)

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/ISqlBulkBatchWriter.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
using System.Collections.Generic;
2-
using System.Data;
1+
using System;
2+
using System.Collections.Generic;
33
using System.Threading.Tasks;
44
using Serilog.Events;
55

66
namespace Serilog.Sinks.MSSqlServer.Platform
77
{
8-
internal interface ISqlBulkBatchWriter
8+
internal interface ISqlBulkBatchWriter : IDisposable
99
{
10-
Task WriteBatch(IEnumerable<LogEvent> events, DataTable dataTable);
10+
Task WriteBatch(IEnumerable<LogEvent> events);
1111
}
1212
}

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/ISqlCommandFactory.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
using Serilog.Sinks.MSSqlServer.Platform.SqlClient;
2+
3+
namespace Serilog.Sinks.MSSqlServer.Platform
4+
{
5+
internal interface ISqlCommandFactory
6+
{
7+
ISqlCommandWrapper CreateCommand(ISqlConnectionWrapper sqlConnection);
8+
ISqlCommandWrapper CreateCommand(string cmdText, ISqlConnectionWrapper sqlConnection);
9+
}
10+
}

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/ISqlLogEventWriter.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
1-
using Serilog.Events;
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Threading.Tasks;
4+
using Serilog.Events;
25

36
namespace Serilog.Sinks.MSSqlServer.Platform
47
{
5-
internal interface ISqlLogEventWriter
8+
internal interface ISqlLogEventWriter : IDisposable
69
{
710
void WriteEvent(LogEvent logEvent);
11+
12+
Task WriteEvents(IEnumerable<LogEvent> events);
813
}
914
}

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlBulkBatchWriter.cs

Lines changed: 54 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Data;
4-
using System.Globalization;
54
using System.Linq;
65
using System.Threading.Tasks;
76
using Serilog.Debugging;
@@ -12,45 +11,52 @@ namespace Serilog.Sinks.MSSqlServer.Platform
1211
{
1312
internal class SqlBulkBatchWriter : ISqlBulkBatchWriter
1413
{
15-
private readonly string _tableName;
16-
private readonly string _schemaName;
1714
private readonly bool _disableTriggers;
15+
private readonly DataTable _dataTable;
1816
private readonly ISqlConnectionFactory _sqlConnectionFactory;
1917
private readonly ILogEventDataGenerator _logEventDataGenerator;
18+
private readonly string _schemaAndTableName;
19+
20+
private bool _disposedValue;
2021

2122
public SqlBulkBatchWriter(
2223
string tableName,
2324
string schemaName,
2425
bool disableTriggers,
26+
IDataTableCreator dataTableCreator,
2527
ISqlConnectionFactory sqlConnectionFactory,
2628
ILogEventDataGenerator logEventDataGenerator)
2729
{
28-
_tableName = tableName ?? throw new ArgumentNullException(nameof(tableName));
29-
_schemaName = schemaName ?? throw new ArgumentNullException(nameof(schemaName));
30+
if (tableName == null) throw new ArgumentNullException(nameof(tableName));
31+
if (schemaName == null) throw new ArgumentNullException(nameof(schemaName));
3032
_disableTriggers = disableTriggers;
3133
_sqlConnectionFactory = sqlConnectionFactory ?? throw new ArgumentNullException(nameof(sqlConnectionFactory));
3234
_logEventDataGenerator = logEventDataGenerator ?? throw new ArgumentNullException(nameof(logEventDataGenerator));
35+
_schemaAndTableName = "[" + schemaName + "].[" + tableName + "]";
36+
37+
_dataTable = dataTableCreator == null
38+
? throw new ArgumentNullException(nameof(dataTableCreator))
39+
: dataTableCreator.CreateDataTable();
3340
}
3441

35-
public async Task WriteBatch(IEnumerable<LogEvent> events, DataTable dataTable)
42+
public async Task WriteBatch(IEnumerable<LogEvent> events)
3643
{
3744
try
3845
{
39-
FillDataTable(events, dataTable);
46+
FillDataTable(events);
4047

4148
using (var cn = _sqlConnectionFactory.Create())
4249
{
4350
await cn.OpenAsync().ConfigureAwait(false);
44-
using (var copy = cn.CreateSqlBulkCopy(_disableTriggers,
45-
string.Format(CultureInfo.InvariantCulture, "[{0}].[{1}]", _schemaName, _tableName)))
51+
using (var copy = cn.CreateSqlBulkCopy(_disableTriggers, _schemaAndTableName))
4652
{
47-
foreach (var column in dataTable.Columns)
53+
for (var i = 0; i < _dataTable.Columns.Count; i++)
4854
{
49-
var columnName = ((DataColumn)column).ColumnName;
55+
var columnName = _dataTable.Columns[i].ColumnName;
5056
copy.AddSqlBulkCopyColumnMapping(columnName, columnName);
5157
}
5258

53-
await copy.WriteToServerAsync(dataTable).ConfigureAwait(false);
59+
await copy.WriteToServerAsync(_dataTable).ConfigureAwait(false);
5460
}
5561
}
5662
}
@@ -62,26 +68,56 @@ public async Task WriteBatch(IEnumerable<LogEvent> events, DataTable dataTable)
6268
}
6369
finally
6470
{
65-
dataTable.Clear();
71+
_dataTable.Clear();
6672
}
6773
}
6874

69-
private void FillDataTable(IEnumerable<LogEvent> events, DataTable dataTable)
75+
private void FillDataTable(IEnumerable<LogEvent> events)
7076
{
71-
// Add the new rows to the collection.
77+
// Add the new rows to the collection.
78+
_dataTable.BeginLoadData();
7279
foreach (var logEvent in events)
7380
{
74-
var row = dataTable.NewRow();
81+
var row = _dataTable.NewRow();
7582

7683
foreach (var field in _logEventDataGenerator.GetColumnsAndValues(logEvent))
7784
{
7885
row[field.Key] = field.Value;
7986
}
8087

81-
dataTable.Rows.Add(row);
88+
_dataTable.Rows.Add(row);
8289
}
8390

84-
dataTable.AcceptChanges();
91+
_dataTable.EndLoadData();
92+
_dataTable.AcceptChanges();
93+
}
94+
95+
96+
/// <summary>
97+
/// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources.
98+
/// </summary>
99+
public void Dispose()
100+
{
101+
Dispose(disposing: true);
102+
GC.SuppressFinalize(this);
103+
}
104+
105+
/// <summary>
106+
/// Releases the unmanaged resources used by the Serilog.Sinks.MSSqlServer.Platform.SqlBulkBatchWriter and optionally
107+
/// releases the managed resources.
108+
/// </summary>
109+
/// <param name="disposing">True to release both managed and unmanaged resources; false to release only unmanaged resources.</param>
110+
protected virtual void Dispose(bool disposing)
111+
{
112+
if (!_disposedValue)
113+
{
114+
if (disposing)
115+
{
116+
_dataTable.Dispose();
117+
}
118+
119+
_disposedValue = true;
120+
}
85121
}
86122
}
87123
}

‎src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/Platform/SqlClient/ISqlCommandWrapper.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Data;
33
using System.Threading.Tasks;
4+
using Microsoft.Data.SqlClient;
45

56
namespace Serilog.Sinks.MSSqlServer.Platform.SqlClient
67
{
@@ -9,6 +10,8 @@ internal interface ISqlCommandWrapper : IDisposable
910
CommandType CommandType { get; set; }
1011
string CommandText { get; set; }
1112

13+
void SetConnection(ISqlConnectionWrapper sqlConnectionWrapper);
14+
void ClearParameters();
1215
void AddParameter(string parameterName, object value);
1316
int ExecuteNonQuery();
1417
Task<int> ExecuteNonQueryAsync();

0 commit comments

Comments
 (0)