----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23305/#review47442 -----------------------------------------------------------
Using a pointer as an identifier isn't safe. Pointer values can be re-used as soon as the in-memory copy of the message is deleted, so they are not unique over time. A global atomic counter might be a possibility but we would need to benchmark for performance consequences before making this the default behavior. - Alan Conway On July 8, 2014, 12:53 p.m., Pavel Moravec wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23305/ > ----------------------------------------------------------- > > (Updated July 8, 2014, 12:53 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 > >
