Skip to content

Commit 71d5e2e

Browse files
NH-4013 - SQL-Server batcher failure after CloseCommands. (#628)
NH-4013 - Test case and fix for SQL-Server batcher failure after CloseCommands.
1 parent 2adba0b commit 71d5e2e

File tree

6 files changed

+164
-41
lines changed

6 files changed

+164
-41
lines changed
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
using System;
2+
using System.Collections;
3+
using System.Collections.Generic;
4+
using NHibernate.Cfg;
5+
using NHibernate.Util;
6+
using NUnit.Framework;
7+
using Environment = NHibernate.Cfg.Environment;
8+
9+
namespace NHibernate.Test.ConnectionTest
10+
{
11+
[TestFixture]
12+
public class BatcherFixture : ConnectionManagementTestCase
13+
{
14+
protected override void Configure(Configuration config)
15+
{
16+
base.Configure(config);
17+
config.SetProperty(Environment.BatchSize, "10");
18+
}
19+
20+
protected override ISession GetSessionUnderTest()
21+
=> OpenSession();
22+
23+
protected override void OnTearDown()
24+
{
25+
using (var s = OpenSession())
26+
{
27+
s.CreateQuery("delete from System.Object").ExecuteUpdate();
28+
}
29+
}
30+
31+
[Test]
32+
public void CanCloseCommandsAndUseBatcher()
33+
{
34+
using (var s = OpenSession())
35+
{
36+
// Need a generator strategy not causing insert at save.
37+
var silly = new YetAnother { Name = "Silly" };
38+
s.Save(silly);
39+
s.GetSessionImplementation().ConnectionManager.Batcher.CloseCommands();
40+
41+
Assert.DoesNotThrow(s.Flush, "Flush failure after closing commands.");
42+
}
43+
}
44+
}
45+
}

src/NHibernate.Test/ConnectionTest/Silly.hbm.xml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,11 @@
1616
</id>
1717
<property name="Name"/>
1818
</class>
19+
20+
<class name="YetAnother">
21+
<id name="Id" type="long">
22+
<generator class="hilo"/>
23+
</id>
24+
<property name="Name"/>
25+
</class>
1926
</hibernate-mapping>
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
using System;
2+
3+
namespace NHibernate.Test.ConnectionTest
4+
{
5+
[Serializable]
6+
public class YetAnother
7+
{
8+
public virtual long Id { get; set; }
9+
10+
public virtual string Name { get; set; }
11+
}
12+
}

src/NHibernate.Test/NHibernate.Test.csproj

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,8 +205,10 @@
205205
<Compile Include="CompositeId\Order.cs" />
206206
<Compile Include="CompositeId\Product.cs" />
207207
<Compile Include="ConnectionStringTest\NamedConnectionStringFixture.cs" />
208+
<Compile Include="ConnectionTest\BatcherFixture.cs" />
208209
<Compile Include="ConnectionTest\AggressiveReleaseTest.cs" />
209210
<Compile Include="ConnectionTest\ConnectionManagementTestCase.cs" />
211+
<Compile Include="ConnectionTest\YetAnother.cs" />
210212
<Compile Include="ConnectionTest\Other.cs" />
211213
<Compile Include="ConnectionTest\Silly.cs" />
212214
<Compile Include="ConnectionTest\MapBasedSessionContextFixture.cs" />

src/NHibernate/AdoNet/MySqlClientBatchingBatcher.cs

Lines changed: 48 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -69,34 +69,48 @@ public override void AddToBatch(IExpectation expectation)
6969

7070
protected override void DoExecuteBatch(DbCommand ps)
7171
{
72-
Log.DebugFormat("Executing batch");
73-
CheckReaders();
74-
if (Factory.Settings.SqlStatementLogger.IsDebugEnabled)
75-
{
76-
Factory.Settings.SqlStatementLogger.LogBatchCommand(currentBatchCommandsLog.ToString());
77-
currentBatchCommandsLog = new StringBuilder().AppendLine("Batch commands:");
78-
}
79-
80-
int rowsAffected;
8172
try
8273
{
83-
rowsAffected = currentBatch.ExecuteNonQuery();
74+
Log.DebugFormat("Executing batch");
75+
CheckReaders();
76+
if (Factory.Settings.SqlStatementLogger.IsDebugEnabled)
77+
{
78+
Factory.Settings.SqlStatementLogger.LogBatchCommand(currentBatchCommandsLog.ToString());
79+
}
80+
81+
int rowsAffected;
82+
try
83+
{
84+
rowsAffected = currentBatch.ExecuteNonQuery();
85+
}
86+
catch (DbException e)
87+
{
88+
throw ADOExceptionHelper.Convert(Factory.SQLExceptionConverter, e, "could not execute batch command.");
89+
}
90+
91+
Expectations.VerifyOutcomeBatched(totalExpectedRowsAffected, rowsAffected);
8492
}
85-
catch (DbException e)
93+
finally
8694
{
87-
throw ADOExceptionHelper.Convert(Factory.SQLExceptionConverter, e, "could not execute batch command.");
95+
ClearCurrentBatch();
8896
}
97+
}
8998

90-
Expectations.VerifyOutcomeBatched(totalExpectedRowsAffected, rowsAffected);
99+
private MySqlClientSqlCommandSet CreateConfiguredBatch()
100+
{
101+
return new MySqlClientSqlCommandSet(batchSize);
102+
}
91103

104+
private void ClearCurrentBatch()
105+
{
92106
currentBatch.Dispose();
93107
totalExpectedRowsAffected = 0;
94108
currentBatch = CreateConfiguredBatch();
95-
}
96109

97-
private MySqlClientSqlCommandSet CreateConfiguredBatch()
98-
{
99-
return new MySqlClientSqlCommandSet(batchSize);
110+
if (Factory.Settings.SqlStatementLogger.IsDebugEnabled)
111+
{
112+
currentBatchCommandsLog = new StringBuilder().AppendLine("Batch commands:");
113+
}
100114
}
101115

102116
public override void CloseCommands()
@@ -105,12 +119,27 @@ public override void CloseCommands()
105119

106120
try
107121
{
108-
currentBatch.Dispose();
122+
ClearCurrentBatch();
109123
}
110124
catch (Exception e)
111125
{
112-
// Prevent exceptions when closing the batch from hiding any original exception
126+
// Prevent exceptions when clearing the batch from hiding any original exception
113127
// (We do not know here if this batch closing occurs after a failure or not.)
128+
Log.Warn("Exception clearing batch", e);
129+
}
130+
}
131+
132+
protected override void Dispose(bool isDisposing)
133+
{
134+
base.Dispose(isDisposing);
135+
// Prevent exceptions when closing the batch from hiding any original exception
136+
// (We do not know here if this batch closing occurs after a failure or not.)
137+
try
138+
{
139+
currentBatch.Dispose();
140+
}
141+
catch (Exception e)
142+
{
114143
Log.Warn("Exception closing batcher", e);
115144
}
116145
}

src/NHibernate/AdoNet/SqlClientBatchingBatcher.cs

Lines changed: 50 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public override void AddToBatch(IExpectation expectation)
6161
{
6262
Log.Debug("Adding to batch:" + lineWithParameters);
6363
}
64-
_currentBatch.Append((System.Data.SqlClient.SqlCommand) batchUpdate);
64+
_currentBatch.Append((System.Data.SqlClient.SqlCommand)batchUpdate);
6565

6666
if (_currentBatch.CountOfCommands >= _batchSize)
6767
{
@@ -71,30 +71,31 @@ public override void AddToBatch(IExpectation expectation)
7171

7272
protected override void DoExecuteBatch(DbCommand ps)
7373
{
74-
Log.DebugFormat("Executing batch");
75-
CheckReaders();
76-
Prepare(_currentBatch.BatchCommand);
77-
if (Factory.Settings.SqlStatementLogger.IsDebugEnabled)
78-
{
79-
Factory.Settings.SqlStatementLogger.LogBatchCommand(_currentBatchCommandsLog.ToString());
80-
_currentBatchCommandsLog = new StringBuilder().AppendLine("Batch commands:");
81-
}
82-
83-
int rowsAffected;
8474
try
8575
{
86-
rowsAffected = _currentBatch.ExecuteNonQuery();
76+
Log.DebugFormat("Executing batch");
77+
CheckReaders();
78+
Prepare(_currentBatch.BatchCommand);
79+
if (Factory.Settings.SqlStatementLogger.IsDebugEnabled)
80+
{
81+
Factory.Settings.SqlStatementLogger.LogBatchCommand(_currentBatchCommandsLog.ToString());
82+
}
83+
int rowsAffected;
84+
try
85+
{
86+
rowsAffected = _currentBatch.ExecuteNonQuery();
87+
}
88+
catch (DbException e)
89+
{
90+
throw ADOExceptionHelper.Convert(Factory.SQLExceptionConverter, e, "could not execute batch command.");
91+
}
92+
93+
Expectations.VerifyOutcomeBatched(_totalExpectedRowsAffected, rowsAffected);
8794
}
88-
catch (DbException e)
95+
finally
8996
{
90-
throw ADOExceptionHelper.Convert(Factory.SQLExceptionConverter, e, "could not execute batch command.");
97+
ClearCurrentBatch();
9198
}
92-
93-
Expectations.VerifyOutcomeBatched(_totalExpectedRowsAffected, rowsAffected);
94-
95-
_currentBatch.Dispose();
96-
_totalExpectedRowsAffected = 0;
97-
_currentBatch = CreateConfiguredBatch();
9899
}
99100

100101
private SqlClientSqlCommandSet CreateConfiguredBatch()
@@ -118,18 +119,45 @@ private SqlClientSqlCommandSet CreateConfiguredBatch()
118119
return result;
119120
}
120121

122+
private void ClearCurrentBatch()
123+
{
124+
_currentBatch.Dispose();
125+
_totalExpectedRowsAffected = 0;
126+
_currentBatch = CreateConfiguredBatch();
127+
128+
if (Factory.Settings.SqlStatementLogger.IsDebugEnabled)
129+
{
130+
_currentBatchCommandsLog = new StringBuilder().AppendLine("Batch commands:");
131+
}
132+
}
133+
121134
public override void CloseCommands()
122135
{
123136
base.CloseCommands();
124137

138+
// Prevent exceptions when closing the batch from hiding any original exception
139+
// (We do not know here if this batch closing occurs after a failure or not.)
140+
try
141+
{
142+
ClearCurrentBatch();
143+
}
144+
catch (Exception e)
145+
{
146+
Log.Warn("Exception clearing batch", e);
147+
}
148+
}
149+
150+
protected override void Dispose(bool isDisposing)
151+
{
152+
base.Dispose(isDisposing);
153+
// Prevent exceptions when closing the batch from hiding any original exception
154+
// (We do not know here if this batch closing occurs after a failure or not.)
125155
try
126156
{
127157
_currentBatch.Dispose();
128158
}
129159
catch (Exception e)
130160
{
131-
// Prevent exceptions when closing the batch from hiding any original exception
132-
// (We do not know here if this batch closing occurs after a failure or not.)
133161
Log.Warn("Exception closing batcher", e);
134162
}
135163
}

0 commit comments

Comments
 (0)