[GitHub] [activemq-artemis] gtully commented on a change in pull request #3204: ARTEMIS-2823 Use datasource with JDBC store db connections

[GitBox <gi...@apache.org>]

gtully commented on a change in pull request #3204:
URL: https://github.com/apache/activemq-artemis/pull/3204#discussion_r482874663



##########
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:
       One thought on this change, to have the same semantic, the connectionPool *must* have a max pool size of 1. That needs to be enforced.
   With the original code, the entire jdbc operation is serialised on the connection, and that will only still hold if the connection pool blocks when it single entry is in use. ie: if it has a max pool size of 1.




----------------------------------------------------------------
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:
users@infra.apache.org