Adding configurator feature to dynamically inject configuration changes #374

iafuture · 2023-01-27T18:05:08Z

jfunston · 2023-01-30T17:27:45Z

Copyright date here and in other new files should be 2023

iafuture · 2023-01-27T18:15:33Z

I would just call the name "k2::config". This name shows up in the logs so something shorter would be better.

iafuture · 2023-01-27T18:17:22Z

log the whole request

iafuture · 2023-01-30T18:14:24Z

We probably need to validate the incoming value here before storing it in the config. you should check the boost map API to see if it does any config-related validation. If not, we should do it ourselves and return the appropriate status back to the RPC caller.

For log-related config, we need to apply the new value(for both set/delete) to the log system. The logic for that is here: https://github.com/futurewei-cloud/chogori-platform/blob/master/src/k2/appbase/Appbase.cpp#L81 and we may have to refactor it to reuse it.

You can also avoid code duplication like so:

return [request=std::move(request)] { auto applier = [request] (auto& config) { config.emplace(key, boost::program_options::variable_value(value, "")); boost::program_options::notify(config); return seastar::make_ready_future(); }; if (request.applyToAll) { return ConfigDist().invoke_on_all(applier); } return applier(); }() .then([] { return RPCResponse(Statuses::S201_Created("ConfigSet accepted"), ConfigSetResponse{}); });

iafuture · 2023-01-30T18:35:55Z

On a further thought, I think the validation and application to config should be delegated to the config here: https://github.com/futurewei-cloud/chogori-platform/blob/master/src/k2/config/Config.h

We should probably add the set/delete methods there which can also hide the all/local dispatch

iafuture · 2023-01-27T18:17:30Z

log the whole request

iafuture · 2023-01-27T18:17:38Z

log the whole request

iafuture · 2023-01-27T18:12:51Z

iafuture · 2023-01-27T18:13:36Z

Not sure what these are for. Probably remove as well as the Config header above

jfunston · 2023-01-30T17:28:42Z

Are these variables used anywhere?

iafuture · 2023-01-27T18:12:38Z

iafuture · 2023-01-27T18:10:50Z

Let's make the structs match the verb naming. So instead of DELETE_ConfiguratorRequest it should be ConfigDeleteRequest. Same for the rest

iafuture · 2023-01-27T18:11:43Z

We supply values for all primitive types in structs. This should be applyToAll{false}. Same for the rest

iafuture · 2023-01-27T18:12:07Z

no need to return the key.

jfunston · 2023-01-30T17:30:13Z

Does the key need to be in the response for any of these?

iafuture · 2023-01-27T18:12:17Z

no need to return key

iafuture · 2023-01-27T18:12:22Z

no need to return key

iafuture · 2023-01-27T18:08:55Z

You should rename these verbs to CONFIG_SET/GET/DELETE since they manage the config, not the configurator. Also, this enum has the MAX_VERB sentinel value of 250. You cannot repeat it. Since we already have a value for 249, You should list your verbs back from 249, so:
CONFIG_DELETE=246
CONFIG_GET=247
CONFIG_SET=248

jfunston · 2023-01-30T17:30:50Z

I'm not sure this is where we want these defined, maybe Ivan has a suggestion.

-Original file line number
+Diff line change
@@ Expand Up / @@ -11,4 +11,5 @@ add_subdirectory (infrastructure) @@
     add_subdirectory (module)
     add_subdirectory (persistence)
     add_subdirectory (transport)
+    add_subdirectory (configurator)
     add_subdirectory (tso)

-Original file line number
+Diff line change
@@ Expand Up / @@ -37,6 +37,7 @@ Copyright(c) 2020 Futurewei Cloud @@
     // k2 transport
     #include <k2/transport/AutoRRDMARPCProtocol.h>
     #include <k2/transport/Discovery.h>
+    #include <k2/configurator/Configurator.h>
     #include <k2/transport/RPCProtocolFactory.h>
     #include <k2/transport/RRDMARPCProtocol.h>
     #include <k2/transport/TCPRPCProtocol.h>
@@ Expand Down Expand Up / @@ -92,6 +93,7 @@ class App { @@
             }
             // add the discovery applet o all apps
             addApplet<k2::Discovery>();
+            addApplet<k2::Configurator>();
         }
         // helper class for positional option adding
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -8,7 +8,7 @@ set_target_properties(appbase PROPERTIES @@
         SOVERSION 1
     )
-    target_link_libraries (appbase PRIVATE config common transport Seastar::seastar )
+    target_link_libraries (appbase PRIVATE config common transport configurator Seastar::seastar )
     # export the library in the common k2Targets
     install(TARGETS appbase EXPORT k2Targets DESTINATION lib/k2)
@@ Expand Down @@

-Original file line number
+Diff line change
@@ -1,5 +1,5 @@
     add_executable (cpo_main cpo_main.cpp)
-    target_link_libraries (cpo_main PRIVATE appbase common transport Seastar::seastar cpo_service infrastructure dto)
+    target_link_libraries (cpo_main PRIVATE appbase common configurator transport Seastar::seastar cpo_service infrastructure dto)
     install (TARGETS cpo_main DESTINATION bin)

-Original file line number
+Diff line change
@@ Expand Up / @@ -3,9 +3,9 @@ add_executable (k2_demo_rpc_echo rpc_echo.cpp) @@
     add_executable (k2_demo_rpc_service rpc_service.cpp)
     add_executable (k2_demo_tso tso_sample_app.cpp)
-    target_link_libraries (k2_demo_service PRIVATE appbase transport common Seastar::seastar)
-    target_link_libraries (k2_demo_rpc_echo PRIVATE appbase transport common Seastar::seastar)
-    target_link_libraries (k2_demo_rpc_service PRIVATE appbase transport common Seastar::seastar)
-    target_link_libraries (k2_demo_tso PRIVATE tso_service tso_client appbase transport common Seastar::seastar)
+    target_link_libraries (k2_demo_service PRIVATE appbase configurator transport common Seastar::seastar)
+    target_link_libraries (k2_demo_rpc_echo PRIVATE appbase configurator transport common Seastar::seastar)
+    target_link_libraries (k2_demo_rpc_service PRIVATE appbase configurator transport common Seastar::seastar)
+    target_link_libraries (k2_demo_tso PRIVATE tso_service tso_client appbase configurator transport common Seastar::seastar)
     #install (TARGETS k2_demo_service k2_demo_rpc_echo k2_demo_rpc_service k2_demo_tso DESTINATION bin)

-Original file line number
+Diff line change
@@ -1,5 +1,5 @@
     add_executable (http_proxy http_proxy_main.cpp)
-    target_link_libraries (http_proxy PRIVATE appbase k23si_client cpo_client tso_client httpproxy infrastructure dto transport Seastar::seastar)
+    target_link_libraries (http_proxy PRIVATE appbase k23si_client cpo_client tso_client httpproxy infrastructure dto configurator transport Seastar::seastar)
     install (TARGETS http_proxy DESTINATION bin)

-Original file line number
+Diff line change
@@ -1,5 +1,5 @@
     add_executable (nodepool nodepool_main.cpp)
-    target_link_libraries (nodepool PRIVATE appbase k23si cpo_client common transport Seastar::seastar assignment_manager tso_client infrastructure dto)
+    target_link_libraries (nodepool PRIVATE appbase k23si cpo_client common configurator transport Seastar::seastar assignment_manager tso_client infrastructure dto)
     install (TARGETS nodepool DESTINATION bin)

-Original file line number
+Diff line change
@@ -1,5 +1,5 @@
     add_executable (persistence persistence_main.cpp)
-    target_link_libraries (persistence PRIVATE cpo_client appbase common transport Seastar::seastar persistence_service)
+    target_link_libraries (persistence PRIVATE cpo_client appbase common configurator transport Seastar::seastar persistence_service)
     install (TARGETS persistence DESTINATION bin)

-Original file line number
+Diff line change
@@ -1,5 +1,5 @@
     add_executable (plog_main plog_main.cpp)
-    target_link_libraries (plog_main PRIVATE appbase common transport Seastar::seastar plog_service)
+    target_link_libraries (plog_main PRIVATE appbase common configurator transport Seastar::seastar plog_service)
     install (TARGETS plog_main DESTINATION bin)

-Original file line number
+Diff line change
@@ Expand Up / @@ -3,6 +3,6 @@ file(GLOB SOURCES "*.cpp") @@
     add_executable (tpcc_client ${HEADERS} ${SOURCES})
-    target_link_libraries (tpcc_client PRIVATE tso_client appbase transport common cpo_client k23si_client dto Seastar::seastar)
+    target_link_libraries (tpcc_client PRIVATE tso_client appbase configurator transport common cpo_client k23si_client dto Seastar::seastar)
     install (TARGETS tpcc_client DESTINATION bin)

-Original file line number
+Diff line change
@@ -1,5 +1,5 @@
     add_executable (tso tso_main.cpp)
-    target_link_libraries (tso PRIVATE tso_service cpo_client appbase common transport Seastar::seastar)
+    target_link_libraries (tso PRIVATE tso_service cpo_client appbase common configurator transport Seastar::seastar)
     install (TARGETS tso DESTINATION bin)

-Original file line number
+Diff line change
@@ Expand Up @@
     add_executable (k23sibench_client k23sibench_client.cpp)
-    target_link_libraries (txbench_client PRIVATE appbase transport common Seastar::seastar)
-    target_link_libraries (txbench_server PRIVATE appbase transport common Seastar::seastar)
-    target_link_libraries (txbench_combine PRIVATE appbase transport common Seastar::seastar)
+    target_link_libraries (txbench_client PRIVATE appbase configurator transport common Seastar::seastar)
+    target_link_libraries (txbench_server PRIVATE appbase configurator transport common Seastar::seastar)
+    target_link_libraries (txbench_combine PRIVATE appbase configurator transport common Seastar::seastar)
-    target_link_libraries (rpcbench_client PRIVATE appbase transport common Seastar::seastar)
-    target_link_libraries (rpcbench_server PRIVATE appbase transport common Seastar::seastar)
+    target_link_libraries (rpcbench_client PRIVATE appbase configurator transport common Seastar::seastar)
+    target_link_libraries (rpcbench_server PRIVATE appbase configurator transport common Seastar::seastar)
-    target_link_libraries (k23sibench_client PRIVATE appbase tso_client cpo_client k23si_client dto transport Seastar::seastar)
+    target_link_libraries (k23sibench_client PRIVATE appbase tso_client cpo_client k23si_client dto configurator transport Seastar::seastar)
     #install (TARGETS txbench_client txbench_server txbench_combine rpcbench_client rpcbench_server k23sibench_client DESTINATION bin)

-Original file line number
+Diff line change
@@ Expand Up / @@ -3,6 +3,6 @@ file(GLOB SOURCES "*.cpp") @@
     add_executable (ycsb_client ${HEADERS} ${SOURCES})
-    target_link_libraries (ycsb_client PRIVATE tso_client appbase common cpo_client k23si_client dto transport Seastar::seastar)
+    target_link_libraries (ycsb_client PRIVATE tso_client appbase common cpo_client k23si_client dto configurator transport Seastar::seastar)
     install (TARGETS ycsb_client DESTINATION bin)

-Original file line number
+Diff line change
@@ -0,0 +1,12 @@
+    file(GLOB HEADERS "*.h")
+    file(GLOB SOURCES "*.cpp")
+    add_library(configurator OBJECT ${HEADERS} ${SOURCES})
+    target_link_libraries (configurator PRIVATE common config Seastar::seastar crc32c skvhttp::common skvhttp::mpack)
+    # export the library in the common k2Targets
+    install(TARGETS configurator EXPORT k2Targets DESTINATION lib/k2)
+    install(FILES ${HEADERS} DESTINATION include/k2/configurator)
+    # export the cmake config in the build tree for any users who want to use this project from source
+    export(TARGETS configurator NAMESPACE k2:: FILE configurator-config.cmake)

-Original file line number
+Diff line change
@@ -0,0 +1,122 @@
+    /*
+    MIT License
+    Copyright(c) 2020 Futurewei Cloud
+        Permission is hereby granted,
+        free of charge, to any person obtaining a copy of this software and associated documentation files(the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and / or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions :
+        The above copyright notice and this permission notice shall be included in all copies
+        or
+        substantial portions of the Software.
+        THE SOFTWARE IS PROVIDED "AS IS",
+        WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+        FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.IN NO EVENT SHALL THE
+        AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
+        DAMAGES OR OTHER
+        LIABILITY,
+        WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+        OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+        SOFTWARE.
+    */
+    #include "Configurator.h"
+    #include "ConfiguratorDTO.h"
+    #include <k2/appbase/AppEssentials.h>
+    #include <k2/appbase/Appbase.h>
+    #include <k2/common/Common.h>
+    #include <k2/logging/Log.h>
+    #include <k2/common/Common.h>
+    #include <k2/transport/PayloadSerialization.h>
+    #include <k2/transport/Status.h>
+    namespace k2 {
+    namespace log {
+    inline thread_local k2::logging::Logger configurator("k2::configurator_server");
+    }
+    Configurator::Configurator() {
+        K2LOG_I(log::configurator, "ctor");
+    }
+    Configurator::~Configurator() {
+        K2LOG_I(log::configurator, "dtor");
+    }
+    seastar::future<> Configurator::start() {
+        K2LOG_I(log::configurator, "Registering message handlers");
+        RPC().registerRPCObserver<SET_ConfiguratorRequest, SET_ConfiguratorResponse>(InternalVerbs::CONFGURATOR_SET, [this](SET_ConfiguratorRequest&& request) {
+                    K2LOG_I(log::configurator, "Received set for key: {}", request.key);
+                    auto key = request.key;
+                    auto value = request.value;
+                    if (request.applyToAll) {
+                        return ConfigDist().invoke_on_all([key, value](auto& config) {
+                            config.emplace(key, boost::program_options::variable_value(value, ""));
+                            boost::program_options::notify(config);
+                        }).then([key] {
+                            SET_ConfiguratorResponse response{.key=std::move(key)};
+                            return RPCResponse(Statuses::S201_Created("Configurator set accepted"), std::move(response));
+                        });
+                    } else {
+                        auto& conf = const_cast<config::BPOVarMap&>(Config());
+                        conf.emplace(key, boost::program_options::variable_value(value, ""));
+                        boost::program_options::notify(conf);
+                        SET_ConfiguratorResponse response{.key=std::move(request.key)};
+                        return RPCResponse(Statuses::S201_Created("Configurator set accepted"), std::move(response));
+                    }
+                });
+        RPC().registerRPCObserver<GET_ConfiguratorRequest, GET_ConfiguratorResponse>(InternalVerbs::CONFGURATOR_GET, [this](GET_ConfiguratorRequest&& request) {
+                    K2LOG_I(log::configurator, "Received get for key: {}", request.key);
+                    auto key = request.key;
+                    GET_ConfiguratorResponse response;
+                    response.key=request.key;
+                    auto iter = Config().find(key);
+                    if (iter != Config().end()) {
+                        response.value = (iter->second).as<String>();
+                    } else {
+                        return RPCResponse(Statuses::S404_Not_Found("key not found"), std::move(response));
+                    }
+                    return RPCResponse(Statuses::S200_OK("get accepted"), std::move(response));
+                });
+        RPC().registerRPCObserver<DELETE_ConfiguratorRequest, DELETE_ConfiguratorResponse>(InternalVerbs::CONFGURATOR_DELETE, [this](DELETE_ConfiguratorRequest&& request) {
+                    K2LOG_I(log::configurator, "Received delete for key: {}", request.key);
+                     auto& key = request.key;
+                    if (request.applyToAll) {
+                        return ConfigDist().invoke_on_all([key](auto& config) {
+                            if (config.count(key)) {
+                                config.erase(key);
+                            }
+                        }).then([key] {
+                            DELETE_ConfiguratorResponse response{.key=std::move(key)};
+                            return RPCResponse(Statuses::S201_Created("Configurator set accepted"), std::move(response));
+                        });
+                    } else {
+                        auto& config = const_cast<config::BPOVarMap&>(Config());
+                        if (config.count(key)) {
+                            config.erase(key);
+                        }
+                        DELETE_ConfiguratorResponse response{.key=std::move(request.key)};
+                        return RPCResponse(Statuses::S201_Created("Configurator erase accepted"), std::move(response));
+                    }
+                });
+        return seastar::make_ready_future();
+    }
+    seastar::future<> Configurator::gracefulStop() {
+        K2LOG_I(log::configurator, "stop");
+        return seastar::make_ready_future<>();
+    }
+    } // namespace k2

-Original file line number
+Diff line change
@@ Expand Up / @@ -38,7 +38,10 @@ typedef uint8_t Verb; @@
     // Verbs used by K2 internally
     enum InternalVerbs : k2::Verb {
         LIST_ENDPOINTS = 249,  // used to discover the endpoints of a node
-        MAX_VERB = 250,  // something we can use to prevent override of internal verbs.
+        CONFGURATOR_GET = 250,  // used to configure get.
+        CONFGURATOR_SET = 251,  // used to configure set.
+        CONFGURATOR_DELETE = 252,  // used to configure clear messages.
+        MAX_VERB = 253,  // something we can use to prevent override of internal verbs.
         NIL              // used for messages where the verb doesn't matter
     };
@@ Expand Down @@

-Original file line number
+Diff line change
@@ -1,6 +1,7 @@
     enable_testing()
     include_directories(include)
     add_subdirectory (cpo)
+    add_subdirectory (configurator)
     add_subdirectory (transport)
     add_subdirectory (k23si)
     add_subdirectory (plog)
@@ Expand Down @@

Adding configurator feature to dynamically inject configuration changes #374

Are you sure you want to change the base?

Uh oh!

Adding configurator feature to dynamically inject configuration changes #374

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!