nifi-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [nifi-minifi-cpp] msharee9 commented on a change in pull request #743: Minificpp 1169 - Simplify C2 metrics collection and reporting
Date Wed, 22 Apr 2020 19:01:28 GMT

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<core::controller::ControllerServiceProvid
         }
         request_mutex.unlock();
       }
-
-      if ( time_since > heart_beat_period_ ) {
-        last_run_ = now;
-        try {
-          performHeartBeat();
-        }
-        catch(const std::exception &e) {
-          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 &e) {
+        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:
users@infra.apache.org



Mime
View raw message