views:

57

answers:

2

I've sort of inherited some code on this scientific modelling project, and my colleagues and I are getting stumped by this problem. The guy who wrote this is now gone, so we can't ask him (go figure).

Inside the data access layer, there is this insert() method. This does what it sounds like -- it inserts records into a database. It is used by the various objects being modeled to tell the database about themselves during the course of the simulation.

However, we noticed that during longer simulations after a fair number of database inserts, we eventually got connection timeouts. So we upped the timeout limits, and then we started getting "out of memory" errors from PostgreSQL. We eventually pinpointed the problem to a line where an IDbCommand object uses Prepare(). Leaving it in causes memory usage to indefinitely go up. Commenting out this line causes the code to work just fine, and eliminates all the memory problems. What does Prepare() do that causes this? I can't find anything in the documentation to explain this.

A compressed version of the code follows.

public virtual void insert(DomainObjects.EntityObject obj)
{
    lock (DataBaseProvider.DataBase.Connection)
    {
        IDbCommand cmd = null; 
        IDataReader noInsertIdReader = null;
        IDataReader reader= null;

        try
        { 
            if (DataBaseProvider.DataBase.Validate)
            {    ...    }

            // create and prepare the insert command
            cmd = createQuery(".toInsert", obj);
            cmd.Prepare();     // This is what is screwing things up

            // get the query to retreive the sequence number
            SqlStatement lastInsertIdSql = DAOLayer...getStatement(this.GetType().ToString() + ".toGetLastInsertId");

            // if the obj insert does not use a sequence, execute the insert command and return
            if (lastInsertIdSql == null)
            {
                noInsertIdReader = cmd.ExecuteReader();
                noInsertIdReader.Close();
                return;
            }

            // append the sequence query to the end of the insert statement
            cmd.CommandText += ";" + lastInsertIdSql.Statement;

            reader = cmd.ExecuteReader();

            // read the sequence number and set the objects id
            ...
        }

        // deal with some specific exceptions
        ...
    }
}

EDIT: (In response to the first given answer) All the database objects do get disposed in a finally block. I just cut that part out here to save space. We've played with that a bit and that didn't make any difference, so I don't think that's the problem.

+1  A: 

You'll notice that IDbCommand and IDataReader both implement IDisposable. Whenever you create an instance of an IDisposable object you should either wrap it in a using statement or call Dispose once you're finished. If you don't you'll end up leaking resources (sometimes resources other than just memory).

Try this in your code

using (IDbCommand cmd = createQuery(".toInsert", obj))
{
    cmd.Prepare();     // This is what is screwing things up
    ...
    //the rest of your example code
    ...
}

EDIT to talk specifically about Prepare

I can see from the code that you're preparing the command and then never reusing it.

The idea behind preparing a command is that it costs extra overhead to prepare, but then each time you use the command it will be more efficient than a non prepared statement. This is good if you've got a command that you're going to reuse a lot, and is a trade off of whether the overhead is worth the performance increase of the command.

So in the code you've shown us you are preparing the command (paying all of the overhead) and getting no benefit because you then immediately throw the command away!

I would either recycle the prepared command, or just ditch the call to the prepare statement.

I have no idea why the prepared commands are leaking, but you shouldn't be preparing so many commands in the first place (especially single use commands).

DoctaJonez
See my edit. :)
jloubert
+1  A: 

The Prepare() method was designed to make the query run more efficiently. It is entirely up to the provider to implement this. A typical one creates a temporary stored procedure, giving the server an opportunity to pre-parse and optimize the query.

There's a couple of ways code like this could leak memory. One is a typical .NET detail, a practical implementation of an IDbCommand class always has a Dispose() method to release resources explicitly before the finalizer thread does it. I don't see it being used in your snippet. But pretty unlikely in this case, it is very hard to consume all memory without ever running the garbage collector. You can tell from Perfmon.exe and observe the performance counters for the garbage collector.

The next candidate is more insidious, you are using a big chunk of native code. Dbase providers are not that simple. The FOSS kind tends to be designed to allow you to get the bugs out of them. Source code is available for a reason. Perfmon.exe again to diagnose that, seeing the managed heaps not growing beyond bounds but private bytes exploding is a dead giveaway.

If you don't feel much like debugging the provider you could just comment the statement.

Hans Passant