On Wed, Nov 9, 2022 at 2:37 AM Amit Kapila <amit.kapil...@gmail.com> wrote:
> On Fri, Nov 4, 2022 at 1:40 PM sirisha chamarthi > <sirichamarth...@gmail.com> wrote: > > > Is the intent of setting restart_lsn to InvalidXLogRecPtr was to > disallow reviving the slot? > > > > I think the intent is to compute the correct value for > replicationSlotMinLSN as we use restart_lsn for it and using the > invalidated slot's restart_lsn value for it doesn't make sense. > Correct. If a slot is invalidated (lost), then shouldn't we ignore the slot from computing the catalog_xmin? I don't see it being set to InvalidTransactionId in ReplicationSlotsComputeRequiredXmin. Attached a small patch to address this and the output after the patch is as shown below. postgres=# select * from pg_replication_slots; slot_name | plugin | slot_type | datoid | database | temporary | active | active_pid | xmin | catalog_xmin | restart_lsn | confirmed_flush_lsn | wal_status | safe_wal_size | two_phase -----------+---------------+-----------+--------+----------+-----------+--------+------------+------+--------------+-------------+---------------------+------------+---------------+----------- s2 | test_decoding | logical | 5 | postgres | f | f | | | 771 | 0/30466368 | 0/304663A0 | reserved | 28903824 | f (1 row) postgres=# create table t2(c int, c1 char(100)); CREATE TABLE postgres=# drop table t2; DROP TABLE postgres=# vacuum pg_class; VACUUM postgres=# select n_dead_tup from pg_stat_all_tables where relname = 'pg_class'; n_dead_tup ------------ 2 (1 row) postgres=# select * from pg_stat_replication; pid | usesysid | usename | application_name | client_addr | client_hostname | client_port | backend_start | backend_xmin | state | sent_lsn | write_lsn | flush_lsn | replay_lsn | write_lag | flush_lag | replay_lag | sync_pri ority | sync_state | reply_time -----+----------+---------+------------------+-------------+-----------------+-------------+---------------+--------------+-------+----------+-----------+-----------+------------+-----------+-----------+------------+--------- ------+------------+------------ (0 rows) postgres=# insert into t1 select * from t1; INSERT 0 2097152 postgres=# checkpoint; CHECKPOINT postgres=# select * from pg_replication_slots; slot_name | plugin | slot_type | datoid | database | temporary | active | active_pid | xmin | catalog_xmin | restart_lsn | confirmed_flush_lsn | wal_status | safe_wal_size | two_phase -----------+---------------+-----------+--------+----------+-----------+--------+------------+------+--------------+-------------+---------------------+------------+---------------+----------- s2 | test_decoding | logical | 5 | postgres | f | f | | | 771 | | 0/304663A0 | lost | | f (1 row) postgres=# vacuum pg_class; VACUUM postgres=# select n_dead_tup from pg_stat_all_tables where relname = 'pg_class'; n_dead_tup ------------ 0 (1 row) > > -- > With Regards, > Amit Kapila. >