fgerlits commented on code in PR #1477:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1477#discussion_r1086575694


##########
libminifi/test/unit/MetricsTests.cpp:
##########
@@ -246,6 +246,38 @@ TEST_CASE("Test ProcessorMetrics", "[ProcessorMetrics]") {
   metrics.addLastOnTriggerRuntime(10ms);
   REQUIRE(metrics.getLastOnTriggerRuntime() == 10ms);
   REQUIRE(metrics.getAverageOnTriggerRuntime() == 37ms);
+
+  REQUIRE(metrics.getLastSessionCommitRuntime() == 0ms);
+  REQUIRE(metrics.getAverageSessionCommitRuntime() == 0ms);
+
+  metrics.addLastSessionCommitRuntime(10ms);
+  metrics.addLastSessionCommitRuntime(20ms);
+  metrics.addLastSessionCommitRuntime(30ms);
+
+  REQUIRE(metrics.getLastSessionCommitRuntime() == 30ms);
+  REQUIRE(metrics.getAverageSessionCommitRuntime() == 20ms);
+
+  for (auto i = 0; i < 7; ++i) {
+    metrics.addLastSessionCommitRuntime(50ms);
+  }
+  REQUIRE(metrics.getAverageSessionCommitRuntime() == 41ms);
+  REQUIRE(metrics.getLastSessionCommitRuntime() == 50ms);
+
+  for (auto i = 0; i < 3; ++i) {
+    metrics.addLastSessionCommitRuntime(50ms);
+  }
+  REQUIRE(metrics.getAverageSessionCommitRuntime() == 50ms);
+  REQUIRE(metrics.getLastSessionCommitRuntime() == 50ms);
+
+  for (auto i = 0; i < 10; ++i) {
+    metrics.addLastSessionCommitRuntime(40ms);
+  }
+  REQUIRE(metrics.getAverageSessionCommitRuntime() == 40ms);
+  REQUIRE(metrics.getLastSessionCommitRuntime() == 40ms);
+
+  metrics.addLastSessionCommitRuntime(10ms);
+  REQUIRE(metrics.getLastSessionCommitRuntime() == 10ms);
+  REQUIRE(metrics.getAverageSessionCommitRuntime() == 37ms);

Review Comment:
   I would put these in a separate `TEST_CASE`



##########
libminifi/src/core/ProcessorMetrics.cpp:
##########
@@ -105,6 +110,17 @@ std::chrono::milliseconds 
ProcessorMetrics::getLastOnTriggerRuntime() const {
   return on_trigger_runtime_averager_.getLastValue();
 }
 
+std::chrono::milliseconds ProcessorMetrics::getAverageSessionCommitRuntime() 
const {
+  return session_commit_runtime_averager_.getAverage();
+}
+
+void ProcessorMetrics::addLastSessionCommitRuntime(std::chrono::milliseconds 
runtime) {
+  session_commit_runtime_averager_.addValue(runtime);
+}
+
+std::chrono::milliseconds ProcessorMetrics::getLastSessionCommitRuntime() 
const {
+  return session_commit_runtime_averager_.getLastValue();
+}
 
 template<typename ValueType>
 requires Summable<ValueType> && DividableByInteger<ValueType>

Review Comment:
   at line 128, why is it safe to check `values_.empty()` outside of the lock?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to