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_r280092297
 
 

 ##########
 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())
 
 Review comment:
   On-the-flight creation of pre-commit action for insertion of batch of 
aggregated enqueue records looks like hack to me.
   I think  that creation of such action in constructor of JBDCTransaction 
would make the code much simpler. Alternately, a special class to aggregate 
enqueued messages can be created where special methods to update and clean 
`_messagesToEnqueue`  can be  added. You can even create tests for the new 
class. You can create an instance of that special class in constructor of 
JDBCTransaction and invoke a method to append enqueued message from 
JBDCTransaction#enqueueMessage().  Potentially, a post commit action can even 
be created and executed in JBDCTransaction#doPreCommitActions. Though, that 
might complicate the code and make it less clean.

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to