> On 2011-10-11 07:51:39, Gordon Sim wrote: > > /trunk/qpid/specs/management-schema.xml, line 106 > > <https://reviews.apache.org/r/2335/diff/1/?file=49296#file49296line106> > > > > 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. > > Kenneth Giusti wrote: > Me too - I'd prefer having "enableTimestamp" as a simple boolean QMF > property with RW access, but QMF doesn't yet support that type of access. > > The ability to turn this feature on and off at runtime is critical - at > best I'll have to go with a set of "get/set" timestamping methods.
I think get/set timestamping methods would be the best option at this point. We already have get/set log-level. We can then try and genericise the approach at some later point. - Gordon ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/2335/#review2505 ----------------------------------------------------------- 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 > >