Commit 402fdc78 authored by Razvan Becheriu's avatar Razvan Becheriu

review changes

parent 798f70f6
......@@ -10,5 +10,5 @@
\"duration\": 1245
}
}",
"cmd-comment": "The server will respond with message about successfully setted limits for all statistics, with a result set to 0 indicating success and an empty parameters field. If an error is encountered, the server will return a status code of 1 (error) and the text field will contain the error description."
"cmd-comment": "The server will respond with message about successfully set limits for all statistics, with a result set to 0 indicating success and an empty parameters field. If an error is encountered, the server will return a status code of 1 (error) and the text field will contain the error description."
}
......@@ -11,5 +11,5 @@
\"duration\": 1245
}
}",
"cmd-comment": "The server will respond with message about successfully setted limit for the given statistic, with a result set to 0 indicating success and an empty parameters field. If an error is encountered (e.g. requested statistic was not found), the server will return a status code of 1 (error) and the text field will contain the error description."
"cmd-comment": "The server will respond with message about successfully set limit for the given statistic, with a result set to 0 indicating success and an empty parameters field. If an error is encountered (e.g. requested statistic was not found), the server will return a status code of 1 (error) and the text field will contain the error description."
}
......@@ -10,5 +10,5 @@
\"max-samples\": 100
}
}",
"cmd-comment": "The server will respond with message about successfully setted limits for all statistics, with a result set to 0 indicating success and an empty parameters field. If an error is encountered, the server will return a status code of 1 (error) and the text field will contain the error description."
"cmd-comment": "The server will respond with message about successfully set limits for all statistics, with a result set to 0 indicating success and an empty parameters field. If an error is encountered, the server will return a status code of 1 (error) and the text field will contain the error description."
}
......@@ -11,5 +11,5 @@
\"max-samples\": 100
}
}",
"cmd-comment": "The server will respond with message about successfully setted limit for the given statistic, with a result set to 0 indicating success and an empty parameters field. If an error is encountered (e.g. requested statistic was not found), the server will return a status code of 1 (error) and the text field will contain the error description."
"cmd-comment": "The server will respond with message about successfully set limit for the given statistic, with a result set to 0 indicating success and an empty parameters field. If an error is encountered (e.g. requested statistic was not found), the server will return a status code of 1 (error) and the text field will contain the error description."
}
......@@ -339,15 +339,14 @@ a status code of 1 (error) and the text field contains the error description.
Time series
====================
Previously, by default, each statistic holded only a single data point. When Kea
attempted to record a new value, the existing previous value was
overwritten. That approach has the benefit of taking up little memory and
it covers most cases reasonably well. However, there may be cases where
you need to have many data points for some process. For example, some
processes, such as received packet size, packet processing time or number
of database queries needed to process a packet, are not cumulative and it
would be useful to keep many data points, perhaps to do some form of
statistical analysis afterwards.
Previously, by default, each statistic held only a single data point. When Kea
attempted to record a new value, the existing previous value was overwritten.
That approach has the benefit of taking up little memory and it covers most
cases reasonably well. However, there may be cases where you need to have many
data points for some process. For example, some processes, such as received
packet size, packet processing time or number of database queries needed to
process a packet, are not cumulative and it would be useful to keep many data
points, perhaps to do some form of statistical analysis afterwards.
Since Kea 1.6, by default, each statistic holds 20 data points. Setting such
......
......@@ -875,8 +875,6 @@ ControlledDhcpv4Srv::ControlledDhcpv4Srv(uint16_t server_port /*= DHCP4_SERVER_P
CommandMgr::instance().registerCommand("statistic-set-max-sample-count-all",
boost::bind(&StatsMgr::statisticSetMaxSampleCountAllHandler, _1, _2));
}
void ControlledDhcpv4Srv::shutdown() {
......@@ -922,7 +920,6 @@ ControlledDhcpv4Srv::~ControlledDhcpv4Srv() {
CommandMgr::instance().deregisterCommand("statistic-set-max-sample-count-all");
CommandMgr::instance().deregisterCommand("version-get");
} catch (...) {
// Don't want to throw exceptions from the destructor. The server
// is shutting down anyway.
......
......@@ -941,7 +941,7 @@ ControlledDhcpv6Srv::~ControlledDhcpv6Srv() {
CommandMgr::instance().deregisterCommand("statistic-set-max-sample-age");
CommandMgr::instance().deregisterCommand("statistic-set-max-sample-count");
CommandMgr::instance().deregisterCommand("statistic-set-max-sample-age-all");
CommandMgr::instance().deregisterCommand("statistic-set-max-sample-count-all");
CommandMgr::instance().deregisterCommand("statistic-set-max-sample-count-all");
CommandMgr::instance().deregisterCommand("version-get");
} catch (...) {
......
......@@ -1089,7 +1089,7 @@ TEST_F(CtrlChannelDhcpv6SrvTest, commandsList) {
checkListCommands(rsp, "statistic-set-max-sample-age");
checkListCommands(rsp, "statistic-set-max-sample-age-all");
checkListCommands(rsp, "statistic-set-max-sample-count");
checkListCommands(rsp, "statistic-set-max-sample-count-all");
checkListCommands(rsp, "statistic-set-max-sample-count-all");
}
// Tests if the server returns its configuration using config-get.
......
......@@ -193,12 +193,12 @@ class Observation {
/// @return size of storage
size_t getSize() const;
/// @brief Returns both value of max_sample_age_ of statistic.
/// @brief Returns both values of max_sample_age_ of statistic.
///
/// @return max_sample_age_.
std::pair<bool, StatsDuration> getMaxSampleAge() const;
/// @brief Returns both value of max_sample_count_ of statistic.
/// @brief Returns both values of max_sample_count_ of statistic.
///
/// @return max_sample_count_.
std::pair<bool, uint32_t> getMaxSampleCount() const;
......
......@@ -195,7 +195,7 @@ StatsMgr::statisticSetMaxSampleAgeHandler(const std::string& /*name*/,
if (!getStatDuration(params, duration, error)) {
return (createAnswer(CONTROL_RESULT_ERROR, error));
}
if (instance().setMaxSampleAge(name, duration)) {
if (setMaxSampleAge(name, duration)) {
return (createAnswer(CONTROL_RESULT_SUCCESS,
"Statistic '" + name + "' duration limit is set."));
} else {
......@@ -215,7 +215,7 @@ StatsMgr::statisticSetMaxSampleCountHandler(const std::string& /*name*/,
if (!getStatMaxSamples(params, max_samples, error)) {
return (createAnswer(CONTROL_RESULT_ERROR, error));
}
if (instance().setMaxSampleCount(name, max_samples)) {
if (setMaxSampleCount(name, max_samples)) {
return (createAnswer(CONTROL_RESULT_SUCCESS,
"Statistic '" + name + "' count limit is set."));
} else {
......@@ -300,7 +300,7 @@ StatsMgr::statisticSetMaxSampleAgeAllHandler(const std::string& /*name*/,
if (!getStatDuration(params, duration, error)) {
return (createAnswer(CONTROL_RESULT_ERROR, error));
}
instance().setMaxSampleAgeAll(duration);
setMaxSampleAgeAll(duration);
return (createAnswer(CONTROL_RESULT_SUCCESS,
"All statistics duration limit are set."));
}
......@@ -313,7 +313,7 @@ StatsMgr::statisticSetMaxSampleCountAllHandler(const std::string& /*name*/,
if (!getStatMaxSamples(params, max_samples, error)) {
return (createAnswer(CONTROL_RESULT_ERROR, error));
}
instance().setMaxSampleCountAll(max_samples);
setMaxSampleCountAll(max_samples);
return (createAnswer(CONTROL_RESULT_SUCCESS,
"All statistics count limit are set."));
}
......@@ -354,16 +354,16 @@ StatsMgr::getStatDuration(const isc::data::ConstElementPtr& params,
return (false);
}
int64_t dur = stat_duration->intValue();
int64_t time_duration = stat_duration->intValue();
int64_t hours = dur/3600;
dur = dur - hours*3600;
int64_t hours = time_duration / 3600;
time_duration -= hours * 3600;
int64_t minutes = dur/60;
dur = dur - minutes*60;
int64_t minutes = time_duration / 60;
time_duration -= minutes * 60;
int64_t seconds = dur;
duration = boost::posix_time::time_duration(hours,minutes,seconds,0);
int64_t seconds = time_duration;
duration = boost::posix_time::time_duration(hours, minutes, seconds, 0);
return (true);
}
......
......@@ -149,7 +149,7 @@ class StatsMgr : public boost::noncopyable {
///
/// Specifies that statistic name should be stored not as single value, but
/// rather as a set of values. In this form, at most max_samples will be kept.
/// When adding max_samples+1 sample, the oldest sample will be discarded.
/// When adding max_samples + 1 sample, the oldest sample will be discarded.
///
///
/// @param name name of the observation
......@@ -256,7 +256,7 @@ class StatsMgr : public boost::noncopyable {
///
/// This method handles statistic-get command, which returns value
/// of a given statistic). It expects one parameter stored in params map:
/// name: name-of-the-statistic
/// name: name of the statistic
///
/// Example params structure:
/// {
......@@ -274,7 +274,7 @@ class StatsMgr : public boost::noncopyable {
///
/// This method handles statistic-reset command, which resets value
/// of a given statistic. It expects one parameter stored in params map:
/// name: name-of-the-statistic
/// name: name of the statistic
///
/// Example params structure:
/// {
......@@ -292,7 +292,7 @@ class StatsMgr : public boost::noncopyable {
///
/// This method handles statistic-reset command, which removes a given
/// statistic completely. It expects one parameter stored in params map:
/// name: name-of-the-statistic
/// name: name of the statistic
///
/// Example params structure:
/// {
......@@ -312,7 +312,7 @@ class StatsMgr : public boost::noncopyable {
/// which set max_sample_age_ limit of a given statistic
/// and leaves max_sample_count_ disabled.
/// It expects two parameters stored in params map:
/// name: name-of-the-statistic
/// name: name of the statistic
/// duration: time limit expressed as a number of seconds
///
/// Example params structure:
......@@ -334,8 +334,8 @@ class StatsMgr : public boost::noncopyable {
/// which set max_sample_count_ limit of a given statistic
/// and leaves max_sample_age_ disabled.
/// It expects two parameters stored in params map:
/// name: name-of-the-statistic
/// max-samples: value of max_sample_count_
/// name: name of the statistic
/// max-samples: count limit
///
/// Example params structure:
/// {
......@@ -348,7 +348,7 @@ class StatsMgr : public boost::noncopyable {
/// @return answer containing information about successfully setup limit of statistic
static isc::data::ConstElementPtr
statisticSetMaxSampleCountHandler(const std::string& name,
const isc::data::ConstElementPtr& params);
const isc::data::ConstElementPtr& params);
/// @brief Handles statistic-get-all command
///
......@@ -403,27 +403,27 @@ class StatsMgr : public boost::noncopyable {
/// @return answer confirming success of this operation
static isc::data::ConstElementPtr
statisticSetMaxSampleAgeAllHandler(const std::string& name,
const isc::data::ConstElementPtr& params);
const isc::data::ConstElementPtr& params);
/// @brief Handles statistic-set-max-sample-count command
/// @brief Handles statistic-set-max-sample-count-all command
///
/// This method handles statistic-set-max-sample-count command,
/// This method handles statistic-set-max-sample-count-all command,
/// which set max_sample_count_ limit of a given statistic
/// and leaves max_sample_age_ disabled.
/// It expects one parameter stored in params map:
/// max-samples: value of max_sample_count_
/// max-samples: count limit
///
/// Example params structure:
/// {
/// "max-samples": 15
/// }
///
/// @param name name of the command (ignored, should be "statistic-set-max-sample-count")
/// @param name name of the command (ignored, should be "statistic-set-max-sample-count-all")
/// @param params structure containing a map that contains "max-samples"
/// @return answer confirming success of this operation
static isc::data::ConstElementPtr
statisticSetMaxSampleCountAllHandler(const std::string& name,
const isc::data::ConstElementPtr& params);
const isc::data::ConstElementPtr& params);
/// @}
......@@ -547,7 +547,7 @@ private:
///
/// This method attempts to extract count limit for a given statistic
/// from the params structure.
/// It is expected to be a map that contains 'max_samples' element,
/// It is expected to be a map that contains 'max-samples' element,
/// that is of type int. If present as expected, statistic count
/// limit (max_samples) is set and true is returned.
/// If missing or is of incorrect type, the reason is specified in reason
......
......@@ -449,14 +449,31 @@ TEST_F(ObservationTest, getLimits) {
EXPECT_EQ(c.getMaxSampleCount().second, 20);
EXPECT_EQ(d.getMaxSampleCount().second, 20);
// Change limit to max_sample_age_
a.setMaxSampleAge(millisec::time_duration(0, 4, 5, 3));
// change limit to time duration
ASSERT_NOT_THROW(a.setMaxSampleAge(millisec::time_duration(0, 4, 5, 3)));
ASSERT_NOT_THROW(b.setMaxSampleAge(millisec::time_duration(0, 4, 5, 3)));
ASSERT_NOT_THROW(c.setMaxSampleAge(millisec::time_duration(0, 4, 5, 3)));
ASSERT_NOT_THROW(d.setMaxSampleAge(millisec::time_duration(0, 4, 5, 3)));
EXPECT_EQ(a.getMaxSampleAge().first, true);
EXPECT_EQ(b.getMaxSampleAge().first, true);
EXPECT_EQ(c.getMaxSampleAge().first, true);
EXPECT_EQ(d.getMaxSampleAge().first, true);
EXPECT_EQ(a.getMaxSampleAge().second, millisec::time_duration(0, 4, 5, 3));
EXPECT_EQ(b.getMaxSampleAge().second, millisec::time_duration(0, 4, 5, 3));
EXPECT_EQ(c.getMaxSampleAge().second, millisec::time_duration(0, 4, 5, 3));
EXPECT_EQ(d.getMaxSampleAge().second, millisec::time_duration(0, 4, 5, 3));
EXPECT_EQ(a.getMaxSampleCount().first, false);
EXPECT_EQ(a.getMaxSampleCount().second, 20);
EXPECT_EQ(b.getMaxSampleCount().first, false);
EXPECT_EQ(c.getMaxSampleCount().first, false);
EXPECT_EQ(d.getMaxSampleCount().first, false);
EXPECT_EQ(a.getMaxSampleCount().second, 20);
EXPECT_EQ(b.getMaxSampleCount().second, 20);
EXPECT_EQ(c.getMaxSampleCount().second, 20);
EXPECT_EQ(d.getMaxSampleCount().second, 20);
}
// Test checks whether timing is reported properly.
......
......@@ -170,54 +170,48 @@ TEST_F(StatsMgrTest, setLimitsAll) {
StatsMgr::instance().setValue("gamma", time_duration(1, 2, 3, 4));
StatsMgr::instance().setValue("delta", "Lorem ipsum");
//First, check the setting of time limit to existing statistics
// check the setting of time limit to existing statistics
EXPECT_NO_THROW(StatsMgr::instance().setMaxSampleAgeAll(time_duration(0, 0, 1, 0)));
// Now check if time limit was set properly
// check if time limit was set properly and whether count limit is disabled
EXPECT_EQ(StatsMgr::instance().getObservation("alpha")->getMaxSampleAge().first, true);
EXPECT_EQ(StatsMgr::instance().getObservation("alpha")->getMaxSampleAge().second,
time_duration(0, 0, 1, 0));
// and whether count limit is disabled
time_duration(0, 0, 1, 0));
EXPECT_EQ(StatsMgr::instance().getObservation("alpha")->getMaxSampleCount().first, false);
EXPECT_EQ(StatsMgr::instance().getObservation("beta")->getMaxSampleAge().first, true);
EXPECT_EQ(StatsMgr::instance().getObservation("beta")->getMaxSampleAge().second,
time_duration(0, 0, 1, 0));
time_duration(0, 0, 1, 0));
EXPECT_EQ(StatsMgr::instance().getObservation("beta")->getMaxSampleCount().first, false);
EXPECT_EQ(StatsMgr::instance().getObservation("gamma")->getMaxSampleAge().first, true);
EXPECT_EQ(StatsMgr::instance().getObservation("gamma")->getMaxSampleAge().second,
time_duration(0, 0, 1, 0));
time_duration(0, 0, 1, 0));
EXPECT_EQ(StatsMgr::instance().getObservation("gamma")->getMaxSampleCount().first, false);
EXPECT_EQ(StatsMgr::instance().getObservation("delta")->getMaxSampleAge().first, true);
EXPECT_EQ(StatsMgr::instance().getObservation("delta")->getMaxSampleAge().second,
time_duration(0, 0, 1, 0));
time_duration(0, 0, 1, 0));
EXPECT_EQ(StatsMgr::instance().getObservation("delta")->getMaxSampleCount().first, false);
//then, check the setting of count limit to existing statistics
// check the setting of count limit to existing statistics
EXPECT_NO_THROW(StatsMgr::instance().setMaxSampleCountAll(1200));
// Now check if count limit was set properly
// check if count limit was set properly and whether count limit is disabled
EXPECT_EQ(StatsMgr::instance().getObservation("alpha")->getMaxSampleCount().first, true);
EXPECT_EQ(StatsMgr::instance().getObservation("alpha")->getMaxSampleCount().second, 1200);
// and whether count limit is disabled
EXPECT_EQ(StatsMgr::instance().getObservation("alpha")->getMaxSampleAge().first, false);
EXPECT_EQ(StatsMgr::instance().getObservation("beta")->getMaxSampleCount().first, true);
EXPECT_EQ(StatsMgr::instance().getObservation("beta")->getMaxSampleCount().second, 1200);
EXPECT_EQ(StatsMgr::instance().getObservation("beta")->getMaxSampleAge().first, false);
EXPECT_EQ(StatsMgr::instance().getObservation("gamma")->getMaxSampleCount().first, true);
EXPECT_EQ(StatsMgr::instance().getObservation("gamma")->getMaxSampleCount().second, 1200);
EXPECT_EQ(StatsMgr::instance().getObservation("gamma")->getMaxSampleAge().first, false);
EXPECT_EQ(StatsMgr::instance().getObservation("delta")->getMaxSampleCount().first, true);
EXPECT_EQ(StatsMgr::instance().getObservation("delta")->getMaxSampleCount().second, 1200);
EXPECT_EQ(StatsMgr::instance().getObservation("delta")->getMaxSampleAge().first, false);
}
......@@ -793,11 +787,10 @@ TEST_F(StatsMgrTest, commandSetMaxSampleAge) {
ASSERT_NO_THROW(parseAnswer(status_code, rsp));
EXPECT_EQ(CONTROL_RESULT_SUCCESS, status_code);
// Now check if time limit was set properly
// check if time limit was set properly and whether count limit is disabled
EXPECT_EQ(StatsMgr::instance().getObservation("alpha")->getMaxSampleAge().first, true);
EXPECT_EQ(StatsMgr::instance().getObservation("alpha")->getMaxSampleAge().second,
time_duration(0, 20, 45, 0));
// and whether count limit is disabled
time_duration(0, 20, 45, 0));
EXPECT_EQ(StatsMgr::instance().getObservation("alpha")->getMaxSampleCount().first, false);
}
......@@ -807,7 +800,6 @@ TEST_F(StatsMgrTest, commandSetMaxSampleAge) {
// - a request with missing statistic name
// - a request for non-existing statistic.
TEST_F(StatsMgrTest, commandSetMaxSampleAgeNegative) {
// Case 1: a request without parameters
ConstElementPtr rsp =
StatsMgr::instance().statisticSetMaxSampleAgeHandler("statistic-set-max-sample-age", ElementPtr());
......@@ -852,29 +844,25 @@ TEST_F(StatsMgrTest, commandSetMaxSampleAgeAll) {
ASSERT_NO_THROW(parseAnswer(status_code, rsp));
EXPECT_EQ(CONTROL_RESULT_SUCCESS, status_code);
// Now check if time limit was set properly
// check if time limit was set properly and whether count limit is disabled
EXPECT_EQ(StatsMgr::instance().getObservation("alpha")->getMaxSampleAge().first, true);
EXPECT_EQ(StatsMgr::instance().getObservation("alpha")->getMaxSampleAge().second,
time_duration(1, 2, 45, 0));
// and whether count limit is disabled
time_duration(1, 2, 45, 0));
EXPECT_EQ(StatsMgr::instance().getObservation("alpha")->getMaxSampleCount().first, false);
EXPECT_EQ(StatsMgr::instance().getObservation("beta")->getMaxSampleAge().first, true);
EXPECT_EQ(StatsMgr::instance().getObservation("beta")->getMaxSampleAge().second,
time_duration(1, 2, 45, 0));
time_duration(1, 2, 45, 0));
EXPECT_EQ(StatsMgr::instance().getObservation("beta")->getMaxSampleCount().first, false);
EXPECT_EQ(StatsMgr::instance().getObservation("gamma")->getMaxSampleAge().first, true);
EXPECT_EQ(StatsMgr::instance().getObservation("gamma")->getMaxSampleAge().second,
time_duration(1, 2, 45, 0));
time_duration(1, 2, 45, 0));
EXPECT_EQ(StatsMgr::instance().getObservation("gamma")->getMaxSampleCount().first, false);
EXPECT_EQ(StatsMgr::instance().getObservation("delta")->getMaxSampleAge().first, true);
EXPECT_EQ(StatsMgr::instance().getObservation("delta")->getMaxSampleAge().second,
time_duration(1, 2, 45, 0));
time_duration(1, 2, 45, 0));
EXPECT_EQ(StatsMgr::instance().getObservation("delta")->getMaxSampleCount().first, false);
}
......@@ -893,21 +881,18 @@ TEST_F(StatsMgrTest, commandSetMaxSampleCount) {
ASSERT_NO_THROW(parseAnswer(status_code, rsp));
EXPECT_EQ(CONTROL_RESULT_SUCCESS, status_code);
// Now check if time limit was set properly
// check if time limit was set properly and whether duration limit is disabled
EXPECT_EQ(StatsMgr::instance().getObservation("alpha")->getMaxSampleCount().first, true);
EXPECT_EQ(StatsMgr::instance().getObservation("alpha")->getMaxSampleCount().second, 15);
// and whether duration limit is disabled
EXPECT_EQ(StatsMgr::instance().getObservation("alpha")->getMaxSampleAge().first, false);
}
// Test checks if statistic-set-max-sample-age is able to handle:
// Test checks if statistic-set-max-sample-count is able to handle:
// - a request without parameters
// - a request without max-samples parameter
// - a request with missing statistic name
// - a request for non-existing statistic.
TEST_F(StatsMgrTest, commandSetMaxSampleCountNegative) {
// Case 1: a request without parameters
ConstElementPtr rsp =
StatsMgr::instance().statisticSetMaxSampleCountHandler("statistic-set-max-sample-count", ElementPtr());
......@@ -951,25 +936,22 @@ TEST_F(StatsMgrTest, commandSetMaxSampleCountAll) {
int status_code;
ASSERT_NO_THROW(parseAnswer(status_code, rsp));
EXPECT_EQ(CONTROL_RESULT_SUCCESS, status_code);
// Now check if count limit was set properly
// check if count limit was set properly and whether count limit is disabled
EXPECT_EQ(StatsMgr::instance().getObservation("alpha")->getMaxSampleCount().first, true);
EXPECT_EQ(StatsMgr::instance().getObservation("alpha")->getMaxSampleCount().second, 200);
// and whether count limit is disabled
EXPECT_EQ(StatsMgr::instance().getObservation("alpha")->getMaxSampleAge().first, false);
EXPECT_EQ(StatsMgr::instance().getObservation("beta")->getMaxSampleCount().first, true);
EXPECT_EQ(StatsMgr::instance().getObservation("beta")->getMaxSampleCount().second, 200);
EXPECT_EQ(StatsMgr::instance().getObservation("beta")->getMaxSampleAge().first, false);
EXPECT_EQ(StatsMgr::instance().getObservation("gamma")->getMaxSampleCount().first, true);
EXPECT_EQ(StatsMgr::instance().getObservation("gamma")->getMaxSampleCount().second, 200);
EXPECT_EQ(StatsMgr::instance().getObservation("gamma")->getMaxSampleAge().first, false);
EXPECT_EQ(StatsMgr::instance().getObservation("delta")->getMaxSampleCount().first, true);
EXPECT_EQ(StatsMgr::instance().getObservation("delta")->getMaxSampleCount().second, 200);
EXPECT_EQ(StatsMgr::instance().getObservation("delta")->getMaxSampleAge().first, false);
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment