ARTEMIS-2090 JDBC Journal is not deleting TX records

JDBCJournalImpl::cleanupTxRecords is not committing the deleteJournalTxRecords statement leaving the TX records into the journal (cherry picked from commit cfa3290)
apache · Sep 19, 2018 · e358fbd · e358fbd
1 parent af98efd
commit e358fbd
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 3 deletions.
diff --git a/...c-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalImpl.java b/...c-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalImpl.java
@@ -252,7 +252,11 @@ public synchronized int sync() {
             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();
@@ -292,15 +296,15 @@ public void handleException(List<JDBCJournalRecord> recordRef, Throwable e) {
 
    /* 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());
 
       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) {
 
@@ -316,9 +320,11 @@ private synchronized void cleanupTxRecords(List<Long> deletedRecords, List<Long>
          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/...ava/org/apache/activemq/artemis/tests/integration/jdbc/store/journal/JDBCJournalTest.java b/...ava/org/apache/activemq/artemis/tests/integration/jdbc/store/journal/JDBCJournalTest.java
@@ -129,6 +129,13 @@ public void testInsertRecords() throws Exception {
       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;