Skip to content

Commit cb1c273

Browse files
committed
feat postgresql: add omit-statement-timeout-for-autocommit for transaction pooler mode
commit_hash:ab8215021bd4f49da2b0c30a6fa26b1617429ee8
1 parent e58cf61 commit cb1c273

7 files changed

Lines changed: 73 additions & 3 deletions

File tree

postgresql/dynamic_configs/POSTGRES_CONNECTION_SETTINGS.yaml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,4 +73,17 @@ schema:
7373
type: boolean
7474
default: true
7575
description: Whether statement timeout is affected by deadline propagation
76+
omit-statement-timeout-for-autocommit:
77+
type: boolean
78+
default: false
79+
description: |-
80+
DANGEROUS, DO NOT ENABLE unless you fully understand the consequences.
81+
82+
Has effect only in `transaction` pooler mode. When enabled, no `SET statement_timeout` is sent
83+
before autocommit statements. This saves one network round trip per query.
84+
85+
The catch: without `statement_timeout` a stuck or slow query is no longer aborted by the framework, so it
86+
can hold the connection indefinitely. Deadline propagation implemented on top of `statement_timeout`
87+
stops working for such queries as well. A single misbehaving query can therefore exhaust the whole
88+
connection pool and take the service down
7689

postgresql/include/userver/storages/postgres/options.hpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,6 +312,11 @@ struct ConnectionSettings {
312312

313313
PoolerMode pooler_mode{PoolerMode::kSession};
314314

315+
/// DANGEROUS. Has effect only in @ref PoolerMode::kTransaction. When set, no `SET statement_timeout` is sent
316+
/// before autocommit statements, saving a round trip at the cost of losing statement_timeout enforcement (and
317+
/// deadline propagation built on top of it) for such queries
318+
bool omit_statement_timeout_for_autocommit = false;
319+
315320
bool operator==(const ConnectionSettings& rhs) const {
316321
return !RequiresConnectionReset(rhs) && recent_errors_threshold == rhs.recent_errors_threshold;
317322
}
@@ -323,7 +328,8 @@ struct ConnectionSettings {
323328
max_prepared_cache_size != rhs.max_prepared_cache_size || pipeline_mode != rhs.pipeline_mode ||
324329
max_ttl != rhs.max_ttl || discard_on_connect != rhs.discard_on_connect ||
325330
omit_describe_mode != rhs.omit_describe_mode || application_name != rhs.application_name ||
326-
pooler_mode != rhs.pooler_mode;
331+
pooler_mode != rhs.pooler_mode ||
332+
omit_statement_timeout_for_autocommit != rhs.omit_statement_timeout_for_autocommit;
327333
}
328334
};
329335

@@ -336,6 +342,7 @@ struct ConnectionSettingsDynamic final {
336342
std::optional<std::chrono::seconds> max_ttl{};
337343
std::optional<ConnectionSettings::DiscardOnConnectOptions> discard_on_connect{};
338344
std::optional<bool> deadline_propagation_enabled{};
345+
std::optional<bool> omit_statement_timeout_for_autocommit{};
339346
};
340347

341348
/// @brief PostgreSQL statements metrics options

postgresql/src/storages/postgres/component.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,10 @@ void MergeConnectionSettings(
9595
MergeField(static_settings.recent_errors_threshold, dynamic_settings.recent_errors_threshold);
9696
MergeField(static_settings.discard_on_connect, dynamic_settings.discard_on_connect);
9797
MergeField(static_settings.deadline_propagation_enabled, dynamic_settings.deadline_propagation_enabled);
98+
MergeField(
99+
static_settings.omit_statement_timeout_for_autocommit,
100+
dynamic_settings.omit_statement_timeout_for_autocommit
101+
);
98102
if (const auto max_ttl = dynamic_settings.max_ttl; max_ttl) {
99103
static_settings.max_ttl = *max_ttl;
100104
}

postgresql/src/storages/postgres/component.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,14 @@ properties:
8686
type: boolean
8787
description: whether statement timeout is affected by deadline propagation
8888
default: true
89+
omit-statement-timeout-for-autocommit:
90+
type: boolean
91+
description: |
92+
DANGEROUS. Has effect only in `transaction` pooler mode. When enabled,
93+
no SET statement_timeout is sent before autocommit statements, saving a round
94+
trip at the cost of losing statement_timeout enforcement for such queries.
95+
Keep it false
96+
default: false
8997
monitoring-dbalias:
9098
type: string
9199
description: |

postgresql/src/storages/postgres/detail/connection_impl.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -453,7 +453,8 @@ Connection::Statistics ConnectionImpl::GetStatsAndReset() {
453453
}
454454

455455
bool ConnectionImpl::ShouldWrapInAutoTransaction(const std::string_view statement) const noexcept {
456-
return IsTransactionPooler() && !IsInTransaction() && !ICaseStartsWith(statement, kStatementVacuum);
456+
return IsTransactionPooler() && !settings_.omit_statement_timeout_for_autocommit && !IsInTransaction() &&
457+
!ICaseStartsWith(statement, kStatementVacuum);
457458
}
458459

459460
ResultSet ConnectionImpl::ExecuteCommandInAutoTransaction(
@@ -513,7 +514,11 @@ ResultSet ConnectionImpl::ExecuteCommand(
513514
return ExecuteCommandInAutoTransaction(query, params, statement_cmd_ctl, deadline);
514515
}
515516

516-
SetStatementTimeout(statement_cmd_ctl);
517+
// In transaction pooler mode with omit_statement_timeout_for_autocommit enabled we deliberately skip
518+
// SET statement_timeout for autocommit statements to save a round trip (see @ref POSTGRES_CONNECTION_SETTINGS)
519+
if (!(IsTransactionPooler() && settings_.omit_statement_timeout_for_autocommit && !IsInTransaction())) {
520+
SetStatementTimeout(statement_cmd_ctl);
521+
}
517522

518523
return PreparedStatementsEnabled(statement_cmd_ctl)
519524
? ExecuteCommand(query, params, deadline, logging::Level::kInfo, true)

postgresql/src/storages/postgres/postgres_config.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,9 @@ ConnectionSettings ParseConnectionSettings(const ConfigType& config) {
9898
? ConnectionSettings::kDiscardAll
9999
: ConnectionSettings::kDiscardNone;
100100
settings.deadline_propagation_enabled = config["deadline-propagation-enabled"].template As<bool>(true);
101+
settings
102+
.omit_statement_timeout_for_autocommit = config["omit-statement-timeout-for-autocommit"].template As<bool>(false
103+
);
101104
settings.application_name =
102105
config["application_name"].template As<std::string>(USERVER_NAMESPACE::utils::GetUserverIdentifier());
103106

@@ -150,6 +153,12 @@ ConnectionSettingsDynamic Parse(const formats::json::Value& config, formats::par
150153
if (const auto dp_enabled = config["deadline-propagation-enabled"].As<std::optional<bool>>(); dp_enabled) {
151154
settings.deadline_propagation_enabled = *dp_enabled;
152155
}
156+
if (const auto omit_statement_timeout_for_autocommit =
157+
config["omit-statement-timeout-for-autocommit"].As<std::optional<bool>>();
158+
omit_statement_timeout_for_autocommit)
159+
{
160+
settings.omit_statement_timeout_for_autocommit = *omit_statement_timeout_for_autocommit;
161+
}
153162

154163
return settings;
155164
}

postgresql/src/storages/postgres/tests/connection_transaction_mode_pgtest.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,30 @@ UTEST_F(PostgreTransactionModeConnection, DuplicatePreparedStatementDirtyConnect
252252
EXPECT_FALSE(conn->IsBroken());
253253
}
254254

255+
UTEST_F(PostgreTransactionModeConnection, OmitStatementTimeoutForAutocommitSkipsAutoTransactionAndStatementTimeout) {
256+
pg::detail::ConnectionPtr conn{nullptr};
257+
auto settings = MakeTransactionPoolerSettings();
258+
settings.omit_statement_timeout_for_autocommit = true;
259+
UEXPECT_NO_THROW(conn = MakeConnection(GetDsnFromEnv(), GetTaskProcessor(), settings));
260+
ASSERT_TRUE(conn);
261+
262+
const DefaultCommandControlScope scope{kTransactionPoolerCmdCtl};
263+
264+
UEXPECT_NO_THROW(ZeroBackendStatementTimeout(conn));
265+
GetLogCapture().Clear();
266+
267+
UEXPECT_NO_THROW(conn->Execute("SELECT pg_sleep(0.3)"));
268+
269+
const auto set_config_logs = GetLogCapture().Filter([&](const utest::LogRecord& log) {
270+
return log.GetTagOptional(tracing::kDatabaseStatementName) == kSetConfigStatementName;
271+
});
272+
EXPECT_THAT(set_config_logs, ::testing::IsEmpty());
273+
UEXPECT_NO_THROW(ExpectBackendStatementTimeout(conn, "0"));
274+
275+
UEXPECT_NO_THROW(conn->CancelAndCleanup(utest::kMaxTestWaitTime));
276+
EXPECT_FALSE(conn->IsBroken());
277+
}
278+
255279
} // namespace
256280

257281
USERVER_NAMESPACE_END

0 commit comments

Comments
 (0)