Uploaded image for project: 'Jira Cloud'
  1. Jira Cloud
  2. JRACLOUD-61818

JiraHostConnectionAccessor does not reuse the same connection in nested calls

    XMLWordPrintable

Details

    Description

      NOTE: This bug report is for JIRA Cloud. Using JIRA Server? See the corresponding bug report.

      I have this use case in Fusion DVCS Connector where I need to execute two DAO calls within the same transaction through com.atlassian.pocketknife.api.querydsl.DatabaseAccessor (atlassian pocketknife query-dsl library) :

              return databaseAccessor.runInTransaction(connection -> {
                  Message<P> createdMessage = messageDao.create(message, tags);
                  requireNonNull(createdMessage.getId());
      
                  for (MessageQueueItem item : messageItems) {
                      messageQueueItemDao.create(item, createdMessage);
                  }
                  return createdMessage;
              });
      

      messageDao.create and messageQueueItemDao.create run the SQL within databaseAccessor.runInTransaction as well, e.g. in MessageDao implementation :

          public Message create(final Message message, final String[] tags) {
              Integer id = databaseAccessor.runInTransaction(connection -> {
                   // SQL query DSL insert code goes here
                   return id;
              });
              message.setTags(tags);
              message.setId(id);
              return message;
          }
      

      Problem in the main code is that messageDao.create and messageQueueItemDao.create run with different connections and therefore NOT in the same transaction as expected.

      Digging out deeper, databaseAccessor.runInTransaction calls JiraHostConnectionAccessor.execute(false, false, callback) which sits in JIRA SAL Plugin :

          public <A> A execute(final boolean readOnly, final boolean newTransaction, @Nonnull final ConnectionCallback<A> callback) {
              if (newTransaction) {
                  // Need to borrow a connection from the pool and start a new transaction
                  return borrowConnectionAndExecute(readOnly, callback);
              } else {
                  final Connection existingConnection = CoreTransactionUtil.getConnection();
                  if (existingConnection == null) {
                      // no current transaction in OfBiz
                      return borrowConnectionAndExecute(readOnly, callback);
                  } else {
                      // Developer has requested to participate in existing transaction, and OfBiz actually has one running ...
                      // I don't think it is a good idea to set read-only on an existing transaction ... ignoring
                      return executeInExistingTransaction(callback, existingConnection);
                  }
              }
          }
      
          private <A> A borrowConnectionAndExecute(final boolean readOnly, final ConnectionCallback<A> callback) {
              return databaseAccessor.executeQuery(connection -> {
                          // By contract must run in a Transaction
                          connection.setAutoCommit(false);
                          try {
                              connection.getJdbcConnection().setReadOnly(readOnly);
                          } catch (SQLException e) {
                              throw new DataAccessException(e);
                          }
                          // If this returns successfully then SAL's DefaultTransactionalExecutor will commit the transaction
                          // If it throws RuntimeException then DatabaseAcccessor and DefaultTransactionalExecutor will both
                          // cause a rollback() ... seems harmless.
                          try {
                              return callback.execute(connection.getJdbcConnection());
                          } finally {
                              if (readOnly) {
                                  // Need to return the connection to the default state of NOT read-only.
                                  try {
                                      connection.getJdbcConnection().setReadOnly(false);
                                  } catch (SQLException ex) {
                                      log.error("Unable to unset the read-only flag on a borrowed DB Connection ... chaos will ensue.", ex);
                                  }
                              }
                              // Connection Pool will set default auto-commit mode
                          }
                      }
              );
          }
      

      The nested call does not reuse the same connection because CoreTransactionUtil.getConnection tries to get "existing" connection from a ThreadLocal variable but borrowConnectionAndExecute code does not put any connection in the ThreadLocal for further re-use.

      I resolved this problem in my code by passing the connection to both DAOs methods.

      Other developers may assume databaseAccessor.runInTransaction and JiraHostConnectionAccessor.execute handle nested calls properly therefore if you do not intend to fix this problem please update the API documentation at least.

      Attachments

        Issue Links

          Activity

            People

              Unassigned Unassigned
              esoares@atlassian.com samurai (Inactive)
              Votes:
              1 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: