Skip to content

Commit 37db170

Browse files
committed
Revert "Refine handling of moving between replay and continue states (#3337)"
This reverts commit 265f522.
1 parent 2c83dc9 commit 37db170

File tree

5 files changed

+47
-57
lines changed

5 files changed

+47
-57
lines changed

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6546,8 +6546,8 @@ private TdsOperationStatus TryReadByteArrayWithContinue(TdsParserStateObject sta
65466546
bytes = null;
65476547
int offset = 0;
65486548
byte[] temp = null;
6549-
(bool canContinue, bool isStarting, bool isContinuing) = stateObj.GetSnapshotStatuses();
6550-
if (canContinue)
6549+
(bool isAvailable, bool isStarting, bool isContinuing) = stateObj.GetSnapshotStatuses();
6550+
if (isAvailable)
65516551
{
65526552
if (isContinuing || isStarting)
65536553
{
@@ -13123,9 +13123,9 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj
1312313123
char[] temp = null;
1312413124
bool buffIsRented = false;
1312513125
int startOffset = 0;
13126-
(bool canContinue, bool isStarting, bool isContinuing) = stateObj.GetSnapshotStatuses();
13126+
(bool isAvailable, bool isStarting, bool isContinuing) = stateObj.GetSnapshotStatuses();
1312713127

13128-
if (canContinue)
13128+
if (isAvailable)
1312913129
{
1313013130
if (isContinuing || isStarting)
1313113131
{
@@ -13144,7 +13144,7 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj
1314413144
length >> 1,
1314513145
stateObj,
1314613146
out length,
13147-
supportRentedBuff: !canContinue, // do not use the arraypool if we are going to keep the buffer in the snapshot
13147+
supportRentedBuff: !isAvailable, // do not use the arraypool if we are going to keep the buffer in the snapshot
1314813148
rentedBuff: ref buffIsRented,
1314913149
startOffset,
1315013150
isStarting || isContinuing

src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/TdsParser.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6742,8 +6742,8 @@ private TdsOperationStatus TryReadByteArrayWithContinue(TdsParserStateObject sta
67426742
bytes = null;
67436743
int offset = 0;
67446744
byte[] temp = null;
6745-
(bool canContinue, bool isStarting, bool isContinuing) = stateObj.GetSnapshotStatuses();
6746-
if (canContinue)
6745+
(bool isAvailable, bool isStarting, bool isContinuing) = stateObj.GetSnapshotStatuses();
6746+
if (isAvailable)
67476747
{
67486748
if (isContinuing || isStarting)
67496749
{
@@ -13304,9 +13304,9 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj
1330413304
char[] temp = null;
1330513305
bool buffIsRented = false;
1330613306
int startOffset = 0;
13307-
(bool canContinue, bool isStarting, bool isContinuing) = stateObj.GetSnapshotStatuses();
13307+
(bool isAvailable, bool isStarting, bool isContinuing) = stateObj.GetSnapshotStatuses();
1330813308

13309-
if (canContinue)
13309+
if (isAvailable)
1331013310
{
1331113311
if (isContinuing || isStarting)
1331213312
{
@@ -13325,7 +13325,7 @@ internal TdsOperationStatus TryReadPlpUnicodeCharsWithContinue(TdsParserStateObj
1332513325
length >> 1,
1332613326
stateObj,
1332713327
out length,
13328-
supportRentedBuff: !canContinue, // do not use the arraypool if we are going to keep the buffer in the snapshot
13328+
supportRentedBuff: !isAvailable, // do not use the arraypool if we are going to keep the buffer in the snapshot
1332913329
rentedBuff: ref buffIsRented,
1333013330
startOffset,
1333113331
isStarting || isContinuing

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlCachedBuffer.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,10 @@ internal static TdsOperationStatus TryCreate(SqlMetaDataPriv metadata, TdsParser
3939
{
4040
buffer = null;
4141

42-
(bool canContinue, bool isStarting, _) = stateObj.GetSnapshotStatuses();
42+
(bool isAvailable, bool isStarting, _) = stateObj.GetSnapshotStatuses();
4343

4444
List<byte[]> cachedBytes = null;
45-
if (canContinue)
45+
if (isAvailable)
4646
{
4747
cachedBytes = stateObj.TryTakeSnapshotStorage() as List<byte[]>;
4848
if (isStarting)
@@ -78,10 +78,10 @@ internal static TdsOperationStatus TryCreate(SqlMetaDataPriv metadata, TdsParser
7878
byte[] byteArr = new byte[cb];
7979
// pass false for the writeDataSizeToSnapshot parameter because we want to only take data
8080
// from the current packet and not try to do a continue-capable multi packet read
81-
result = stateObj.TryReadPlpBytes(ref byteArr, 0, cb, out cb, canContinue, writeDataSizeToSnapshot: false, compatibilityMode: false);
81+
result = stateObj.TryReadPlpBytes(ref byteArr, 0, cb, out cb, writeDataSizeToSnapshot: false, compatibilityMode: false);
8282
if (result != TdsOperationStatus.Done)
8383
{
84-
if (result == TdsOperationStatus.NeedMoreData && canContinue && cb == byteArr.Length)
84+
if (result == TdsOperationStatus.NeedMoreData && isAvailable && cb == byteArr.Length)
8585
{
8686
// succeeded in getting the data but failed to find the next plp length
8787
returnAfterAdd = true;

src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/TdsParserStateObject.cs

Lines changed: 33 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1922,7 +1922,7 @@ internal TdsOperationStatus TryReadStringWithEncoding(int length, System.Text.En
19221922
}
19231923
byte[] buf = null;
19241924
int offset = 0;
1925-
(bool canContinue, bool isStarting, bool isContinuing) = GetSnapshotStatuses();
1925+
(bool isAvailable, bool isStarting, bool isContinuing) = GetSnapshotStatuses();
19261926

19271927
if (isPlp)
19281928
{
@@ -1940,7 +1940,7 @@ internal TdsOperationStatus TryReadStringWithEncoding(int length, System.Text.En
19401940
if (((_inBytesUsed + length) > _inBytesRead) || (_inBytesPacket < length))
19411941
{
19421942
int startOffset = 0;
1943-
if (canContinue)
1943+
if (isAvailable)
19441944
{
19451945
if (isContinuing || isStarting)
19461946
{
@@ -1958,7 +1958,7 @@ internal TdsOperationStatus TryReadStringWithEncoding(int length, System.Text.En
19581958
buf = new byte[length];
19591959
}
19601960

1961-
TdsOperationStatus result = TryReadByteArray(buf, length, out _, startOffset, canContinue);
1961+
TdsOperationStatus result = TryReadByteArray(buf, length, out _, startOffset, isAvailable);
19621962

19631963
if (result != TdsOperationStatus.Done)
19641964
{
@@ -2088,23 +2088,22 @@ internal int ReadPlpBytesChunk(byte[] buff, int offset, int len)
20882088

20892089
internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len, out int totalBytesRead)
20902090
{
2091-
bool canContinue = false;
20922091
bool isStarting = false;
20932092
bool isContinuing = false;
20942093
bool compatibilityMode = LocalAppContextSwitches.UseCompatibilityAsyncBehaviour;
20952094
if (!compatibilityMode)
20962095
{
2097-
(canContinue, isStarting, isContinuing) = GetSnapshotStatuses();
2096+
(_, isStarting, isContinuing) = GetSnapshotStatuses();
20982097
}
2099-
return TryReadPlpBytes(ref buff, offset, len, out totalBytesRead, canContinue, canContinue, compatibilityMode);
2098+
return TryReadPlpBytes(ref buff, offset, len, out totalBytesRead, isStarting || isContinuing, compatibilityMode);
21002099
}
21012100
// Reads the requested number of bytes from a plp data stream, or the entire data if
21022101
// requested length is -1 or larger than the actual length of data. First call to this method
21032102
// should be preceeded by a call to ReadPlpLength or ReadDataLength.
21042103
// Returns the actual bytes read.
21052104
// NOTE: This method must be retriable WITHOUT replaying a snapshot
21062105
// Every time you call this method increment the offset and decrease len by the value of totalBytesRead
2107-
internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len, out int totalBytesRead, bool canContinue, bool writeDataSizeToSnapshot, bool compatibilityMode)
2106+
internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len, out int totalBytesRead, bool writeDataSizeToSnapshot, bool compatibilityMode)
21082107
{
21092108
totalBytesRead = 0;
21102109

@@ -2129,16 +2128,9 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len
21292128
// If total length is known up front, allocate the whole buffer in one shot instead of realloc'ing and copying over each time
21302129
if (buff == null && _longlen != TdsEnums.SQL_PLP_UNKNOWNLEN)
21312130
{
2132-
if (compatibilityMode && _snapshot != null && _snapshotStatus != SnapshotStatus.NotActive)
2133-
{
2134-
// legacy replay path perf optimization
2135-
// if there is a snapshot which contains a stored plp buffer take it
2136-
// and try to use it if it is the right length
2137-
buff = TryTakeSnapshotStorage() as byte[];
2138-
}
2139-
else if (writeDataSizeToSnapshot && canContinue && _snapshot != null)
2131+
if (writeDataSizeToSnapshot)
21402132
{
2141-
// if there is a snapshot which it contains a stored plp buffer take it
2133+
// if there is a snapshot and it contains a stored plp buffer take it
21422134
// and try to use it if it is the right length
21432135
buff = TryTakeSnapshotStorage() as byte[];
21442136
if (buff != null)
@@ -2147,7 +2139,13 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len
21472139
totalBytesRead = offset;
21482140
}
21492141
}
2150-
2142+
else if (compatibilityMode && _snapshot != null && _snapshotStatus != SnapshotStatus.NotActive)
2143+
{
2144+
// legacy replay path perf optimization
2145+
// if there is a snapshot and it contains a stored plp buffer take it
2146+
// and try to use it if it is the right length
2147+
buff = TryTakeSnapshotStorage() as byte[];
2148+
}
21512149

21522150
if ((ulong)(buff?.Length ?? 0) != _longlen)
21532151
{
@@ -2199,49 +2197,43 @@ internal TdsOperationStatus TryReadPlpBytes(ref byte[] buff, int offset, int len
21992197
_longlenleft -= (ulong)bytesRead;
22002198
if (result != TdsOperationStatus.Done)
22012199
{
2202-
if (compatibilityMode && _snapshot != null)
2200+
if (writeDataSizeToSnapshot)
22032201
{
2204-
// legacy replay path perf optimization
22052202
// a partial read has happened so store the target buffer in the snapshot
22062203
// so it can be re-used when another packet arrives and we read again
22072204
SetSnapshotStorage(buff);
2205+
SetSnapshotDataSize(bytesRead);
2206+
22082207
}
2209-
else if (canContinue)
2208+
else if (compatibilityMode && _snapshot != null)
22102209
{
2210+
// legacy replay path perf optimization
22112211
// a partial read has happened so store the target buffer in the snapshot
22122212
// so it can be re-used when another packet arrives and we read again
22132213
SetSnapshotStorage(buff);
2214-
if (writeDataSizeToSnapshot)
2215-
{
2216-
SetSnapshotDataSize(bytesRead);
2217-
}
22182214
}
22192215
return result;
22202216
}
2221-
if (writeDataSizeToSnapshot && canContinue)
2222-
{
2223-
SetSnapshotDataSize(bytesRead);
2224-
}
22252217

22262218
if (_longlenleft == 0)
22272219
{
22282220
// Read the next chunk or cleanup state if hit the end
22292221
result = TryReadPlpLength(false, out _);
22302222
if (result != TdsOperationStatus.Done)
22312223
{
2232-
if (compatibilityMode && _snapshot != null)
2233-
{
2234-
// a partial read has happened so store the target buffer in the snapshot
2235-
// so it can be re-used when another packet arrives and we read again
2236-
SetSnapshotStorage(buff);
2237-
}
2238-
else if (canContinue && result == TdsOperationStatus.NeedMoreData)
2224+
if (writeDataSizeToSnapshot)
22392225
{
2240-
SetSnapshotStorage(buff);
2241-
if (writeDataSizeToSnapshot)
2226+
if (result == TdsOperationStatus.NeedMoreData)
22422227
{
2228+
SetSnapshotStorage(buff);
22432229
SetSnapshotDataSize(bytesRead);
22442230
}
2231+
}
2232+
else if (compatibilityMode && _snapshot != null)
2233+
{
2234+
// a partial read has happened so store the target buffer in the snapshot
2235+
// so it can be re-used when another packet arrives and we read again
2236+
SetSnapshotStorage(buff);
22452237
}
22462238
return result;
22472239
}
@@ -3516,17 +3508,17 @@ internal bool IsSnapshotContinuing()
35163508
_snapshotStatus == TdsParserStateObject.SnapshotStatus.ContinueRunning;
35173509
}
35183510

3519-
internal (bool CanContinue, bool IsStarting, bool IsContinuing) GetSnapshotStatuses()
3511+
internal (bool IsAvailable, bool IsStarting, bool IsContinuing) GetSnapshotStatuses()
35203512
{
3521-
bool canContinue = _snapshot != null && _snapshot.ContinueEnabled && _snapshotStatus != SnapshotStatus.NotActive;
3513+
bool isAvailable = _snapshot != null && _snapshot.ContinueEnabled;
35223514
bool isStarting = false;
35233515
bool isContinuing = false;
3524-
if (canContinue)
3516+
if (isAvailable)
35253517
{
35263518
isStarting = _snapshotStatus == SnapshotStatus.ReplayStarting;
35273519
isContinuing = _snapshotStatus == SnapshotStatus.ContinueRunning;
35283520
}
3529-
return (canContinue, isStarting, isContinuing);
3521+
return (isAvailable, isStarting, isContinuing);
35303522
}
35313523

35323524
internal int GetSnapshotStorageLength<T>()

src/Microsoft.Data.SqlClient/tests/ManualTests/SQL/DataReaderTest/DataReaderTest.cs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@
66
using System.Collections.Generic;
77
using System.Data;
88
using System.Data.SqlTypes;
9-
using System.Diagnostics;
10-
using System.Linq;
119
using System.Reflection;
1210
using System.Text;
1311
using System.Threading;

0 commit comments

Comments
 (0)