Re: POC: enable logical decoding when wal_level = 'replica' without a server restart - Mailing list pgsql-hackers

On Tue, Oct 21, 2025 at 3:52 PM Peter Smith <smithpb2250@gmail.com> wrote:
>
> Hi Sawada-San.
>
> I reviewed the test code of patch v19; below are some minor comments.
>
> ======
>
> 1.
> +# Initialize the primary server with wal_level = 'replica'.
> +my $primary = PostgreSQL::Test::Cluster->new('primary');
> +$primary->init(allows_streaming => 1);
> +$primary->append_conf('postgresql.conf', "log_min_messages = debug1");
> +$primary->start();
>
> Would it be nicer to set wal_level  explicitly? Otherwise, maybe the
> comment should say that 'replica' is the default.
>
> ~~~
>
> 2.
> +# Check both initial wal_level and effective_wal_level values.
> +test_wal_level($primary, "replica|replica",
> + "wal_level and effective_wal_level starts with the same value 'replica'");
> +
>
> /starts/start/
>
> ~~~
>
> 3.
> +# 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);
> +
>
> 3a.
> typo? /and exits/but exit/
>
> ~
>
> 3b.
> I had expected to see the wal_level|effective_wal_level values getting
> checked before/after that deactivation.
>
> ~~~
>
> 4.
> +# Promote the standby1 node that doesn't have any logical slot. So
> +# effective_wal_level should be decreased to 'replica' at promotion.
> +$standby1->promote;
> +test_wal_level($standby1, "replica|replica",
> + "effective_wal_level got decrased to 'replica' during promotion");
> +$standby1->stop;
> +
>
> typo: /decrased/decreased/
>
> ~~~
>
> 5.
> +# Initialize standby3 node and starts it with wal_level = 'logical'.
> +my $standby3 = PostgreSQL::Test::Cluster->new('standby3');
> +$standby3->init_from_backup($primary, 'my_backup', has_streaming => 1);
> +$standby3->set_standby_mode();
> +$standby3->append_conf('postgresql.conf', qq[wal_level = 'logical']);
> +$standby3->start();
> +$standby3->backup('my_backup3');
> +
>
> /starts/start/
>
> ~~~
>
> 6.
> +# Drop the primary's last logical slot, decreasing effective_wal_level to
> +# replica on all nodes.
> +$primary->safe_psql('postgres',
> + qq[select pg_drop_replication_slot('test_slot')]);
> +wait_for_logical_decoding_deactivation($primary);
> +
> +$primary->wait_for_replay_catchup($standby3);
> +$standby3->wait_for_replay_catchup($cascade, $primary);
> +
> +test_wal_level($primary, "replica|replica",
> + "effective_wal_level got decreased to 'replica' on primary");
> +test_wal_level($standby3, "logical|replica",
> + "effective_wal_level got decreased to 'replica' on standby");
> +test_wal_level($cascade, "replica|replica",
> + "effective_wal_level got decreased to 'replica' on cascaded standby");
> +
>
> The "logical|replica" combination looks a bit weird; maybe it needs
> more explanation in the comment.
>
> ~~~
>
> 7.
> +# 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();
> +$standby4->start;
> +
>
> 7a.
> /starts/start/
>
> ~
>
> 7b.
> I got confused here. Isn't the $primary still wal_level=replica? But
> this comment says starting wal_level = 'logical'. Is it correct?
>
> ~~~
>
> 8.
> +# Drop the logical slot from the primary, decreasing effective_wal_level to
> +# 'replica' on the primary, which leads to invalidate the logical
> slot on the standby
> +# due to 'wal_level_insufficient'.
> +$primary->safe_psql('postgres',
> + qq[select pg_drop_replication_slot('test_slot')]);
> +wait_for_logical_decoding_deactivation($primary);
> +test_wal_level($primary, "replica|replica",
> + "effective_wal_level got decreased to 'replica' on the primary to
> invalidate standby's slots"
> +);
> +$standby4->poll_query_until(
> + 'postgres', qq[
> +select invalidation_reason = 'wal_level_insufficient' from
> pg_replication_slots where slot_name = 'standby4_slot'
> +     ]);
> +
>
> 8a.
> /invalidate/invalidating/
>
> ~
>
> 8b.
> Should this part also be showing the expected
> wal_level|effective_wal_level on the $standby4?
>
> ~~~
>
> 9.
> +# 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");
> +
>
> /Check if/Check that/
>
> ~~~
>
> 10.
> + # Initialize standby5 and starts it with wal_level = 'logical'.
> + my $standby5 = PostgreSQL::Test::Cluster->new('standby5');
> + $standby5->init_from_backup($primary, 'my_backup', has_streaming => 1);
> + $standby5->set_standby_mode();
> + $standby5->start;
> +
>
> /starts it/start it/
>
> ~~~
>
> 11.
> + # Start the logical decoding activation process upon creating the logical
> + # slot, but it will wait due to the injection point.
> + $psql_create_slot_1->query_until(
> + qr/create_slot_cancelled/,
> + q(\echo create_slot_cancelled
> +select injection_points_set_local();
> +select injection_points_attach('logical-decoding-activation', 'wait');
> +select pg_create_logical_replication_slot('slot_cancelled', 'pgoutput');
> +\q
> +));
> +
>
> I found everywhere in the PG source uses the American spelling
> "canceled" (one L), so this patch should too in all places.

Thanks again. I've addressed all comments in the next version patch.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com