Hi Amit, Ashutosh, On Fri, 12 Sept 2025 at 17:28, Ashutosh Sharma <[email protected]> wrote: > > Hi Amit, > > On Fri, Sep 12, 2025 at 4:24 PM Amit Kapila <[email protected]> wrote: > > > > On Fri, Sep 12, 2025 at 1:07 PM Ashutosh Sharma <[email protected]> > > wrote: > > > > > > On Mon, Sep 8, 2025 at 9:52 AM Ashutosh Sharma <[email protected]> > > > 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
v2_approach1-0001-Add-stats-related-to-slot-sync-skip.patch
Description: Binary data
v2_approach2-0001-Add-stats-related-to-slot-sync-skip.patch
Description: Binary data
