Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit80b727e

Browse files
committed
Use the same cmd_context throughout a walsender's lifetime.
exec_replication_command created a cmd_context to work in andthen deleted it on exit. This is pretty dangerous becausesome replication commands start/finish transactions. In thewake of commit1afe31f, that could lead to re-selecting aCurrentMemoryContext that's already been deleted, leading tohilarity such as a memory context that is its own parent.To fix, let's make the cmd_context persist acrossexec_replication_command calls; instead of deleting it, we'll justreset it each time. In this way it retains the same identity andthere's no problem if transaction abort restores it as the workingcontext. It probably even saves a few microseconds to do this.This fix also ensures that exec_replication_command returns to thecaller (PostgresMain) with the same context active that had beenwhen it was called (probably MessageContext). The previouscoding could get that wrong too.Reported-by: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>Author: Anthonin Bonnefoy <anthonin.bonnefoy@datadoghq.com>Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>Discussion:https://postgr.es/m/CAO6_XqoJA7-_G6t7Uqe5nWF3nj+QBGn4F6Ptp=rUGDr0zo+KvA@mail.gmail.com
1 parent5ec8b01 commit80b727e

File tree

2 files changed

+51
-9
lines changed

2 files changed

+51
-9
lines changed

‎src/backend/replication/walsender.c

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1973,8 +1973,10 @@ exec_replication_command(const char *cmd_string)
19731973
intparse_rc;
19741974
Node*cmd_node;
19751975
constchar*cmdtag;
1976-
MemoryContextcmd_context;
1977-
MemoryContextold_context;
1976+
MemoryContextold_context=CurrentMemoryContext;
1977+
1978+
/* We save and re-use the cmd_context across calls */
1979+
staticMemoryContextcmd_context=NULL;
19781980

19791981
/*
19801982
* If WAL sender has been told that shutdown is getting close, switch its
@@ -2003,11 +2005,30 @@ exec_replication_command(const char *cmd_string)
20032005

20042006
/*
20052007
* Prepare to parse and execute the command.
2008+
*
2009+
* Because replication command execution can involve beginning or ending
2010+
* transactions, we need a working context that will survive that, so we
2011+
* make it a child of TopMemoryContext. That in turn creates a hazard of
2012+
* long-lived memory leaks if we lose track of the working context. We
2013+
* deal with that by creating it only once per walsender, and resetting it
2014+
* for each new command. (Normally this reset is a no-op, but if the
2015+
* prior exec_replication_command call failed with an error, it won't be.)
2016+
*
2017+
* This is subtler than it looks. The transactions we manage can extend
2018+
* across replication commands, indeed SnapBuildClearExportedSnapshot
2019+
* might have just ended one. Because transaction exit will revert to the
2020+
* memory context that was current at transaction start, we need to be
2021+
* sure that that context is still valid. That motivates re-using the
2022+
* same cmd_context rather than making a new one each time.
20062023
*/
2007-
cmd_context=AllocSetContextCreate(CurrentMemoryContext,
2008-
"Replication command context",
2009-
ALLOCSET_DEFAULT_SIZES);
2010-
old_context=MemoryContextSwitchTo(cmd_context);
2024+
if (cmd_context==NULL)
2025+
cmd_context=AllocSetContextCreate(TopMemoryContext,
2026+
"Replication command context",
2027+
ALLOCSET_DEFAULT_SIZES);
2028+
else
2029+
MemoryContextReset(cmd_context);
2030+
2031+
MemoryContextSwitchTo(cmd_context);
20112032

20122033
replication_scanner_init(cmd_string,&scanner);
20132034

@@ -2020,7 +2041,7 @@ exec_replication_command(const char *cmd_string)
20202041
replication_scanner_finish(scanner);
20212042

20222043
MemoryContextSwitchTo(old_context);
2023-
MemoryContextDelete(cmd_context);
2044+
MemoryContextReset(cmd_context);
20242045

20252046
/* XXX this is a pretty random place to make this check */
20262047
if (MyDatabaseId==InvalidOid)
@@ -2180,9 +2201,12 @@ exec_replication_command(const char *cmd_string)
21802201
cmd_node->type);
21812202
}
21822203

2183-
/* done */
2204+
/*
2205+
* Done. Revert to caller's memory context, and clean out the cmd_context
2206+
* to recover memory right away.
2207+
*/
21842208
MemoryContextSwitchTo(old_context);
2185-
MemoryContextDelete(cmd_context);
2209+
MemoryContextReset(cmd_context);
21862210

21872211
/*
21882212
* We need not update ps display or pg_stat_activity, because PostgresMain

‎src/test/subscription/t/100_bugs.pl

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,24 @@
477477
is($result,qq(2|f
478478
3|t),'check replicated update on subscriber');
479479

480+
# Test create and immediate drop of replication slot via replication commands
481+
# (this exposed a memory-management bug in v18)
482+
my$publisher_host =$node_publisher->host;
483+
my$publisher_port =$node_publisher->port;
484+
my$connstr_db =
485+
"host=$publisher_host port=$publisher_port replication=database dbname=postgres";
486+
487+
is($node_publisher->psql(
488+
'postgres',
489+
qq[
490+
CREATE_REPLICATION_SLOT test_slot LOGICAL pgoutput (SNAPSHOT export);
491+
DROP_REPLICATION_SLOT test_slot;
492+
],
493+
timeout=>$PostgreSQL::Test::Utils::timeout_default,
494+
extra_params=> ['-d',$connstr_db ]),
495+
0,
496+
'create and immediate drop of replication slot');
497+
480498
$node_publisher->stop('fast');
481499
$node_subscriber->stop('fast');
482500

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp