From 57e72842776cd69b26454c4eb6d5639a55aca4df Mon Sep 17 00:00:00 2001 From: Andrea Barbadoro Date: Fri, 10 May 2024 17:21:33 +0200 Subject: [PATCH 1/2] admin/server: fix rearm_log_level_timer _log_level_resets is a map, we want to extract the next expiring level_reset to rearm the _log_level_timer. previously we would extract the min_element based on the key (the logger name) so this sequence of actions set log-level kafka trace 10 set log-level raft trace 10 only kafka would be expired, because once expired the next run would pick again kafka, see that there is no expiration, and not rearm the timer. --- src/v/redpanda/admin_server.cc | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/v/redpanda/admin_server.cc b/src/v/redpanda/admin_server.cc index def53bff09a7..eec400091b58 100644 --- a/src/v/redpanda/admin_server.cc +++ b/src/v/redpanda/admin_server.cc @@ -134,6 +134,7 @@ #include #include #include +#include #include #include #include @@ -622,16 +623,14 @@ void admin_server::log_exception( void admin_server::rearm_log_level_timer() { _log_level_timer.cancel(); - auto next = std::min_element( - _log_level_resets.begin(), - _log_level_resets.end(), - [](const auto& a, const auto& b) { - return a.second.expires < b.second.expires; - }); - - if (next != _log_level_resets.end()) { - _log_level_timer.arm(next->second.expires); + if (_log_level_resets.empty()) { + return; } + + auto reset_values = _log_level_resets | std::views::values; + auto& lvl_rst = *std::ranges::min_element( + reset_values, std::less<>{}, &level_reset::expires); + _log_level_timer.arm(lvl_rst.expires); } void admin_server::log_level_timer_handler() { From e7c299a66c18ef34acd685304234f3bd3f4637db Mon Sep 17 00:00:00 2001 From: Andrea Barbadoro Date: Fri, 10 May 2024 17:32:59 +0200 Subject: [PATCH 2/2] tests/log_level_test: test_log_level_multiple_expiry the test shows that this sequence of actions set log-level admin-api-server trace 10 set log-level raft trace 10 works and both logger are reset after the expiry period --- tests/rptest/tests/log_level_test.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/rptest/tests/log_level_test.py b/tests/rptest/tests/log_level_test.py index 9a7e66e42dee..6821a37f1bc9 100644 --- a/tests/rptest/tests/log_level_test.py +++ b/tests/rptest/tests/log_level_test.py @@ -7,10 +7,12 @@ # the Business Source License, use of this software will be governed # by the Apache License, Version 2.0 +import time import ducktape.errors import requests.exceptions import urllib.parse +from ducktape.mark import parametrize from ducktape.utils.util import wait_until from rptest.services.cluster import cluster from rptest.tests.redpanda_test import RedpandaTest @@ -94,6 +96,32 @@ def test_log_level_control(self): backoff_sec=1, err_msg="Never saw message") + @cluster(num_nodes=1) + @parametrize(loggers=("admin_api_server", "raft")) + @parametrize(loggers=("raft", "admin_api_server")) + def test_log_level_multiple_expiry(self, loggers=tuple[str, str]): + """ + Check that more than one logger can be in a modified level and be expired correctly + see https://redpandadata.atlassian.net/browse/CORE-96 + """ + admin = Admin(self.redpanda) + node = self.redpanda.nodes[0] + + first_logger, second_logger = loggers + # set two loggers to trace, expect that both of them expires in a timely fashion + with self.redpanda.monitor_log(node) as mon: + admin.set_log_level(first_logger, "trace", expires=10) + time.sleep(1) + admin.set_log_level(second_logger, "trace", expires=10) + mon.wait_until(f"Expiring log level for {{{first_logger}}}", + timeout_sec=15, + backoff_sec=1, + err_msg=f"Never saw Expiring for {first_logger}") + mon.wait_until(f"Expiring log level for {{{second_logger}}}", + timeout_sec=15, + backoff_sec=1, + err_msg=f"Never saw Expiring for {second_logger}") + @cluster(num_nodes=3) def test_invalid_logger_name(self): admin = Admin(self.redpanda)