From 76c6b03cc118b91cc494d00a4adaad301e456e78 Mon Sep 17 00:00:00 2001 From: Souvik Roy Date: Wed, 8 Jan 2025 00:04:17 -0600 Subject: [PATCH] Remove redundant async call in FRU collection threads (#588) This commit removes the redundant std:async call in the detached thread launched for parsing and publishing the VPD for an individual FRU. Since we have a dedicated detached thread for each FRU, we can do VPD parse and publish in a synchronous manner from the detached thread itself, instead of launching a asynchronous task which adds unnecessary performance cost. This commit also handles any exception thrown while launching the detached thread for a FRU. Incase launching detached thread for a FRU fails, we log a PEL and continue launching threads for other FRUs. This commit addresses issue #558. Test: ``` 1. Install bitbaked image on Everest (ever6bmc) 2. After initial boot, check: - BMC State Ready - vpd-manager's CollectionStatus property should be "Completed" busctl get-property com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager CollectionStatus s "Completed" - vpd-manager status should be running with no restarts - vpd-manager should have only 1 thread running: check "ls -la /proc//task" 3. Reboot the BMC several times and repeat Step 2. ``` Change-Id: I603c64dc9b5057429a2288f0edfde6086755b851 Signed-off-by: Souvik Roy --- vpd-manager/include/worker.hpp | 1 + vpd-manager/src/worker.cpp | 38 +++++++++++++++++++++------------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/vpd-manager/include/worker.hpp b/vpd-manager/include/worker.hpp index 7689aea4f..de77a88af 100644 --- a/vpd-manager/include/worker.hpp +++ b/vpd-manager/include/worker.hpp @@ -80,6 +80,7 @@ class Worker * Note: Config JSON file path should be passed to worker class constructor * to make use of this API. * + * @throw std::runtime_error */ void collectFrusFromJson(); diff --git a/vpd-manager/src/worker.cpp b/vpd-manager/src/worker.cpp index 6fc901d0f..980e1d027 100644 --- a/vpd-manager/src/worker.cpp +++ b/vpd-manager/src/worker.cpp @@ -5,6 +5,7 @@ #include "backup_restore.hpp" #include "configuration.hpp" #include "constants.hpp" +#include "event_logger.hpp" #include "exceptions.hpp" #include "logger.hpp" #include "parser.hpp" @@ -1512,22 +1513,31 @@ void Worker::collectFrusFromJson() continue; } - std::thread{[vpdFilePath, this]() { - auto l_futureObject = std::async(&Worker::parseAndPublishVPD, this, - vpdFilePath); - - std::tuple l_threadInfo = l_futureObject.get(); + try + { + std::thread{[vpdFilePath, this]() { + const auto& l_parseResult = parseAndPublishVPD(vpdFilePath); - // thread returned. - m_mutex.lock(); - m_activeCollectionThreadCount--; - m_mutex.unlock(); + m_mutex.lock(); + m_activeCollectionThreadCount--; + m_mutex.unlock(); - if (!m_activeCollectionThreadCount) - { - m_isAllFruCollected = true; - } - }}.detach(); + if (!m_activeCollectionThreadCount) + { + m_isAllFruCollected = true; + } + }}.detach(); + } + catch (const std::exception& l_ex) + { + // TODO: Should we re-try launching thread for this FRU? + EventLogger::createSyncPel( + types::ErrorType::InvalidVpdMessage, types::SeverityType::Alert, + __FILE__, __FUNCTION__, 0, + std::string("Failed to start collection thread for FRU :[" + + vpdFilePath + "]. Error: " + l_ex.what()), + std::nullopt, std::nullopt, std::nullopt, std::nullopt); + } } }