Skip to content

Commit 911e2c6

Browse files
author
p34k1n
committed
feat userver: set trx_tracker start/end in transactions
Awaiting long requests during an active transaction (e.g. PostgreSQL, YDB, Clickhouse) is a frequently occuring bug that can easily use up the DB connection pool. This PR adds calls of StartTransaction and EndTransaction in Begin/Commit/Rollback methods of transactional databases (MySQL, PostgreSQL, SQLite, YDB). commit_hash:435e51a1fd2734b964359d980c171b02503409ff
1 parent 98715b8 commit 911e2c6

16 files changed

Lines changed: 405 additions & 95 deletions

File tree

.mapping.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1970,8 +1970,6 @@
19701970
"core/src/utils/text.cpp":"taxi/uservices/userver/core/src/utils/text.cpp",
19711971
"core/src/utils/text_test.cpp":"taxi/uservices/userver/core/src/utils/text_test.cpp",
19721972
"core/src/utils/trx_tracker.cpp":"taxi/uservices/userver/core/src/utils/trx_tracker.cpp",
1973-
"core/src/utils/trx_tracker_internal.cpp":"taxi/uservices/userver/core/src/utils/trx_tracker_internal.cpp",
1974-
"core/src/utils/trx_tracker_internal.hpp":"taxi/uservices/userver/core/src/utils/trx_tracker_internal.hpp",
19751973
"core/src/utils/trx_tracker_test.cpp":"taxi/uservices/userver/core/src/utils/trx_tracker_test.cpp",
19761974
"core/src/utils/userver_info.cpp":"taxi/uservices/userver/core/src/utils/userver_info.cpp",
19771975
"core/static_configs/dns_client.yaml":"taxi/uservices/userver/core/static_configs/dns_client.yaml",

core/include/userver/utils/trx_tracker.hpp

Lines changed: 58 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
/// @file
44
/// @brief Tracking for heavy operations while having active transactions.
55

6+
#include <optional>
7+
#include <thread>
8+
69
#include <userver/utils/impl/source_location.hpp>
710
#include <userver/utils/statistics/rate.hpp>
811

@@ -22,13 +25,62 @@ USERVER_NAMESPACE_BEGIN
2225
/// @snippet utils/trx_tracker_test.cpp Sample TransactionTracker usage
2326
namespace utils::trx_tracker {
2427

25-
/// @brief Increment transaction counter.
26-
void StartTransaction();
28+
namespace impl {
29+
30+
/// @brief Global enabler for transaction tracker.
31+
class GlobalEnabler final {
32+
public:
33+
explicit GlobalEnabler(bool enable = true);
34+
~GlobalEnabler();
35+
36+
GlobalEnabler(const GlobalEnabler&) = delete;
37+
GlobalEnabler& operator=(const GlobalEnabler&) = delete;
38+
};
39+
40+
/// @brief Check if transaction tracker is enabled.
41+
bool IsEnabled() noexcept;
2742

28-
/// @brief Decrement transaction counter.
43+
/// @brief Unique ID for every task.
2944
///
30-
/// If called before StartTransaction, the behavior is undefined.
31-
void EndTransaction() noexcept;
45+
/// Sometimes transactions start and end in different coroutines.
46+
/// To prevent transaction from incrementing and decrementing different
47+
/// transaction counters, TransactionLock stores TaskId on Lock and
48+
/// checks that stored TaskId is the same as current TaskId in Unlock.
49+
class TaskId final {
50+
public:
51+
TaskId();
52+
53+
bool operator==(const TaskId& other) const;
54+
bool operator!=(const TaskId& other) const;
55+
56+
private:
57+
std::thread::id created_thread_id_;
58+
std::uint64_t thread_local_counter_;
59+
};
60+
61+
} // namespace impl
62+
63+
/// @brief Class for incrementing and decrementing transaction counter.
64+
class TransactionLock final {
65+
public:
66+
TransactionLock() = default;
67+
TransactionLock(const TransactionLock&) = delete;
68+
TransactionLock(TransactionLock&&) noexcept;
69+
TransactionLock operator=(const TransactionLock&) = delete;
70+
TransactionLock& operator=(TransactionLock&&) noexcept;
71+
72+
/// @brief Decrement transaction counter on destruction.
73+
~TransactionLock();
74+
75+
/// @brief Manually increment transaction counter.
76+
void Lock() noexcept;
77+
78+
/// @brief Manually decrement transaction counter.
79+
void Unlock() noexcept;
80+
81+
private:
82+
std::optional<impl::TaskId> task_id_;
83+
};
3284

3385
/// @brief Check for active transactions.
3486
void CheckNoTransactions(utils::impl::SourceLocation location = utils::impl::SourceLocation::Current());
@@ -45,7 +97,7 @@ void CheckNoTransactions(std::string_view location);
4597
class CheckDisabler final {
4698
public:
4799
/// @brief Disable check for active transactions.
48-
CheckDisabler();
100+
explicit CheckDisabler();
49101

50102
/// @brief Reenable check for active transactions on destruction.
51103
~CheckDisabler();

core/src/components/run.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#include <userver/utils/strerror.hpp>
2828
#include <userver/utils/string_literal.hpp>
2929
#include <userver/utils/traceful_exception.hpp>
30+
#include <userver/utils/trx_tracker.hpp>
3031

3132
#include <components/manager.hpp>
3233
#include <components/manager_config.hpp>
@@ -36,7 +37,6 @@
3637
#include <utils/ignore_signal_scope.hpp>
3738
#include <utils/jemalloc.hpp>
3839
#include <utils/signal_catcher.hpp>
39-
#include <utils/trx_tracker_internal.hpp>
4040

4141
USERVER_NAMESPACE_BEGIN
4242

@@ -203,7 +203,7 @@ void DoRun(
203203
utils::impl::UserverExperimentsScope experiments_scope;
204204
std::optional<impl::Manager> manager;
205205

206-
const utils::trx_tracker::GlobalEnabler enabler{manager_config.enable_trx_tracker};
206+
const utils::trx_tracker::impl::GlobalEnabler enabler{manager_config.enable_trx_tracker};
207207

208208
try {
209209
experiments_scope.EnableOnly(manager_config.enabled_experiments);

core/src/utils/trx_tracker.cpp

Lines changed: 78 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
#include <userver/utils/trx_tracker.hpp>
22

3-
#include "trx_tracker_internal.hpp"
4-
3+
#include <userver/compiler/thread_local.hpp>
54
#include <userver/engine/task/local_variable.hpp>
65
#include <userver/formats/json.hpp>
76
#include <userver/logging/log.hpp>
@@ -16,9 +15,12 @@ namespace utils::trx_tracker {
1615

1716
namespace {
1817

18+
compiler::ThreadLocal local_task_counter = []() -> std::uint64_t { return 0; };
19+
1920
struct TransactionTracker final {
2021
std::uint32_t trx_count = 0;
2122
std::uint32_t disabler_count = 0;
23+
impl::TaskId task_id;
2224
};
2325

2426
struct TransactionTrackerStatisticsInternal final {
@@ -29,12 +31,12 @@ engine::TaskLocalVariable<TransactionTracker> transaction_tracker{};
2931
TransactionTrackerStatisticsInternal transaction_tracker_statistics{};
3032

3133
void CheckNoTransactions(utils::function_ref<std::string()> get_location) {
32-
if (!IsEnabled()) {
34+
if (!impl::IsEnabled()) {
3335
return;
3436
}
3537

3638
auto* tracker = transaction_tracker.GetOptional();
37-
if (tracker && !tracker->disabler_count && tracker->trx_count) {
39+
if (tracker && !tracker->disabler_count && tracker->trx_count > 0) {
3840
const std::string location = get_location();
3941
logging::LogExtra log_extra;
4042
log_extra.Extend("location", location);
@@ -49,19 +51,84 @@ void CheckNoTransactions(utils::function_ref<std::string()> get_location) {
4951
}
5052
}
5153

52-
} // namespace
53-
54-
void StartTransaction() {
55-
if (IsEnabled()) {
54+
std::optional<impl::TaskId> StartTransaction() {
55+
if (impl::IsEnabled()) {
5656
++transaction_tracker->trx_count;
57+
return transaction_tracker->task_id;
5758
}
59+
return std::nullopt;
5860
}
5961

60-
void EndTransaction() noexcept {
61-
if (IsEnabled()) {
62-
UASSERT(transaction_tracker.GetOptional() != nullptr);
62+
bool EndTransaction(const impl::TaskId& task_id) noexcept {
63+
if (impl::IsEnabled()) {
64+
if (transaction_tracker.GetOptional() == nullptr || transaction_tracker->task_id != task_id) {
65+
// EndTransaction is called in different task than StartTransaction
66+
return false;
67+
}
6368
--transaction_tracker->trx_count;
69+
return true;
6470
}
71+
return true;
72+
}
73+
74+
bool trx_tracker_enabled = false;
75+
bool trx_tracker_enabler_exists = false;
76+
77+
} // namespace
78+
79+
namespace impl {
80+
81+
GlobalEnabler::GlobalEnabler(bool enable) {
82+
UASSERT(!trx_tracker_enabler_exists);
83+
trx_tracker_enabled = enable;
84+
trx_tracker_enabler_exists = true;
85+
}
86+
87+
GlobalEnabler::~GlobalEnabler() {
88+
trx_tracker_enabled = false;
89+
trx_tracker_enabler_exists = false;
90+
}
91+
92+
bool IsEnabled() noexcept { return trx_tracker_enabled; }
93+
94+
TaskId::TaskId() : created_thread_id_{std::this_thread::get_id()} {
95+
auto counter_scope{local_task_counter.Use()};
96+
thread_local_counter_ = (*counter_scope)++;
97+
}
98+
99+
bool TaskId::operator==(const TaskId& other) const {
100+
return created_thread_id_ == other.created_thread_id_ && thread_local_counter_ == other.thread_local_counter_;
101+
}
102+
103+
bool TaskId::operator!=(const TaskId& other) const { return !(*this == other); }
104+
105+
} // namespace impl
106+
107+
TransactionLock::~TransactionLock() { Unlock(); }
108+
109+
void TransactionLock::Lock() noexcept {
110+
if (!task_id_.has_value()) {
111+
task_id_ = StartTransaction();
112+
}
113+
}
114+
115+
void TransactionLock::Unlock() noexcept {
116+
if (task_id_.has_value() && EndTransaction(task_id_.value())) {
117+
task_id_ = std::nullopt;
118+
}
119+
}
120+
121+
TransactionLock::TransactionLock(TransactionLock&& other) noexcept
122+
: task_id_(std::exchange(other.task_id_, std::nullopt)) {}
123+
124+
TransactionLock& TransactionLock::operator=(TransactionLock&& other) noexcept {
125+
if (&other == this) {
126+
return *this;
127+
}
128+
129+
Unlock();
130+
task_id_ = std::exchange(other.task_id_, std::nullopt);
131+
return *this;
65132
}
66133

67134
void CheckNoTransactions(utils::impl::SourceLocation location) {

core/src/utils/trx_tracker_internal.cpp

Lines changed: 0 additions & 21 deletions
This file was deleted.

core/src/utils/trx_tracker_internal.hpp

Lines changed: 0 additions & 20 deletions
This file was deleted.

0 commit comments

Comments
 (0)