From e092a8ad173486e582e98253890212ae5890315a Mon Sep 17 00:00:00 2001 From: Leif Hedstrom Date: Wed, 18 Oct 2023 15:23:22 -0600 Subject: [PATCH 1/2] Improve the TSMgmtUpdateRegister with an optional file name Providing this filename makes the update registry keep track of file changes automatically. When used, the continuation provided will only be called if changes to the configuration file is detected. Also, this eliminates the superflous code for calling a named plugin reload. That code is never used, and I don't like having code there "just because", when nothing uses it. --- .../api/functions/TSMgmtUpdateRegister.en.rst | 2 +- include/api/InkAPIInternal.h | 7 ++-- include/ts/ts.h | 2 +- src/api/ConfigUpdateCbTable.cc | 34 +++++++++++-------- src/api/InkAPI.cc | 4 +-- src/mgmt/config/FileManager.cc | 3 +- 6 files changed, 28 insertions(+), 24 deletions(-) diff --git a/doc/developer-guide/api/functions/TSMgmtUpdateRegister.en.rst b/doc/developer-guide/api/functions/TSMgmtUpdateRegister.en.rst index b841466bae1..41e1b0dcd3e 100644 --- a/doc/developer-guide/api/functions/TSMgmtUpdateRegister.en.rst +++ b/doc/developer-guide/api/functions/TSMgmtUpdateRegister.en.rst @@ -28,7 +28,7 @@ Synopsis #include -.. function:: void TSMgmtUpdateRegister(TSCont contp, const char * plugin_name) +.. function:: void TSMgmtUpdateRegister(TSCont contp, const char *plugin_name, const char *plugin_file_name=nullptr) Description =========== diff --git a/include/api/InkAPIInternal.h b/include/api/InkAPIInternal.h index 086bcffd84b..9025bc688a2 100644 --- a/include/api/InkAPIInternal.h +++ b/include/api/InkAPIInternal.h @@ -41,6 +41,7 @@ #include "ts/experimental.h" #include +#include /* Some defines that might be candidates for configurable settings later. */ @@ -143,12 +144,12 @@ class ConfigUpdateCbTable ConfigUpdateCbTable(); ~ConfigUpdateCbTable(); - void insert(INKContInternal *contp, const char *name); - void invoke(const char *name); + void insert(INKContInternal *contp, const char *name, const char *file_name = nullptr); + void invoke(); void invoke(INKContInternal *contp); private: - std::unordered_map cb_table; + std::unordered_map> cb_table; }; #include "HttpAPIHooks.h" diff --git a/include/ts/ts.h b/include/ts/ts.h index 734aa6d77c0..0bbd3163602 100644 --- a/include/ts/ts.h +++ b/include/ts/ts.h @@ -1255,7 +1255,7 @@ namespace c /* -------------------------------------------------------------------------- Management */ - void TSMgmtUpdateRegister(TSCont contp, const char *plugin_name); + void TSMgmtUpdateRegister(TSCont contp, const char *plugin_name, const char *plugin_file_name = nullptr); TSReturnCode TSMgmtIntGet(const char *var_name, TSMgmtInt *result); TSReturnCode TSMgmtCounterGet(const char *var_name, TSMgmtCounter *result); TSReturnCode TSMgmtFloatGet(const char *var_name, TSMgmtFloat *result); diff --git a/src/api/ConfigUpdateCbTable.cc b/src/api/ConfigUpdateCbTable.cc index 19f6dfa50e8..4c3de899d6f 100644 --- a/src/api/ConfigUpdateCbTable.cc +++ b/src/api/ConfigUpdateCbTable.cc @@ -28,31 +28,35 @@ ConfigUpdateCbTable::ConfigUpdateCbTable() {} ConfigUpdateCbTable::~ConfigUpdateCbTable() {} void -ConfigUpdateCbTable::insert(INKContInternal *contp, const char *name) +ConfigUpdateCbTable::insert(INKContInternal *contp, const char *name, const char *file_name) { - if (contp && name) { - cb_table.emplace(name, contp); + std::filesystem::file_time_type timestamp; + std::string fname = file_name; + + ink_assert(contp != nullptr); + ink_assert(name != nullptr); + + if (fname.size() > 0) { + timestamp = std::filesystem::last_write_time(fname); } + cb_table.emplace(name, std::make_tuple(contp, fname, timestamp)); } void -ConfigUpdateCbTable::invoke(const char *name) +ConfigUpdateCbTable::invoke() { - INKContInternal *contp; + for (auto &&it : cb_table) { + auto &[contp, file_name, timestamp] = it.second; + + if (file_name.size() > 0) { + auto newtime = std::filesystem::last_write_time(file_name); - if (name != nullptr) { - if (strcmp(name, "*") == 0) { - for (auto &&it : cb_table) { - contp = it.second; - ink_assert(contp != nullptr); + if (newtime > timestamp) { + timestamp = newtime; invoke(contp); } } else { - if (auto it = cb_table.find(name); it != cb_table.end()) { - contp = it->second; - ink_assert(contp != nullptr); - invoke(contp); - } + invoke(contp); } } } diff --git a/src/api/InkAPI.cc b/src/api/InkAPI.cc index 9de791cbfb6..5172a82bab3 100644 --- a/src/api/InkAPI.cc +++ b/src/api/InkAPI.cc @@ -4096,12 +4096,12 @@ tsapi::c::TSConfigDataGet(TSConfig configp) //////////////////////////////////////////////////////////////////// void -tsapi::c::TSMgmtUpdateRegister(TSCont contp, const char *plugin_name) +tsapi::c::TSMgmtUpdateRegister(TSCont contp, const char *plugin_name, const char *plugin_file_name) { sdk_assert(sdk_sanity_check_iocore_structure(contp) == TS_SUCCESS); sdk_assert(sdk_sanity_check_null_ptr((void *)plugin_name) == TS_SUCCESS); - global_config_cbs->insert((INKContInternal *)contp, plugin_name); + global_config_cbs->insert((INKContInternal *)contp, plugin_name, plugin_file_name); } TSReturnCode diff --git a/src/mgmt/config/FileManager.cc b/src/mgmt/config/FileManager.cc index d96332eef51..c379355c4e0 100644 --- a/src/mgmt/config/FileManager.cc +++ b/src/mgmt/config/FileManager.cc @@ -193,9 +193,8 @@ void FileManager::invokeConfigPluginCallbacks() { Debug("filemanager", "invoke plugin callbacks"); - static const std::string_view s{"*"}; if (_pluginCallbackList) { - _pluginCallbackList->invoke(s.data()); + _pluginCallbackList->invoke(); } } From 4cad488e57f922bb5b892ff2daf7fd8a93080f85 Mon Sep 17 00:00:00 2001 From: Leif Hedstrom Date: Fri, 20 Oct 2023 13:23:11 -0600 Subject: [PATCH 2/2] Changes from std::filesystem to swoc::file --- include/api/InkAPIInternal.h | 4 ++-- src/api/ConfigUpdateCbTable.cc | 35 ++++++++++++++++++++++------------ 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/include/api/InkAPIInternal.h b/include/api/InkAPIInternal.h index 9025bc688a2..e3a7f0e2b17 100644 --- a/include/api/InkAPIInternal.h +++ b/include/api/InkAPIInternal.h @@ -37,11 +37,11 @@ #include "api/APIHooks.h" #include "api/FeatureAPIHooks.h" +#include "swoc/swoc_file.h" #include "ts/InkAPIPrivateIOCore.h" #include "ts/experimental.h" #include -#include /* Some defines that might be candidates for configurable settings later. */ @@ -149,7 +149,7 @@ class ConfigUpdateCbTable void invoke(INKContInternal *contp); private: - std::unordered_map> cb_table; + std::unordered_map> cb_table; }; #include "HttpAPIHooks.h" diff --git a/src/api/ConfigUpdateCbTable.cc b/src/api/ConfigUpdateCbTable.cc index 4c3de899d6f..1b1e5d33980 100644 --- a/src/api/ConfigUpdateCbTable.cc +++ b/src/api/ConfigUpdateCbTable.cc @@ -30,30 +30,41 @@ ConfigUpdateCbTable::~ConfigUpdateCbTable() {} void ConfigUpdateCbTable::insert(INKContInternal *contp, const char *name, const char *file_name) { - std::filesystem::file_time_type timestamp; - std::string fname = file_name; - ink_assert(contp != nullptr); ink_assert(name != nullptr); - if (fname.size() > 0) { - timestamp = std::filesystem::last_write_time(fname); + if (nullptr != file_name) { + swoc::file::path file_path{file_name}; + std::error_code ec; + auto timestamp = swoc::file::last_write_time(file_path, ec); + + if (!ec) { + cb_table.emplace(name, std::make_tuple(contp, file_path, timestamp)); + } else { + Error("Failed to stat %s: %s", file_path.c_str(), ec.message().c_str()); + } + } else { + cb_table.emplace(name, std::make_tuple(contp, swoc::file::path{}, swoc::file::file_time_type{})); } - cb_table.emplace(name, std::make_tuple(contp, fname, timestamp)); } void ConfigUpdateCbTable::invoke() { for (auto &&it : cb_table) { - auto &[contp, file_name, timestamp] = it.second; + auto &[contp, file_path, timestamp] = it.second; - if (file_name.size() > 0) { - auto newtime = std::filesystem::last_write_time(file_name); + if (!file_path.empty()) { + std::error_code ec; + auto newtime = swoc::file::last_write_time(file_path, ec); - if (newtime > timestamp) { - timestamp = newtime; - invoke(contp); + if (!ec) { + if (newtime > timestamp) { + timestamp = newtime; + invoke(contp); + } + } else { + Error("Failed to stat %s: %s", file_path.c_str(), ec.message().c_str()); } } else { invoke(contp);