You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by GitBox <gi...@apache.org> on 2019/05/01 14:50:05 UTC

[GitHub] [qpid-broker-j] alex-rufous commented on a change in pull request #30: QPID-8305: [Broker-J][JDBC Message Store] Performance regression when increasing the number of queues linked to a topic

alex-rufous commented on a change in pull request #30: QPID-8305: [Broker-J][JDBC Message Store] Performance regression when increasing the number of queues linked to a topic
URL: https://github.com/apache/qpid-broker-j/pull/30#discussion_r280094393
 
 

 ##########
 File path: broker-plugins/jdbc-store/src/main/java/org/apache/qpid/server/store/jdbc/AbstractJDBCMessageStore.java
 ##########
 @@ -1151,25 +1153,27 @@ public MessageEnqueueRecord enqueueMessage(TransactionLogResource queue, Enqueue
             final StoredMessage storedMessage = message.getStoredMessage();
             if(storedMessage instanceof StoredJDBCMessage)
             {
-                _preCommitActions.add(new Runnable()
-                {
-                    @Override
-                    public void run()
+                _preCommitActions.add(() -> {
+                    try
                     {
-                        try
-                        {
-                            ((StoredJDBCMessage) storedMessage).store(_connWrapper.getConnection());
-                            _storeSizeIncrease += storedMessage.getContentSize();
-                        }
-                        catch (SQLException e)
-                        {
-                            throw new StoreException("Exception on enqueuing message into message store" + _messageId,
-                                                     e);
-                        }
+                        ((StoredJDBCMessage) storedMessage).store(_connWrapper.getConnection());
+                        _storeSizeIncrease += storedMessage.getContentSize();
                     }
+                    catch (SQLException e)
+                    {
+                        throw new StoreException("Exception on enqueuing message into message store" + _messageId, e);
+                    }
+                });
+            }
+            if(_messagesToEnqueue.isEmpty())
+            {
+                _preCommitActions.add(() -> {
+                    AbstractJDBCMessageStore.this.enqueueMessages(_connWrapper, _messagesToEnqueue);
+                    _messagesToEnqueue.clear();
 
 Review comment:
   I think that `_messagesToEnqueue` can be cleaned in JBDCTransaction#doPreCommitActions or  JBDCTransaction#commit* similar to `abortTran`. IMHO, that would make code more consistent and cleaner. If special class for aggregation/accumulation of enqueued messages is introduced, you can introduce a method to clean up the map and invoke that method from commit/abort methods.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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