----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2335/#review2505 -----------------------------------------------------------
/trunk/qpid/cpp/src/qpid/broker/Message.cpp <https://reviews.apache.org/r/2335/#comment5669> Might be worth renaming this method. It should have been renamed much earlier, but now it seems if anything even more odd given there is a parameter that effectively indicates whether to set the timestamp. Maybe just routed() or handled()? /trunk/qpid/specs/management-schema.xml <https://reviews.apache.org/r/2335/#comment5670> I'm torn on this approach to management. On the one hand I like having a relatively loose schema that can be evolved more easily, and I like having generic mechanisms rather than lots of specific methods. On the other hand the use of the map as the value feels clunky. It also feels as if the control of attributes should be a more intrinsic part of QMF. And finally I'm concerned that the vision for management at a higher level isn't really clear and our piecemeal changes (not just this but over many other changes) is lacking coherence. - Gordon On 2011-10-10 23:14:39, Kenneth Giusti wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/2335/ > ----------------------------------------------------------- > > (Updated 2011-10-10 23:14:39) > > > Review request for qpid, Gordon Sim and Ted Ross. > > > Summary > ------- > > A slight deviation from the design originally proposed in QPID-3417 - this > change adds the timestamp delivery property to messages using the relatively > simple approach as described in AMQP-0.10. > > Other than the approach itself, the QMF management interface & schema changes > seem like I could be guilty of overkill - I'd like feedback before I go too > far down that hole... > > > This addresses bug qpid-3417. > https://issues.apache.org/jira/browse/qpid-3417 > > > Diffs > ----- > > /trunk/qpid/cpp/src/qpid/broker/Broker.h 1180888 > /trunk/qpid/cpp/src/qpid/broker/Broker.cpp 1180888 > /trunk/qpid/cpp/src/qpid/broker/Message.h 1180888 > /trunk/qpid/cpp/src/qpid/broker/Message.cpp 1180888 > /trunk/qpid/cpp/src/qpid/broker/SemanticState.cpp 1180888 > /trunk/qpid/cpp/src/qpid/client/amqp0_10/IncomingMessages.cpp 1180888 > /trunk/qpid/cpp/src/tests/BrokerOptions.cpp PRE-CREATION > /trunk/qpid/cpp/src/tests/Makefile.am 1180888 > /trunk/qpid/specs/management-schema.xml 1180888 > > Diff: https://reviews.apache.org/r/2335/diff > > > Testing > ------- > > One unit test to verify the timestamp is being added. No dynamic control via > mgmt yet. > > Simple perf testing didn't seem like the hit wasn't too bad so far: > > Pre patch, from trunk: > [root@mrg44 tests]# qpid-cpp-benchmark --repeat=5 --summarize > --messages=5000000 > send-tp recv-tp l-min l-max l-avg > 68428 68388 0.16 69.59 4.16 > 68238 68201 0.17 44.18 3.82 > 68622 68581 0.16 102.52 4.60 > 68688 68647 0.18 117.33 5.29 > 69142 69104 0.19 103.30 4.50 > > > Patched, no timestamping: > [root@mrg44 src]# qpid-cpp-benchmark --repeat=5 --summarize --messages=5000000 > send-tp recv-tp l-min l-max l-avg > 67543 67471 0.17 79.76 4.37 > 69069 69028 0.15 42.92 3.78 > 68481 68439 0.17 45.91 3.98 > 68674 68636 0.18 41.30 3.74 > 67588 67587 0.17 60.23 4.21 > > > Patched, timestamping enabled: > [root@mrg44 src]# qpid-cpp-benchmark --repeat=5 --summarize --messages=5000000 > send-tp recv-tp l-min l-max l-avg > 67228 67227 0.21 41.80 3.97 > 67697 67659 0.19 43.01 4.19 > 67405 67368 0.19 101.61 4.99 > 66515 66511 0.15 41.85 4.10 > 67664 67622 0.17 47.35 4.01 > > > Thanks, > > Kenneth > >
