ARTEMIS-2090 JDBC Journal is not deleting TX records

JDBCJournalImpl::cleanupTxRecords is not committing
the deleteJournalTxRecords statement leaving the
TX records into the journal


Project: http://git-wip-us.apache.org/repos/asf/activemq-artemis/repo
Commit: http://git-wip-us.apache.org/repos/asf/activemq-artemis/commit/cfa32902
Tree: http://git-wip-us.apache.org/repos/asf/activemq-artemis/tree/cfa32902
Diff: http://git-wip-us.apache.org/repos/asf/activemq-artemis/diff/cfa32902

Branch: refs/heads/master
Commit: cfa3290209ae6eddc88808e8c0c391e3d4f54be8
Parents: 46dfb8a
Author: Francesco Nigro <[email protected]>
Authored: Fri Sep 14 17:22:53 2018 +0200
Committer: Clebert Suconic <[email protected]>
Committed: Wed Sep 19 15:32:03 2018 -0400

----------------------------------------------------------------------
 .../artemis/jdbc/store/journal/JDBCJournalImpl.java     | 12 +++++++++---
 .../integration/jdbc/store/journal/JDBCJournalTest.java |  7 +++++++
 2 files changed, 16 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/cfa32902/artemis-jdbc-store/src/main/java/org/apache/activemq/artemis/jdbc/store/journal/JDBCJournalImpl.java
----------------------------------------------------------------------
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 334bc46..8ec451d 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) {

http://git-wip-us.apache.org/repos/asf/activemq-artemis/blob/cfa32902/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jdbc/store/journal/JDBCJournalTest.java
----------------------------------------------------------------------
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 1661df9..018a3de 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
@@ -130,6 +130,13 @@ public class JDBCJournalTest extends ActiveMQTestBase {
    }
 
    @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;
       final CountDownLatch done = new CountDownLatch(noRecords);

Reply via email to