[ 
https://issues.apache.org/jira/browse/ARTEMIS-2823?focusedWorklogId=483330&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-483330
 ]

ASF GitHub Bot logged work on ARTEMIS-2823:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 12/Sep/20 21:47
            Start Date: 12/Sep/20 21:47
    Worklog Time Spent: 10m 
      Work Description: uomik commented on a change in pull request #3204:
URL: https://github.com/apache/activemq-artemis/pull/3204#discussion_r486843904



##########
File path: 
artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/Db2SequentialFileDriver.java
##########
@@ -30,33 +31,30 @@ public Db2SequentialFileDriver() {
       super();
    }
 
-   public Db2SequentialFileDriver(DataSource dataSource, SQLProvider provider) 
{
-      super(dataSource, provider);
-   }
-
-   public Db2SequentialFileDriver(Connection connection, SQLProvider provider) 
{
-      super(connection, provider);
+   public Db2SequentialFileDriver(JDBCConnectionProvider connectionProvider, 
SQLProvider provider) {
+      super(connectionProvider, provider);
    }
 
    @Override
-   protected void prepareStatements() throws SQLException {
-      this.deleteFile = 
connection.prepareStatement(sqlProvider.getDeleteFileSQL());
-      this.createFile = 
connection.prepareStatement(sqlProvider.getInsertFileSQL(), new String[]{"ID"});
-      this.selectFileByFileName = 
connection.prepareStatement(sqlProvider.getSelectFileByFileName());
-      this.copyFileRecord = 
connection.prepareStatement(sqlProvider.getCopyFileRecordByIdSQL());
-      this.renameFile = 
connection.prepareStatement(sqlProvider.getUpdateFileNameByIdSQL());
-      this.readLargeObject = 
connection.prepareStatement(sqlProvider.getReadLargeObjectSQL());
-      this.appendToLargeObject = 
connection.prepareStatement(sqlProvider.getAppendToLargeObjectSQL());
-      this.selectFileNamesByExtension = 
connection.prepareStatement(sqlProvider.getSelectFileNamesByExtensionSQL());
+   protected void prepareStatements() {
+      this.deleteFile = sqlProvider.getDeleteFileSQL();
+      this.createFile = sqlProvider.getInsertFileSQL();
+      this.createFileColumnNames = new String[]{"ID"};
+      this.selectFileByFileName = sqlProvider.getSelectFileByFileName();
+      this.copyFileRecord = sqlProvider.getCopyFileRecordByIdSQL();
+      this.renameFile = sqlProvider.getUpdateFileNameByIdSQL();
+      this.readLargeObject = sqlProvider.getReadLargeObjectSQL();
+      this.appendToLargeObject = sqlProvider.getAppendToLargeObjectSQL();
+      this.selectFileNamesByExtension = 
sqlProvider.getSelectFileNamesByExtensionSQL();
    }
 
    @Override
    public int writeToFile(JDBCSequentialFile file, byte[] data) throws 
SQLException {
       if (data == null || data.length == 0) {
          return 0;
       }
-      synchronized (connection) {
-         try {

Review comment:
       I did a quick check by just starting "stock" Artemis and it at least 
opens several connections to the database on startup. After a while it seems to 
settle down to two active connections. And I have to correct myself, I forgot 
that I actually decided to drop the setting form the configs as well as I came 
to conclusion that removing it has no negative effect. But correct me if I'm 
wrong.

##########
File path: 
artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/Db2SequentialFileDriver.java
##########
@@ -30,33 +31,30 @@ public Db2SequentialFileDriver() {
       super();
    }
 
-   public Db2SequentialFileDriver(DataSource dataSource, SQLProvider provider) 
{
-      super(dataSource, provider);
-   }
-
-   public Db2SequentialFileDriver(Connection connection, SQLProvider provider) 
{
-      super(connection, provider);
+   public Db2SequentialFileDriver(JDBCConnectionProvider connectionProvider, 
SQLProvider provider) {
+      super(connectionProvider, provider);
    }
 
    @Override
-   protected void prepareStatements() throws SQLException {
-      this.deleteFile = 
connection.prepareStatement(sqlProvider.getDeleteFileSQL());
-      this.createFile = 
connection.prepareStatement(sqlProvider.getInsertFileSQL(), new String[]{"ID"});
-      this.selectFileByFileName = 
connection.prepareStatement(sqlProvider.getSelectFileByFileName());
-      this.copyFileRecord = 
connection.prepareStatement(sqlProvider.getCopyFileRecordByIdSQL());
-      this.renameFile = 
connection.prepareStatement(sqlProvider.getUpdateFileNameByIdSQL());
-      this.readLargeObject = 
connection.prepareStatement(sqlProvider.getReadLargeObjectSQL());
-      this.appendToLargeObject = 
connection.prepareStatement(sqlProvider.getAppendToLargeObjectSQL());
-      this.selectFileNamesByExtension = 
connection.prepareStatement(sqlProvider.getSelectFileNamesByExtensionSQL());
+   protected void prepareStatements() {
+      this.deleteFile = sqlProvider.getDeleteFileSQL();
+      this.createFile = sqlProvider.getInsertFileSQL();
+      this.createFileColumnNames = new String[]{"ID"};
+      this.selectFileByFileName = sqlProvider.getSelectFileByFileName();
+      this.copyFileRecord = sqlProvider.getCopyFileRecordByIdSQL();
+      this.renameFile = sqlProvider.getUpdateFileNameByIdSQL();
+      this.readLargeObject = sqlProvider.getReadLargeObjectSQL();
+      this.appendToLargeObject = sqlProvider.getAppendToLargeObjectSQL();
+      this.selectFileNamesByExtension = 
sqlProvider.getSelectFileNamesByExtensionSQL();
    }
 
    @Override
    public int writeToFile(JDBCSequentialFile file, byte[] data) throws 
SQLException {
       if (data == null || data.length == 0) {
          return 0;
       }
-      synchronized (connection) {
-         try {

Review comment:
       I did a quick check by just starting "stock" Artemis and it at least 
opens several connections to the database on startup. After a while it seems to 
settle down to two active connections. And I have to correct myself, I forgot 
that I actually decided to drop the setting form the configs as well as I came 
to conclusion that removing it has no negative effect. But correct me if I'm 
wrong.

##########
File path: 
artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/Db2SequentialFileDriver.java
##########
@@ -30,33 +31,30 @@ public Db2SequentialFileDriver() {
       super();
    }
 
-   public Db2SequentialFileDriver(DataSource dataSource, SQLProvider provider) 
{
-      super(dataSource, provider);
-   }
-
-   public Db2SequentialFileDriver(Connection connection, SQLProvider provider) 
{
-      super(connection, provider);
+   public Db2SequentialFileDriver(JDBCConnectionProvider connectionProvider, 
SQLProvider provider) {
+      super(connectionProvider, provider);
    }
 
    @Override
-   protected void prepareStatements() throws SQLException {
-      this.deleteFile = 
connection.prepareStatement(sqlProvider.getDeleteFileSQL());
-      this.createFile = 
connection.prepareStatement(sqlProvider.getInsertFileSQL(), new String[]{"ID"});
-      this.selectFileByFileName = 
connection.prepareStatement(sqlProvider.getSelectFileByFileName());
-      this.copyFileRecord = 
connection.prepareStatement(sqlProvider.getCopyFileRecordByIdSQL());
-      this.renameFile = 
connection.prepareStatement(sqlProvider.getUpdateFileNameByIdSQL());
-      this.readLargeObject = 
connection.prepareStatement(sqlProvider.getReadLargeObjectSQL());
-      this.appendToLargeObject = 
connection.prepareStatement(sqlProvider.getAppendToLargeObjectSQL());
-      this.selectFileNamesByExtension = 
connection.prepareStatement(sqlProvider.getSelectFileNamesByExtensionSQL());
+   protected void prepareStatements() {
+      this.deleteFile = sqlProvider.getDeleteFileSQL();
+      this.createFile = sqlProvider.getInsertFileSQL();
+      this.createFileColumnNames = new String[]{"ID"};
+      this.selectFileByFileName = sqlProvider.getSelectFileByFileName();
+      this.copyFileRecord = sqlProvider.getCopyFileRecordByIdSQL();
+      this.renameFile = sqlProvider.getUpdateFileNameByIdSQL();
+      this.readLargeObject = sqlProvider.getReadLargeObjectSQL();
+      this.appendToLargeObject = sqlProvider.getAppendToLargeObjectSQL();
+      this.selectFileNamesByExtension = 
sqlProvider.getSelectFileNamesByExtensionSQL();
    }
 
    @Override
    public int writeToFile(JDBCSequentialFile file, byte[] data) throws 
SQLException {
       if (data == null || data.length == 0) {
          return 0;
       }
-      synchronized (connection) {
-         try {

Review comment:
       I did a quick check by just starting "stock" Artemis and it at least 
opens several connections to the database on startup. After a while it seems to 
settle down to two active connections. And I have to correct myself, I forgot 
that I actually decided to drop the setting form the configs as well as I came 
to conclusion that removing it has no negative effect. But correct me if I'm 
wrong.

##########
File path: 
artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/Db2SequentialFileDriver.java
##########
@@ -30,33 +31,30 @@ public Db2SequentialFileDriver() {
       super();
    }
 
-   public Db2SequentialFileDriver(DataSource dataSource, SQLProvider provider) 
{
-      super(dataSource, provider);
-   }
-
-   public Db2SequentialFileDriver(Connection connection, SQLProvider provider) 
{
-      super(connection, provider);
+   public Db2SequentialFileDriver(JDBCConnectionProvider connectionProvider, 
SQLProvider provider) {
+      super(connectionProvider, provider);
    }
 
    @Override
-   protected void prepareStatements() throws SQLException {
-      this.deleteFile = 
connection.prepareStatement(sqlProvider.getDeleteFileSQL());
-      this.createFile = 
connection.prepareStatement(sqlProvider.getInsertFileSQL(), new String[]{"ID"});
-      this.selectFileByFileName = 
connection.prepareStatement(sqlProvider.getSelectFileByFileName());
-      this.copyFileRecord = 
connection.prepareStatement(sqlProvider.getCopyFileRecordByIdSQL());
-      this.renameFile = 
connection.prepareStatement(sqlProvider.getUpdateFileNameByIdSQL());
-      this.readLargeObject = 
connection.prepareStatement(sqlProvider.getReadLargeObjectSQL());
-      this.appendToLargeObject = 
connection.prepareStatement(sqlProvider.getAppendToLargeObjectSQL());
-      this.selectFileNamesByExtension = 
connection.prepareStatement(sqlProvider.getSelectFileNamesByExtensionSQL());
+   protected void prepareStatements() {
+      this.deleteFile = sqlProvider.getDeleteFileSQL();
+      this.createFile = sqlProvider.getInsertFileSQL();
+      this.createFileColumnNames = new String[]{"ID"};
+      this.selectFileByFileName = sqlProvider.getSelectFileByFileName();
+      this.copyFileRecord = sqlProvider.getCopyFileRecordByIdSQL();
+      this.renameFile = sqlProvider.getUpdateFileNameByIdSQL();
+      this.readLargeObject = sqlProvider.getReadLargeObjectSQL();
+      this.appendToLargeObject = sqlProvider.getAppendToLargeObjectSQL();
+      this.selectFileNamesByExtension = 
sqlProvider.getSelectFileNamesByExtensionSQL();
    }
 
    @Override
    public int writeToFile(JDBCSequentialFile file, byte[] data) throws 
SQLException {
       if (data == null || data.length == 0) {
          return 0;
       }
-      synchronized (connection) {
-         try {

Review comment:
       I did a quick check by just starting "stock" Artemis and it at least 
opens several connections to the database on startup. After a while it seems to 
settle down to two active connections. And I have to correct myself, I forgot 
that I actually decided to drop the setting form the configs as well as I came 
to conclusion that removing it has no negative effect. But correct me if I'm 
wrong.

##########
File path: 
artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/Db2SequentialFileDriver.java
##########
@@ -30,33 +31,30 @@ public Db2SequentialFileDriver() {
       super();
    }
 
-   public Db2SequentialFileDriver(DataSource dataSource, SQLProvider provider) 
{
-      super(dataSource, provider);
-   }
-
-   public Db2SequentialFileDriver(Connection connection, SQLProvider provider) 
{
-      super(connection, provider);
+   public Db2SequentialFileDriver(JDBCConnectionProvider connectionProvider, 
SQLProvider provider) {
+      super(connectionProvider, provider);
    }
 
    @Override
-   protected void prepareStatements() throws SQLException {
-      this.deleteFile = 
connection.prepareStatement(sqlProvider.getDeleteFileSQL());
-      this.createFile = 
connection.prepareStatement(sqlProvider.getInsertFileSQL(), new String[]{"ID"});
-      this.selectFileByFileName = 
connection.prepareStatement(sqlProvider.getSelectFileByFileName());
-      this.copyFileRecord = 
connection.prepareStatement(sqlProvider.getCopyFileRecordByIdSQL());
-      this.renameFile = 
connection.prepareStatement(sqlProvider.getUpdateFileNameByIdSQL());
-      this.readLargeObject = 
connection.prepareStatement(sqlProvider.getReadLargeObjectSQL());
-      this.appendToLargeObject = 
connection.prepareStatement(sqlProvider.getAppendToLargeObjectSQL());
-      this.selectFileNamesByExtension = 
connection.prepareStatement(sqlProvider.getSelectFileNamesByExtensionSQL());
+   protected void prepareStatements() {
+      this.deleteFile = sqlProvider.getDeleteFileSQL();
+      this.createFile = sqlProvider.getInsertFileSQL();
+      this.createFileColumnNames = new String[]{"ID"};
+      this.selectFileByFileName = sqlProvider.getSelectFileByFileName();
+      this.copyFileRecord = sqlProvider.getCopyFileRecordByIdSQL();
+      this.renameFile = sqlProvider.getUpdateFileNameByIdSQL();
+      this.readLargeObject = sqlProvider.getReadLargeObjectSQL();
+      this.appendToLargeObject = sqlProvider.getAppendToLargeObjectSQL();
+      this.selectFileNamesByExtension = 
sqlProvider.getSelectFileNamesByExtensionSQL();
    }
 
    @Override
    public int writeToFile(JDBCSequentialFile file, byte[] data) throws 
SQLException {
       if (data == null || data.length == 0) {
          return 0;
       }
-      synchronized (connection) {
-         try {

Review comment:
       I did a quick check by just starting "stock" Artemis and it at least 
opens several connections to the database on startup. After a while it seems to 
settle down to two active connections. And I have to correct myself, I forgot 
that I actually decided to drop the setting form the configs as well as I came 
to conclusion that removing it has no negative effect. But correct me if I'm 
wrong.

##########
File path: 
artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/Db2SequentialFileDriver.java
##########
@@ -30,33 +31,30 @@ public Db2SequentialFileDriver() {
       super();
    }
 
-   public Db2SequentialFileDriver(DataSource dataSource, SQLProvider provider) 
{
-      super(dataSource, provider);
-   }
-
-   public Db2SequentialFileDriver(Connection connection, SQLProvider provider) 
{
-      super(connection, provider);
+   public Db2SequentialFileDriver(JDBCConnectionProvider connectionProvider, 
SQLProvider provider) {
+      super(connectionProvider, provider);
    }
 
    @Override
-   protected void prepareStatements() throws SQLException {
-      this.deleteFile = 
connection.prepareStatement(sqlProvider.getDeleteFileSQL());
-      this.createFile = 
connection.prepareStatement(sqlProvider.getInsertFileSQL(), new String[]{"ID"});
-      this.selectFileByFileName = 
connection.prepareStatement(sqlProvider.getSelectFileByFileName());
-      this.copyFileRecord = 
connection.prepareStatement(sqlProvider.getCopyFileRecordByIdSQL());
-      this.renameFile = 
connection.prepareStatement(sqlProvider.getUpdateFileNameByIdSQL());
-      this.readLargeObject = 
connection.prepareStatement(sqlProvider.getReadLargeObjectSQL());
-      this.appendToLargeObject = 
connection.prepareStatement(sqlProvider.getAppendToLargeObjectSQL());
-      this.selectFileNamesByExtension = 
connection.prepareStatement(sqlProvider.getSelectFileNamesByExtensionSQL());
+   protected void prepareStatements() {
+      this.deleteFile = sqlProvider.getDeleteFileSQL();
+      this.createFile = sqlProvider.getInsertFileSQL();
+      this.createFileColumnNames = new String[]{"ID"};
+      this.selectFileByFileName = sqlProvider.getSelectFileByFileName();
+      this.copyFileRecord = sqlProvider.getCopyFileRecordByIdSQL();
+      this.renameFile = sqlProvider.getUpdateFileNameByIdSQL();
+      this.readLargeObject = sqlProvider.getReadLargeObjectSQL();
+      this.appendToLargeObject = sqlProvider.getAppendToLargeObjectSQL();
+      this.selectFileNamesByExtension = 
sqlProvider.getSelectFileNamesByExtensionSQL();
    }
 
    @Override
    public int writeToFile(JDBCSequentialFile file, byte[] data) throws 
SQLException {
       if (data == null || data.length == 0) {
          return 0;
       }
-      synchronized (connection) {
-         try {

Review comment:
       I did a quick check by just starting "stock" Artemis and it at least 
opens several connections to the database on startup. After a while it seems to 
settle down to two active connections. And I have to correct myself, I forgot 
that I actually decided to drop the setting form the configs as well as I came 
to conclusion that removing it has no negative effect. But correct me if I'm 
wrong.

##########
File path: 
artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/file/Db2SequentialFileDriver.java
##########
@@ -30,33 +31,30 @@ public Db2SequentialFileDriver() {
       super();
    }
 
-   public Db2SequentialFileDriver(DataSource dataSource, SQLProvider provider) 
{
-      super(dataSource, provider);
-   }
-
-   public Db2SequentialFileDriver(Connection connection, SQLProvider provider) 
{
-      super(connection, provider);
+   public Db2SequentialFileDriver(JDBCConnectionProvider connectionProvider, 
SQLProvider provider) {
+      super(connectionProvider, provider);
    }
 
    @Override
-   protected void prepareStatements() throws SQLException {
-      this.deleteFile = 
connection.prepareStatement(sqlProvider.getDeleteFileSQL());
-      this.createFile = 
connection.prepareStatement(sqlProvider.getInsertFileSQL(), new String[]{"ID"});
-      this.selectFileByFileName = 
connection.prepareStatement(sqlProvider.getSelectFileByFileName());
-      this.copyFileRecord = 
connection.prepareStatement(sqlProvider.getCopyFileRecordByIdSQL());
-      this.renameFile = 
connection.prepareStatement(sqlProvider.getUpdateFileNameByIdSQL());
-      this.readLargeObject = 
connection.prepareStatement(sqlProvider.getReadLargeObjectSQL());
-      this.appendToLargeObject = 
connection.prepareStatement(sqlProvider.getAppendToLargeObjectSQL());
-      this.selectFileNamesByExtension = 
connection.prepareStatement(sqlProvider.getSelectFileNamesByExtensionSQL());
+   protected void prepareStatements() {
+      this.deleteFile = sqlProvider.getDeleteFileSQL();
+      this.createFile = sqlProvider.getInsertFileSQL();
+      this.createFileColumnNames = new String[]{"ID"};
+      this.selectFileByFileName = sqlProvider.getSelectFileByFileName();
+      this.copyFileRecord = sqlProvider.getCopyFileRecordByIdSQL();
+      this.renameFile = sqlProvider.getUpdateFileNameByIdSQL();
+      this.readLargeObject = sqlProvider.getReadLargeObjectSQL();
+      this.appendToLargeObject = sqlProvider.getAppendToLargeObjectSQL();
+      this.selectFileNamesByExtension = 
sqlProvider.getSelectFileNamesByExtensionSQL();
    }
 
    @Override
    public int writeToFile(JDBCSequentialFile file, byte[] data) throws 
SQLException {
       if (data == null || data.length == 0) {
          return 0;
       }
-      synchronized (connection) {
-         try {

Review comment:
       I did a quick check by just starting "stock" Artemis and it at least 
opens several connections to the database on startup. After a while it seems to 
settle down to two active connections. And I have to correct myself, I forgot 
that I actually decided to drop the setting form the configs as well as I came 
to conclusion that removing it has no negative effect. But correct me if I'm 
wrong.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 483330)
    Time Spent: 3h  (was: 2h 50m)

> Improve JDBC connection management
> ----------------------------------
>
>                 Key: ARTEMIS-2823
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-2823
>             Project: ActiveMQ Artemis
>          Issue Type: Improvement
>          Components: Broker
>            Reporter: Mikko
>            Priority: Major
>          Time Spent: 3h
>  Remaining Estimate: 0h
>
> I have a case where the whole clustering reliability and HA must rely on HA 
> capabilities of clustered database, and running on top of application server 
> is not an option.
> The current JDBC store implementation is rather bare bones on the connection 
> management side. JDBC driver is used directly with no management layer. At 
> startup, the broker just opens couple of direct connections to database and 
> expects them to be available forever. This is something that cannot be 
> expected in HA production environment. So, similarly to the discussion linked 
> below, in our case we lose the db connection after one hour, and all the 
> brokers need to be restared to get new connections:
> [http://activemq.2283324.n4.nabble.com/Artemis-does-not-reconnect-to-MySQL-after-connection-timeout-td4751956.html]
>  
> This is something that could be resolved by simply using JDBC4 isValid 
> checks, but proper connection handling and pooling through datasource would 
> be preferrable.
> I have implemented a solution for this by using DBCP2 datasource. Our test 
> cluster has been successfully running this forked version since the release 
> of Artemis 2.13.0. I will prepare of pull request if this is seen to be 
> something that can be useful.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to