Query method not closing the reader? #129

Closed
jotte opened this Issue Feb 16, 2012 · 4 comments

Comments

Projects
None yet
3 participants
@jotte

jotte commented Feb 16, 2012

I may be wrong, but I'm using Massive against Oracle, and the Query(string sql, params object[] args) method called many times would increase the number of opened cursors close to thousand in my case!
I changed the line:
var rdr = CreateCommand(sql, conn, args).ExecuteReader();
to
using (var rdr = CreateCommand(sql, conn, args).ExecuteReader()) {...}
and the number of opened cursors never exceeded 12!
This is line #198

Thank you for your time!

@robconery

This comment has been minimized.

Show comment Hide comment
@robconery

robconery Feb 22, 2012

Contributor

The code is using a yield - which means that yes, the connection is open when the record is returned. This enables you to stream many records if you need. I don't care much for this design as the yield operation can take a while and leave the connection open - however many people like it. You can use "ToList()" if you want to get your records in a batch.

That said - one that is very helpful is to actually try and see if there's a problem before you report it as an issue. What would be even more helpful is to show me a failing test.

I can look at this code, however, and tell you that the connection is instantiated in a using statement - which means that once the using statement goes out of scope through error or exit - the connection (and the reader) are closed.

Unless you see otherwise - that's the deal here (and with Massive in general).

Contributor

robconery commented Feb 22, 2012

The code is using a yield - which means that yes, the connection is open when the record is returned. This enables you to stream many records if you need. I don't care much for this design as the yield operation can take a while and leave the connection open - however many people like it. You can use "ToList()" if you want to get your records in a batch.

That said - one that is very helpful is to actually try and see if there's a problem before you report it as an issue. What would be even more helpful is to show me a failing test.

I can look at this code, however, and tell you that the connection is instantiated in a using statement - which means that once the using statement goes out of scope through error or exit - the connection (and the reader) are closed.

Unless you see otherwise - that's the deal here (and with Massive in general).

@robconery robconery closed this Feb 22, 2012

@jotte

This comment has been minimized.

Show comment Hide comment
@jotte

jotte Feb 23, 2012

Rob, I fully understand your point and I do agree with it.
The project I'm working on is processing records that are in a 1->n->n relationship according to the business rules. When number of children is small (under 200) there are no issues. But occasionally there are records with 2000-7000 children at first level of inheritance and then I am getting "ORA-01000: maximum open cursors exceeded " error. The DBA increased the max number of cursor to 4000 if I'm not mistaken.
I asked the DBA to monitor the number of cursors and he confirmed that it was spiking very high (over 1000)!
After making the change in code as I mentioned, and run the same batch again, this time the DBA confirmed the number of open cursors did not exceed 12.
I was thinking to avoid using table.Query for that step but the tables have over 100 columns and for my purpose I need only a few of them. Here is a sample code where I have this issue, most of the time the ORA error will occur when querying for rs2 or rs3

var tbl = new ORA_TABLE1();

var rs1 = tbl.Query("Select fld1, fld2, fld3 from T1 where...");
foreach (dynamic element1 in rs1) {
var rs2 = tbl.Query("Select fld1, fld2... from T2 where ...");
foreach (dynamic elem2 in rs2) {
if (some_condition) {
var rs3 = tbl.Query(string.Format("select {0} from T3 where fld1={1} and fld2='{2}'", elem2.Val1, var2, var3);
foreach (dynamic elem3 in rs3) {
//do something
}
}
}
}

Thank you for your time.

jotte commented Feb 23, 2012

Rob, I fully understand your point and I do agree with it.
The project I'm working on is processing records that are in a 1->n->n relationship according to the business rules. When number of children is small (under 200) there are no issues. But occasionally there are records with 2000-7000 children at first level of inheritance and then I am getting "ORA-01000: maximum open cursors exceeded " error. The DBA increased the max number of cursor to 4000 if I'm not mistaken.
I asked the DBA to monitor the number of cursors and he confirmed that it was spiking very high (over 1000)!
After making the change in code as I mentioned, and run the same batch again, this time the DBA confirmed the number of open cursors did not exceed 12.
I was thinking to avoid using table.Query for that step but the tables have over 100 columns and for my purpose I need only a few of them. Here is a sample code where I have this issue, most of the time the ORA error will occur when querying for rs2 or rs3

var tbl = new ORA_TABLE1();

var rs1 = tbl.Query("Select fld1, fld2, fld3 from T1 where...");
foreach (dynamic element1 in rs1) {
var rs2 = tbl.Query("Select fld1, fld2... from T2 where ...");
foreach (dynamic elem2 in rs2) {
if (some_condition) {
var rs3 = tbl.Query(string.Format("select {0} from T3 where fld1={1} and fld2='{2}'", elem2.Val1, var2, var3);
foreach (dynamic elem3 in rs3) {
//do something
}
}
}
}

Thank you for your time.

@johnmbaughman

This comment has been minimized.

Show comment Hide comment
@johnmbaughman

johnmbaughman Jun 21, 2012

I'm just wondering if this is related...

var model = new DynamicModel("database");
dynamic table1 = model.Query("INSERT INTO table1(field1, field2) VALUES (@0, @1); SELECT @@IDENTITY AS NewId FROM table1;",1, 2).FirstOrDefault();
for ((int i = 0; i < 10; i++) {
    model.Query("INSERT INTO table2 (field1, field2) VALUES (@0, @1);", 1, i);
}

Basically, nothing happens in the second query.

And... If I add .FirstOrDefault() to the second query, to try and return the @@identity for instance, I get that model.Query() is now missing that method.

Any ideas? Is it related or is a new bug?

-John

I'm just wondering if this is related...

var model = new DynamicModel("database");
dynamic table1 = model.Query("INSERT INTO table1(field1, field2) VALUES (@0, @1); SELECT @@IDENTITY AS NewId FROM table1;",1, 2).FirstOrDefault();
for ((int i = 0; i < 10; i++) {
    model.Query("INSERT INTO table2 (field1, field2) VALUES (@0, @1);", 1, i);
}

Basically, nothing happens in the second query.

And... If I add .FirstOrDefault() to the second query, to try and return the @@identity for instance, I get that model.Query() is now missing that method.

Any ideas? Is it related or is a new bug?

-John

@johnmbaughman

This comment has been minimized.

Show comment Hide comment
@johnmbaughman

johnmbaughman Jun 21, 2012

And just as soon as I asked it I solved it (at least for myself):

var model = new DynamicModel("database", tableName: "table1");    
dynamic table1 = model.Insert(new { field1 = 1, field2 = 2 });
var model1 = new DynamicModel("database", tableName: "table2");
for ((int i = 0; i < 10; i++) {
    model2.Insert( new {field1 = 2, field2 = i} );
}

I still see a potential issue here, but the work around is pretty nice. Thanks for reading; now, hopefully, someone else can learn from my "mistake"

And just as soon as I asked it I solved it (at least for myself):

var model = new DynamicModel("database", tableName: "table1");    
dynamic table1 = model.Insert(new { field1 = 1, field2 = 2 });
var model1 = new DynamicModel("database", tableName: "table2");
for ((int i = 0; i < 10; i++) {
    model2.Insert( new {field1 = 2, field2 = i} );
}

I still see a potential issue here, but the work around is pretty nice. Thanks for reading; now, hopefully, someone else can learn from my "mistake"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment