Skip to content

Removed soft cancellation for the reader's sequential methods #3276

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.

Sign up for GitHub

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

Merged
roji merged 10 commits into from
Oct 28, 2020
Merged

Removed soft cancellation for the reader's sequential methods #3276

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
59e3467
Removed soft cancellation for the reader's sequential methods
vonzshik Oct 26, 2020
5b04782
Made block a readonly struct
vonzshik Oct 27, 2020
c9fb4a5
Rolled back previous changes
vonzshik Oct 27, 2020
59f04b8
Added a variable, to control the flow of the cancellation on a timeout
vonzshik Oct 27, 2020
67d36f7
Flipped cancellation flag
vonzshik Oct 27, 2020
0df8ecc
Added tests for a timeout
vonzshik Oct 28, 2020
771806d
Added one more test + fixed sequential cancellation
vonzshik Oct 28, 2020
32049d6
Small fix
vonzshik Oct 28, 2020
32484fc
Another small fix
vonzshik Oct 28, 2020
2424d20
Final test fixes
vonzshik Oct 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Final test fixes
  • Loading branch information
@vonzshik
vonzshik committed Oct 28, 2020
commit 2424d20df3c74485ae7e3d688fcace247631b6d1
2 changes: 1 addition & 1 deletion src/Npgsql/NpgsqlConnector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ internal void FlagAsWritableForMultiplexing()
volatile bool _cancellationRequested;
volatile bool _userCancellationRequested;

internal bool UserCancellationRequesed => _userCancellationRequested;
internal bool UserCancellationRequested => _userCancellationRequested;

internal CancellationToken UserCancellationToken { get; set; }

Expand Down
2 changes: 1 addition & 1 deletion src/Npgsql/NpgsqlReadBuffer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ async Task EnsureLong()

// There is a case, when we might call a cancellable method (NpgsqlDataReader.NextResult)
// but it times out on a sequential read (NpgsqlDataReader.ConsumeRow)
if (Connector.UserCancellationRequesed)
if (Connector.UserCancellationRequested)
{
// User requested the cancellation and it timed out (or we didn't send it)
throw Connector.Break(new OperationCanceledException("Query was cancelled", TimeoutException(),
Expand Down
29 changes: 23 additions & 6 deletions test/Npgsql.Tests/ReaderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1729,6 +1729,9 @@ public async Task NextResultAsync_sequential_cancel()
if (IsMultiplexing)
return; // Multiplexing, cancellation

if (!IsSequential)
return;

await using var postmasterMock = PgPostmasterMock.Start(ConnectionString);
using var _ = CreateTempPool(postmasterMock.ConnectionString, out var connectionString);
await using var conn = await OpenConnectionAsync(connectionString);
Expand All @@ -1742,7 +1745,7 @@ await pgMock
.WriteDataRowWithFlush(new byte[10000]);

using var cmd = new NpgsqlCommand("SELECT some_bytea FROM some_table", conn);
await using var reader = await cmd.ExecuteReaderAsync(CommandBehavior.SequentialAccess);
await using var reader = await cmd.ExecuteReaderAsync(Behavior);

// Successfully read the first resultset
Assert.True(await reader.ReadAsync());
Expand All @@ -1757,7 +1760,7 @@ await pgMock

// Send no response from server, wait for the cancellation attempt to time out
var exception = Assert.ThrowsAsync<OperationCanceledException>(async () => await task);
Assert.That(exception.InnerException, Is.Null);
Assert.That(exception.InnerException, Is.TypeOf<TimeoutException>());
Assert.That(exception.CancellationToken, Is.EqualTo(cancellationSource.Token));

Assert.That(conn.FullState, Is.EqualTo(ConnectionState.Broken));
Expand All @@ -1769,6 +1772,9 @@ public async Task GetFieldValueAsync_sequential_cancel()
if (IsMultiplexing)
return; // Multiplexing, cancellation

if (!IsSequential)
return;

await using var postmasterMock = PgPostmasterMock.Start(ConnectionString);
using var _ = CreateTempPool(postmasterMock.ConnectionString, out var connectionString);
await using var conn = await OpenConnectionAsync(connectionString);
Expand All @@ -1782,7 +1788,7 @@ await pgMock
.WriteDataRowWithFlush(new byte[10000]);

using var cmd = new NpgsqlCommand("SELECT some_bytea FROM some_table", conn);
await using var reader = await cmd.ExecuteReaderAsync(CommandBehavior.SequentialAccess);
await using var reader = await cmd.ExecuteReaderAsync(Behavior);

await reader.ReadAsync();

Expand All @@ -1791,6 +1797,7 @@ await pgMock
cts.Cancel();

var exception = Assert.ThrowsAsync<OperationCanceledException>(async () => await task);
Copy link
Copy Markdown
Member

@roji roji Oct 28, 2020

Choose a reason for hiding this comment

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

For completeness we can assert that no PG cancellation request reached the server (also in the other relevant tests).

Assert.That(exception.InnerException, Is.Null);

Assert.That(conn.FullState, Is.EqualTo(ConnectionState.Broken));
}
Expand All @@ -1801,6 +1808,9 @@ public async Task IsDBNullAsync_sequential_cancel()
if (IsMultiplexing)
return; // Multiplexing, cancellation

if (!IsSequential)
return;

await using var postmasterMock = PgPostmasterMock.Start(ConnectionString);
using var _ = CreateTempPool(postmasterMock.ConnectionString, out var connectionString);
await using var conn = await OpenConnectionAsync(connectionString);
Expand All @@ -1814,7 +1824,7 @@ await pgMock
.WriteDataRowWithFlush(new byte[10000], new byte[4]);

using var cmd = new NpgsqlCommand("SELECT some_bytea, some_int FROM some_table", conn);
await using var reader = await cmd.ExecuteReaderAsync(CommandBehavior.SequentialAccess);
await using var reader = await cmd.ExecuteReaderAsync(Behavior);

await reader.ReadAsync();

Expand All @@ -1823,6 +1833,7 @@ await pgMock
cts.Cancel();

var exception = Assert.ThrowsAsync<OperationCanceledException>(async () => await task);
Assert.That(exception.InnerException, Is.Null);

Assert.That(conn.FullState, Is.EqualTo(ConnectionState.Broken));
}
Expand All @@ -1838,6 +1849,9 @@ public async Task GetFieldValueAsync_sequential_timeout()
if (IsMultiplexing)
return; // Multiplexing, cancellation

if (!IsSequential)
return;

var csb = new NpgsqlConnectionStringBuilder(ConnectionString);
csb.CommandTimeout = 3;
csb.CancellationTimeout = 15000;
Expand All @@ -1855,7 +1869,7 @@ await pgMock
.WriteDataRowWithFlush(new byte[10000]);

using var cmd = new NpgsqlCommand("SELECT some_bytea FROM some_table", conn);
await using var reader = await cmd.ExecuteReaderAsync(CommandBehavior.SequentialAccess);
await using var reader = await cmd.ExecuteReaderAsync(Behavior);

await reader.ReadAsync();

Expand All @@ -1874,6 +1888,9 @@ public async Task IsDBNullAsync_sequential_timeout()
if (IsMultiplexing)
return; // Multiplexing, cancellation

if (!IsSequential)
return;

var csb = new NpgsqlConnectionStringBuilder(ConnectionString);
csb.CommandTimeout = 3;
csb.CancellationTimeout = 15000;
Expand All @@ -1891,7 +1908,7 @@ await pgMock
.WriteDataRowWithFlush(new byte[10000], new byte[4]);

using var cmd = new NpgsqlCommand("SELECT some_bytea, some_int FROM some_table", conn);
await using var reader = await cmd.ExecuteReaderAsync(CommandBehavior.SequentialAccess);
await using var reader = await cmd.ExecuteReaderAsync(Behavior);

await reader.ReadAsync();

Expand Down