[
https://issues.apache.org/jira/browse/JCR-940?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12525653
]
Thomas Mueller commented on JCR-940:
------------------------------------
> store: log.warn("storing and committing changes failed: " + e.getMessage());
exception stack traces must be logged as well:
log.warn("storing and committing changes failed: " + e.getMessage(), e);
> stmt = connectionManager.getConnection().prepareStatement("select NODE_ID,
> BUNDLE_DATA from BUNDLE");
The SQL statement should stored in one of the member variables in my view (but
it was also not done before...).
+ if (!connectionManager.setAutoCommit(false)) {
+ throw new ItemStateException("disabling autoCommit failed");
Using a return value for an exception.
+ Object[] keys = getKey(bundle.getId().getUUID());
+ Object[] params = new Object[keys.length + 1];
+ params[0] = out.toByteArray();
+ for (int i = 1; i < params.length; i++) {
+ params[i] = keys[i-1];
}
What about using an ArrayList:
List params = getKey(bundle.getId().getUUID());
params.add(out.toByteArray());
- stmt.setBytes(2, bundle.getId().getUUID().getRawBytes());
This is code you removed... Strange, how did this work with getStorageModel()
!= SM_BINARY_KEYS?
+ Connection con = connectionManager.getConnection();
+ if (con != null) {
+ DatabaseMetaData metaData = con.getMetaData();
+ if (metaData.getDriverMajorVersion() < 10) {
+ Connection con = connectionManager.getConnection();
+ if (con != null) {
+ try {
+ con.rollback();
In some places the return value is checked, in other places not. I would make
sure getConnection never returns null, otherwise you get NullPointerExceptions
in strange places, and you don't really know what was the reason.
+ /**
+ * @return the database connection that is managed, possibly null
+ */
+ public synchronized Connection getConnection() {
+ int trials = TRIALS;
+ while (trials-- > 0) {
+
+ // First, try to reconnect if needed
+ if (isClosed && autoReconnect) {
+ reestablishConnection();
+ }
+
+ // Then, try to return the connection
+ if (!isClosed) {
+ return connection;
+ }
+ }
+ log.warn("failed to get connection");
+ return null;
+ }
I would write:
+ /**
+ * @return the database connection that is managed, possibly null
+ */
+ public synchronized Connection getConnection() throws SQLException {
+ // reconnect if needed
+ if (isClosed && autoReconnect) {
+ reestablishConnection();
+ }
+ return connection;
+ }
In my view, reestablishConnection should loop, and if it can't connect, throw
an exception. And once it failed, it should set autoReconnect to false,
otherwise other code will try to re-connect again and again (maybe endlessly).
You have used reestablishConnection in many places in the code, I think it
should only be called in one place.
Thomas
> 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.
