Changeset: 2103689a0acf for MonetDB
URL: http://dev.monetdb.org/hg/MonetDB?cmd=changeset;node=2103689a0acf
Modified Files:
        java/src/main/java/nl/cwi/monetdb/jdbc/MonetDatabaseMetaData.java
Branch: default
Log Message:

Detected a memory leak in the MonetDatabaseMetaData.java cache object:
   private static Map<Connection, Map<String,String>> envs = new 
HashMap<Connection, Map<String,String>>();
This Map caches environment table values per Connection object.
As this cache is defined as static (so global) and it only grows, the 
references to the MonetConnection object are kept (till the JVM stops).
This causes using up memory for the all the Connection (and associated objects 
as DatabaseMetaData, Statement and ResultSet) objects.
A test showed that after 10000 iterations of sequence:
        Connection con = DriverManager.getConnection(con_str);
        DatabaseMetaData md1 = con.getMetaData();
        String cu = md1.getUserName();  // this tiggers the filling of the cache
        String mv = md1.getDatabaseProductVersion();
        int mc = md1.getMaxConnections();
        con.close();
456654848 bytes of memory was used and calling the garbage collector explicitly 
(System.gc();) could not free the new used memory.
The memory consumption was growing with each iteration, so eventually would 
reach the max size of the allocated heap and return OutOfMemoryError.

Solved the issue by creating a much simpler non static cache (of only 3 String 
values) without keeping a reference to the Connection object.
Also instead of caching all 22 environment values (from sys.environment table) 
we only need and therefore cache now 3 values, so cache much less data.

After the fix the 10000 iterations test showed that the memory consumpting did 
not grow as fast as before
 and that the garbage collector successfully freed up the allocated objects, so 
resolving the memory leak.

I also removed the usage of getEnv("gdk_dbname") for retrieving and returning 
as catalog name.
MonetDB does not support catalog in SQL qualifiers as in 
<catalog>.<schema>.<table>
 we should not return a value for the CATALOG column in the DatabaseMetaData 
methods which return a ResultSet with a CATALOG column.
MonetDB only supports <schema>.<table> qualifiers in SQL.
It is not possible to use <databasename>.<schema>.<table> constructs in SQL, so 
also not to return the databasename as SQL catalog name.


diffs (truncated from 437 to 300 lines):

diff --git a/java/src/main/java/nl/cwi/monetdb/jdbc/MonetDatabaseMetaData.java 
b/java/src/main/java/nl/cwi/monetdb/jdbc/MonetDatabaseMetaData.java
--- a/java/src/main/java/nl/cwi/monetdb/jdbc/MonetDatabaseMetaData.java
+++ b/java/src/main/java/nl/cwi/monetdb/jdbc/MonetDatabaseMetaData.java
@@ -8,19 +8,30 @@
 
 package nl.cwi.monetdb.jdbc;
 
-import java.sql.*;
-import java.util.*;
+import java.sql.Connection;
+import java.sql.DatabaseMetaData;
+import java.sql.Driver;
+import java.sql.Statement;
+import java.sql.SQLException;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.RowIdLifetime;
+import java.sql.Types;
+
+import java.util.ArrayList;
 
 /**
  * A DatabaseMetaData object suitable for the MonetDB database.
  * 
  *
- * @author Fabian Groffen
- * @version 0.5
+ * @author Fabian Groffen, Martin van Dinther
+ * @version 0.6
  */
 public class MonetDatabaseMetaData extends MonetWrapper implements 
DatabaseMetaData {
        private Connection con;
-       private static Map<Connection, Map<String,String>> envs = new 
HashMap<Connection, Map<String,String>>();
+       private String env_current_user;
+       private String env_monet_version;
+       private String env_max_clients;
 
        public MonetDatabaseMetaData(Connection parent) {
                con = parent;
@@ -36,58 +47,50 @@ public class MonetDatabaseMetaData exten
        }
 
        /**
-        * Internal cache for environment properties retrieved from the
-        * server.  To avoid querying the server over and over again, once a
-        * value is read, it is kept in a Map for reuse.
+        * Internal cache for 3 environment properties retrieved from the
+        * server, to avoid querying the server over and over again.
+        * Once a value is read, it is kept in the private env_* variables for 
reuse.
+        * We currently only need the env values of: current_user, 
monet_version and max_clients.
         */
-       private synchronized String getEnv(String key) {
-               // if due to concurrency on this Class envs is assigned twice, I
-               // just don't care here
-               Map<String,String> menvs = envs.get(con);
-               if (menvs == null) {
-                       // make the env map, insert all entries from env()
-                       menvs = new HashMap<String,String>();
-                       try {
-                               ResultSet env = getStmt().executeQuery(
-                                               "SELECT \"name\", \"value\" 
FROM sys.env() as env");
+       private synchronized void getEnvProperties() {
+               Statement st = null;
+               ResultSet rs = null;
+               try {
+                       st = getStmt();
+                       rs = st.executeQuery(
+                               "SELECT \"name\", \"value\" FROM 
\"sys\".\"environment\"" +
+                               " WHERE \"name\" IN ('monet_version', 
'max_clients')" +
+                               " UNION SELECT 'current_user' as \"name\", 
current_user as \"value\"");
+                       if (rs != null) {
+                               while (rs.next()) {
+                                       String prop = rs.getString("name");
+                                       String value = rs.getString("value");
+                                       if ("current_user".equals(prop)) {
+                                               env_current_user = value;
+                                       } else
+                                       if ("monet_version".equals(prop)) {
+                                               env_monet_version = value;
+                                       } else
+                                       if ("max_clients".equals(prop)) {
+                                               env_max_clients = value;
+                                       }
+                               }
+                       }
+               } catch (SQLException e) {
+                       // ignore
+               } finally {
+                       if (rs != null) {
                                try {
-                                       while (env.next()) {
-                                               
menvs.put(env.getString("name"), env.getString("value"));
-                                       }
-                               } finally {
-                                       env.close();
-                               }
-                       } catch (SQLException e) {
-                               // ignore
+                                       rs.close();
+                               } catch (SQLException e) { /* ignore */ }
                        }
-                       envs.put(con, menvs);
-               }
-               
-               String ret = menvs.get(key);
-               if (ret == null) {
-                       // It might be some key from the "sessions" table, 
which is
-                       // no longer there, but available as variables.  Just 
query
-                       // for the variable, and hope that it is ok (not a user
-                       // variable, etc.)  In general, it should be ok, because
-                       // it's only used inside this class.
-                       // The effects of caching a variable, might be
-                       // undesirable... TODO
-                       try {
-                               ResultSet env = getStmt().executeQuery(
-                                               "SELECT @\"" + key + "\" AS 
\"value\"");
+                       if (st != null) {
                                try {
-                                       if (env.next()) {
-                                               ret = env.getString("value");
-                                               menvs.put(key, ret);
-                                       }
-                               } finally {
-                                       env.close();
-                               }
-                       } catch (SQLException e) {
-                               // ignore
+                                        st.close();
+                               } catch (SQLException e) { /* ignore */ }
                        }
                }
-               return ret;
+// for debug: System.out.println("Read: env_current_user: " + env_current_user 
+ "  env_monet_version: " + env_monet_version + "  env_max_clients: " + 
env_max_clients);
        }
 
        /**
@@ -131,7 +134,9 @@ public class MonetDatabaseMetaData exten
         */
        @Override
        public String getUserName() throws SQLException {
-               return getEnv("current_user");
+               if (env_current_user == null)
+                       getEnvProperties();
+               return env_current_user;
        }
 
        /**
@@ -204,7 +209,9 @@ public class MonetDatabaseMetaData exten
         */
        @Override
        public String getDatabaseProductVersion() throws SQLException {
-               return getEnv("monet_version");
+               if (env_monet_version == null)
+                       getEnvProperties();
+               return env_monet_version;
        }
 
        /**
@@ -552,7 +559,7 @@ public class MonetDatabaseMetaData exten
        /**
         * Get all the "extra" characters that can be used in unquoted
         * identifier names (those beyond a-zA-Z0-9 and _)
-        * MonetDB has no extras
+        * MonetDB has no extra characters (verified it for chars: 
!@#$%^&*()~{}[]?
         *
         * @return a string containing the extra characters
         */
@@ -1290,11 +1297,15 @@ public class MonetDatabaseMetaData exten
         */
        @Override
        public int getMaxConnections() {
+               if (env_max_clients == null)
+                       getEnvProperties();
+
                int max_clients = 16;
-               try {
-                       String max_clients_val = getEnv("max_clients");
-                       max_clients = Integer.parseInt(max_clients_val);
-               } catch (NumberFormatException nfe) { /* ignore */ }
+               if (env_max_clients != null) {
+                       try {
+                               max_clients = Integer.parseInt(env_max_clients);
+                       } catch (NumberFormatException nfe) { /* ignore */ }
+               }
                return max_clients;
        }
 
@@ -1714,7 +1725,6 @@ public class MonetDatabaseMetaData exten
                String types[]
        ) throws SQLException
        {
-               String cat = getEnv("gdk_dbname");
                // as of Jul2015 release the sys.tables.type values (0 through 
6) is extended with new values 10, 11, 20, and 30 (for system and temp 
tables/views).
                // for correct behavior we need to know if the server is using 
the old (pre Jul2015) or new sys.tables.type values
                boolean preJul2015 = 
("11.19.15".compareTo(getDatabaseProductVersion()) >= 0);
@@ -1722,7 +1732,7 @@ public class MonetDatabaseMetaData exten
 
                String query =
                        "SELECT * FROM ( " +
-                       "SELECT '" + cat + "' AS \"TABLE_CAT\", 
\"schemas\".\"name\" AS \"TABLE_SCHEM\", \"tables\".\"name\" AS \"TABLE_NAME\", 
" +
+                       "SELECT cast(null as char(1)) AS \"TABLE_CAT\", 
\"schemas\".\"name\" AS \"TABLE_SCHEM\", \"tables\".\"name\" AS \"TABLE_NAME\", 
" +
                                "CASE WHEN \"tables\".\"system\" = true AND 
\"tables\".\"type\" = " + (preJul2015 ? "0" : "10") + " AND 
\"tables\".\"temporary\" = 0 THEN 'SYSTEM TABLE' " +
                                "WHEN \"tables\".\"system\" = true AND 
\"tables\".\"type\" = " + (preJul2015 ? "1" : "11") + " AND 
\"tables\".\"temporary\" = 0 THEN 'SYSTEM VIEW' " +
                                "WHEN \"tables\".\"system\" = false AND 
\"tables\".\"type\" = 0 AND \"tables\".\"temporary\" = 0 THEN 'TABLE' " +
@@ -1780,17 +1790,13 @@ public class MonetDatabaseMetaData exten
        public ResultSet getSchemas(String catalog, String schemaPattern)
                throws SQLException
        {
-               String cat = getEnv("gdk_dbname");
                String query =
                        "SELECT \"name\" AS \"TABLE_SCHEM\", " +
-                               "'" + cat + "' AS \"TABLE_CATALOG\", " +
-                               "'" + cat + "' AS \"TABLE_CAT\" " +     // 
SquirrelSQL requests this one...
-                       "FROM \"sys\".\"schemas\" " +
-                       "WHERE 1 = 1 ";
-               if (catalog != null)
-                       query += "AND '" + cat + "' ILIKE '" + 
escapeQuotes(catalog) + "' ";
+                               "cast(null as char(1)) AS \"TABLE_CATALOG\", " +
+                               "cast(null as char(1)) AS \"TABLE_CAT\" " +     
// SquirrelSQL requests this one...
+                       "FROM \"sys\".\"schemas\" ";
                if (schemaPattern != null)
-                       query += "AND \"name\" ILIKE '" + 
escapeQuotes(schemaPattern) + "' ";
+                       query += "WHERE \"name\" ILIKE '" + 
escapeQuotes(schemaPattern) + "' ";
                query += "ORDER BY \"TABLE_SCHEM\"";
 
                return getStmt().executeQuery(query);
@@ -1830,7 +1836,7 @@ public class MonetDatabaseMetaData exten
 
                columns[0] = "TABLE_CAT";
                types[0] = "varchar";
-               results[0][0] = getEnv("gdk_dbname");
+               results[0][0] = "";
 
                try {
                        return new MonetVirtualResultSet(columns, types, 
results);
@@ -1941,9 +1947,8 @@ public class MonetDatabaseMetaData exten
                String columnNamePattern
        ) throws SQLException
        {
-               String cat = getEnv("gdk_dbname");
                String query =
-                       "SELECT '" + cat + "' AS \"TABLE_CAT\", 
\"schemas\".\"name\" AS \"TABLE_SCHEM\", " +
+                       "SELECT cast(null as char(1)) AS \"TABLE_CAT\", 
\"schemas\".\"name\" AS \"TABLE_SCHEM\", " +
                        "\"tables\".\"name\" AS \"TABLE_NAME\", 
\"columns\".\"name\" AS \"COLUMN_NAME\", " +
                        "cast(" + 
MonetDriver.getSQLTypeMap("\"columns\".\"type\"") + " " +
                        "AS smallint) AS \"DATA_TYPE\", " +
@@ -2024,9 +2029,8 @@ public class MonetDatabaseMetaData exten
                String columnNamePattern
        ) throws SQLException
        {
-               String cat = getEnv("gdk_dbname");
                String query =
-               "SELECT '" + cat + "' AS \"TABLE_CAT\", " +
+               "SELECT cast(null as char(1)) AS \"TABLE_CAT\", " +
                        "\"schemas\".\"name\" AS \"TABLE_SCHEM\", " +
                        "\"tables\".\"name\" AS \"TABLE_NAME\", " +
                        "\"columns\".\"name\" AS \"COLUMN_NAME\", " +
@@ -2107,9 +2111,8 @@ public class MonetDatabaseMetaData exten
                String tableNamePattern
        ) throws SQLException
        {
-               String cat = getEnv("gdk_dbname");
                String query =
-               "SELECT '" + cat + "' AS \"TABLE_CAT\", " +
+               "SELECT cast(null as char(1)) AS \"TABLE_CAT\", " +
                        "\"schemas\".\"name\" AS \"TABLE_SCHEM\", " +
                        "\"tables\".\"name\" AS \"TABLE_NAME\", " +
                        "\"grantors\".\"name\" AS \"GRANTOR\", " +
@@ -2419,8 +2422,7 @@ public class MonetDatabaseMetaData exten
        public ResultSet getImportedKeys(String catalog, String schema, String 
table)
                throws SQLException
        {
-               String cat = getEnv("gdk_dbname");
-               String query = keyQuery(cat);
+               String query = keyQuery("");
 
                if (schema != null) {
                        query += "AND \"fkschema\".\"name\" ILIKE '" + 
escapeQuotes(schema) + "' ";
@@ -2490,8 +2492,7 @@ public class MonetDatabaseMetaData exten
        public ResultSet getExportedKeys(String catalog, String schema, String 
table)
                throws SQLException
        {
-               String cat = getEnv("gdk_dbname");
-               String query = keyQuery(cat);
+               String query = keyQuery("");
 
                if (schema != null) {
                        query += "AND \"pkschema\".\"name\" ILIKE '" + 
escapeQuotes(schema) + "' ";
@@ -2576,8 +2577,7 @@ public class MonetDatabaseMetaData exten
                String ftable
        ) throws SQLException
        {
-               String cat = getEnv("gdk_dbname");
-               String query = keyQuery(cat);
+               String query = keyQuery("");
 
                if (pschema != null) {
                        query += "AND \"pkschema\".\"name\" ILIKE '" + 
escapeQuotes(pschema) + "' ";
@@ -2750,10 +2750,9 @@ public class MonetDatabaseMetaData exten
                boolean approximate
_______________________________________________
checkin-list mailing list
[email protected]
https://www.monetdb.org/mailman/listinfo/checkin-list

Reply via email to