diff options
author | HÃ¥vard Pettersen <3535158+havardpe@users.noreply.github.com> | 2021-10-07 14:21:50 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-10-07 14:21:50 +0200 |
commit | ad60a4df3210a28f00e17dcadbb7761ef2f04283 (patch) | |
tree | 5520cc3979cd8237e80a19cbf6a9d6cf32612412 /vespalib | |
parent | eefe62a61741eaebcc77ae3dded3124f309933e8 (diff) | |
parent | e1a26413bd20ef269d53dc56552a8d8809ab4e58 (diff) |
Merge pull request #19457 from vespa-engine/havardpe/capture-issues
capture issues by binding handlers to threads
Diffstat (limited to 'vespalib')
-rw-r--r-- | vespalib/CMakeLists.txt | 1 | ||||
-rw-r--r-- | vespalib/src/tests/issue/CMakeLists.txt | 9 | ||||
-rw-r--r-- | vespalib/src/tests/issue/issue_test.cpp | 68 | ||||
-rw-r--r-- | vespalib/src/vespa/vespalib/util/CMakeLists.txt | 1 | ||||
-rw-r--r-- | vespalib/src/vespa/vespalib/util/issue.cpp | 65 | ||||
-rw-r--r-- | vespalib/src/vespa/vespalib/util/issue.h | 66 |
6 files changed, 210 insertions, 0 deletions
diff --git a/vespalib/CMakeLists.txt b/vespalib/CMakeLists.txt index 1b226c84753..472696131d2 100644 --- a/vespalib/CMakeLists.txt +++ b/vespalib/CMakeLists.txt @@ -64,6 +64,7 @@ vespa_define_module( src/tests/hwaccelrated src/tests/io/fileutil src/tests/io/mapped_file_input + src/tests/issue src/tests/latch src/tests/left_right_heap src/tests/make_fixture_macros diff --git a/vespalib/src/tests/issue/CMakeLists.txt b/vespalib/src/tests/issue/CMakeLists.txt new file mode 100644 index 00000000000..7c8147979d7 --- /dev/null +++ b/vespalib/src/tests/issue/CMakeLists.txt @@ -0,0 +1,9 @@ +# Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +vespa_add_executable(vespalib_issue_test_app TEST + SOURCES + issue_test.cpp + DEPENDS + vespalib + GTest::GTest +) +vespa_add_test(NAME vespalib_issue_test_app COMMAND vespalib_issue_test_app) diff --git a/vespalib/src/tests/issue/issue_test.cpp b/vespalib/src/tests/issue/issue_test.cpp new file mode 100644 index 00000000000..01b151da482 --- /dev/null +++ b/vespalib/src/tests/issue/issue_test.cpp @@ -0,0 +1,68 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include <vespa/vespalib/util/issue.h> +#include <vespa/vespalib/gtest/gtest.h> + +using namespace vespalib; + +struct MyHandler : Issue::Handler { + std::vector<vespalib::string> list; + void handle(const Issue &issue) override { + list.push_back(issue.message()); + } +}; + +std::vector<vespalib::string> make_list(std::vector<vespalib::string> list) { + return list; +} + +TEST(IssueTest, log_issue_not_captured) { + Issue::report(Issue("this should be logged")); +} + +TEST(IssueTest, capture_an_issue) { + MyHandler my_handler; + { + Issue::report(Issue("this should be logged")); + Issue::Binding my_binding = Issue::listen(my_handler); + Issue::report(Issue("this should be captured")); + } + Issue::report(Issue("this should also be logged")); + EXPECT_EQ(my_handler.list, make_list({"this should be captured"})); +} + +TEST(IssueTest, capture_issues_with_nested_bindings) { + MyHandler my_handler1; + MyHandler my_handler2; + { + Issue::report(Issue("this should be logged")); + auto my_binding1 = Issue::listen(my_handler1); + Issue::report(Issue("issue1")); + { + auto my_binding2 = Issue::listen(my_handler2); + Issue::report(Issue("issue2")); + } + Issue::report(Issue("issue3")); + } + Issue::report(Issue("this should also be logged")); + EXPECT_EQ(my_handler1.list, make_list({"issue1", "issue3"})); + EXPECT_EQ(my_handler2.list, make_list({"issue2"})); +} + +TEST(IssueTest, handler_can_be_bound_multiple_times) { + MyHandler my_handler; + { + Issue::report(Issue("this should be logged")); + auto my_binding1 = Issue::listen(my_handler); + Issue::report(Issue("issue1")); + { + auto my_binding2 = Issue::listen(my_handler); + Issue::report(Issue("issue2")); + } + Issue::report(Issue("issue3")); + } + Issue::report(Issue("this should also be logged")); + EXPECT_EQ(my_handler.list, make_list({"issue1", "issue2", "issue3"})); +} + +GTEST_MAIN_RUN_ALL_TESTS() diff --git a/vespalib/src/vespa/vespalib/util/CMakeLists.txt b/vespalib/src/vespa/vespalib/util/CMakeLists.txt index 8db65b58b04..9e2917775f0 100644 --- a/vespalib/src/vespa/vespalib/util/CMakeLists.txt +++ b/vespalib/src/vespa/vespalib/util/CMakeLists.txt @@ -29,6 +29,7 @@ vespa_add_library(vespalib_vespalib_util OBJECT generationholder.cpp hdr_abort.cpp host_name.cpp + issue.cpp joinable.cpp latch.cpp left_right_heap.cpp diff --git a/vespalib/src/vespa/vespalib/util/issue.cpp b/vespalib/src/vespa/vespalib/util/issue.cpp new file mode 100644 index 00000000000..9526d8d6bcd --- /dev/null +++ b/vespalib/src/vespa/vespalib/util/issue.cpp @@ -0,0 +1,65 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#include "issue.h" + +#include <vespa/log/log.h> +LOG_SETUP(".vespalib.issue"); + +namespace vespalib { + +namespace { + +using Link = Issue::Binding::Link; + +struct LogIssues : Issue::Handler { + void handle(const Issue &issue) override { + LOG(warning, "issue not captured: %s", issue.message().c_str()); + } +}; + +Link *get_root() { + static LogIssues log_issues; + static Link root{log_issues, nullptr}; + return &root; +} + +Link **get_head() { + thread_local Link *head = get_root(); + return &head; +} + +} // <unnamed> + +Issue::Issue(const vespalib::string &message) + : _message(message) +{ +} + +Issue::Binding::Binding(Handler &handler) + : _link{handler, nullptr} +{ + Link **head = get_head(); + _link.next = *head; + *head = &_link; +} + +Issue::Binding::~Binding() +{ + Link **head = get_head(); + LOG_ASSERT(*head == &_link); + *head = (*head)->next; +} + +void +Issue::report(const Issue &issue) +{ + (*get_head())->handler.handle(issue); +} + +Issue::Binding +Issue::listen(Handler &handler) +{ + return Binding(handler); +} + +} diff --git a/vespalib/src/vespa/vespalib/util/issue.h b/vespalib/src/vespa/vespalib/util/issue.h new file mode 100644 index 00000000000..7cb4ebaff59 --- /dev/null +++ b/vespalib/src/vespa/vespalib/util/issue.h @@ -0,0 +1,66 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +#pragma once + +#include <vespa/vespalib/stllike/string.h> + +namespace vespalib { + +/** + * An Issue is an error encountered during program execution that will + * not affect the program flow. It is like an exception that is not + * (re-)thrown, but rather logged as a warning, or like an error that + * augments the computation result instead of replacing it. Issues are + * reported by the code identifying that something is wrong; similar + * to logging a warning. Issues are handled by an Issue::Handler that + * is bound to the current thread; similar to try/catch used by + * exceptions, but rather than disrupting the program flow we only + * capture any issues encountered along the way. Thread-local data + * structures are used to match the reporting of an issue with its + * appropriate handler, making it possible to handle issues across + * arbitrary synchronous API boundaries without changing the APIs to + * explicitly wire all issues through them. + * + * An Issue object represents a single issue. The static 'report' + * function is used to report an issue. The static 'listen' function + * is used to bind an Issue::Handler to the current thread using a + * special object that should reside on the stack and unbinds the + * handler when destructed. Handler bindings can be nested. Issue + * reports will be routed to the last handler bound to the thread. + * + * Note that the objects binding handlers to threads must not be + * destructed out of order; just let them go out of scope. + **/ +class Issue +{ +private: + vespalib::string _message; +public: + Issue(const vespalib::string &message); + const vespalib::string &message() const { return _message; } + struct Handler { + virtual void handle(const Issue &issue) = 0; + virtual ~Handler() = default; + }; + class Binding + { + public: + struct Link { + Handler &handler; + Link *next; + }; + private: + Link _link; + public: + Binding(Handler &handler); + Binding(Binding &&) = delete; + Binding(const Binding &) = delete; + Binding &operator=(Binding &&) = delete; + Binding &operator=(const Binding &) = delete; + ~Binding(); + }; + static void report(const Issue &issue); + static Binding listen(Handler &handler); +}; + +} |