----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9824/#review17616 -----------------------------------------------------------
Ship it! Aside from the minor nits below, looks good. Removing the existing threshold event and config will be visible externally. Should it be left in as deprecated for one release cycle? trunk/qpid/cpp/src/qpid/broker/ThresholdAlerts.cpp <https://reviews.apache.org/r/9824/#comment37363> Overriding a bad value (down > up) with a valid default is good, but it's probably a good idea to drop a log message when doing so. trunk/qpid/tests/src/py/qpid_tests/broker_0_10/threshold.py <https://reviews.apache.org/r/9824/#comment37364> what happens if depth toggles between 9 and 10? It appears the code would prevent generating events. This test should explicitly trigger that case to ensure the code doesn't get broken over time. [and also toggle between 4 and 5 to cover the down event] - Kenneth Giusti On March 8, 2013, 3:13 p.m., Ted Ross wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9824/ > ----------------------------------------------------------- > > (Updated March 8, 2013, 3:13 p.m.) > > > Review request for qpid. > > > Description > ------- > > Change the queue-depth threshold alerts to be edge-triggered rather than > level-sensitive. Provide upper and lower thresholds for both message count > and byte count. Two events are generated: "Crossing-Upward" events are > raised when the upper threshold is crossed lower-to-higher and > "Crossing-Downward" events are raised when the lower threshold is crossed > higher-to-lower. > > This removes the need for a mechanism to throttle events and it also removes > the need for special handling for recursion (thresholds-on-the-event-queue). > > If the lower thresholds are not specified or invalid, they are set to half of > the upper threshold. > > > This addresses bug QPID-4632. > https://issues.apache.org/jira/browse/QPID-4632 > > > Diffs > ----- > > trunk/qpid/cpp/src/qpid/broker/QueueSettings.h 1454413 > trunk/qpid/cpp/src/qpid/broker/QueueSettings.cpp 1454413 > trunk/qpid/cpp/src/qpid/broker/ThresholdAlerts.h 1454413 > trunk/qpid/cpp/src/qpid/broker/ThresholdAlerts.cpp 1454413 > trunk/qpid/specs/management-schema.xml 1454413 > trunk/qpid/tests/src/py/qpid_tests/broker_0_10/threshold.py 1454413 > > Diff: https://reviews.apache.org/r/9824/diff/ > > > Testing > ------- > > The existing tests were modified and a new test for threshold hysteresis was > added. > > > Thanks, > > Ted Ross > >
