[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743:
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r413242502
##
File path: libminifi/src/c2/C2Agent.cpp
##
@@ -89,44 +93,68 @@ C2Agent::C2Agent(const
std::shared_ptr heart_beat_period_ ) {
-last_run_ = now;
-try {
- performHeartBeat();
-}
-catch(const std::exception ) {
- logger_->log_error("Exception occurred while performing heartbeat.
error: %s", e.what());
-}
-catch(...) {
- logger_->log_error("Unknonwn exception occurred while performing
heartbeat.");
-}
+ try {
+performHeartBeat();
+ }
+ catch(const std::exception ) {
+logger_->log_error("Exception occurred while performing heartbeat.
error: %s", e.what());
+ }
+ catch(...) {
+logger_->log_error("Unknonwn exception occurred while performing
heartbeat.");
}
checkTriggers();
- std::this_thread::sleep_for(std::chrono::milliseconds(heart_beat_period_
> 500 ? 500 : heart_beat_period_));
- return
state::Update(state::UpdateStatus(state::UpdateState::READ_COMPLETE, false));
+ return
utils::TaskRescheduleInfo::RetryIn(std::chrono::milliseconds(heart_beat_period_));
};
-
functions_.push_back(c2_producer_);
c2_consumer_ = [&]() {
auto now = std::chrono::steady_clock::now();
if ( queue_mutex.try_lock_until(now + std::chrono::seconds(1)) ) {
- if (responses.size() > 0) {
-const C2Payload payload(std::move(responses.back()));
-responses.pop_back();
-extractPayload(std::move(payload));
+ if (responses.empty()) {
Review comment:
It had to change because of a potential deadlock situation there.
The deadlock happens as follows:
Thread performing heartbeat acquires request_mutex and while holding lock on
this mutex acquires lock on queue_mutex in enqueue_c2_server_response
Thread handling C2 tries to acquire both locks in the reverse order creating
a deadlock.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743:
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r413240661
##
File path: libminifi/src/c2/C2Agent.cpp
##
@@ -89,44 +93,68 @@ C2Agent::C2Agent(const
std::shared_ptr heart_beat_period_ ) {
-last_run_ = now;
-try {
- performHeartBeat();
-}
-catch(const std::exception ) {
- logger_->log_error("Exception occurred while performing heartbeat.
error: %s", e.what());
-}
-catch(...) {
- logger_->log_error("Unknonwn exception occurred while performing
heartbeat.");
-}
+ try {
+performHeartBeat();
+ }
+ catch(const std::exception ) {
+logger_->log_error("Exception occurred while performing heartbeat.
error: %s", e.what());
+ }
+ catch(...) {
+logger_->log_error("Unknonwn exception occurred while performing
heartbeat.");
}
checkTriggers();
- std::this_thread::sleep_for(std::chrono::milliseconds(heart_beat_period_
> 500 ? 500 : heart_beat_period_));
- return
state::Update(state::UpdateStatus(state::UpdateState::READ_COMPLETE, false));
+ return
utils::TaskRescheduleInfo::RetryIn(std::chrono::milliseconds(heart_beat_period_));
};
-
functions_.push_back(c2_producer_);
c2_consumer_ = [&]() {
auto now = std::chrono::steady_clock::now();
if ( queue_mutex.try_lock_until(now + std::chrono::seconds(1)) ) {
- if (responses.size() > 0) {
-const C2Payload payload(std::move(responses.back()));
-responses.pop_back();
-extractPayload(std::move(payload));
+ if (responses.empty()) {
+queue_mutex.unlock();
+return
utils::TaskRescheduleInfo::RetryIn(std::chrono::milliseconds(100));
Review comment:
Defined a const.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743:
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r413228089
##
File path: libminifi/include/c2/C2Agent.h
##
@@ -52,39 +55,28 @@ namespace c2 {
* 0 HeartBeat -- RESERVED
* 1-255 Defined by the configuration file.
*/
-class C2Agent : public state::UpdateController, public
state::response::ResponseNodeSink, public std::enable_shared_from_this
{
+class C2Agent : public state::UpdateController {
public:
- C2Agent(const std::shared_ptr
, const std::shared_ptr , const
std::shared_ptr );
+ C2Agent(const std::shared_ptr
,
+ const std::shared_ptr ,
+ const std::shared_ptr ,
+ utils::ThreadPool );
Review comment:
Used a separate threadpool
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r408289302
##
File path: libminifi/src/c2/C2Agent.cpp
##
@@ -89,44 +93,68 @@ C2Agent::C2Agent(const
std::shared_ptr heart_beat_period_ ) {
-last_run_ = now;
-try {
- performHeartBeat();
-}
-catch(const std::exception ) {
- logger_->log_error("Exception occurred while performing heartbeat.
error: %s", e.what());
-}
-catch(...) {
- logger_->log_error("Unknonwn exception occurred while performing
heartbeat.");
-}
+ try {
+performHeartBeat();
+ }
+ catch(const std::exception ) {
+logger_->log_error("Exception occurred while performing heartbeat.
error: %s", e.what());
+ }
+ catch(...) {
+logger_->log_error("Unknonwn exception occurred while performing
heartbeat.");
}
checkTriggers();
- std::this_thread::sleep_for(std::chrono::milliseconds(heart_beat_period_
> 500 ? 500 : heart_beat_period_));
- return
state::Update(state::UpdateStatus(state::UpdateState::READ_COMPLETE, false));
+ return
utils::TaskRescheduleInfo::RetryIn(std::chrono::milliseconds(heart_beat_period_));
};
-
functions_.push_back(c2_producer_);
c2_consumer_ = [&]() {
auto now = std::chrono::steady_clock::now();
if ( queue_mutex.try_lock_until(now + std::chrono::seconds(1)) ) {
- if (responses.size() > 0) {
-const C2Payload payload(std::move(responses.back()));
-responses.pop_back();
-extractPayload(std::move(payload));
+ if (responses.empty()) {
+queue_mutex.unlock();
+return utils::TaskRescheduleInfo::RetryImmediately();
Review comment:
> @msharee9 Tried lightweight and normal heartbeat and different flows, they
seem to work fine.
>
> However, even when running with no flow, but C2 enabled, minifi consumes
150% CPU both on macOS and Linux.
>
> This is very likely caused by the issue pointed out by Arpad, that is, we
are busy looping because of an incorrect usage of the Thread Pool.
>
> This callgrind profiling of minifi also seems to reinforce that suspicion:
> https://user-images.githubusercontent.com/9108564/79242487-b1a66680-7e74-11ea-9a6b-f07f61bccc54.png;>
Changed it to Retry in 100 ms. I guess then the RetryImmediately() cannot be
used anywhere. We should either remove it altogether or have some delay before
we call continue at this line.
https://github.com/apache/nifi-minifi-cpp/blob/3116eb817a7182486bf3e50b87c3032ac2074031/libminifi/src/utils/ThreadPool.cpp#L53
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r406886579
##
File path: libminifi/include/utils/BackTrace.h
##
@@ -81,13 +83,32 @@ void pull_trace(uint8_t frames_to_skip = 1);
*/
void emplace_handler();
+class Lock {
Review comment:
std::unique_lock could be used here. Removed the custom Lock class. my bad.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r401721628
##
File path: libminifi/src/utils/BackTrace.cpp
##
@@ -80,25 +80,21 @@ BackTrace TraceResolver::getBackTrace(std::string
thread_name, std::thread::nati
// lock so that we only perform one backtrace at a time.
#ifdef HAS_EXECINFO
std::lock_guard lock(mutex_);
-
- caller_handle_ = pthread_self();
- thread_handle_ = thread_handle;
trace_ = BackTrace(std::move(thread_name));
- if (0 == thread_handle_ || pthread_equal(caller_handle_, thread_handle)) {
+ if (0 == thread_handle || pthread_equal(pthread_self(), thread_handle)) {
pull_trace();
} else {
-if (thread_handle_ == 0) {
+if (thread_handle == 0) {
return std::move(trace_);
}
emplace_handler();
-if (pthread_kill(thread_handle_, SIGUSR2) != 0) {
+std::unique_lock ulock(trace_mutex_);
+if (pthread_kill(thread_handle, SIGUSR2) != 0) {
return std::move(trace_);
}
-sigset_t mask;
-sigfillset();
-sigdelset(, SIGUSR2);
-sigsuspend();
Review comment:
There was a race condition here which lead to a deadlock.
Caller thread (Thread that is interested in pulling backtrace of other
thread) acquired lock on a mutex (worker_queue_mutex_), and sent a SIGUSR2
signal to another thread (thread_handle_) but before the caller thread goes
into a suspended state or may be even before it removes the SIGUSR2 from its
blocking signal set (sigdelset) , the callee while executing its thread
handler, sent a SIGUSR2 signal back to the caller and at this time the caller
thread will run through its signal handler and after returning from signal
handler it goes back to suspended state. From this state there is no way of
waking it up.
The callee was waiting on a condition variable tied to the mutex caller was
holding thus creating a deadlock.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r401721628
##
File path: libminifi/src/utils/BackTrace.cpp
##
@@ -80,25 +80,21 @@ BackTrace TraceResolver::getBackTrace(std::string
thread_name, std::thread::nati
// lock so that we only perform one backtrace at a time.
#ifdef HAS_EXECINFO
std::lock_guard lock(mutex_);
-
- caller_handle_ = pthread_self();
- thread_handle_ = thread_handle;
trace_ = BackTrace(std::move(thread_name));
- if (0 == thread_handle_ || pthread_equal(caller_handle_, thread_handle)) {
+ if (0 == thread_handle || pthread_equal(pthread_self(), thread_handle)) {
pull_trace();
} else {
-if (thread_handle_ == 0) {
+if (thread_handle == 0) {
return std::move(trace_);
}
emplace_handler();
-if (pthread_kill(thread_handle_, SIGUSR2) != 0) {
+std::unique_lock ulock(trace_mutex_);
+if (pthread_kill(thread_handle, SIGUSR2) != 0) {
return std::move(trace_);
}
-sigset_t mask;
-sigfillset();
-sigdelset(, SIGUSR2);
-sigsuspend();
Review comment:
There was a race condition here which lead to a deadlock.
Caller thread (Thread that is interested in pulling backtrace of other
thread) acquired lock on a mutex (worker_queue_mutex_), that is outside this
function and sent a SIGUSR2 signal to another thread (thread_handle_) but
before the caller thread goes into a suspended state or may be even before it
removes the SIGUSR2 from its blocking signal set (sigdelset) , the callee while
executing its thread handler, sent a SIGUSR2 signal back to the caller and at
this time the caller thread will run through its signal handler and after
returning from signal handler it goes back to suspended state. From this state
there is no way of waking it up.
The callee was waiting on a condition variable tied to the mutex caller was
holding thus creating a deadlock.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r398046871
##
File path: nanofi/include/cxx/C2CallbackAgent.h
##
@@ -70,6 +69,7 @@ class C2CallbackAgent : public c2::C2Agent {
c2_ag_stop_callback *stop;
private:
+utils::ThreadPool thread_pool_;
Review comment:
Oh well, the thread_pool_ shouldn't be here at all. This file requires no
changes. I forgot to remove it from the previous commit.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r397589151
##
File path: libminifi/src/c2/C2Agent.cpp
##
@@ -32,24 +32,34 @@
#include "utils/file/FileUtils.h"
#include "utils/file/FileManager.h"
#include "utils/HTTPClient.h"
+#include "utils/GeneralUtils.h"
+#include "utils/Monitors.h"
+
namespace org {
namespace apache {
namespace nifi {
namespace minifi {
namespace c2 {
-C2Agent::C2Agent(const
std::shared_ptr , const
std::shared_ptr ,
- const std::shared_ptr )
+std::shared_ptr C2Agent::id_generator_ =
utils::IdGenerator::getIdGenerator();
Review comment:
Yep, that remained to be fixed. I forgot to make correction here. There is
one more in FlowController.cpp.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r397588367
##
File path: extensions/http-curl/tests/C2JstackTest.cpp
##
@@ -16,152 +16,63 @@
* limitations under the License.
*/
-#include
#undef NDEBUG
-#include
-#include
-#include
-#include
-#include
#include
-#include
-#include
-#include
-#include
-#include
-#include "HTTPClient.h"
-#include "InvokeHTTP.h"
#include "TestBase.h"
-#include "utils/StringUtils.h"
-#include "core/Core.h"
-#include "core/logging/Logger.h"
-#include "core/ProcessGroup.h"
-#include "core/yaml/YamlConfiguration.h"
-#include "FlowController.h"
-#include "properties/Configure.h"
-#include "unit/ProvenanceTestHelper.h"
-#include "io/StreamFactory.h"
-#include "c2/C2Agent.h"
-#include "CivetServer.h"
-#include
-#include "protocols/RESTSender.h"
+#include "HTTPIntegrationBase.h"
+#include "HTTPHandlers.h"
-void waitToVerifyProcessor() {
- std::this_thread::sleep_for(std::chrono::seconds(10));
-}
-
-
-class ConfigHandler : public CivetHandler {
+class VerifyC2DescribeJstack : public VerifyC2Describe {
public:
- ConfigHandler() {
-calls_ = 0;
- }
- bool handlePost(CivetServer *server, struct mg_connection *conn) {
-calls_++;
-std::string heartbeat_response = "{\"operation\" :
\"heartbeat\",\"requested_operations\": [ {"
- "\"operation\" : \"describe\", "
- "\"operationid\" : \"8675309\", "
- "\"name\": \"jstack\""
- "}]}";
- mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: "
-"text/plain\r\nContent-Length: %lu\r\nConnection:
close\r\n\r\n",
-heartbeat_response.length());
- mg_printf(conn, "%s", heartbeat_response.c_str());
-
-
-return true;
+ explicit VerifyC2DescribeJstack(bool isSecure)
+ : VerifyC2Describe(isSecure) {
}
- bool handleGet(CivetServer *server, struct mg_connection *conn) {
-std::ifstream myfile(test_file_location_.c_str());
-
-if (myfile.is_open()) {
- std::stringstream buffer;
- buffer << myfile.rdbuf();
- std::string str = buffer.str();
- myfile.close();
- mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: "
-"text/plain\r\nContent-Length: %lu\r\nConnection:
close\r\n\r\n",
-str.length());
- mg_printf(conn, "%s", str.c_str());
-} else {
- mg_printf(conn, "HTTP/1.1 500 Internal Server Error\r\n");
-}
-
-return true;
+ virtual void runAssertions() {
+assert(LogTestController::getInstance().contains("SchedulingAgent") ==
true);
}
- std::string test_file_location_;
- std::atomic calls_;
};
-int main(int argc, char **argv) {
- mg_init_library(0);
- LogTestController::getInstance().setInfo();
- LogTestController::getInstance().setDebug();
- LogTestController::getInstance().setDebug();
- LogTestController::getInstance().setTrace();
+class DescribeJstackHandler : public HeartbeatHandler {
+ public:
+ explicit DescribeJstackHandler(bool isSecure)
+ : HeartbeatHandler(isSecure) {
+ }
- const char *options[] = { "document_root", ".", "listening_ports", "0", 0 };
- std::vector cpp_options;
- for (int i = 0; i < (sizeof(options) / sizeof(options[0]) - 1); i++) {
-cpp_options.push_back(options[i]);
+ virtual void handleHeartbeat(const rapidjson::Document& root, struct
mg_connection * conn) {
+sendHeartbeatResponse("DESCRIBE", "jstack", "889398", conn);
}
- CivetServer server(cpp_options);
+ virtual void handleAcknowledge(const rapidjson::Document& root) {
+assert(root.HasMember("Flowcontroller threadpool #0") == true);
+ }
- std::string port_str = std::to_string(server.getListeningPorts()[0]);
+};
- ConfigHandler h_ex;
- server.addHandler("/update", h_ex);
- std::string key_dir, test_file_location;
+int main(int argc, char **argv) {
+ std::string key_dir, test_file_location, url;
+ url = "http://localhost:0/api/heartbeat;;
if (argc > 1) {
-h_ex.test_file_location_ = test_file_location = argv[1];
-key_dir = argv[2];
+test_file_location = argv[1];
+if (argc > 2) {
+ url = "https://localhost:0/api/heartbeat;;
+ key_dir = argv[2];
+}
}
+ bool isSecure = false;
+ if (url.find("https") != std::string::npos) {
+isSecure = true;
+ }
- std::shared_ptr configuration =
std::make_shared();
-
- std::string c2_rest_url = "http://localhost:; + port_str + "/update";
-
- configuration->set("c2.rest.url", c2_rest_url);
- configuration->set("c2.agent.heartbeat.period", "1000");
-
- std::shared_ptr test_repo =
std::make_shared();
- std::shared_ptr test_flow_repo =
std::make_shared();
-
- configuration->set(minifi::Configure::nifi_flow_configuration_file,
test_file_location);
-
- std::shared_ptr stream_factory =
minifi::io::StreamFactory::getInstance(configuration);
- std::shared_ptr content_repo =
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r395838661
##
File path: extensions/http-curl/tests/C2JstackTest.cpp
##
@@ -16,152 +16,63 @@
* limitations under the License.
*/
-#include
#undef NDEBUG
-#include
-#include
-#include
-#include
-#include
#include
-#include
-#include
-#include
-#include
-#include
-#include "HTTPClient.h"
-#include "InvokeHTTP.h"
#include "TestBase.h"
-#include "utils/StringUtils.h"
-#include "core/Core.h"
-#include "core/logging/Logger.h"
-#include "core/ProcessGroup.h"
-#include "core/yaml/YamlConfiguration.h"
-#include "FlowController.h"
-#include "properties/Configure.h"
-#include "unit/ProvenanceTestHelper.h"
-#include "io/StreamFactory.h"
-#include "c2/C2Agent.h"
-#include "CivetServer.h"
-#include
-#include "protocols/RESTSender.h"
+#include "HTTPIntegrationBase.h"
+#include "HTTPHandlers.h"
-void waitToVerifyProcessor() {
- std::this_thread::sleep_for(std::chrono::seconds(10));
-}
-
-
-class ConfigHandler : public CivetHandler {
+class VerifyC2DescribeJstack : public VerifyC2Describe {
public:
- ConfigHandler() {
-calls_ = 0;
- }
- bool handlePost(CivetServer *server, struct mg_connection *conn) {
-calls_++;
-std::string heartbeat_response = "{\"operation\" :
\"heartbeat\",\"requested_operations\": [ {"
- "\"operation\" : \"describe\", "
- "\"operationid\" : \"8675309\", "
- "\"name\": \"jstack\""
- "}]}";
- mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: "
-"text/plain\r\nContent-Length: %lu\r\nConnection:
close\r\n\r\n",
-heartbeat_response.length());
- mg_printf(conn, "%s", heartbeat_response.c_str());
-
-
-return true;
+ explicit VerifyC2DescribeJstack(bool isSecure)
+ : VerifyC2Describe(isSecure) {
}
- bool handleGet(CivetServer *server, struct mg_connection *conn) {
-std::ifstream myfile(test_file_location_.c_str());
-
-if (myfile.is_open()) {
- std::stringstream buffer;
- buffer << myfile.rdbuf();
- std::string str = buffer.str();
- myfile.close();
- mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: "
-"text/plain\r\nContent-Length: %lu\r\nConnection:
close\r\n\r\n",
-str.length());
- mg_printf(conn, "%s", str.c_str());
-} else {
- mg_printf(conn, "HTTP/1.1 500 Internal Server Error\r\n");
-}
-
-return true;
+ virtual void runAssertions() {
+assert(LogTestController::getInstance().contains("SchedulingAgent") ==
true);
}
- std::string test_file_location_;
- std::atomic calls_;
};
-int main(int argc, char **argv) {
- mg_init_library(0);
- LogTestController::getInstance().setInfo();
- LogTestController::getInstance().setDebug();
- LogTestController::getInstance().setDebug();
- LogTestController::getInstance().setTrace();
+class DescribeJstackHandler : public HeartbeatHandler {
+ public:
+ explicit DescribeJstackHandler(bool isSecure)
+ : HeartbeatHandler(isSecure) {
+ }
- const char *options[] = { "document_root", ".", "listening_ports", "0", 0 };
- std::vector cpp_options;
- for (int i = 0; i < (sizeof(options) / sizeof(options[0]) - 1); i++) {
-cpp_options.push_back(options[i]);
+ virtual void handleHeartbeat(const rapidjson::Document& root, struct
mg_connection * conn) {
+sendHeartbeatResponse("DESCRIBE", "jstack", "889398", conn);
}
- CivetServer server(cpp_options);
+ virtual void handleAcknowledge(const rapidjson::Document& root) {
+assert(root.HasMember("Flowcontroller threadpool #0") == true);
+ }
- std::string port_str = std::to_string(server.getListeningPorts()[0]);
+};
- ConfigHandler h_ex;
- server.addHandler("/update", h_ex);
- std::string key_dir, test_file_location;
+int main(int argc, char **argv) {
+ std::string key_dir, test_file_location, url;
+ url = "http://localhost:0/api/heartbeat;;
if (argc > 1) {
-h_ex.test_file_location_ = test_file_location = argv[1];
-key_dir = argv[2];
+test_file_location = argv[1];
+if (argc > 2) {
+ url = "https://localhost:0/api/heartbeat;;
+ key_dir = argv[2];
+}
}
+ bool isSecure = false;
+ if (url.find("https") != std::string::npos) {
+isSecure = true;
+ }
- std::shared_ptr configuration =
std::make_shared();
-
- std::string c2_rest_url = "http://localhost:; + port_str + "/update";
-
- configuration->set("c2.rest.url", c2_rest_url);
- configuration->set("c2.agent.heartbeat.period", "1000");
-
- std::shared_ptr test_repo =
std::make_shared();
- std::shared_ptr test_flow_repo =
std::make_shared();
-
- configuration->set(minifi::Configure::nifi_flow_configuration_file,
test_file_location);
-
- std::shared_ptr stream_factory =
minifi::io::StreamFactory::getInstance(configuration);
- std::shared_ptr content_repo =
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r393707126
##
File path: extensions/http-curl/tests/C2JstackTest.cpp
##
@@ -16,152 +16,63 @@
* limitations under the License.
*/
-#include
#undef NDEBUG
-#include
-#include
-#include
-#include
-#include
#include
-#include
-#include
-#include
-#include
-#include
-#include "HTTPClient.h"
-#include "InvokeHTTP.h"
#include "TestBase.h"
-#include "utils/StringUtils.h"
-#include "core/Core.h"
-#include "core/logging/Logger.h"
-#include "core/ProcessGroup.h"
-#include "core/yaml/YamlConfiguration.h"
-#include "FlowController.h"
-#include "properties/Configure.h"
-#include "unit/ProvenanceTestHelper.h"
-#include "io/StreamFactory.h"
-#include "c2/C2Agent.h"
-#include "CivetServer.h"
-#include
-#include "protocols/RESTSender.h"
+#include "HTTPIntegrationBase.h"
+#include "HTTPHandlers.h"
-void waitToVerifyProcessor() {
- std::this_thread::sleep_for(std::chrono::seconds(10));
-}
-
-
-class ConfigHandler : public CivetHandler {
+class VerifyC2DescribeJstack : public VerifyC2Describe {
public:
- ConfigHandler() {
-calls_ = 0;
- }
- bool handlePost(CivetServer *server, struct mg_connection *conn) {
-calls_++;
-std::string heartbeat_response = "{\"operation\" :
\"heartbeat\",\"requested_operations\": [ {"
- "\"operation\" : \"describe\", "
- "\"operationid\" : \"8675309\", "
- "\"name\": \"jstack\""
- "}]}";
- mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: "
-"text/plain\r\nContent-Length: %lu\r\nConnection:
close\r\n\r\n",
-heartbeat_response.length());
- mg_printf(conn, "%s", heartbeat_response.c_str());
-
-
-return true;
+ explicit VerifyC2DescribeJstack(bool isSecure)
+ : VerifyC2Describe(isSecure) {
}
- bool handleGet(CivetServer *server, struct mg_connection *conn) {
-std::ifstream myfile(test_file_location_.c_str());
-
-if (myfile.is_open()) {
- std::stringstream buffer;
- buffer << myfile.rdbuf();
- std::string str = buffer.str();
- myfile.close();
- mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: "
-"text/plain\r\nContent-Length: %lu\r\nConnection:
close\r\n\r\n",
-str.length());
- mg_printf(conn, "%s", str.c_str());
-} else {
- mg_printf(conn, "HTTP/1.1 500 Internal Server Error\r\n");
-}
-
-return true;
+ virtual void runAssertions() {
+assert(LogTestController::getInstance().contains("SchedulingAgent") ==
true);
}
- std::string test_file_location_;
- std::atomic calls_;
};
-int main(int argc, char **argv) {
- mg_init_library(0);
- LogTestController::getInstance().setInfo();
- LogTestController::getInstance().setDebug();
- LogTestController::getInstance().setDebug();
- LogTestController::getInstance().setTrace();
+class DescribeJstackHandler : public HeartbeatHandler {
+ public:
+ explicit DescribeJstackHandler(bool isSecure)
+ : HeartbeatHandler(isSecure) {
+ }
- const char *options[] = { "document_root", ".", "listening_ports", "0", 0 };
- std::vector cpp_options;
- for (int i = 0; i < (sizeof(options) / sizeof(options[0]) - 1); i++) {
-cpp_options.push_back(options[i]);
+ virtual void handleHeartbeat(const rapidjson::Document& root, struct
mg_connection * conn) {
+sendHeartbeatResponse("DESCRIBE", "jstack", "889398", conn);
}
- CivetServer server(cpp_options);
+ virtual void handleAcknowledge(const rapidjson::Document& root) {
+assert(root.HasMember("Flowcontroller threadpool #0") == true);
+ }
- std::string port_str = std::to_string(server.getListeningPorts()[0]);
+};
- ConfigHandler h_ex;
- server.addHandler("/update", h_ex);
- std::string key_dir, test_file_location;
+int main(int argc, char **argv) {
+ std::string key_dir, test_file_location, url;
+ url = "http://localhost:0/api/heartbeat;;
if (argc > 1) {
-h_ex.test_file_location_ = test_file_location = argv[1];
-key_dir = argv[2];
+test_file_location = argv[1];
+if (argc > 2) {
+ url = "https://localhost:0/api/heartbeat;;
+ key_dir = argv[2];
+}
}
+ bool isSecure = false;
+ if (url.find("https") != std::string::npos) {
+isSecure = true;
+ }
- std::shared_ptr configuration =
std::make_shared();
-
- std::string c2_rest_url = "http://localhost:; + port_str + "/update";
-
- configuration->set("c2.rest.url", c2_rest_url);
- configuration->set("c2.agent.heartbeat.period", "1000");
-
- std::shared_ptr test_repo =
std::make_shared();
- std::shared_ptr test_flow_repo =
std::make_shared();
-
- configuration->set(minifi::Configure::nifi_flow_configuration_file,
test_file_location);
-
- std::shared_ptr stream_factory =
minifi::io::StreamFactory::getInstance(configuration);
- std::shared_ptr content_repo =
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r392347808
##
File path: libminifi/src/FlowController.cpp
##
@@ -907,29 +908,44 @@ int16_t FlowController::clearConnection(const
std::string ) {
return -1;
}
-int16_t
FlowController::getResponseNodes(std::vector>
_vector, uint16_t metricsClass) {
+std::shared_ptr
FlowController::getMetricsNode(const std::string& metricsClass) const {
std::lock_guard lock(metrics_mutex_);
-
- for (auto metric : root_response_nodes_) {
-metric_vector.push_back(metric.second);
+ if (!metricsClass.empty()) {
+const auto citer = component_metrics_.find(metricsClass);
+if (citer != component_metrics_.end()) {
+ return citer->second;
+}
+ } else {
+const auto iter = root_response_nodes_.find("metrics");
+if (iter != root_response_nodes_.end()) {
+ return iter->second;
+}
Review comment:
There is no code duplication as far as I see. we are looking up different
maps in if and else branches.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r392346281
##
File path: libminifi/src/FlowController.cpp
##
@@ -172,6 +173,9 @@ void FlowController::initializePaths(const std::string
) {
FlowController::~FlowController() {
stop(true);
+ if (c2_agent_)
+c2_agent_->stop();
+ thread_pool_.shutdown();
Review comment:
We do stop the schedulers before stopping threadpool here. Look at line #
175.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r391746461
##
File path: libminifi/src/c2/C2Agent.cpp
##
@@ -342,6 +342,31 @@ void C2Agent::serializeMetrics(C2Payload _payload,
const std::string
}
}
+/*
+void C2Agent::serializeMetrics(C2Payload _payload, const std::string
, const std::vector ,
bool is_container, bool is_collapsible) {
Review comment:
Committed that accidentally. Removed the comments in latest commit.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r391745333
##
File path: libminifi/src/FlowController.cpp
##
@@ -173,9 +173,9 @@ void FlowController::initializePaths(const std::string
) {
FlowController::~FlowController() {
stop(true);
+ c2_agent_->stop();
Review comment:
No we cannot avoid null check here. Fixed it in latest commit
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r391670955
##
File path: nanofi/include/cxx/C2CallbackAgent.h
##
@@ -47,7 +47,10 @@ class C2CallbackAgent : public c2::C2Agent {
public:
- explicit C2CallbackAgent(const
std::shared_ptr , const
std::shared_ptr , const
std::shared_ptr );
+ explicit C2CallbackAgent(const
std::shared_ptr ,
Review comment:
This class is not used anywhere in the code today. It will be removed later
when nanofi c2 work gets completed.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r390669052
##
File path: extensions/http-curl/tests/HTTPHandlers.h
##
@@ -343,4 +345,104 @@ class DeleteTransactionResponder : public CivetHandler {
std::string response_code;
};
+class HeartbeatHandler : public CivetHandler {
+ public:
+ explicit HeartbeatHandler(bool isSecure)
+ : isSecure(isSecure) {
+ }
+
+ std::string readPost(struct mg_connection *conn) {
+std::string response;
+int blockSize = 1024 * sizeof(char), readBytes;
Review comment:
Not sure why, but makes sense to simplify this. We are just reading 1024
bytes from the connection in each call. The variable blockSize is extraneous.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r390614132 ## File path: libminifi/include/FlowController.h ## @@ -304,23 +305,23 @@ class FlowController : public core::controller::ControllerServiceProvider, publi virtual void enableAllControllerServices(); /** - * Retrieves all root response nodes from this source. - * @param metric_vector -- metrics will be placed in this vector. - * @return result of the get operation. - * 0 Success - * 1 No error condition, but cannot obtain lock in timely manner. - * -1 failure + * Retrieves metrics node + * @return metrics response node */ - virtual int16_t getResponseNodes(std::vector> _vector, uint16_t metricsClass); + virtual std::shared_ptr getMetricsNode() const; + + /** + * Retrieves root nodes configured to be included in heartbeat + * @param includeManifest -- determines if manifest is to be included + * @return a list of response nodes + */ + virtual std::vector> getHeartbeatNodes(bool includeManifest) const; Review comment: We can definitely have raw pointers in root_response_nodes_ but is there a reason not to use shared pointers? I understand we cannot use unique_ptrs here as we want to have the FlowController maintain ownership of the nodes and share it with C2Agent. But, given the choices of smart pointers we have shared_ptr is the best one we can use in this case. Having said that, I am not a strong adherent of using shared_ptr universally. In this case I think it is more of a coding guidelines/style/consistency in the code base. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r390665167
##
File path: extensions/http-curl/tests/HTTPHandlers.h
##
@@ -343,4 +345,104 @@ class DeleteTransactionResponder : public CivetHandler {
std::string response_code;
};
+class HeartbeatHandler : public CivetHandler {
+ public:
+ explicit HeartbeatHandler(bool isSecure)
+ : isSecure(isSecure) {
+ }
+
+ std::string readPost(struct mg_connection *conn) {
+std::string response;
+int blockSize = 1024 * sizeof(char), readBytes;
+
+char buffer[1024];
+while ((readBytes = mg_read(conn, buffer, blockSize)) > 0) {
+ response.append(buffer, 0, (readBytes / sizeof(char)));
+}
+return response;
+ }
+
+ void sendStopOperation(struct mg_connection *conn) {
+std::string resp = "{\"operation\" : \"heartbeat\",
\"requested_operations\" : [{ \"operationid\" : 41, \"operation\" : \"stop\",
\"operand\" : \"invoke\" }, "
+"{ \"operationid\" : 42, \"operation\" : \"stop\", \"operand\" :
\"FlowController\" } ]}";
+mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: "
+ "text/plain\r\nContent-Length: %lu\r\nConnection: close\r\n\r\n",
+ resp.length());
+mg_printf(conn, "%s", resp.c_str());
+ }
+
+ void sendHeartbeatResponse(const std::string& operation, const std::string&
operand, const std::string& operationId, struct mg_connection * conn) {
+std::string heartbeat_response = "{\"operation\" :
\"heartbeat\",\"requested_operations\": [ {"
+ "\"operation\" : \"" + operation + "\","
+ "\"operationid\" : \"" + operationId + "\","
+ "\"operand\": \"" + operand + "\"}]}";
+
+ mg_printf(conn, "HTTP/1.1 200 OK\r\nContent-Type: "
+"text/plain\r\nContent-Length: %lu\r\nConnection:
close\r\n\r\n",
+heartbeat_response.length());
+ mg_printf(conn, "%s", heartbeat_response.c_str());
+ }
+
+ void verifyJsonHasAgentManifest(const rapidjson::Document& root) {
+bool found = false;
+assert(root.HasMember("agentInfo") == true);
+assert(root["agentInfo"].HasMember("agentManifest") == true);
+assert(root["agentInfo"]["agentManifest"].HasMember("bundles") == true);
+
+for (auto :
root["agentInfo"]["agentManifest"]["bundles"].GetArray()) {
+ assert(bundle.HasMember("artifact"));
+ std::string str = bundle["artifact"].GetString();
+ if (str == "minifi-system") {
+
+std::vector classes;
+for (auto :
bundle["componentManifest"]["processors"].GetArray()) {
+ classes.push_back(proc["type"].GetString());
+}
+
+auto group = minifi::BuildDescription::getClassDescriptions(str);
+for (auto proc : group.processors_) {
+ assert(std::find(classes.begin(), classes.end(), proc.class_name_)
!= std::end(classes));
+ found = true;
+}
+
+ }
+}
+assert(found == true);
+ }
+
+ virtual void handleHeartbeat(const rapidjson::Document& root, struct
mg_connection * conn) {
+(void)conn;
Review comment:
Good point. Will remove the variable name from the argument list.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r390664802
##
File path: extensions/http-curl/tests/HTTPHandlers.h
##
@@ -343,4 +345,104 @@ class DeleteTransactionResponder : public CivetHandler {
std::string response_code;
};
+class HeartbeatHandler : public CivetHandler {
+ public:
+ explicit HeartbeatHandler(bool isSecure)
+ : isSecure(isSecure) {
+ }
+
+ std::string readPost(struct mg_connection *conn) {
+std::string response;
+int blockSize = 1024 * sizeof(char), readBytes;
+
+char buffer[1024];
+while ((readBytes = mg_read(conn, buffer, blockSize)) > 0) {
+ response.append(buffer, 0, (readBytes / sizeof(char)));
+}
+return response;
+ }
+
+ void sendStopOperation(struct mg_connection *conn) {
+std::string resp = "{\"operation\" : \"heartbeat\",
\"requested_operations\" : [{ \"operationid\" : 41, \"operation\" : \"stop\",
\"operand\" : \"invoke\" }, "
+"{ \"operationid\" : 42, \"operation\" : \"stop\", \"operand\" :
\"FlowController\" } ]}";
Review comment:
I like the neatness of using string literals however, I would rather build a
json with some json library and serialize it. But I would keep it as it is for
the test code. If we ever want to reuse this response string elsewhere, I would
be happy to refactor this.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r390615024 ## File path: libminifi/include/FlowController.h ## @@ -406,22 +409,27 @@ class FlowController : public core::controller::ControllerServiceProvider, publi std::chrono::steady_clock::time_point start_time_; - std::mutex metrics_mutex_; + mutable std::mutex metrics_mutex_; // root_nodes cache std::map> root_response_nodes_; + // metrics cache std::map> device_information_; // metrics cache std::map> component_metrics_; std::map>> component_metrics_by_id_; + // metrics last run std::chrono::steady_clock::time_point last_metrics_capture_; private: std::shared_ptr logger_; std::string serial_number_; + + std::shared_ptr c2_agent_; Review comment: You have a valid point. Made it a unique_ptr 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r390614132 ## File path: libminifi/include/FlowController.h ## @@ -304,23 +305,23 @@ class FlowController : public core::controller::ControllerServiceProvider, publi virtual void enableAllControllerServices(); /** - * Retrieves all root response nodes from this source. - * @param metric_vector -- metrics will be placed in this vector. - * @return result of the get operation. - * 0 Success - * 1 No error condition, but cannot obtain lock in timely manner. - * -1 failure + * Retrieves metrics node + * @return metrics response node */ - virtual int16_t getResponseNodes(std::vector> _vector, uint16_t metricsClass); + virtual std::shared_ptr getMetricsNode() const; + + /** + * Retrieves root nodes configured to be included in heartbeat + * @param includeManifest -- determines if manifest is to be included + * @return a list of response nodes + */ + virtual std::vector> getHeartbeatNodes(bool includeManifest) const; Review comment: We can definitely have raw pointers in root_response_nodes_ but is there a reason not to use shared pointers? I understand we cannot use unique_ptrs here as we want to have the FlowController maintain ownership of the nodes. But, given the choices of smart pointers we have shared_ptr is the best one we can use in this case. Having said that, I am not a strong adherent of using shared_ptr universally. In this case I think it is more of a coding guidelines/style/consistency in the code base. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r389004654
##
File path: libminifi/src/FlowController.cpp
##
@@ -449,13 +451,9 @@ void FlowController::initializeC2() {
if (!c2_initialized_) {
configuration_->setAgentIdentifier(identifier_str);
-state::StateManager::initialize();
-std::shared_ptr agent =
std::make_shared(std::dynamic_pointer_cast(shared_from_this()),
std::dynamic_pointer_cast(shared_from_this()),
+c2_agent_ =
std::make_shared(std::dynamic_pointer_cast(shared_from_this()),
std::dynamic_pointer_cast(shared_from_this()),
Review comment:
As discussed, we will take this in another PR as it requires some major
changes that might break interfaces.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r389004074
##
File path: libminifi/include/utils/ThreadPool.h
##
@@ -412,6 +414,22 @@ class ThreadPool {
void run_tasks(std::shared_ptr thread);
};
+template
+bool ThreadPool::execute(Worker &) {
Review comment:
Reverted this change.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify
C2 metrics collection and reporting
URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r387232949
##
File path: conf/minifi.properties
##
@@ -52,6 +52,8 @@
nifi.database.content.repository.directory.default=${MINIFI_HOME}/content_reposi
#nifi.c2.rest.url=
#nifi.c2.rest.url.ack=
nifi.c2.root.classes=DeviceInfoNode,AgentInformation,FlowInformation
+## Minimize heartbeat payload size by excluding agent manifest from the
heartbeat
Review comment:
This will not be true anymore
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting URL: https://github.com/apache/nifi-minifi-cpp/pull/743#discussion_r387232703 ## File path: C2.md ## @@ -49,6 +49,11 @@ Release 0.6.0: Please note that all c2 properties now exist as `nifi.c2.*`. If y files contain the former naming convention of `c2.*`, we will continue to support that as an alternate key, but you are encouraged to switch your configuration options as soon as possible. Review comment: This will not be true anymore. Will take care of this 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
