qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gordon Sim" <g...@redhat.com>
Subject Re: Review Request 23305: [C++ broker] Make memory usage consistent after broker restart
Date Mon, 07 Jul 2014 17:17:57 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/23305/#review47397
-----------------------------------------------------------


Having the broker set the persistence id in some cases, where the current expectation is that
the store sets it, makes me nervous. How does this affect the windows store(s) for example?
Does any store make any assumptions about the id that would no longer hold?


/trunk/qpid/cpp/src/qpid/broker/QueueSettings.h
<https://reviews.apache.org/r/23305/#comment83126>

    Should be camel case rather than underscore, like other option members.



/trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp
<https://reviews.apache.org/r/23305/#comment83127>

    Odd option name, qpid.store_msg_id would be neater. I don't feel the option name really
properly describes the function though. It's really more something like 'share recovered messages'



/trunk/qpid/cpp/src/qpid/legacystore/MessageStoreImpl.cpp
<https://reviews.apache.org/r/23305/#comment83128>

    Would be nicer to contain this all in the broker code, e.g. perhaps in qpid/broker/RecoveryManagerImpl.cpp?


- Gordon Sim


On July 7, 2014, 4:59 p.m., Pavel Moravec wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23305/
> -----------------------------------------------------------
> 
> (Updated July 7, 2014, 4:59 p.m.)
> 
> 
> Review request for qpid, Gordon Sim and Kim van der Riet.
> 
> 
> Bugs: QPID-5880
>     https://issues.apache.org/jira/browse/QPID-5880
> 
> 
> Repository: qpid
> 
> 
> Description
> -------
> 
> Simple idea:
> - in Queue::enqueue, set PersistableMessage::persistencyID "manually" to some unique
number that is identical to all message instances that has common SharedState - e.g. to the
pointer to SharedState
> - during journal recovery, if we recover a message with already seen persistencyID, use
the previous seen instead with its SharedState and PersistableMessage bits
> 
> Known limitation:
> - message annotations added to some queue (e.g. due to queue sequencing enabled) will
be either over-written or shared to all other queues during recovery
> 
> The patch contains a new QueueSettings option to enable (by default disabled) this feature
on per queue basis. This somehow limits the limitation above.
> 
> Isn't storing pointer to SharedState to the disk (via persistencyID) some sort of security
breach? (I dont think so, but worth to ask)
> 
> Can't manual setup of persistencyID break something in store? (AFAIK no as uniqueness
of the ID is assured: 1) a new / different message with the same persistencyID can appear
only after the previous instance is gone from memory, and 2) only queues with the option enabled
are checked for message coupling)
> 
> Will it help in cluster? No, it won't. As when primary broker gets 1 message to an exchange
that distributes it to 100 queues, th broker updates backup brokers via 100 individual "enqueue
1 message to queue q[1-100]" events. So backup brokers consume more memory than primary -
the same amount like primary does not share SharedState at all.
> 
> So it is reasonable for standalone brokers only.
> 
> 
> Diffs
> -----
> 
>   /trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1608083 
>   /trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1608083 
>   /trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1608083 
>   /trunk/qpid/cpp/src/qpid/legacystore/MessageStoreImpl.h 1608083 
>   /trunk/qpid/cpp/src/qpid/legacystore/MessageStoreImpl.cpp 1608083 
>   /trunk/qpid/cpp/src/qpid/linearstore/MessageStoreImpl.h 1608083 
>   /trunk/qpid/cpp/src/qpid/linearstore/MessageStoreImpl.cpp 1608083 
> 
> Diff: https://reviews.apache.org/r/23305/diff/
> 
> 
> Testing
> -------
> 
> No significant difference in memory consumption before & after restart (in setup
of 500 queues with qpid.store_msgID=true and thousands of messages sent via fanout exchange
to all of them).
> 
> Automated tests passed.
> 
> 
> Thanks,
> 
> Pavel Moravec
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message