[jira] [Commented] (QPID-8385) [Broker-J] Improve performance of operation to dequeue queue entries on message expiration or triggering ring policy

["Alex Rudyy (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/QPID-8385?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16984588#comment-16984588 ] 

Alex Rudyy commented on QPID-8385:
----------------------------------

Attached patch  [^0001-QPID-8385-Broker-J-Utilize-AsyncAutoCommitTransactio.patch]  changing {{AbstractQueue#dequeueEntry(QueueEntry)}} to use {{AsyncAutoCommitTransaction}} for entry deletion. The  changes made in patch add {{FutureCallback}}  to {{ListenableFuture}} returned by store  on async transaction commit. When {{Future}} is set (committer thread synced the transaction),  the task to perform post commit action is executed using {{VirtualHost}} house keeping thread. This approach frees committer thread from performing dequeue post-commit action. Otherwise, execution of post-comit accition might result in persistent message deletion and a new job being added into a committer queue from commither thread. 

The usage of {{VirtualHost}} house keeping threads creates an extra delay in dequeue functionality. As result, the ring policy can delete more messages than actually needed (as dequeueing is asynchronous, and arriving messages can cause dequeueing  of more older messages,  due to queue depths not being changed immediately).

It seems,  that with suggested approach the {{RingOverflowPolicyHandler}} needs to be changed  to take into consideration  the messages which are still in the in process of dequeue (to avoid unnecessary deletion of queue entries).

The alternative approach could be to invoke the post transaction action immediately without waiting for committer thread. The change would be quite simple: 

{code}
diff --git a/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java b/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java
index cc6608d0d..27f9f5fc1 100644
--- a/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java
+++ b/broker-core/src/main/java/org/apache/qpid/server/queue/AbstractQueue.java
@@ -113,7 +113,7 @@ import org.apache.qpid.server.store.MessageEnqueueRecord;
 import org.apache.qpid.server.store.StorableMessageMetaData;
 import org.apache.qpid.server.store.StoredMessage;
 import org.apache.qpid.server.transport.AMQPConnection;
-import org.apache.qpid.server.txn.AutoCommitTransaction;
+import org.apache.qpid.server.txn.AsyncAutoCommitTransaction;
 import org.apache.qpid.server.txn.LocalTransaction;
 import org.apache.qpid.server.txn.ServerTransaction;
 import org.apache.qpid.server.txn.TransactionMonitor;
@@ -1760,7 +1760,8 @@ public abstract class AbstractQueue<X extends AbstractQueue<X>>
 
     private void dequeueEntry(final QueueEntry node)
     {
-        ServerTransaction txn = new AutoCommitTransaction(getVirtualHost().getMessageStore());
+        ServerTransaction txn = new AsyncAutoCommitTransaction(getVirtualHost().getMessageStore(),
+                                                               (future, action) -> action.postCommit());
         dequeueEntry(node, txn);
     }
 
{code}

I think that the changes in the diff above should do the job. Though, the dequeue transaction failure in committer thread would be unseen with this approach. I think, it should be OK for non replicated environment, as it gets closed anyway on exception in the committer thread. The replicated environment would end-up in  {{ConnectionScopeRuntimeException}} r being thrown from the committer thread.

At the moment, I do not see other problems, which might be caused by the approach provided in the diff above.



> [Broker-J] Improve performance of operation to dequeue queue entries on message expiration or triggering ring policy
> --------------------------------------------------------------------------------------------------------------------
>
>                 Key: QPID-8385
>                 URL: https://issues.apache.org/jira/browse/QPID-8385
>             Project: Qpid
>          Issue Type: Improvement
>          Components: Broker-J
>            Reporter: Alex Rudyy
>            Priority: Major
>             Fix For: qpid-java-broker-8.0.0, qpid-java-broker-7.1.6
>
>         Attachments: 0001-QPID-8385-Broker-J-Utilize-AsyncAutoCommitTransactio.patch
>
>
> AutoCommitTransaction is used to dequeue  entries on message expiration or breaching ring policy threshold. As result, the dequeueing operation for persistent queue entry blocks until queue entry record is removed from the store and  the underlying store transaction is synced to disk. The sequential removal of multiple entries can results in unnecessary delays due to  syncing disk on every dequeue.
> The  broker performance for the corner cases described above can be significantly improved by using  asynchronous  transactions in dequeue operations.
> Asynchronous transaction does not wait for the store dequeue  transaction to sync to disk. As result, the performance of removal unneeded queue entries can increase.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@qpid.apache.org
For additional commands, e-mail: dev-help@qpid.apache.org