Balazs, Balazs Michnay wrote: > recently I found > out that my memory consumption of my > application is nothing compared to the memory consumption of my database > server (MySQL). > > I'm theoretically using connection pool to save resources of my > database server, but each time I make a query, I have a brand new > connection.
Well... are you /theoretically/ or /actually/ using a JDBC connection pool?
> Why doesn't PID 11 use the connection used by PID 10?
> There might be
> some errors in my code, however, it was previously
> reviewed and said to be correct.
Do you trust the reviewer? Looking at the code you posted, I would not
trust them any more :(
> private Statement getConnectionStatement(Connection conn) throws Exception {
>
> Context ctx = new InitialContext();
> if(ctx == null )
> throw new Exception("Boom - No Context");
>
> Context envCtx = (Context) ctx.lookup("java:comp/env");
> DataSource ds = (DataSource) envCtx.lookup("jdbc/akr_db");
>
> if (ds != null) {
>
> conn = ds.getConnection();
> if(conn == null) throw new Exception();
>
> } else throw new Exception();
>
> return conn.createStatement();
> }
Do you pass an active connection to this method? If you do, then you are
leaking connections every time you try to create a statement. If you are
always passing "null", then why do you have the Connection parameter to
this method?
A proper method with the above signature should be implemented like this:
private Statement getConnectionStatement(Connection conn)
{
return conn.createStatement();
}
As you can see, this method is completely worthless. It looks like your
method ought to be this instead:
private Connection getConnection()
{
Context ctx = new InitialContext();
if(ctx == null )
throw new Exception("Boom - No Context");
Context envCtx = (Context) ctx.lookup("java:comp/env");
// Consider checking envCtx against null, here, too
// Consider making the JNDI resource name configurable
// instead of hard-coding.
DataSource ds = (DataSource) envCtx.lookup("jdbc/akr_db");
if (ds != null) {
conn = ds.getConnection();
if(conn == null) throw new Exception();
} else throw new Exception();
return conn;
}
The closeConnection method looks fine except for these style comments:
1. Call this method "close"... it closes much more than the connection.
2. Don't swallow the SQLExceptions. At least log them, but never
swallow an exception!
3. Don't bother setting each reference to null. This is a waste of time,
and clutters the code.
Selected lines from your code:
> Connection conn = null;
> try {
> stmt = getConnectionStatement(conn);
// Note that conn is still null here.
> } catch (Exception e) {
> System.err.println(e.getMessage());
> } finally {
> closeConnection(conn,stmt,rst);
> }
Conn is still null, so it will never be closed.
YOU ARE LEAKING CONNECTIONS. FIX YOUR CODE.
It should look like this:
Connection conn = null;
Statement stmt = null; // Or PreparedStatement if needed
ResultSet rst = null;
String query = "SELECT * FROM MyTable";
try {
conn = getConnection();
stmt = conn.createStatement();
rst = stmt.executeQuery(query);
// YOU SHOULD CHECK THIS FOR false:
rst.next();
this.setTartam(rst.getString("tartam"));
} catch (Exception e) {
System.err.println(e.getMessage());
} finally {
closeConnection(conn,stmt,rst);
}
Note that PreparedStatements are universally better than bare Statements
when using parameterized queries (which you are not using, here).
These changes should help A LOT. You are probably leaking connections
all over the place. I'm curious as to why you aren't getting messages on
stderr that your connection pool is empty...
-chris
signature.asc
Description: OpenPGP digital signature
