From cfa3290209ae6eddc88808e8c0c391e3d4f54be8 Mon Sep 17 00:00:00 2001
From: Francesco Nigro <nigro.fra@gmail.com>
Date: Fri, 14 Sep 2018 17:22:53 +0200
Subject: [PATCH] ARTEMIS-2090 JDBC Journal is not deleting TX records

JDBCJournalImpl::cleanupTxRecords is not committing
the deleteJournalTxRecords statement leaving the
TX records into the journal
---
 .../artemis/jdbc/store/journal/JDBCJournalImpl.java  | 12 +++++++++---
 .../jdbc/store/journal/JDBCJournalTest.java          |  7 +++++++
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalImpl.java b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalImpl.java
index 334bc46f90..8ec451de0f 100644
--- a/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalImpl.java
+++ b/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalImpl.java
@@ -251,7 +251,11 @@ public class JDBCJournalImpl extends AbstractJDBCDriver implements Journal {
             logger.trace("JDBC commit worked");
          }
 
-         cleanupTxRecords(deletedRecords, committedTransactions);
+         if (cleanupTxRecords(deletedRecords, committedTransactions)) {
+            deleteJournalTxRecords.executeBatch();
+            connection.commit();
+            logger.trace("JDBC commit worked on cleanupTxRecords");
+         }
          executeCallbacks(recordRef, true);
 
          return recordRef.size();
@@ -291,7 +295,7 @@ public class JDBCJournalImpl extends AbstractJDBCDriver implements Journal {
 
    /* We store Transaction reference in memory (once all records associated with a Tranascation are Deleted,
       we remove the Tx Records (i.e. PREPARE, COMMIT). */
-   private synchronized void cleanupTxRecords(List<Long> deletedRecords, List<Long> committedTx) throws SQLException {
+   private synchronized boolean cleanupTxRecords(List<Long> deletedRecords, List<Long> committedTx) throws SQLException {
       List<RecordInfo> iterableCopy;
       List<TransactionHolder> iterableCopyTx = new ArrayList<>();
       iterableCopyTx.addAll(transactions.values());
@@ -299,7 +303,7 @@ public class JDBCJournalImpl extends AbstractJDBCDriver implements Journal {
       for (Long txId : committedTx) {
          transactions.get(txId).committed = true;
       }
-
+      boolean hasDeletedJournalTxRecords = false;
       // TODO (mtaylor) perhaps we could store a reverse mapping of IDs to prevent this O(n) loop
       for (TransactionHolder h : iterableCopyTx) {
 
@@ -315,9 +319,11 @@ public class JDBCJournalImpl extends AbstractJDBCDriver implements Journal {
          if (h.recordInfos.isEmpty() && h.committed) {
             deleteJournalTxRecords.setLong(1, h.transactionID);
             deleteJournalTxRecords.addBatch();
+            hasDeletedJournalTxRecords = true;
             transactions.remove(h.transactionID);
          }
       }
+      return hasDeletedJournalTxRecords;
    }
 
    private void executeCallbacks(final List<JDBCJournalRecord> records, final boolean success) {
diff --git a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jdbc/store/journal/JDBCJournalTest.java b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jdbc/store/journal/JDBCJournalTest.java
index 1661df91b8..018a3de816 100644
--- a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jdbc/store/journal/JDBCJournalTest.java
+++ b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jdbc/store/journal/JDBCJournalTest.java
@@ -129,6 +129,13 @@ public class JDBCJournalTest extends ActiveMQTestBase {
       assertEquals(noRecords, journal.getNumberOfRecords());
    }
 
+   @Test
+   public void testCleanupTxRecords() throws Exception {
+      journal.appendDeleteRecordTransactional(1, 1);
+      journal.appendCommitRecord(1, true);
+      assertEquals(0, journal.getNumberOfRecords());
+   }
+
    @Test
    public void testCallbacks() throws Exception {
       final int noRecords = 10;