Browse Source

YT-21233: Split error into error and stripped_error to rid the latter of deps on global variables

What happened:
1. error contents has been split into stripped_error and error. stripped_error contains the error itself (with attributes for now) and macros; error contains stripped_error and some extensions, namely, functions to get fiberId, hostname and traceid/spanid and all functions used to (de-)serialize error. This means that you cannot print error if you only include stripped_error, therefore you are likely to still require the entire error.h at the moment.
2. Mechanic for gathering origin attributes has been moved to newly created library/cpp/yt/error thus having no dependency on fibers, net or tracing. stripped_error uses these attributes as extendable semi-erased (meaning, you still would have to add a field and recompile the entire thing, but you don't have to introduce an extra dependency) storage for a bunch of attributes
3. Parsing of said attributes is done in error file (and not stripped_error).

P.S. So far the plan is to eventually move stripped_error (once dependency on core/ytree/attributes is eliminated) without any actual change to dependency graph of anything outside of core (e.g. you would still have to include misc/error.h to use it). Next step would be re-teaching the error how to print, which would move some more methods from core to the standalone module. After that one could finally depend on the error itself and not the entire core.

Annotations: [nodiff:caesar]
66615172181355821241d2e5f8e4a0f15e0ea791
arkady-e1ppa 6 months ago
parent
commit
28ff4da78a

+ 112 - 0
library/cpp/yt/error/origin_attributes.cpp

@@ -0,0 +1,112 @@
+#include "origin_attributes.h"
+
+#include <library/cpp/yt/assert/assert.h>
+
+#include <library/cpp/yt/misc/thread_name.h>
+#include <library/cpp/yt/misc/tls.h>
+
+#include <library/cpp/yt/string/format.h>
+
+#include <util/system/thread.h>
+
+namespace NYT {
+
+////////////////////////////////////////////////////////////////////////////////
+
+YT_DEFINE_THREAD_LOCAL(bool, ErrorSanitizerEnabled, false);
+YT_DEFINE_THREAD_LOCAL(TInstant, ErrorSanitizerDatetimeOverride);
+YT_DEFINE_THREAD_LOCAL(TSharedRef, ErrorSanitizerLocalHostNameOverride);
+
+TErrorSanitizerGuard::TErrorSanitizerGuard(TInstant datetimeOverride, TSharedRef localHostNameOverride)
+    : SavedEnabled_(ErrorSanitizerEnabled())
+    , SavedDatetimeOverride_(ErrorSanitizerDatetimeOverride())
+    , SavedLocalHostNameOverride_(ErrorSanitizerLocalHostNameOverride())
+{
+    ErrorSanitizerEnabled() = true;
+    ErrorSanitizerDatetimeOverride() = datetimeOverride;
+    ErrorSanitizerLocalHostNameOverride() = std::move(localHostNameOverride);
+}
+
+TErrorSanitizerGuard::~TErrorSanitizerGuard()
+{
+    YT_ASSERT(ErrorSanitizerEnabled());
+
+    ErrorSanitizerEnabled() = SavedEnabled_;
+    ErrorSanitizerDatetimeOverride() = SavedDatetimeOverride_;
+    ErrorSanitizerLocalHostNameOverride() = std::move(SavedLocalHostNameOverride_);
+}
+
+bool IsErrorSanitizerEnabled() noexcept
+{
+    return ErrorSanitizerEnabled();
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
+bool TOriginAttributes::operator==(const TOriginAttributes& other) const noexcept
+{
+    return
+        Host == other.Host &&
+        Datetime == other.Datetime &&
+        Pid == other.Pid &&
+        Tid == other.Tid &&
+        ExtensionData == other.ExtensionData;
+}
+
+void TOriginAttributes::Capture()
+{
+    if (ErrorSanitizerEnabled()) {
+        Datetime = ErrorSanitizerDatetimeOverride();
+        HostHolder = ErrorSanitizerLocalHostNameOverride();
+        Host = HostHolder.empty() ? TStringBuf() : TStringBuf(HostHolder.Begin(), HostHolder.End());
+        return;
+    }
+
+    Datetime = TInstant::Now();
+    Pid = GetPID();
+    Tid = TThread::CurrentThreadId();
+    ThreadName = GetCurrentThreadName();
+    ExtensionData = NDetail::GetExtensionData();
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
+namespace NDetail {
+
+std::optional<TOriginAttributes::TErasedExtensionData> GetExtensionData()
+{
+    using TFunctor = TOriginAttributes::TErasedExtensionData(*)();
+
+    if (auto strong = NGlobal::GetErasedVariable(GetExtensionDataTag)) {
+        return strong->AsConcrete<TFunctor>()();
+    }
+    return std::nullopt;
+}
+
+TString FormatOrigin(const TOriginAttributes& attributes)
+{
+    using TFunctor = TString(*)(const TOriginAttributes&);
+
+    if (auto strong = NGlobal::GetErasedVariable(FormatOriginTag)) {
+        return strong->AsConcrete<TFunctor>()(attributes);
+    }
+
+    return Format(
+        "%v (pid %v, thread %v)",
+        attributes.Host,
+        attributes.Pid,
+        MakeFormatterWrapper([&] (auto* builder) {
+            auto threadName = attributes.ThreadName.ToStringBuf();
+            if (threadName.empty()) {
+                FormatValue(builder, attributes.Tid, "v");
+                return;
+            }
+            FormatValue(builder, threadName, "v");
+        }));
+}
+
+} // namespace NDetail
+
+////////////////////////////////////////////////////////////////////////////////
+
+} // namespace NYT

+ 83 - 0
library/cpp/yt/error/origin_attributes.h

@@ -0,0 +1,83 @@
+#pragma once
+
+#include <library/cpp/yt/global/access.h>
+
+#include <library/cpp/yt/memory/ref.h>
+
+#include <library/cpp/yt/misc/guid.h>
+#include <library/cpp/yt/misc/thread_name.h>
+
+#include <library/cpp/yt/threading/public.h>
+
+#include <util/datetime/base.h>
+
+#include <util/system/getpid.h>
+
+namespace NYT {
+
+////////////////////////////////////////////////////////////////////////////////
+
+//! When this guard is set, newly created errors do not have non-deterministic
+//! system attributes and have "datetime" and "host" attributes overridden with a given values.
+class TErrorSanitizerGuard
+    : public TNonCopyable
+{
+public:
+    TErrorSanitizerGuard(TInstant datetimeOverride, TSharedRef localHostNameOverride);
+    ~TErrorSanitizerGuard();
+
+private:
+    const bool SavedEnabled_;
+    const TInstant SavedDatetimeOverride_;
+    const TSharedRef SavedLocalHostNameOverride_;
+};
+
+bool IsErrorSanitizerEnabled() noexcept;
+
+////////////////////////////////////////////////////////////////////////////////
+
+struct TOriginAttributes
+{
+    static constexpr size_t ExtensionDataByteSizeCap = 64;
+    using TErasedExtensionData = TErasedStorage<ExtensionDataByteSizeCap>;
+
+    TProcessId Pid;
+
+    NThreading::TThreadId Tid;
+    TThreadName ThreadName;
+
+    TInstant Datetime;
+
+    TSharedRef HostHolder;
+    mutable TStringBuf Host;
+
+    // Opaque storage for data from yt/yt/core.
+    // Currently may contain FiberId, TraceId, SpandId.
+    std::optional<TErasedExtensionData> ExtensionData;
+
+    bool operator==(const TOriginAttributes& other) const noexcept;
+
+    void Capture();
+};
+
+////////////////////////////////////////////////////////////////////////////////
+
+namespace NDetail {
+
+inline constexpr NGlobal::TVariableTag GetExtensionDataTag = {};
+inline constexpr NGlobal::TVariableTag FormatOriginTag = {};
+inline constexpr NGlobal::TVariableTag ExtractFromDictionaryTag = {};
+
+////////////////////////////////////////////////////////////////////////////////
+
+// These are "weak" symbols.
+// NB(arkady-e1ppa): ExtractFromDictionary symbol is left in yt/yt/core/misc/origin_attributes
+// because it depends on ytree for now.
+std::optional<TOriginAttributes::TErasedExtensionData> GetExtensionData();
+TString FormatOrigin(const TOriginAttributes& attributes);
+
+} // namespace NDetail
+
+////////////////////////////////////////////////////////////////////////////////
+
+} // namespace NYT

+ 18 - 0
library/cpp/yt/error/ya.make

@@ -0,0 +1,18 @@
+LIBRARY()
+
+INCLUDE(${ARCADIA_ROOT}/library/cpp/yt/ya_cpp.make.inc)
+
+PEERDIR(
+    library/cpp/yt/assert
+    library/cpp/yt/global
+    library/cpp/yt/memory
+    library/cpp/yt/misc
+    library/cpp/yt/threading
+    library/cpp/yt/string
+)
+
+SRCS(
+    origin_attributes.cpp
+)
+
+END()

+ 80 - 0
library/cpp/yt/global/access.h

@@ -0,0 +1,80 @@
+#pragma once
+
+#include <library/cpp/yt/memory/erased_storage.h>
+
+#include <library/cpp/yt/misc/strong_typedef.h>
+
+#include <atomic>
+
+namespace NYT::NGlobal {
+
+////////////////////////////////////////////////////////////////////////////////
+
+namespace NDetail {
+
+class TGlobalVariablesRegistry;
+
+} // namespace NDetail
+
+////////////////////////////////////////////////////////////////////////////////
+
+inline constexpr size_t GlobalVariableMaxByteSize = 32;
+using TErasedStorage = NYT::TErasedStorage<GlobalVariableMaxByteSize>;
+
+// NB(arkady-e1ppa): Accessor must ensure thread-safety on its own.
+using TAccessor = TErasedStorage(*)() noexcept;
+
+////////////////////////////////////////////////////////////////////////////////
+
+// Usage:
+/*
+ * // public.h file:
+ * // NB: It's important to mark it inline for linker to deduplicate
+ * // addresses accross different UT's.
+ * inline constexpr NGlobal::TVariableTag MyGlobalVarTag = {};
+ *
+ *
+ *
+ * // some_stuff.cpp file
+ *
+ * TErasedStorage GetMyVar()
+ * {
+ * // definition here
+ * }
+ * NGlobal::Variable<int> MyGlobalVar{MyGlobalVarTag, &GetMyVar};
+ *
+ *
+ *
+ * // other_stuff.cpp file
+ *
+ *
+ * int ReadMyVar()
+ * {
+ *   auto erased = NGlobal::GetErasedVariable(MyGlobalVarTag);
+ *   return erased->AsConcrete<int>();
+ * }
+ */
+class TVariableTag
+{
+public:
+    TVariableTag() = default;
+
+    TVariableTag(const TVariableTag& other) = delete;
+    TVariableTag& operator=(const TVariableTag& other) = delete;
+
+private:
+    friend class ::NYT::NGlobal::NDetail::TGlobalVariablesRegistry;
+
+    mutable std::atomic<bool> Initialized_ = false;
+    mutable std::atomic<int> Key_ = -1;
+};
+
+////////////////////////////////////////////////////////////////////////////////
+
+// Defined in impl.cpp.
+// |std::nullopt| iff global variable with a given tag is not present.
+std::optional<TErasedStorage> GetErasedVariable(const TVariableTag& tag);
+
+////////////////////////////////////////////////////////////////////////////////
+
+} // namespace NYT::NGlobal

+ 126 - 0
library/cpp/yt/global/impl.cpp

@@ -0,0 +1,126 @@
+#include "access.h"
+#include "variable.h"
+
+#include <library/cpp/yt/memory/leaky_singleton.h>
+
+#include <array>
+
+namespace NYT::NGlobal {
+
+////////////////////////////////////////////////////////////////////////////////
+
+namespace NDetail {
+
+inline constexpr int MaxTrackedGlobalVariables = 32;
+
+////////////////////////////////////////////////////////////////////////////////
+
+class TGlobalVariablesRegistry
+{
+public:
+    static TGlobalVariablesRegistry* Get()
+    {
+        return LeakySingleton<TGlobalVariablesRegistry>();
+    }
+
+    void RegisterAccessor(const TVariableTag& tag, TAccessor accessor)
+    {
+        if (!tag.Initialized_.exchange(true, std::memory_order::relaxed)) { // (a)
+            DoRegisterAccessor(tag, accessor);
+            return;
+        }
+
+        TryVerifyingExistingAccessor(tag, accessor);
+    }
+
+    std::optional<TErasedStorage> GetVariable(const TVariableTag& tag)
+    {
+        auto key = tag.Key_.load(std::memory_order::acquire); // (e)
+        if (key != -1) {
+            return Accessors_[key](); // (f)
+        }
+
+        return std::nullopt;
+    }
+
+private:
+    std::atomic<int> KeyGenerator_ = 0;
+    std::array<TAccessor, MaxTrackedGlobalVariables> Accessors_;
+
+    void DoRegisterAccessor(const TVariableTag& tag, TAccessor accessor)
+    {
+        // Get id -> place accessor -> store id
+        auto key = KeyGenerator_.fetch_add(1, std::memory_order::relaxed); // (b)
+
+        YT_VERIFY(key < MaxTrackedGlobalVariables);
+
+        Accessors_[key] = accessor; // (c)
+
+        tag.Key_.store(key, std::memory_order::release); // (d)
+    }
+
+    void TryVerifyingExistingAccessor(const TVariableTag& tag, TAccessor accessor)
+    {
+        auto key = tag.Key_.load(std::memory_order::acquire); // (e')
+        if (key == -1) {
+            // Accessor is about to be set.
+
+            // In order to avoid deadlock caused by forks
+            // we just leave. We could try acquiring fork
+            // locks here but this makes our check too expensive
+            // to be bothered.
+            return;
+        }
+
+        // Accessor has been already set -> safe to read it.
+        YT_VERIFY(Accessors_[key] == accessor); // (f')
+    }
+};
+
+// (arkady-e1ppa): Memory orders:
+/*
+    We have two scenarios: 2 writes and write & read:
+
+    2 writes: Accessors_ is protected via Initialized_ flag
+    and KeyGenerator_ counter.
+    1) RMW (a) reads the last value in modification order
+    thus relaxed is enough to ensure <= 1 threads registering
+    per Tag.
+    2) KeyGenerator_ uses the same logic (see (b))
+    to ensure <= 1 threads registering per index in array.
+
+    If there are two writes per tag, then there is a "losing"
+    thread which read Initialized_ // true. For all intents
+    and purposes TryVerifyingExistingAccessor call is identical
+    to GetVariable call.
+
+    write & read: Relevant execution is below
+                    W^na(Accessors_[id], 0x0) // Ctor
+        T1(Register)                    T2(Read)
+    W^na(Accessors_[id], 0x42) (c)  R^acq(Key_, id)              (e)
+    W^rel(Key_, id)             (d)  R^na(Accessors_[id], 0x42)  (f)
+
+    (d) -rf-> (e) => (d) -SW-> (e). Since (c) -SB-> (d) and (e) -SB-> (f)
+    we have (c) -strongly HB-> (f) (strongly happens before). Thus we must
+    read 0x42 from Accessors_[id] (and not 0x0 which was written in ctor).
+ */
+
+////////////////////////////////////////////////////////////////////////////////
+
+void RegisterVariable(const TVariableTag& tag, TAccessor accessor)
+{
+    TGlobalVariablesRegistry::Get()->RegisterAccessor(tag, accessor);
+}
+
+} // namespace NDetail
+
+////////////////////////////////////////////////////////////////////////////////
+
+std::optional<TErasedStorage> GetErasedVariable(const TVariableTag& tag)
+{
+    return NDetail::TGlobalVariablesRegistry::Get()->GetVariable(tag);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
+} // namespace NYT::NGlobal

+ 17 - 0
library/cpp/yt/global/mock_modules/module1_defs/direct_access.h

@@ -0,0 +1,17 @@
+#pragma once
+
+#include <cstdint>
+
+namespace NYT {
+
+////////////////////////////////////////////////////////////////////////////////
+
+int GetTestVariable1();
+void SetTestVariable1(int val);
+
+int GetTlsVariable();
+void SetTlsVariable(int val);
+
+////////////////////////////////////////////////////////////////////////////////
+
+} // namespace NYT

+ 38 - 0
library/cpp/yt/global/mock_modules/module1_defs/test_variable.cpp

@@ -0,0 +1,38 @@
+#include "direct_access.h"
+
+#include <library/cpp/yt/global/mock_modules/module1_public/test_tag.h>
+
+#include <library/cpp/yt/global/variable.h>
+
+namespace NYT {
+
+////////////////////////////////////////////////////////////////////////////////
+
+YT_DEFINE_TRACKED_GLOBAL(int, TestVariable1, TestTag1, 42);
+YT_DEFINE_TRACKED_THREAD_LOCAL(int, TlsVariable, ThreadLocalTag, 0);
+
+////////////////////////////////////////////////////////////////////////////////
+
+int GetTestVariable1()
+{
+    return TestVariable1.Get();
+}
+
+void SetTestVariable1(int val)
+{
+    TestVariable1.Set(val);
+}
+
+int GetTlsVariable()
+{
+    return TlsVariable();
+}
+
+void SetTlsVariable(int val)
+{
+    TlsVariable() = val;
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
+} // namespace NYT

+ 14 - 0
library/cpp/yt/global/mock_modules/module1_defs/ya.make

@@ -0,0 +1,14 @@
+LIBRARY()
+
+INCLUDE(${ARCADIA_ROOT}/library/cpp/yt/ya_cpp.make.inc)
+
+SRC(
+    test_variable.cpp
+)
+
+PEERDIR(
+    library/cpp/yt/global
+    library/cpp/yt/global/mock_modules/module1_public
+)
+
+END()

+ 14 - 0
library/cpp/yt/global/mock_modules/module1_public/test_tag.h

@@ -0,0 +1,14 @@
+#pragma once
+
+#include <library/cpp/yt/global/access.h>
+
+namespace NYT {
+
+////////////////////////////////////////////////////////////////////////////////
+
+inline constexpr NGlobal::TVariableTag TestTag1 = {};
+inline constexpr NGlobal::TVariableTag ThreadLocalTag = {};
+
+////////////////////////////////////////////////////////////////////////////////
+
+} // namespace NYT

+ 9 - 0
library/cpp/yt/global/mock_modules/module1_public/ya.make

@@ -0,0 +1,9 @@
+LIBRARY()
+
+INCLUDE(${ARCADIA_ROOT}/library/cpp/yt/ya_cpp.make.inc)
+
+PEERDIR(
+    library/cpp/yt/global
+)
+
+END()

Some files were not shown because too many files changed in this diff