You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by jh...@apache.org on 2023/01/17 21:55:53 UTC

[james-project] 01/01: [JAMES-3877] enables configuration of jdbc pool max connections

This is an automated email from the ASF dual-hosted git repository.

jhelou pushed a commit to branch james-3877
in repository https://gitbox.apache.org/repos/asf/james-project.git

commit a7aa5f133a2d78b67dfaaa2f01312d9f603d8997
Author: Jean Helou <jh...@codamens.fr>
AuthorDate: Sun Jan 15 08:07:52 2023 +0100

    [JAMES-3877] enables configuration of jdbc pool max connections
---
 .../james-database-mariadb.properties              |  2 +
 .../sample-configuration/james-database.properties |  2 +
 .../james-database-mariadb.properties              |  2 +
 .../sample-configuration/james-database.properties |  2 +
 .../james/modules/data/JPAConfiguration.java       | 30 ++++++++++---
 .../james/modules/data/JPAEntityManagerModule.java |  6 ++-
 .../james/modules/data/JPAConfigurationTest.java   | 51 ++++++++++++++++++++--
 src/site/xdoc/server/config-system.xml             | 21 ++++++---
 8 files changed, 99 insertions(+), 17 deletions(-)

diff --git a/server/apps/jpa-app/sample-configuration/james-database-mariadb.properties b/server/apps/jpa-app/sample-configuration/james-database-mariadb.properties
index 8060df0018..cf091319a4 100644
--- a/server/apps/jpa-app/sample-configuration/james-database-mariadb.properties
+++ b/server/apps/jpa-app/sample-configuration/james-database-mariadb.properties
@@ -40,3 +40,5 @@ openjpa.streaming=false
 # datasource.validationQueryTimeoutSec=2
 # This is different per database. See https://stackoverflow.com/questions/10684244/dbcp-validationquery-for-different-databases#10684260
 # datasource.validationQuery=select 1
+# The maximum number of active connections that can be allocated from this pool at the same time, or negative for no limit.
+# datasource.maxTotal=8
diff --git a/server/apps/jpa-app/sample-configuration/james-database.properties b/server/apps/jpa-app/sample-configuration/james-database.properties
index 0da699d9b8..d569d6d5d6 100644
--- a/server/apps/jpa-app/sample-configuration/james-database.properties
+++ b/server/apps/jpa-app/sample-configuration/james-database.properties
@@ -44,3 +44,5 @@ openjpa.streaming=false
 # datasource.validationQueryTimeoutSec=2
 # This is different per database. See https://stackoverflow.com/questions/10684244/dbcp-validationquery-for-different-databases#10684260
 # datasource.validationQuery=select 1
+# The maximum number of active connections that can be allocated from this pool at the same time, or negative for no limit.
+# datasource.maxTotal=8
diff --git a/server/apps/jpa-smtp-app/sample-configuration/james-database-mariadb.properties b/server/apps/jpa-smtp-app/sample-configuration/james-database-mariadb.properties
index 8060df0018..cf091319a4 100644
--- a/server/apps/jpa-smtp-app/sample-configuration/james-database-mariadb.properties
+++ b/server/apps/jpa-smtp-app/sample-configuration/james-database-mariadb.properties
@@ -40,3 +40,5 @@ openjpa.streaming=false
 # datasource.validationQueryTimeoutSec=2
 # This is different per database. See https://stackoverflow.com/questions/10684244/dbcp-validationquery-for-different-databases#10684260
 # datasource.validationQuery=select 1
+# The maximum number of active connections that can be allocated from this pool at the same time, or negative for no limit.
+# datasource.maxTotal=8
diff --git a/server/apps/jpa-smtp-app/sample-configuration/james-database.properties b/server/apps/jpa-smtp-app/sample-configuration/james-database.properties
index 0da699d9b8..d569d6d5d6 100644
--- a/server/apps/jpa-smtp-app/sample-configuration/james-database.properties
+++ b/server/apps/jpa-smtp-app/sample-configuration/james-database.properties
@@ -44,3 +44,5 @@ openjpa.streaming=false
 # datasource.validationQueryTimeoutSec=2
 # This is different per database. See https://stackoverflow.com/questions/10684244/dbcp-validationquery-for-different-databases#10684260
 # datasource.validationQuery=select 1
+# The maximum number of active connections that can be allocated from this pool at the same time, or negative for no limit.
+# datasource.maxTotal=8
diff --git a/server/container/guice/jpa-common/src/main/java/org/apache/james/modules/data/JPAConfiguration.java b/server/container/guice/jpa-common/src/main/java/org/apache/james/modules/data/JPAConfiguration.java
index 46f7309a1e..bed6c9041a 100644
--- a/server/container/guice/jpa-common/src/main/java/org/apache/james/modules/data/JPAConfiguration.java
+++ b/server/container/guice/jpa-common/src/main/java/org/apache/james/modules/data/JPAConfiguration.java
@@ -19,6 +19,7 @@
 package org.apache.james.modules.data;
 
 import static org.apache.james.modules.data.JPAConfiguration.Credential.NO_CREDENTIAL;
+import static org.apache.james.modules.data.JPAConfiguration.ReadyToBuild.NO_MAX_CONNECTIONS;
 import static org.apache.james.modules.data.JPAConfiguration.ReadyToBuild.NO_TEST_ON_BORROW;
 import static org.apache.james.modules.data.JPAConfiguration.ReadyToBuild.NO_VALIDATION_QUERY;
 import static org.apache.james.modules.data.JPAConfiguration.ReadyToBuild.NO_VALIDATION_QUERY_TIMEOUT_SEC;
@@ -33,7 +34,6 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 
 public class JPAConfiguration {
-
     public static class Credential {
         private static final Logger LOGGER = LoggerFactory.getLogger(Credential.class);
         static final Optional<Credential> NO_CREDENTIAL = Optional.empty();
@@ -80,6 +80,7 @@ public class JPAConfiguration {
         static final Optional<Boolean> NO_TEST_ON_BORROW = Optional.empty();
         static final Optional<Integer> NO_VALIDATION_QUERY_TIMEOUT_SEC = Optional.empty();
         static final Optional<String> NO_VALIDATION_QUERY = Optional.empty();
+        static final Optional<Integer> NO_MAX_CONNECTIONS = Optional.empty();
 
         private final String driverName;
         private final String driverURL;
@@ -88,26 +89,29 @@ public class JPAConfiguration {
         private Optional<Boolean> testOnBorrow;
         private Optional<Integer> validationQueryTimeoutSec;
         private Optional<String> validationQuery;
+        private Optional<Integer> maxConnections;
 
 
         private ReadyToBuild(String driverName, String driverURL, Optional<Credential> credential,
                             Optional<Boolean> testOnBorrow, Optional<Integer> validationQueryTimeoutSec,
-                            Optional<String> validationQuery) {
+                            Optional<String> validationQuery,Optional<Integer> maxConnections
+        ) {
             this.driverName = driverName;
             this.driverURL = driverURL;
             this.credential = credential;
             this.testOnBorrow = testOnBorrow;
             this.validationQueryTimeoutSec = validationQueryTimeoutSec;
             this.validationQuery = validationQuery;
+            this.maxConnections = maxConnections;
         }
 
         public JPAConfiguration build() {
-            return new JPAConfiguration(driverName, driverURL, credential, testOnBorrow, validationQueryTimeoutSec, validationQuery);
+            return new JPAConfiguration(driverName, driverURL, credential, testOnBorrow, validationQueryTimeoutSec, validationQuery, maxConnections);
         }
 
         public RequirePassword username(String username) {
             return password -> new ReadyToBuild(driverName, driverURL, Credential.of(username, password),
-                testOnBorrow, validationQueryTimeoutSec, validationQuery);
+                testOnBorrow, validationQueryTimeoutSec, validationQuery, maxConnections);
         }
 
         public ReadyToBuild testOnBorrow(Boolean testOnBorrow) {
@@ -124,6 +128,11 @@ public class JPAConfiguration {
             this.validationQuery = Optional.ofNullable(validationQuery);
             return this;
         }
+
+        public ReadyToBuild maxConnections(Integer maxConnections) {
+            this.maxConnections = Optional.ofNullable(maxConnections);
+            return this;
+        }
     }
 
     @FunctionalInterface
@@ -133,7 +142,7 @@ public class JPAConfiguration {
 
     public static RequireDriverName builder() {
         return driverName -> driverURL -> new ReadyToBuild(driverName, driverURL, NO_CREDENTIAL, NO_TEST_ON_BORROW,
-            NO_VALIDATION_QUERY_TIMEOUT_SEC, NO_VALIDATION_QUERY);
+            NO_VALIDATION_QUERY_TIMEOUT_SEC, NO_VALIDATION_QUERY, NO_MAX_CONNECTIONS);
     }
 
     private final String driverName;
@@ -142,14 +151,17 @@ public class JPAConfiguration {
     private final Optional<Integer> validationQueryTimeoutSec;
     private final Optional<Credential> credential;
     private final Optional<String> validationQuery;
+    private final Optional<Integer> maxConnections;
 
     @VisibleForTesting
     JPAConfiguration(String driverName, String driverURL, Optional<Credential> credential, Optional<Boolean> testOnBorrow,
-                     Optional<Integer> validationQueryTimeoutSec, Optional<String> validationQuery) {
+                     Optional<Integer> validationQueryTimeoutSec, Optional<String> validationQuery, Optional<Integer> maxConnections) {
         Preconditions.checkNotNull(driverName, "driverName cannot be null");
         Preconditions.checkNotNull(driverURL, "driverURL cannot be null");
         validationQueryTimeoutSec.ifPresent(timeoutInSec ->
             Preconditions.checkArgument(timeoutInSec > 0, "validationQueryTimeoutSec is required to be greater than 0"));
+        maxConnections.ifPresent(maxCon ->
+            Preconditions.checkArgument(maxCon == -1 || maxCon > 0, "maxConnections is required to be -1 (no limit) or  greater than 0"));
 
         this.driverName = driverName;
         this.driverURL = driverURL;
@@ -157,6 +169,7 @@ public class JPAConfiguration {
         this.testOnBorrow = testOnBorrow;
         this.validationQueryTimeoutSec = validationQueryTimeoutSec;
         this.validationQuery = validationQuery;
+        this.maxConnections = maxConnections;
     }
 
     public String getDriverName() {
@@ -182,4 +195,9 @@ public class JPAConfiguration {
     public Optional<Credential> getCredential() {
         return credential;
     }
+
+
+    public Optional<Integer> getMaxConnections() {
+        return maxConnections;
+    }
 }
diff --git a/server/container/guice/jpa-common/src/main/java/org/apache/james/modules/data/JPAEntityManagerModule.java b/server/container/guice/jpa-common/src/main/java/org/apache/james/modules/data/JPAEntityManagerModule.java
index 0823ae11bb..fec3e2422c 100644
--- a/server/container/guice/jpa-common/src/main/java/org/apache/james/modules/data/JPAEntityManagerModule.java
+++ b/server/container/guice/jpa-common/src/main/java/org/apache/james/modules/data/JPAEntityManagerModule.java
@@ -40,7 +40,7 @@ public class JPAEntityManagerModule extends AbstractModule {
     @Singleton
     public EntityManagerFactory provideEntityManagerFactory(JPAConfiguration jpaConfiguration) {
         HashMap<String, String> properties = new HashMap<>();
-        
+
         properties.put("openjpa.ConnectionDriverName", jpaConfiguration.getDriverName());
         properties.put("openjpa.ConnectionURL", jpaConfiguration.getDriverURL());
         jpaConfiguration.getCredential()
@@ -55,6 +55,9 @@ public class JPAEntityManagerModule extends AbstractModule {
             .ifPresent(timeoutSecond -> connectionFactoryProperties.add("ValidationTimeout=" + timeoutSecond * 1000));
         jpaConfiguration.getValidationQuery()
             .ifPresent(validationQuery -> connectionFactoryProperties.add("ValidationSQL='" + validationQuery + "'"));
+        jpaConfiguration.getMaxConnections().ifPresent(maxConnections ->
+                connectionFactoryProperties.add("MaxTotal="+ maxConnections)
+        );
 
         properties.put("openjpa.ConnectionFactoryProperties", Joiner.on(", ").join(connectionFactoryProperties));
 
@@ -71,6 +74,7 @@ public class JPAEntityManagerModule extends AbstractModule {
                 .testOnBorrow(dataSource.getBoolean("datasource.testOnBorrow", false))
                 .validationQueryTimeoutSec(dataSource.getInteger("datasource.validationQueryTimeoutSec", null))
                 .validationQuery(dataSource.getString("datasource.validationQuery", null))
+                .maxConnections(dataSource.getInteger("datasource.maxTotal",null))
                 .username(dataSource.getString("database.username"))
                 .password(dataSource.getString("database.password"))
                 .build();
diff --git a/server/container/guice/jpa-common/src/test/java/org/apache/james/modules/data/JPAConfigurationTest.java b/server/container/guice/jpa-common/src/test/java/org/apache/james/modules/data/JPAConfigurationTest.java
index 967d252d63..b81fd7fb52 100644
--- a/server/container/guice/jpa-common/src/test/java/org/apache/james/modules/data/JPAConfigurationTest.java
+++ b/server/container/guice/jpa-common/src/test/java/org/apache/james/modules/data/JPAConfigurationTest.java
@@ -32,6 +32,7 @@ class JPAConfigurationTest {
     private static final boolean TEST_ON_BORROW = true;
     private static final String VALIDATION_QUERY = "validationQuery";
     private static final int VALIDATION_TIMEOUT_SEC = 1;
+    private static final int MAX_CONNECTIONS = 5;
     private static final String USER_NAME = "username";
     private static final String PASSWORD = "password";
     private static final String EMPTY_STRING = "";
@@ -46,6 +47,7 @@ class JPAConfigurationTest {
             .validationQueryTimeoutSec(VALIDATION_TIMEOUT_SEC)
             .username(USER_NAME)
             .password(PASSWORD)
+            .maxConnections(MAX_CONNECTIONS)
             .build();
 
         SoftAssertions.assertSoftly(softly -> {
@@ -55,9 +57,10 @@ class JPAConfigurationTest {
             softly.assertThat(configuration.getValidationQuery()).contains(VALIDATION_QUERY);
             softly.assertThat(configuration.getValidationQueryTimeoutSec()).contains(VALIDATION_TIMEOUT_SEC);
             softly.assertThat(configuration.getCredential()).hasValueSatisfying(credential -> {
-                    softly.assertThat(credential.getPassword()).isEqualTo(PASSWORD);
-                    softly.assertThat(credential.getUsername()).isEqualTo(USER_NAME);
-                });
+                softly.assertThat(credential.getPassword()).isEqualTo(PASSWORD);
+                softly.assertThat(credential.getUsername()).isEqualTo(USER_NAME);
+            });
+            softly.assertThat(configuration.getMaxConnections()).contains(MAX_CONNECTIONS);
         });
     }
 
@@ -76,6 +79,7 @@ class JPAConfigurationTest {
             softly.assertThat(configuration.getValidationQuery()).isEmpty();
             softly.assertThat(configuration.getValidationQueryTimeoutSec()).isEmpty();
             softly.assertThat(configuration.getCredential()).isEmpty();
+            softly.assertThat(configuration.getMaxConnections()).isEmpty();
         });
     }
 
@@ -90,6 +94,47 @@ class JPAConfigurationTest {
             .hasMessage("validationQueryTimeoutSec is required to be greater than 0");
     }
 
+    @Test
+    void buildShouldThrowWhenMaxConnectionIsZero() {
+        assertThatThrownBy(() -> JPAConfiguration.builder()
+                .driverName(DRIVER_NAME)
+                .driverURL(DRIVER_URL)
+                .maxConnections(0)
+                .build())
+            .isInstanceOf(IllegalArgumentException.class)
+            .hasMessage("maxConnections is required to be -1 (no limit) or  greater than 0");
+    }
+    @Test
+    void buildShouldThrowWhenMaxConnectionIsLesThanMinusOne() {
+        assertThatThrownBy(() -> JPAConfiguration.builder()
+                .driverName(DRIVER_NAME)
+                .driverURL(DRIVER_URL)
+                .maxConnections(-10)
+                .build())
+            .isInstanceOf(IllegalArgumentException.class)
+            .hasMessage("maxConnections is required to be -1 (no limit) or  greater than 0");
+    }
+
+    @Test
+    void buildShouldBuildWhenMaxConnectionIsMinusOne() {
+        JPAConfiguration configuration = JPAConfiguration.builder()
+                .driverName(DRIVER_NAME)
+                .driverURL(DRIVER_URL)
+                .maxConnections(-1)
+                .build();
+
+        SoftAssertions.assertSoftly(softly -> {
+            softly.assertThat(configuration.getDriverName()).isEqualTo(DRIVER_NAME);
+            softly.assertThat(configuration.getDriverURL()).isEqualTo(DRIVER_URL);
+
+            softly.assertThat(configuration.isTestOnBorrow()).isEmpty();
+            softly.assertThat(configuration.getValidationQuery()).isEmpty();
+            softly.assertThat(configuration.getValidationQueryTimeoutSec()).isEmpty();
+            softly.assertThat(configuration.getCredential()).isEmpty();
+            softly.assertThat(configuration.getMaxConnections()).contains(-1);
+        });
+    }
+
     @Test
     void buildShouldThrowWhenValidationQueryTimeoutSecIsNegative() {
         assertThatThrownBy(() -> JPAConfiguration.builder()
diff --git a/src/site/xdoc/server/config-system.xml b/src/site/xdoc/server/config-system.xml
index ecdefb34e1..799797ac79 100644
--- a/src/site/xdoc/server/config-system.xml
+++ b/src/site/xdoc/server/config-system.xml
@@ -56,7 +56,7 @@
 
                 <p>This configuration file is only relevant when using JPA, with Spring or Guice.</p>
 
-                <p>Consult <a href="https://github.com/apache/james-project/tree/master/server/apps/spring-app/src/main/resources/james-database.properties">james.properties</a> in GIT to get some examples and hints.</p>
+                <p>Consult <a href="https://github.com/apache/james-project/tree/master/server/apps/spring-app/src/main/resources/james-database.properties">james-database.properties</a> in GIT to get some examples and hints.</p>
 
                 <p>The database connection in database.properties</p>
 
@@ -72,10 +72,10 @@
                 <p>Also, since James will use JDBC to access the database, an appropriate JDBC driver must be
                     available for installation. You can place the JDBC driver jar in the conf/lib folder, it will
                     be automatically loaded.</p>
-
+                <p>Database configuration</p>
                 <dl>
                     <dt><strong>database.driverClassName</strong></dt>
-                    <dd>he class name of the database driver to be used.</dd>
+                    <dd>The class name of the database driver to be used.</dd>
                     <dt><strong>database.url</strong></dt>
                     <dd>The JDBC connection URL for your database/driver.</dd>
                     <dt><strong>database.username</strong></dt>
@@ -88,12 +88,19 @@
                     <dd>true or false - Use streaming for Blobs. This is only supported on a limited set of databases atm. You
                         should check if its supported by your DB before enable it. See <a href="http://openjpa.apache.org/builds/latest/docs/manual/ref_guide_mapping_jpa.html">http://openjpa.apache.org/builds/latest/docs/manual/ref_guide_mapping_jpa.html</a> (#7.11. LOB Streaming).</dd>
                 </dl>
-
-                <p>Note for postgresql databases: Add standard_conforming_strings=off to your postgresql.xml, otherwise you
-                    will get ""Invalid escape string Hint: Escape string must be empty or one character. {prepstmnt 174928937
+                <p>The JPA datasource handling is delegated to commons-dbcp, some of dbcp properties can be configured through james-database.properties. The corresponding definitions and default values can be found in the <a href="https://commons.apache.org/proper/commons-dbcp/configuration.html">dbcp documentation</a></p>
+                <ul>
+                    <li>datasource.testOnBorrow</li>
+                    <li>datasource.validationQueryTimeoutSec</li>
+                    <li>datasource.validationQuery</li>
+                    <li>datasource.maxTotal</li>
+
+                </ul>
+                <p>Note for postgresql databases: Add <code>standard_conforming_strings=off</code> to your postgresql.xml, otherwise you
+                    will get <code>""Invalid escape string Hint: Escape string must be empty or one character. {prepstmnt 174928937
                     SELECT t0.mailbox_id, t0.mailbox_highest_modseq, t0.mailbox_last_uid, t0.mailbox_name, t0.mailbox_namespace,
                     t0.mailbox_uid_validity, t0.user_name FROM public.james_mailbox t0 WHERE (t0.mailbox_name LIKE ?
-                    ESCAPE '\\' AND t0.user_name = ? AND t0.mailbox_namespace = ?) [params=?, ?, ?]} [code=0, state=22025]"</p>
+                        ESCAPE '\\' AND t0.user_name = ? AND t0.mailbox_namespace = ?) [params=?, ?, ?]} [code=0, state=22025]"</code></p>
 
             </subsection>
 


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org