Hi Amit, Ashutosh,
On Fri, 12 Sept 2025 at 17:28, Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
>
> Hi Amit,
>
> On Fri, Sep 12, 2025 at 4:24 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Fri, Sep 12, 2025 at 1:07 PM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> > >
> > > On Mon, Sep 8, 2025 at 9:52 AM Ashutosh Sharma <ashu.coek88@gmail.com> wrote:
> > > >
> > > > I think we can do that, since sync_skip_reason appears to be a
> > > > descriptive metadata rather than statistical data like
> > > > slot_sync_skip_count and last_slot_sync_skip. However, it's also true
> > > > that all three pieces of data are transient by nature - they will just
> > > > be present in the runtime.
> > > >
> > >
> > > After spending some more time on this, I found that maintaining
> > > sync_skip_reason in pg_replication_slots would make the code changes a
> > > bit messy and harder to maintain.
> > >
> >
> > What exactly is your worry? It seems more logical to have
> > sync_skip_reason in pg_replication_slots and other two in
> > pg_stat_replication_slots as the latter is purely a stats view and the
> > sync_skip_count/last_sync_skip suits there better.
> >
>
> The code changes for adding the skip reason to pg_replication_slots
> feel a bit hacky compared to the approach for incorporating all three
> pieces of information in pg_stat_replication_slots. I thought many
> might prefer simplicity over hackiness, which is why having everything
> in pg_stat_replication_slots could be more acceptable. That said, we
> could maybe prepare a POC patch with this approach as well, compare
> the two, and then decide which path to take.
>
Here are my thought on this:
I believe that if we decide to keep skip_reason in
pg_stat_replication_slot, it should mean "reason of last slot sync
skip" as I think it would make more sense in the sense of statistics.
And if we decide to keep skip_reason in pg_replication_slot, it would
be more appropriate to keep the latest slot data ( It should display
skip_reason only if the current slot sync cycle is skipped).
This is my observation based on the behaviour of current columns in
these views. Thoughts?
I have also attached POC patches for both approaches as per discussion above.
v2_approach1 : It adds all columns 'slot_sync_skip_reason',
'slot_sync_skip_count' and 'last_slot_sync_skip' to
pg_stat_replication_slots
v2_approach2 : It adds column 'slot_sync_skip_reason' to
pg_replication_slots and columns 'slot_sync_skip_count' and
'last_slot_sync_skip' to pg_stat_replication_slots
Thanks,
Shlok Kyal
