adamdebreceni commented on a change in pull request #1259:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1259#discussion_r806660908
##########
File path: libminifi/include/properties/Properties.h
##########
@@ -29,16 +29,23 @@
#include "core/logging/Logger.h"
#include "utils/ChecksumCalculator.h"
+#include "utils/StringUtils.h"
namespace org {
namespace apache {
namespace nifi {
namespace minifi {
+enum class PropertyChangeLifetime {
+ TRANSIENT, // the changed value will not be committed to disk
+ PERSISTENT // the changed value will be written to the source file
+};
+
class Properties {
struct PropertyValue {
- std::string value;
- bool changed;
+ std::string persisted_value;
+ std::string active_value;
+ bool changed{false}; // persisted_value should be written to disk if
requested
Review comment:
renamed
##########
File path: libminifi/src/core/logging/LoggerConfiguration.cpp
##########
@@ -359,6 +323,57 @@ void LoggerConfiguration::initializeCompression(const
std::lock_guard<std::mutex
}
}
+std::shared_ptr<spdlog::sinks::rotating_file_sink_mt>
LoggerConfiguration::getRotatingFileSink(const std::string& appender_key, const
std::shared_ptr<LoggerProperties>& properties) {
+ static std::map<std::filesystem::path,
std::shared_ptr<spdlog::sinks::rotating_file_sink_mt>> rotating_file_sinks;
+ static std::mutex sink_map_mtx;
+
+ std::string file_name;
+ if (!properties->getString(appender_key + ".file_name", file_name)) {
+ file_name = "minifi-app.log";
+ }
+ std::string directory;
+ if (!properties->getString(appender_key + ".directory", directory)) {
+ // The below part assumes logger_properties->getHome() is existing
+ // Cause minifiHome must be set at MiNiFiMain.cpp?
+ directory = properties->getHome() +
utils::file::FileUtils::get_separator() + "logs";
+ }
+
+ file_name = directory + utils::file::FileUtils::get_separator() + file_name;
+ if (utils::file::FileUtils::create_dir(directory) == -1) {
+ std::cerr << directory << " cannot be created\n";
+ exit(1);
+ }
+
+ int max_files = 3;
+ std::string max_files_str = "";
+ if (properties->getString(appender_key + ".max_files", max_files_str)) {
+ try {
+ max_files = std::stoi(max_files_str);
+ } catch (const std::invalid_argument &) {
+ } catch (const std::out_of_range &) {
+ }
+ }
+
+ int max_file_size = 5_MiB;
+ std::string max_file_size_str = "";
+ if (properties->getString(appender_key + ".max_file_size",
max_file_size_str)) {
+ try {
+ max_file_size = std::stoi(max_file_size_str);
+ } catch (const std::invalid_argument &) {
+ } catch (const std::out_of_range &) {
+ }
+ }
+
+ std::lock_guard<std::mutex> guard(sink_map_mtx);
+ auto it = rotating_file_sinks.find(file_name);
+ if (it != rotating_file_sinks.end()) {
+ return it->second;
+ }
Review comment:
added comment: in the spdlog implementation I found no guarantee that
multiple sinks to the same directory won't interfere with each other (it might
be guaranteed at the OS level though), also found this
https://github.com/gabime/spdlog/discussions/2151
##########
File path: libminifi/src/core/logging/LoggerConfiguration.cpp
##########
@@ -359,6 +323,57 @@ void LoggerConfiguration::initializeCompression(const
std::lock_guard<std::mutex
}
}
+std::shared_ptr<spdlog::sinks::rotating_file_sink_mt>
LoggerConfiguration::getRotatingFileSink(const std::string& appender_key, const
std::shared_ptr<LoggerProperties>& properties) {
+ static std::map<std::filesystem::path,
std::shared_ptr<spdlog::sinks::rotating_file_sink_mt>> rotating_file_sinks;
+ static std::mutex sink_map_mtx;
+
+ std::string file_name;
+ if (!properties->getString(appender_key + ".file_name", file_name)) {
+ file_name = "minifi-app.log";
+ }
+ std::string directory;
+ if (!properties->getString(appender_key + ".directory", directory)) {
+ // The below part assumes logger_properties->getHome() is existing
+ // Cause minifiHome must be set at MiNiFiMain.cpp?
+ directory = properties->getHome() +
utils::file::FileUtils::get_separator() + "logs";
+ }
+
+ file_name = directory + utils::file::FileUtils::get_separator() + file_name;
+ if (utils::file::FileUtils::create_dir(directory) == -1) {
+ std::cerr << directory << " cannot be created\n";
+ exit(1);
+ }
+
+ int max_files = 3;
+ std::string max_files_str = "";
+ if (properties->getString(appender_key + ".max_files", max_files_str)) {
+ try {
+ max_files = std::stoi(max_files_str);
+ } catch (const std::invalid_argument &) {
+ } catch (const std::out_of_range &) {
+ }
+ }
+
+ int max_file_size = 5_MiB;
+ std::string max_file_size_str = "";
+ if (properties->getString(appender_key + ".max_file_size",
max_file_size_str)) {
+ try {
+ max_file_size = std::stoi(max_file_size_str);
+ } catch (const std::invalid_argument &) {
+ } catch (const std::out_of_range &) {
+ }
+ }
+
+ std::lock_guard<std::mutex> guard(sink_map_mtx);
+ auto it = rotating_file_sinks.find(file_name);
+ if (it != rotating_file_sinks.end()) {
+ return it->second;
+ }
Review comment:
this also might be relevant: https://github.com/gabime/spdlog/issues/243
> If you want multiple loggers to share the same file, they must share the
same sink. Otherwise bad things will happen as you have noticed.
##########
File path: libminifi/src/core/logging/LoggerConfiguration.cpp
##########
@@ -359,6 +323,57 @@ void LoggerConfiguration::initializeCompression(const
std::lock_guard<std::mutex
}
}
+std::shared_ptr<spdlog::sinks::rotating_file_sink_mt>
LoggerConfiguration::getRotatingFileSink(const std::string& appender_key, const
std::shared_ptr<LoggerProperties>& properties) {
+ static std::map<std::filesystem::path,
std::shared_ptr<spdlog::sinks::rotating_file_sink_mt>> rotating_file_sinks;
+ static std::mutex sink_map_mtx;
+
+ std::string file_name;
+ if (!properties->getString(appender_key + ".file_name", file_name)) {
+ file_name = "minifi-app.log";
+ }
+ std::string directory;
+ if (!properties->getString(appender_key + ".directory", directory)) {
+ // The below part assumes logger_properties->getHome() is existing
+ // Cause minifiHome must be set at MiNiFiMain.cpp?
+ directory = properties->getHome() +
utils::file::FileUtils::get_separator() + "logs";
+ }
+
+ file_name = directory + utils::file::FileUtils::get_separator() + file_name;
+ if (utils::file::FileUtils::create_dir(directory) == -1) {
+ std::cerr << directory << " cannot be created\n";
+ exit(1);
+ }
+
+ int max_files = 3;
+ std::string max_files_str = "";
+ if (properties->getString(appender_key + ".max_files", max_files_str)) {
+ try {
+ max_files = std::stoi(max_files_str);
+ } catch (const std::invalid_argument &) {
+ } catch (const std::out_of_range &) {
+ }
+ }
+
+ int max_file_size = 5_MiB;
+ std::string max_file_size_str = "";
+ if (properties->getString(appender_key + ".max_file_size",
max_file_size_str)) {
+ try {
+ max_file_size = std::stoi(max_file_size_str);
+ } catch (const std::invalid_argument &) {
+ } catch (const std::out_of_range &) {
+ }
+ }
+
+ std::lock_guard<std::mutex> guard(sink_map_mtx);
+ auto it = rotating_file_sinks.find(file_name);
+ if (it != rotating_file_sinks.end()) {
+ return it->second;
+ }
Review comment:
and this indeed means that some updates will not take effect until
restart, e.g. changing the rotating file count/size
##########
File path: libminifi/src/core/logging/LoggerConfiguration.cpp
##########
@@ -359,6 +323,57 @@ void LoggerConfiguration::initializeCompression(const
std::lock_guard<std::mutex
}
}
+std::shared_ptr<spdlog::sinks::rotating_file_sink_mt>
LoggerConfiguration::getRotatingFileSink(const std::string& appender_key, const
std::shared_ptr<LoggerProperties>& properties) {
+ static std::map<std::filesystem::path,
std::shared_ptr<spdlog::sinks::rotating_file_sink_mt>> rotating_file_sinks;
+ static std::mutex sink_map_mtx;
+
+ std::string file_name;
+ if (!properties->getString(appender_key + ".file_name", file_name)) {
+ file_name = "minifi-app.log";
+ }
+ std::string directory;
+ if (!properties->getString(appender_key + ".directory", directory)) {
+ // The below part assumes logger_properties->getHome() is existing
+ // Cause minifiHome must be set at MiNiFiMain.cpp?
+ directory = properties->getHome() +
utils::file::FileUtils::get_separator() + "logs";
+ }
+
+ file_name = directory + utils::file::FileUtils::get_separator() + file_name;
+ if (utils::file::FileUtils::create_dir(directory) == -1) {
+ std::cerr << directory << " cannot be created\n";
+ exit(1);
+ }
+
+ int max_files = 3;
+ std::string max_files_str = "";
+ if (properties->getString(appender_key + ".max_files", max_files_str)) {
+ try {
+ max_files = std::stoi(max_files_str);
+ } catch (const std::invalid_argument &) {
+ } catch (const std::out_of_range &) {
+ }
+ }
+
+ int max_file_size = 5_MiB;
+ std::string max_file_size_str = "";
+ if (properties->getString(appender_key + ".max_file_size",
max_file_size_str)) {
+ try {
+ max_file_size = std::stoi(max_file_size_str);
+ } catch (const std::invalid_argument &) {
+ } catch (const std::out_of_range &) {
+ }
+ }
+
+ std::lock_guard<std::mutex> guard(sink_map_mtx);
+ auto it = rotating_file_sinks.find(file_name);
+ if (it != rotating_file_sinks.end()) {
+ return it->second;
+ }
Review comment:
changed comment
--
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]
