Fix several issues in recent alarms changes

The patch fixes several issues introduced in ff5e042: - dcache-common imports hikaricp, which means that several components that don't make use of databases now pull in database related code (like the srm client). - Missing @OverRide annotations in AlarmEnabledDataSource - Redundant shutdown method in AlarmEnabledDataSource (use close) - Infinite loop in uncaught exception handler (on IOException that isn't an out of file descriptor error, the code would loop forever) - Complex code before kill on fatal JVM error (which in case of OOM may lead to another OOM rather than a shutdown) To fix the HikariCP dependency I had to update to HikariCP 2.0.1. Target: trunk Require-notes: no Require-book: no Acked-by: Albert Rossi <arossi@fnal.gov> Patch: https://rb.dcache.org/r/7188/
dCache · Aug 8, 2014 · 4b10fe5 · 4b10fe5
1 parent e547954
commit 4b10fe5
Show file tree

Hide file tree

Showing 14 changed files with 61 additions and 55 deletions.
diff --git a/modules/acl/pom.xml b/modules/acl/pom.xml
@@ -24,7 +24,7 @@
     </dependency>
     <dependency>
       <groupId>com.zaxxer</groupId>
-      <artifactId>HikariCP</artifactId>
+      <artifactId>HikariCP-java6</artifactId>
     </dependency>
     <dependency>
       <groupId>org.dcache</groupId>

diff --git a/modules/cells/src/main/java/dmg/cells/nucleus/SystemCell.java b/modules/cells/src/main/java/dmg/cells/nucleus/SystemCell.java
@@ -1,9 +1,10 @@
 package dmg.cells.nucleus ;
 
+import com.google.common.base.Throwables;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.io.IOException;
+import java.io.FileNotFoundException;
 import java.io.PrintWriter;
 import java.io.Serializable;
 import java.net.InetAddress;
@@ -320,27 +321,25 @@ public void uncaughtException(Thread t, Throwable e)
          */
         if (e instanceof VirtualMachineError) {
             _oomSafetyBuffer = null;
+            kill();
             _log.error(AlarmMarkerFactory.getMarker(Severity.CRITICAL,
                                                     "FATAL_JVM_ERROR",
                                                     getCellDomainName(),
                                                     getCellName()),
-                                                    "Fatal JVM error", e);
-            _log.error("Shutting down...");
-            kill();
+                       "Restarting due to fatal JVM error", e);
+            return;
         }
 
-        if (e instanceof IOException) {
-            Throwable cause = e;
-            while(cause != null) {
-                if (cause.toString().contains("Too many open files")) {
-                    _log.error(AlarmMarkerFactory.getMarker(Severity.CRITICAL,
-                                                            "OUT_OF_FILE_DESCRIPTORS",
-                                                            getCellDomainName(),
-                                                            getCellName()),
-                                     "Uncaught exception in thread " + t.getName(),
-                                     e);
-                    return;
-                }
+        Throwable root = Throwables.getRootCause(e);
+        if (root instanceof FileNotFoundException) {
+            if (root.getMessage().contains("Too many open files")) {
+                _log.error(AlarmMarkerFactory.getMarker(Severity.CRITICAL,
+                                                        "OUT_OF_FILE_DESCRIPTORS",
+                                                        getCellDomainName(),
+                                                        getCellName()),
+                           "Uncaught exception in thread " + t.getName(),
+                           e);
+                return;
             }
         }
 

diff --git a/modules/common/pom.xml b/modules/common/pom.xml
@@ -46,10 +46,5 @@
       <groupId>org.bouncycastle</groupId>
       <artifactId>${bouncycastle.bcprov}</artifactId>
     </dependency>
-
-    <dependency>
-      <groupId>com.zaxxer</groupId>
-      <artifactId>HikariCP</artifactId>
-    </dependency>
   </dependencies>
 </project>
diff --git a/modules/common/src/main/java/org/dcache/db/AlarmEnabledDataSource.java b/modules/common/src/main/java/org/dcache/db/AlarmEnabledDataSource.java
@@ -59,7 +59,6 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
  */
 package org.dcache.db;
 
-import com.zaxxer.hikari.HikariDataSource;
 import org.slf4j.LoggerFactory;
 
 import javax.sql.DataSource;
@@ -76,6 +75,8 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
 import org.dcache.alarms.Severity;
 import org.dcache.util.NetworkUtils;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 /**
  * A decorator which allows exception thrown by the connection
  * factory to be marked as alarms.
@@ -93,34 +94,46 @@ public class AlarmEnabledDataSource implements DataSource, Closeable {
     private final String url;
 
     /**
-     * @param serviceName alarms will be identified by a combination of
+     * @param connectorName alarms will be identified by a combination of
      *        this name, the connection url and the canonical name of
      *        this client host.
      */
     public AlarmEnabledDataSource(String url,
                                   String connectorName,
                                   DataSource delegate) {
-        this.connectorName = connectorName;
-        this.url = url;
-        this.delegate = delegate;
+        this.connectorName = checkNotNull(connectorName);
+        this.url = checkNotNull(url);
+        this.delegate = checkNotNull(delegate);
     }
 
+    @Override
     public PrintWriter getLogWriter() throws SQLException {
         return delegate.getLogWriter();
     }
 
+    @Override
+    @SuppressWarnings("unchecked")
     public <T> T unwrap(Class<T> iface) throws SQLException {
+        if (iface.isInstance(delegate)) {
+            return (T) delegate;
+        }
         return delegate.unwrap(iface);
     }
 
+    @Override
     public void setLogWriter(PrintWriter out) throws SQLException {
         delegate.setLogWriter(out);
     }
 
+    @Override
     public boolean isWrapperFor(Class<?> iface) throws SQLException {
+        if (iface.isInstance(delegate)) {
+            return true;
+        }
         return delegate.isWrapperFor(iface);
     }
 
+    @Override
     public Connection getConnection() throws SQLException {
         try {
             return delegate.getConnection();
@@ -135,10 +148,12 @@ public Connection getConnection() throws SQLException {
         }
     }
 
+    @Override
     public void setLoginTimeout(int seconds) throws SQLException {
         delegate.setLoginTimeout(seconds);
     }
 
+    @Override
     public Connection getConnection(String username, String password)
                     throws SQLException {
 
@@ -155,33 +170,20 @@ public Connection getConnection(String username, String password)
         }
     }
 
+    @Override
     public int getLoginTimeout() throws SQLException {
         return delegate.getLoginTimeout();
     }
 
+    @Override
     public Logger getParentLogger() throws SQLFeatureNotSupportedException {
         return delegate.getParentLogger();
     }
 
-    public DataSource getDelegate() {
-        return delegate;
-    }
-
+    @Override
     public void close() throws IOException {
-        if (delegate instanceof HikariDataSource) {
-            ((HikariDataSource) delegate).shutdown();
-        } else if (delegate instanceof Closeable) {
+        if (delegate instanceof Closeable) {
             ((Closeable) delegate).close();
         }
     }
-
-    public void shutdown() {
-        try {
-            close();
-        } catch (IOException t) {
-            LOGGER.warn("{}: shutdown encountered a problem: {}",
-                            connectorName,
-                            t.getMessage() );
-        }
-    }
 }
diff --git a/modules/dcache-chimera/pom.xml b/modules/dcache-chimera/pom.xml
@@ -49,7 +49,7 @@
     </dependency>
     <dependency>
       <groupId>com.zaxxer</groupId>
-      <artifactId>HikariCP</artifactId>
+      <artifactId>HikariCP-java6</artifactId>
     </dependency>
     <dependency>
         <groupId>org.postgresql</groupId>

diff --git a/modules/dcache-chimera/src/main/java/org/dcache/chimera/namespace/ChimeraCleaner.java b/modules/dcache-chimera/src/main/java/org/dcache/chimera/namespace/ChimeraCleaner.java
@@ -13,6 +13,7 @@
 import org.springframework.jdbc.core.RowMapper;
 import org.springframework.jdbc.datasource.DriverManagerDataSource;
 
+import java.io.IOException;
 import java.io.PrintWriter;
 import java.net.URI;
 import java.net.URISyntaxException;
@@ -299,7 +300,11 @@ public void cleanUp()
             _broadcastRegistration.unregister();
         }
         if (_dataSource != null) {
-            _dataSource.shutdown();
+            try {
+                _dataSource.close();
+            } catch (IOException e) {
+                _log.debug("Failed to shutdown database connection pool: {}", e.getMessage());
+            }
         }
     }
 

diff --git a/modules/dcache-chimera/src/main/resources/diskCacheV111/namespace/pnfsmanager-chimera.xml b/modules/dcache-chimera/src/main/resources/diskCacheV111/namespace/pnfsmanager-chimera.xml
@@ -75,7 +75,7 @@
 
   <bean id="file-system" class="org.dcache.chimera.JdbcFs" depends-on="liquibase">
       <constructor-arg>
-        <bean class="org.dcache.db.AlarmEnabledDataSource" destroy-method="shutdown">
+        <bean class="org.dcache.db.AlarmEnabledDataSource" destroy-method="close">
             <description>Connection pool decorator</description>
             <constructor-arg value="${pnfsmanager.db.url}"/>
             <constructor-arg value="JdbcFs"/>

diff --git a/modules/dcache-nfs/src/main/resources/org/dcache/chimera/nfsv41/door/nfsv41-common.xml b/modules/dcache-nfs/src/main/resources/org/dcache/chimera/nfsv41/door/nfsv41-common.xml
@@ -77,7 +77,7 @@
         </constructor-arg>
     </bean>
 
-    <bean id="dataSource" class="org.dcache.db.AlarmEnabledDataSource" destroy-method="shutdown">
+    <bean id="dataSource" class="org.dcache.db.AlarmEnabledDataSource" destroy-method="close">
         <description>Connection pool decorator</description>
         <constructor-arg value="${nfs.db.url}"/>
         <constructor-arg value="DCacheAwareJdbcFs"/>

diff --git a/modules/dcache/pom.xml b/modules/dcache/pom.xml
@@ -76,7 +76,7 @@
     </dependency>
     <dependency>
       <groupId>com.zaxxer</groupId>
-      <artifactId>HikariCP</artifactId>
+      <artifactId>HikariCP-java6</artifactId>
     </dependency>
     <dependency>
         <groupId>org.postgresql</groupId>

diff --git a/modules/dcache/src/main/java/diskCacheV111/services/TransferManager.java b/modules/dcache/src/main/java/diskCacheV111/services/TransferManager.java
@@ -14,6 +14,7 @@
 import javax.jdo.PersistenceManager;
 import javax.jdo.Transaction;
 
+import java.io.IOException;
 import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.HashSet;
@@ -181,7 +182,11 @@ public void cleanUp()
     {
         super.cleanUp();
         if (ds != null) {
-            ds.shutdown();
+            try {
+                ds.close();
+            } catch (IOException e) {
+                log.debug("Failed to shutdown database connection pool: {}", e.getMessage());
+            }
         }
         executor.shutdown();
     }

diff --git a/modules/dcache/src/main/resources/diskCacheV111/services/space/spacemanager.xml b/modules/dcache/src/main/resources/diskCacheV111/services/space/spacemanager.xml
@@ -71,7 +71,7 @@
         </constructor-arg>
     </bean>
 
-    <bean id="data-source" class="org.dcache.db.AlarmEnabledDataSource" destroy-method="shutdown">
+    <bean id="data-source" class="org.dcache.db.AlarmEnabledDataSource" destroy-method="close">
             <description>Connection pool decorator</description>
             <constructor-arg value="${spacemanager.db.url}"/>
             <constructor-arg value="SpaceManager"/>

diff --git a/modules/dcache/src/main/resources/diskCacheV111/srm/srm.xml b/modules/dcache/src/main/resources/diskCacheV111/srm/srm.xml
@@ -457,7 +457,7 @@
         </constructor-arg>
     </bean>
 
-    <bean id="data-source" class="org.dcache.db.AlarmEnabledDataSource" destroy-method="shutdown">
+    <bean id="data-source" class="org.dcache.db.AlarmEnabledDataSource" destroy-method="close">
             <description>Connection pool decorator</description>
             <constructor-arg value="${srm.db.url}"/>
             <constructor-arg value="Srm"/>

diff --git a/modules/srm-server/pom.xml b/modules/srm-server/pom.xml
@@ -104,7 +104,7 @@
     </dependency>
     <dependency>
       <groupId>com.zaxxer</groupId>
-      <artifactId>HikariCP</artifactId>
+      <artifactId>HikariCP-java6</artifactId>
     </dependency>
 
     <dependency>

diff --git a/pom.xml b/pom.xml
@@ -286,8 +286,8 @@
             </dependency>
             <dependency>
                 <groupId>com.zaxxer</groupId>
-                <artifactId>HikariCP</artifactId>
-                <version>1.3.9</version>
+                <artifactId>HikariCP-java6</artifactId>
+                <version>2.0.1</version>
             </dependency>
             <dependency>
                 <groupId>org.postgresql</groupId>