jackrabbit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Mueller (JIRA)" <j...@apache.org>
Subject [jira] Commented: (JCR-940) add db connection autoConnect for BundleDbPersistenceManager.
Date Fri, 07 Sep 2007 07:55:31 GMT

    [ https://issues.apache.org/jira/browse/JCR-940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12525632
] 

Thomas Mueller commented on JCR-940:
------------------------------------

Hi,
I have a few comments:

SLEEP_BEFORE_RECONNECT = 10000;
TRIALS = 1;
I would use:
SLEEP_BEFORE_RECONNECT = 500;
TRIALS = 20;

public synchronized boolean addStatement(String sql) {
...
        log.warn("failed to add statement");
        return false;
    }

It's easy to forget checking the return value. Actually, you did _not_ check it.
I would throw an exception if preparing a statement failed.

initPreparedStatements: why not prepare the statements when they are first used?
Like that you only prepare statements that are actually used. This saves time. So I would
change:

PreparedStatement stmt = (PreparedStatement) preparedStatements.get(sql);
if (stmt == null) {
  throw new Exception("unknown SQL statement: " + sql);
}

to

PreparedStatement stmt = (PreparedStatement) preparedStatements.get(sql);
if (stmt == null) {
  stmt = connection.prepareStatement(sql);
  preparedStatements.add(sql, stmt);
}

There is anyway a loop to reconnect if this failed. So you can get rid of addStatement(..),
rePrepareStatements, and sqlStatements.

+                        if (params[i] instanceof Long) {
+                            stmt.setLong(i + 1, ((Long) params[i]).longValue());
+                        } else if (params[i] instanceof byte[]) {
+                            stmt.setBytes(i + 1, (byte[]) params[i]);
+                        } else if (params[i] instanceof Blob) {
+                            stmt.setBlob(i + 1, (Blob) params[i]);
+                        } else {
+                            stmt.setObject(i + 1, params[i]);

Only the last line is required.

// close shared prepared statements

This is not required. If the connection is closed, the prepared statements are closed. Just
calling preparedStatements.clear() is enough.

boolean reestablishConnection(..)

Again, using a return value...


private Set sqlStatements = new HashSet();


-                stmt = con.prepareStatement(
-                        "select NODE_ID, BUNDLE_DATA from "
-                        + schemaObjectPrefix + "BUNDLE");
+                stmt = connectionManager.getConnection().prepareStatement("select NODE_ID,
BUNDLE_DATA from BUNDLE");

The schemaObjectPrefix is lost here.


> add db connection autoConnect for BundleDbPersistenceManager.
> -------------------------------------------------------------
>
>                 Key: JCR-940
>                 URL: https://issues.apache.org/jira/browse/JCR-940
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: core
>    Affects Versions: 1.3
>            Reporter: Xiaohua Lu
>         Attachments: JCR-940-v2.patch, JCR-940.patch
>
>
> Since bundled db pm doesn't inherited from database pm, it can't reconnect once database
is bounced. it would be nice to add this feature. 

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message