From: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
---|---|
To: | 'Masahiko Sawada' <sawada(dot)mshk(at)gmail(dot)com> |
Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | RE: POC: enable logical decoding when wal_level = 'replica' without a server restart |
Date: | 2025-10-22 08:30:41 |
Message-ID: | OSCPR01MB149661D3BD2B2308CA8794F61F5F3A@OSCPR01MB14966.jpnprd01.prod.outlook.com |
Views: | Whole Thread | Raw Message | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Dear Sawada-san,
Thanks for updating the patch. Here are comments for v19.
I spent sometime to find race condition but nothing found. Below were for test code.
```
# Copyright (c) 2024-2025, PostgreSQL Global Development Group
```
2024 can be removed.
```
# Create a temporary logical slot and exits without releasing it explicitly.
# This enables logical decoding but skips disabling it and delegates to the
# checkpointer.
$primary->safe_psql('postgres',
qq[select pg_create_logical_replication_slot('test_tmp_slot', 'test_decoding', true)]
);
# Wait for the checkpointer to disable logical decoding.
wait_for_logical_decoding_deactivation($primary);
```
Can we ensure the log has the line "requested disabling logical decoding"?
The test looks like we spend checkpoint_timeout seconds here, but actually
backend kicks the checkpointer.
```
$primary->adjust_conf('postgresql.conf', 'wal_level', 'minimal');
```
To clarify, adjust_conf() is used for wal_level and max_wal_senders. Is there
a reason you choose?
```
my $res = run_log(
[
'pg_ctl',
'--pgdata' => $primary->data_dir,
'--log' => $primary->logfile,
'start',
]);
ok(!$res,
"cannot server with wal_level='minimal' as there is in-use logical slot");
```
We can use command_fails() to just confirm the command fails. $res is not needed
if we use it.
```
ok( $logfile =~
qr/logical replication slot "test_slot" exists, but "wal_level" < "replica"/,
'logical slots requires logical decoding enabled at server startup');
```
Per [1], we should use like() instead of ok(). Same thing can be said for other
ok().
```
```
$standby1->set_standby_mode();
```
This seems not needed because init_from_backup() with has_streaming does the same
thing. Same thing can be said for other standbys.
```
# Initialize standby4 node and starts it with wal_level = 'logical'.
my $standby4 = PostgreSQL::Test::Cluster->new('standby4');
$standby4->init_from_backup($primary, 'my_backup', has_streaming => 1);
$standby4->set_standby_mode();
```
The comment looks not correct. Same thing can be said for standby5.
```
# Check if we cannot use the invalidated slot on the standby.
($result, $stdout, $stderr) = $standby4->psql('postgres',
qq[select pg_logical_slot_get_changes('standby4_slot', null, null)]);
ok( $stderr =~
/ERROR: can no longer access replication slot "standby4_slot"/,
"cannot use invalidated logical slot");
```
Not sure it should be tested in the file becasue 035 seems to verify that invalidated
slots are no longer usable.
```
# Test the abort process of logical decoding activation. We drop the the primary's
# slot to decrease its effective_wal_level to 'replica'.
```
s/drop the the primary's/drop the primary's/.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
From | Date | Subject | |
---|---|---|---|
Next Message | Jelte Fennema-Nio | 2025-10-22 08:41:49 | Re: CI: Add task that runs pgindent |
Previous Message | jian he | 2025-10-22 08:08:49 | Re: [PATCH] Add pg_get_trigger_ddl() to retrieve the CREATE TRIGGER statement |