On Fri, Apr 29, 2022 at 4:02 PM Alvaro Herrera <alvherre@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.