Programming is interesting!
Here's a bit of problem solving to do with coding practices, #PostgreSQL (or perhaps any database) and #Java.
Some customers (or at least their alerts) have been now and again reporting being unable to open a database connection as the available connections had run out. There's a lot of db activity, but I'd set the limits relatively high so was surprised.
However waking up this morning I had an idea (first tip - sleep on it!)
It might be that in some cases when an exception is raised, the transaction isn't rolled back before being closed. Take a method like this
```
public int getSomething() throws SomeException {
int something;
Connection conn = null;
try {
conn = Util.getConnection(); // Gets a connection and sets auto-commit to false
something = // do some work
conn.commit();
} catch (SQLException | OtherException ex) {
try {
conn.rollback();
} catch (SQLException sqlex) {
// Log error
}
// Handle exception
} finally {
if (conn != null) {
try {
conn.close();
} catch (SQLException sqlex) {
// Log error
}
}
}
return something;
}
```
That's a typical sort of pattern for our code. It looks like everything's handled. What happens though if SomeException is thrown, as per the method signature? It could have been thrown in the 'do some work' bit. Now in that case, the connection will be closed in the finally block but the transaction won't have been committed or rolled back.
(1 of 2 or 3...)
This is one of the few cases of a bug pattern where the Java compiler and static analysis tools don't complain.
You might think .close() should roll it back before closing. Apparently some JDBC drivers do that but it's not part of the spec. so it's undefined what happens. I seem to remember from past experience that the Postgres JDBC driver doesn't.
Trying a rollback before close as a fallback doesn't have many downsides, so my solution was to replace make the finally block with
```
finally {
if (conn != null) {
try {
conn.rollback();
} catch (SQLException sqlex) {
// Log error
}
try {
conn.close();
} catch (SQLException sqlex) {
// Log error
}
}
}
```
You may think I'm being a bit over the top with the trys, but in the real code there's often other stuff happening afterwards in the finally block that really has to run every time, otherwise the app will grind to a halt, like releasing locks. Also as all the methods have that bit it's now been extracted out into a helper method.
Then an audit of all one to two hundred methods where a database connection (and transaction) is opened, to make that change!
As well as the above change, I found a small number of methods, often called though, which returned the value immediately after the commit, before the catch block.
That's another problematic pattern - everything should work fine, but if the calling method and stack take a long time to complete, which they could well do, that will delay finally closing and returning the connection to the pool. Lots of simultaneous requests could exhaust the pool.
Some also used the auto-closeable property of a Connection so had no finally block at all. Those were replaced with the above.
One or maybe two two were just missing a finally block because it had been forgotten - doh!
So hopefully one of those things was the cause of the problem. We'll see soon enough. At least the code's been improved, whatever happens and this post should be a reminder to myself to stick to the rules!
@okohll I'd highly recommend using the try with resources syntax, it makes a lot of this much less error prone and cleaner.
Especially when using a pooler, calling close on the connection can leave the connection idle in a transaction.
Often it's worth just issuing the rollback and back to autocommit in a finally, small trade off for simplicity.
I tend to use a wrapper class which does the lower level connection management. Where auto close will issue a rollback and release the connection.
@intrbiz I would by preference use try with resources and skip the finally (generally do for prepared statements), but I don’t think the official Postgres JDBC driver does do rollback before close. I could be wrong - if there are any docs which say otherwise, they’d be good to see.