On Fri, Apr 29, 2022 at 4:02 PM Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote: > > If I read the code right, this patch emits logs when > pg_logical_slot_get_changes and pg_replication_slot_advance SQL > functions are called. Is this desirable/useful, for the use case you > stated at start of thread? I think it is most likely pointless. If you > get rid of those, then the only acquisitions that would log messages are > those in StartReplication and StartLogicalReplication. So I wonder if > it would be better to leave the API of ReplicationSlotAcquire() alone, > and instead make StartReplication and StartLogicalReplication > responsible for those messages. > > I didn't look at the release-side messages you're adding, but I suppose > it should be symmetrical.
Adding the messages right after ReplicationSlotAcquire in the StartReplication and StartLogicalReplication looks okay, but, if we just add "released replication slot \"%s\" for logical/physical replication". in StartReplication and StartLogicalReplication right after ReplicationSlotRelease, how about ReplicationSlotRelease in WalSndErrorCleanup and ReplicationSlotShmemExit? The whole idea is to get to know when and how much time a slot was inactive/unused when log_replication_commands is set to true. We can think of adding a timestamp column to on-disk replication slot data but that will be too much. Regards, Bharath Rupireddy.
