D'oh - no idea what happened there. Re-attached with an actual diff this time (I hope :) )
https://issues.apache.org/jira/secure/attachment/12941192/QPID-8242.patch -- Rob On Wed, 19 Sep 2018 at 01:23, Rob Godfrey <rob.j.godf...@gmail.com> wrote: > As an alternative approach, how about the (totally untested) patch I've > attached to QPID-8242 [1] > > Rather than trying to consolidate the commits to delete the queue entry > and the message content it instead attempts to delete the message content > asynchronously and batch up all message contents awaiting deletion. From a > performance point of view, on the broker it takes the message content > deletion off the connection thread, and if the database is being the > bottleneck it should start batching up the deletions of content. > > -- Rob > > [1] > https://issues.apache.org/jira/secure/attachment/12940318/QPID-8242.patch > > On Mon, 17 Sep 2018 at 15:22, Rob Godfrey <rob.j.godf...@gmail.com> wrote: > >> The reference counting is done in AbstractServerMessageImpl.java. In >> general instances of ServerMessage should not be passed around, rather a >> MessageReference (obtained by calling newReference(..) on the message >> object. Then if the message is no longer required in that context, >> release() can be called on the reference. Within the context of a queue >> this is normally encapsulated by the notion of the QueueEntry (which holds >> the relevant MessageReference). When delete() is called on the QueueEntry >> (delete being defined in the MessageInstance interface), this releases the >> reference, which decrements the reference count, and if the count has gone >> to zero the message calls the store to delete itself. >> >> I think trying to change this logic would be a bit of a nightmare >> (understatement). I think a better alternative in terms of reducing the >> number of commits in the JDBC store might instead be for the remove() >> method on StoredJDBCMessage to (rather than immediately commit the message >> delete) schedule the message removal to be picked up by the next commit >> that the store is asked to perform. This would make the behaviour more >> like the BDB store (where we schedule the commit but don't actually force >> the sync to disk on message deletion). >> >> -- Rob >> >> On Mon, 17 Sep 2018 at 14:55, VERMEULEN Olivier < >> olivier.vermeu...@murex.com> wrote: >> >>> Ok then I missed something. >>> Whan/Where is the reference-counting you were talking about in your >>> first mail happening? >>> >>> Olivier >>> >>> -----Original Message----- >>> From: Rob Godfrey <rob.j.godf...@gmail.com> >>> Sent: lundi 17 septembre 2018 14:05 >>> To: users@qpid.apache.org >>> Subject: Re: [Broker-J] JDBC message store performance >>> >>> Hi Olivier, >>> >>> the approach you are attempting will not work for the reasons I >>> described previously. If the message has (for instance) been placed in two >>> durable subscription queues (because there are two durable subscriptions) >>> then as soon as the message is consumed from the first queue it would be >>> removed from the store, leading to problems when the second consumer tries >>> to read the message. >>> >>> -- Rob >>> >>> On Mon, 17 Sep 2018 at 11:39, VERMEULEN Olivier < >>> olivier.vermeu...@murex.com> >>> wrote: >>> >>> > Hello Rob, >>> > >>> > Thanks for the answer. >>> > I started looking at the code to see if there is something I can do >>> > about these 2 commits. >>> > But before going any further I'd like your input on the below, to see >>> > if what I'm trying to do could work or if I'm missing something (which >>> > I'm surely are) >>> > >>> > https://github.com/apache/qpid-broker-j/compare/master...overmeulen:fe >>> > ature/jdbc-message-store-commits >>> > >>> > Regards, >>> > Olivier >>> > >>> > -----Original Message----- >>> > From: Rob Godfrey <rob.j.godf...@gmail.com> >>> > Sent: vendredi 14 septembre 2018 16:06 >>> > To: users@qpid.apache.org >>> > Cc: AYOUBI Ali <ali.ayo...@murex.com> >>> > Subject: Re: [Broker-J] JDBC message store performance >>> > >>> > On Fri, 14 Sep 2018 at 15:30, VERMEULEN Olivier < >>> > olivier.vermeu...@murex.com> >>> > wrote: >>> > >>> > > Hello, >>> > > >>> > > We ran a performance test with a bunch of brokers and an Oracle >>> > > database to store the messages. >>> > > We noticed that the database was a bit overloaded with commits. >>> > > Looking at the logs we saw that sending a message was triggering 1 >>> > > commit for 3 operations (QPID_QUEUE_ENTRIES, QPID_MESSAGE_METADATA, >>> > > QPID_MESSAGE_CONTENT) which is what we were expecting but receiving >>> > > a message was triggering 2 commits (1 for QPID_QUEUE_ENTRIES and 1 >>> > > for QPID_MESSAGE_METADATA and QPID_MESSAGE_CONTENT). >>> > > I debugged a bit the code and saw that in >>> > > AbstractVirtualHost.executeTransaction the delete on >>> > > QPID_MESSAGE_METADATA and QPID_MESSAGE_CONTENT was defined as a >>> > > "post commit" operation, explaining why we have 2 commits. >>> > > Is it something expected? Do you think we could reduce this to 1 >>> > > commit when receiving a message? >>> > > >>> > >>> > It's not unexpected - basically the issue is that the broker needs to >>> > cope with the possibility that the same message is being stored on >>> > multiple queues. The first commit is deleting the referencing of the >>> > message from the given queue. The second commit is occuring after the >>> > message has been definitively removed from the queue, and the store >>> > has determined that there are no more references, so it is ok to >>> > remove the message. This is driven by reference-counting of the >>> > message, and has historically been a place of many potential race >>> > conditions. I'm sure it is possible to optimise the code in some way, >>> > but it may not be "easy". For the BDB store this doesn't matter as >>> > much as the actual synchronisation to disk of these operations is >>> > coalesced, obviously this is more of an issue for the JDBC store. >>> > >>> > -- Rob >>> > >>> > >>> > > >>> > > Thanks, >>> > > Olivier >>> > > ******************************* >>> > > >>> > > This e-mail contains information for the intended recipient only. It >>> > > may contain proprietary material or confidential information. If you >>> > > are not the intended recipient you are not authorised to distribute, >>> > > copy or use this e-mail or any attachment to it. Murex cannot >>> > > guarantee that it is virus free and accepts no responsibility for >>> > > any loss or damage arising from its use. If you have received this >>> > > e-mail in error please notify immediately the sender and delete the >>> > > original email received, any attachments and all copies from your >>> system. >>> > > >>> > ******************************* >>> > >>> > This e-mail contains information for the intended recipient only. It >>> > may contain proprietary material or confidential information. If you >>> > are not the intended recipient you are not authorised to distribute, >>> > copy or use this e-mail or any attachment to it. Murex cannot >>> > guarantee that it is virus free and accepts no responsibility for any >>> > loss or damage arising from its use. If you have received this e-mail >>> > in error please notify immediately the sender and delete the original >>> > email received, any attachments and all copies from your system. >>> > >>> ******************************* >>> >>> This e-mail contains information for the intended recipient only. It may >>> contain proprietary material or confidential information. If you are not >>> the intended recipient you are not authorised to distribute, copy or use >>> this e-mail or any attachment to it. Murex cannot guarantee that it is >>> virus free and accepts no responsibility for any loss or damage arising >>> from its use. If you have received this e-mail in error please notify >>> immediately the sender and delete the original email received, any >>> attachments and all copies from your system. >>> >>
