brbzull0 commented on code in PR #13904:
URL: https://github.com/apache/trafficserver/pull/13904#discussion_r4379679726
##########
include/mgmt/config/ConfigContext.h:
##########
@@ -186,7 +189,8 @@ class ConfigContext
/// @code
/// if (auto yaml = ctx.supplied_yaml()) { /* use yaml node */ }
/// @endcode
- /// @return copy of the supplied YAML node (cheap - YAML::Node is internally
reference-counted).
+ /// @return copy of the supplied YAML node (cheap - YAML::Node is internally
reference-counted),
+ /// or a new Undefined node on each call when none was supplied.
Review Comment:
Both getters' `@return` now say they return a handle on a node that copies
and dependents of the context share, so a write through it is seen by all of
them.
##########
src/records/unit_tests/test_ConfigReloadProgress.cc:
##########
@@ -86,3 +93,55 @@ TEST_CASE("Progress checker releases a task that timed out",
"[config][reload][l
REQUIRE(wait_for_sole_owner(task, std::chrono::seconds{10}));
REQUIRE(task->get_state() == ConfigReloadTask::State::TIMEOUT);
}
+
+// execute_reload() gives a handler a context with a task only while a reload
is in
+// progress, and starting one needs ET_TASK for its progress checker, so this
case
+// lives in this binary.
+TEST_CASE("A dependent context inherits its parent's RPC content",
"[config][reload][context]")
+{
+ static std::optional<ConfigContext> parent;
+ static std::optional<ConfigContext> child;
+
+ std::string token;
+ REQUIRE(ReloadCoordinator::Get_Instance().prepare_reload(token,
"test-dep-").is_ok());
+ auto const main_task = ReloadCoordinator::Get_Instance().get_current_task();
+ REQUIRE(main_task);
+ // prepare_reload() started a progress checker, which holds one of these.
+ auto const owners_with_checker = main_task.use_count();
+
+ auto ® = config::ConfigRegistry::Get_Instance();
+ reg.register_config("test_dependent_payload", "", "",
+ [](ConfigContext ctx) {
+ parent.emplace(ctx);
+ child.emplace(ctx.add_dependent_ctx("dependent"));
+ child->complete();
+ ctx.complete();
+ },
+ config::ConfigSource::FileAndRpc, {});
+ reg.set_passed_config("test_dependent_payload", YAML::Load("{_reload: {id:
foo}, data: x}"));
+ reg.execute_reload("test_dependent_payload");
+
+ REQUIRE(parent.has_value());
+ REQUIRE(child.has_value());
+ REQUIRE(static_cast<bool>(*parent));
+ REQUIRE(static_cast<bool>(*child));
+
+ YAML::Node const content = child->supplied_yaml();
+ REQUIRE(content);
+ REQUIRE(content["data"].as<std::string>() == "x");
+ YAML::Node const directives = child->reload_directives();
+ REQUIRE(directives);
+ REQUIRE(directives["id"].as<std::string>() == "foo");
+ REQUIRE(content.is(parent->supplied_yaml()));
+ REQUIRE(directives.is(parent->reload_directives()));
+
+ child.reset();
+ parent.reset();
+
+ // Like the cases above, don't leave the checker running past the test.
+ auto const deadline = std::chrono::steady_clock::now() +
std::chrono::seconds{15};
+ while (main_task.use_count() >= owners_with_checker &&
std::chrono::steady_clock::now() < deadline) {
Review Comment:
The loop now uses `wait_for_owners_below()`, which `wait_for_sole_owner()`
also calls. The 15 s is the same bound the terminal case uses for the same
checker, and the comment says so.
##########
src/records/unit_tests/test_ConfigRegistry.cc:
##########
@@ -309,3 +314,131 @@ TEST_CASE("ConfigRegistry is_reloadable distinguishes
handler-less entries", "[c
// An unknown key is not reloadable.
CHECK_FALSE(reg.is_reloadable("test_reloadable_unknown_xyz"));
}
+
+// ─── ConfigContext content on the execute_reload() path
───────────────────────
+//
+// No reload is in progress here, so execute_reload() hands the handler a
task-less
+// context, and the RPC-supplied content is still applied to it.
+
+namespace
+{
+ConfigContext
+reload_with(std::string const &key, YAML::Node const &content)
Review Comment:
Keeping `reload_with`: at the call sites it reads as `reload_with(key,
content)`, and the comment above the helper says what it returns.
--
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]
