rohiroy / dapper-dot-net

Automatically exported from code.google.com/p/dapper-dot-net
Other
0 stars 0 forks source link

Error while iterating rows results in TimeoutException #106

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
This applies only to the case where you do Query(buffered: false).
Non-buffered queries basically return a IEnumerable over the DbDataReader.

The problem goes like this:
If during your processing of the results, e.g. inside a 
foreach(var row in connection.Query("...", buffered: false))
an exception is raised, the thread seems to freeze for sometimes and then 
raises a Timeout exception on the connection.
Raising an exception inside the loop could be an expected use-case, unexpected 
error, or simply cancelToken.ThrowIfCancellationRequired (which could be a 
common use-case if you have lots of row).

The reason behind is this:
This happens with SQL Server if you call Close() (which is called by Dispose()) 
on a datareader before it has read all its data. Apparently it's linked to the 
networking protocol.
For your reference, here's a discussion of the problem:
http://stackoverflow.com/questions/133374/net-sqldatareader-close-or-dispose-res
ults-in-timeout-expired-exception

The solution:
The official, recommended solution is to call Cancel() on the DbCommand before 
disposing the DbDataReader.

Suggested fix: 
inside QueryInternal() instead of simply wrapping the reader with:
using(var reader = cmd.ExecuteReader())
use this: 
try
{
// ... existing code ...
reader.Close();
}
finally
{
if (!reader.IsClosed()) cmd.Cancel();
reader.Dispose();
}

Original issue reported on code.google.com by joel.du...@gmail.com on 6 Jul 2012 at 4:16

GoogleCodeExporter commented 9 years ago
I would like to add that an exception is not strictly required. Quiting a 
foreach early should exhibit the same behavior:

foreach (var row in conn.Query("...", buffered: false))
{
  if (row.Value == "That's what I want!")
    break;
}

Original comment by joel.du...@gmail.com on 6 Jul 2012 at 4:19

GoogleCodeExporter commented 9 years ago
Given that I did provide both the problem and a fix, is there any chance to see 
that code merged into Dapper proper?

I'm currently using my own modified version of Dapper, which is kind of 
annoying to keep in sync with new releases.

Original comment by joel.du...@gmail.com on 19 Sep 2012 at 5:59

GoogleCodeExporter commented 9 years ago

Original comment by marc.gravell on 19 Sep 2012 at 6:54

GoogleCodeExporter commented 9 years ago
Aplogies; I will look at this for the next deploy. Or you have my permission to 
hunt me down and yell at me!

Original comment by marc.gravell on 19 Sep 2012 at 6:55

GoogleCodeExporter commented 9 years ago
This seems to have caused a regression error for the Dapper.Contrib tests, and 
it is also causing issues for Postgres.

The SqlCe implmentation of IDbCommand doesn't appear to implement Cancel() at 
all, and throws an exception when called.

As for Postgres, after a short time of continually running queries, a 'user 
requested cancel' happens for the currently running query, from what appears to 
be a previous query's cancel.  I still haven't tracked down if it is Dapper or 
Npgsql causing it.

Original comment by scotlor...@gmail.com on 9 Oct 2012 at 3:51

GoogleCodeExporter commented 9 years ago
I'll add some try/catch around the Cancel, which should fix SqlCe; can you be 
more specific about the Postgres? I'm unclear what is happening there.

Original comment by marc.gravell on 9 Oct 2012 at 7:19

GoogleCodeExporter commented 9 years ago
I'm still trying to track down what is going on, but I altered the test suite 
for the contrib section to run for both CE and Postgresql.  The CE section 
failed from the non-implemented cancel.  Postgresql randomly failed while 
executing some statements saying "ERROR: canceling statement due to user 
request".  If I comment out the section that does the cmd canceling, it runs 
fine.

If I can't track down the bug, I will sumbit the work that I've done and 
someone else can have a go.

Original comment by scotlor...@gmail.com on 9 Oct 2012 at 7:11

GoogleCodeExporter commented 9 years ago
Have submitted a pull request with a small patch that should at least fix up 
Postgres.

I'm still unsure of what is going on in its entirety, but I think there is a 
race condition between the connection being torn down and disposed of, and the 
previous connection's cancel command.  

If you're interested in having a look, I've attached the part of the crash log 
that might explain some of what was going on.  It has a lot of the logging from 
Npgsql turned on and some custom stuff.  :D

Original comment by scotlor...@gmail.com on 10 Oct 2012 at 3:12

Attachments:

GoogleCodeExporter commented 9 years ago
See pull request notes; applied manually, but with some minor tweaks - you may 
want to check that this still fixes Postgres.

Original comment by marc.gravell on 10 Oct 2012 at 6:34

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Seems to work fine for my test cases, and no spurious command cancels.  Looks 
good.

Original comment by scotlor...@gmail.com on 10 Oct 2012 at 8:12