Application hangs when using Prepared Statements
I've come across this problem before, but never really resolved it. I have written an app to do some batch processing on a database, but after just ten records it hangs when trying to execute the update using a prepared statement.
This is the code:
Statement st = conn.createStatement();
ResultSet rs = st.executeQuery("SELECT * FROM " + table);
while(rs.next()){// For each row
try{
File originalURL =new File(rs.getString("OriginalURL"));
String path = originalURL.getParent();
StringBuffer b = fixField(new StringBuffer(rs.getString(field)), 0, path);
PreparedStatement ps = conn.prepareStatement("UPDATE " + table +" SET " + field +"=? WHERE " + primaryKey +"=?");
ps.setString(1, b.toString());
ps.setString(2, rs.getString(primaryKey));
ps.executeUpdate();
System.out.println(rs.getString(primaryKey) +" updated okay");
}catch(NullPointerException npe){
System.out.println("Empty URL at: " + rs.getString(primaryKey));
}
}
Anyone know what I'm doing wrong?
[1640 byte] By [
RJSanga] at [2007-10-2 5:30:03]
I'm not sure if I am correctly understanding your problem but these are my observations.
1. Calling rs.getString(primaryKey) several times may not work with most of the drivers.
2. Your code handling a NullPointerException is a significantly huge block. This might have some strange results. Handling NullPointerException is not a good idea. If there is an exception occuring, your code needs to have a check for null on the object references before they invoke the methods.
3. You are defeating the purpose of having a PreparedStatement by keeping the creation of prepared statement inside the while loop.
I don't know what's wrong, but checking the update return might help, and I presume you're wrapping this whole thing in a try/catch for SQLException, but it's not throwing anything?
Statement st = conn.createStatement();
ResultSet rs = st.executeQuery("SELECT * FROM " + table);
while(rs.next()) {// For each row
try {
File originalURL = new File(rs.getString("OriginalURL"));
String path = originalURL.getParent();
StringBuffer b = fixField(new StringBuffer(rs.getString(field)), 0, path);
PreparedStatement ps = conn.prepareStatement("UPDATE " + table + " SET " + field + "=? WHERE " + primaryKey + "=?");
ps.setString(1, b.toString());
ps.setString(2, rs.getString(primaryKey));
int updtCount = ps.executeUpdate(); // check return
if (updtCount > 0 )// add check
System.out.println(rs.getString(primaryKey) + " updated okay");
else
System.out.println(rs.getString(primaryKey) + " NOT updated!");
} catch(NullPointerException npe) {
System.out.println("Empty URL at: " + rs.getString(primaryKey));
}
}
I wouldn't try to keep a ResultSet open and then use the same Connection for other things. Not guaranteed to work.
That code could, depending on the database, deadlock with itself.
Also you are not closing your statements. A big no-no.
What I'd do:
Plan A: do it with a single SQL statement if possible:
update whatever set originalurl = 'http://' || originalurl;
If it isn't possible to calculate the new value like that then that won't work. Speed-wise the best though if can be done.
Plan B: if your database has stored procedures consider doing it with those. Has downsides too, though (procedure languages are ***** and lock you to that db vendor).
Plan C: read the entire ResultSet into a LinkedList. Close the ResultSet and the Statement. Then loop over the LinkedList and do the updates.
> (procedure languages are ***** and lock you to> that db vendor).What, the filter doesn't like f-u-g-l-y?
Okay, randomly it has now stopped hanging and started throwing an error:
java.sql.SQLException: Network error: Connection reset
at net.sourceforge.jtds.jdbc.TdsStatement.getMoreResults(Unkown Source)
at net.sourceforge.jtds.jdbc.TdsStatement.skipToEnd(Unkown Source)
at net.sourceforge.jtds.jdbc.TdsStatement.close(Unkown Source)
at net.sourceforge.jtds.jdbc.TdsConnection.close(Unkown Source)
Weird. I'll try some of your suggestions and see if it changes :)
"I wouldn't try to keep a ResultSet open and then use the same Connection for other things. Not guaranteed to work."
If you're right about that I've learned something. The above is a very common approach when coding with cursors in PL/SQL, Pro*C, Pro*Cobol, SQR, etc.
I've adapted the method to use in a few Java JDBC pgms too and it has always appeared to work fine.
Mind you, I'm not arguing. My own experience has not been with extremely large numbers of rows (thousands, but not hundreds of thousands or so), so I will defer to those who work on large Enterprise apps.
It's just surprising to me that this oft used technique in cursor/resultset processing is incorrect.
~Bill
Okay, modified my code but no change. Still get the connection reset error
Statement st = conn.createStatement();
ResultSet rs = st.executeQuery("SELECT * FROM " + table);
PreparedStatement ps = conn.prepareStatement("UPDATE " + table + " SET " + field + "=? WHERE " + primaryKey + "=?");
while(rs.next()) {// For each row
String pk = rs.getString(primaryKey);
String URLString = rs.getString("OriginalURL");
if(URLString == null) {
System.out.println("Empty URL at: " + pk);
} else {
File originalURL = new File(URLString);
String path = originalURL.getParent();
StringBuffer b = fixField(new StringBuffer(rs.getString(field)), 0, path);
ps.setString(1, b.toString());
ps.setString(2, pk);
int udok = ps.executeUpdate();
if(udok > 0) System.out.println(pk + " updated okay");
else System.out.println(pk + " didn't update");
}
}
ps.close();
rs.close();
st.close();
If I were you, I'd like to see what's in 'b' and 'path' before the executeUpdate.
> "I wouldn't try to keep a ResultSet open and then use
> the same Connection for other things. Not guaranteed
> to work."
>
> If you're right about that I've learned something.
I'm starting to be convinced I was wrong there. The JDBC standard explicitly mentions multiple Statements are ok; can't find a mention of ResultSets but I'll assume mixing those in concurrent access is ok too.
I wonder where I got that...
> > "I wouldn't try to keep a ResultSet open and then
> use
> > the same Connection for other things. Not
> guaranteed
> > to work."
> >
> > If you're right about that I've learned something.
>
> I'm starting to be convinced I was wrong there. The
> JDBC standard explicitly mentions multiple Statements
> are ok; can't find a mention of ResultSets but I'll
> assume mixing those in concurrent access is ok too.
>
Thanks for the update. I hope it's okay, as it makes pgm'g simpler.
> I wonder where I got that...
Been there, done that>;o)
Moving the PreparedStaement out of the loop was the better thing to do but it wasn't the reason your app was hanging. Though inefficient you could still have the PreparedStatement inside the loop. The problem was you weren't closing the prepared statement inside the loop. This caused a whole lot of preparedstatements to be quickly created and not closed before the garbage collector could clean them up. So many that the database couldn't keep up. Some databases associate a cursor (a type of database resource) with each statement and there is a limit to the number of cursors you can have open at one time. The database can either throw an error if it runs out of resources or pause until resources are freed. Either way your application wouldn't run correctly and would explain the hanging you experienced.
You mean my suggestion worked by fluke? :) That was some useful information you provided.
> Moving the PreparedStaement out of the loop was the
> better thing to do but it wasn't the reason your app
> was hanging. Though inefficient you could still have
> the PreparedStatement inside the loop. The problem
> was you weren't closing the prepared statement inside
> the loop. This caused a whole lot of
> preparedstatements to be quickly created and not
> closed before the garbage collector could clean them
> up. So many that the database couldn't keep up.
> Some databases associate a cursor (a type of
> f database resource) with each statement and there is
> a limit to the number of cursors you can have open at
> one time. The database can either throw an error if
> it runs out of resources or pause until resources are
> freed. Either way your application wouldn't run
> correctly and would explain the hanging you
> experienced.
Ah, good catch - that probably is it.
But it's still not working, even with the preparedstatement outside the loop :(
> But it's still not working, even with the> preparedstatement outside the loop :(What is not working?
The tenth row will not update, but does not throw any form of exception. I think that what happens after that is the cursor is not reset and the app fails to read the rest of the result set.
I tried using a separate connection to do the updates to the one that reads the resultset. The result of this was that after attempting to update the tenth record the exception thrown was "Network error: broken pipe".
I'm baffled!
Then you might have a database deadlock between the result set you are reading in and the update you are doing.
1. At the time of the pause, check your database for locks.
2. Use two loops. The first reads your ResultSet into a List. Then close your Statement and ResultSet. If it locks up during this loop then there is something wrong with reading from the database. In the second loop iterate the List and use your prepared statement to update the database. See if it pauses during the second loop.
3. If you are still pausing trying closing the connection and opening a new connection between the two loops in step 2.
On a side note, be sure to close your ResultSets, Statements, PreparedStatements, Connections in finally blocks.
Try this:
Statement st = conn.createStatement();
ResultSet rs = st.executeQuery("SELECT * FROM " + table);
while(rs.next()) {// For each row
PreparedStatement ps = conn.prepareStatement("UPDATE " + table + " SET " + field + "=? WHERE " + primaryKey + "=?");
String pk = rs.getString(primaryKey);
String URLString = rs.getString("OriginalURL");
if(URLString == null) {
System.out.println("Empty URL at: " + pk);
} else {
File originalURL = new File(URLString);
String path = originalURL.getParent();
StringBuffer b = fixField(new StringBuffer(rs.getString(field)), 0, path);
ps.setString(1, b.toString());
ps.setString(2, pk);
int udok = ps.executeUpdate();
if(udok > 0) System.out.println(pk + " updated okay");
else System.out.println(pk + " didn't update");
}
ps.close();
}
rs.close();
st.close();
I made my statements unprepared (took them by surprise?) and they work fine.La la la la la la la
Good! But just to satisfy curiosity, did you try that last bit I posted?
