Thread

Commits

  1. Add comments explaining overflow entries in the replication lag tracker.

  2. Fix stalled lag columns in pg_stat_replication when replay LSN stops advancing.

  1. Fix lag columns in pg_stat_replication not advancing when replay LSN stalls

    Fujii Masao <masao.fujii@gmail.com> — 2025-10-17T03:56:55Z

    Hi,
    
    While testing, I noticed that write_lag and flush_lag in pg_stat_replication
    initially advanced but eventually stopped updating. This happened when
    I started pg_receivewal, ran pgbench, and periodically monitored
    pg_stat_replication.
    
    My analysis shows that this issue occurs when any of the write, flush,
    or replay LSNs in the standby’s feedback message stop updating for some time.
    In the case of pg_receivewal, the replay LSN is always invalid (never updated),
    which triggers the problem. Similarly, in regular streaming replication,
    if the replay LSN remains unchanged for a long time—such as during
    a recovery conflict—the lag values for both write and flush can stop advancing.
    
    The root cause seems to be that when any of the LSNs stop updating,
    the lag tracker's cyclic buffer becomes full (the write head reaches
    the slowest read head). In this situation, LagTrackerWrite() and
    LagTrackerRead() didn't handle the full-buffer condition properly.
    For instance, if the replay LSN stalls, the buffer fills up and the read heads
    for "write" and "flush" end up at the same position as the write head.
    This causes LagTrackerRead() to return -1 for both, preventing write_lag
    and flush_lag from advancing.
    
    The attached patch fixes the problem by treating the slowest read entry
    (the one causing the buffer to fill up) as a separate overflow entry,
    allowing the lag tracker to continue operating correctly.
    
    Thoughts?
    
    Regards,
    
    -- 
    Fujii Masao
    
  2. Re: Fix lag columns in pg_stat_replication not advancing when replay LSN stalls

    Chao Li <li.evan.chao@gmail.com> — 2025-10-17T08:11:20Z

    
    > On Oct 17, 2025, at 11:56, Fujii Masao <masao.fujii@gmail.com> wrote:
    > 
    > Hi,
    > 
    > While testing, I noticed that write_lag and flush_lag in pg_stat_replication
    > initially advanced but eventually stopped updating. This happened when
    > I started pg_receivewal, ran pgbench, and periodically monitored
    > pg_stat_replication.
    > 
    > My analysis shows that this issue occurs when any of the write, flush,
    > or replay LSNs in the standby’s feedback message stop updating for some time.
    > In the case of pg_receivewal, the replay LSN is always invalid (never updated),
    > which triggers the problem. Similarly, in regular streaming replication,
    > if the replay LSN remains unchanged for a long time—such as during
    > a recovery conflict—the lag values for both write and flush can stop advancing.
    > 
    > The root cause seems to be that when any of the LSNs stop updating,
    > the lag tracker's cyclic buffer becomes full (the write head reaches
    > the slowest read head). In this situation, LagTrackerWrite() and
    > LagTrackerRead() didn't handle the full-buffer condition properly.
    > For instance, if the replay LSN stalls, the buffer fills up and the read heads
    > for "write" and "flush" end up at the same position as the write head.
    > This causes LagTrackerRead() to return -1 for both, preventing write_lag
    > and flush_lag from advancing.
    > 
    > The attached patch fixes the problem by treating the slowest read entry
    > (the one causing the buffer to fill up) as a separate overflow entry,
    > allowing the lag tracker to continue operating correctly.
    > 
    > -- 
    > Fujii Masao
    > <v1-0001-Fix-lag-columns-in-pg_stat_replication-not-advanc.patch>
    
    It took me some time to understand this fix. My most confusing was that once overwrite happens, how a reader head to catch up again? Finally I figured it out:
    
    ```
    +		lag_tracker->read_heads[head] =
    +			(lag_tracker->write_head + 1) % LAG_TRACKER_BUFFER_SIZE;
    ```
    
    "(lag_tracker->write_head + 1) % LAG_TRACKER_BUFFER_SIZE” points to the oldest LSN in the ring, from where an overflowed reader head starts to catch up.
    
    I have no comment on the code change. Nice patch!
    
    All I wonder is if we can add a TAP test for this fix?
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
    
    
    
  3. Re: Fix lag columns in pg_stat_replication not advancing when replay LSN stalls

    Fujii Masao <masao.fujii@gmail.com> — 2025-10-17T14:28:17Z

    On Fri, Oct 17, 2025 at 5:11 PM Chao Li <li.evan.chao@gmail.com> wrote:
    > It took me some time to understand this fix. My most confusing was that once overwrite happens, how a reader head to catch up again? Finally I figured it out:
    >
    > ```
    > +               lag_tracker->read_heads[head] =
    > +                       (lag_tracker->write_head + 1) % LAG_TRACKER_BUFFER_SIZE;
    > ```
    >
    > "(lag_tracker->write_head + 1) % LAG_TRACKER_BUFFER_SIZE” points to the oldest LSN in the ring, from where an overflowed reader head starts to catch up.
    >
    > I have no comment on the code change. Nice patch!
    
    Thanks for the review!
    
    I've updated the source comment to make the code easier to understand.
    The updated patch is attached.
    
    
    > All I wonder is if we can add a TAP test for this fix?
    
    I think it would be good to add a test for this fix, but reproducing
    the condition
    where the buffer fills up and the slowest read entry overflows takes a time.
    Because of that, I'm not sure adding such a potentially slow test is a
    good idea.
    
    Regards,
    
    -- 
    Fujii Masao
    
  4. Re: Fix lag columns in pg_stat_replication not advancing when replay LSN stalls

    Chao Li <li.evan.chao@gmail.com> — 2025-10-18T00:16:01Z

    
    > On Oct 17, 2025, at 22:28, Fujii Masao <masao.fujii@gmail.com> wrote:
    > 
    > On Fri, Oct 17, 2025 at 5:11 PM Chao Li <li.evan.chao@gmail.com> wrote:
    >> It took me some time to understand this fix. My most confusing was that once overwrite happens, how a reader head to catch up again? Finally I figured it out:
    >> 
    >> ```
    >> +               lag_tracker->read_heads[head] =
    >> +                       (lag_tracker->write_head + 1) % LAG_TRACKER_BUFFER_SIZE;
    >> ```
    >> 
    >> "(lag_tracker->write_head + 1) % LAG_TRACKER_BUFFER_SIZE” points to the oldest LSN in the ring, from where an overflowed reader head starts to catch up.
    >> 
    >> I have no comment on the code change. Nice patch!
    > 
    > Thanks for the review!
    > 
    > I've updated the source comment to make the code easier to understand.
    > The updated patch is attached.
    > 
    > <v2-0001-Fix-stalled-lag-columns-in-pg_stat_replication-wh.patch>
    
    Thanks for adding the comment.
    
    I think I put all concentration on the big picture yesterday, so I ignored this implementation detail:
    
    ```
    +		if (lag_tracker->overflowed[head].lsn > lsn)
    +			return now - lag_tracker->overflowed[head].time;
    ```
    
    Should this “>” be “>=“?
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
    
    
    
  5. Re: Fix lag columns in pg_stat_replication not advancing when replay LSN stalls

    Shinya Kato <shinya11.kato@gmail.com> — 2025-10-18T17:04:18Z

    Hi,
    
    On Fri, Oct 17, 2025 at 12:57 PM Fujii Masao <masao.fujii@gmail.com> wrote:
    >
    > Hi,
    >
    > While testing, I noticed that write_lag and flush_lag in pg_stat_replication
    > initially advanced but eventually stopped updating. This happened when
    > I started pg_receivewal, ran pgbench, and periodically monitored
    > pg_stat_replication.
    
    Nice catch! I reproduced the same issue.
    
    >
    > My analysis shows that this issue occurs when any of the write, flush,
    > or replay LSNs in the standby’s feedback message stop updating for some time.
    > In the case of pg_receivewal, the replay LSN is always invalid (never updated),
    > which triggers the problem. Similarly, in regular streaming replication,
    > if the replay LSN remains unchanged for a long time—such as during
    > a recovery conflict—the lag values for both write and flush can stop advancing.
    >
    > The root cause seems to be that when any of the LSNs stop updating,
    > the lag tracker's cyclic buffer becomes full (the write head reaches
    > the slowest read head). In this situation, LagTrackerWrite() and
    > LagTrackerRead() didn't handle the full-buffer condition properly.
    > For instance, if the replay LSN stalls, the buffer fills up and the read heads
    > for "write" and "flush" end up at the same position as the write head.
    > This causes LagTrackerRead() to return -1 for both, preventing write_lag
    > and flush_lag from advancing.
    >
    > The attached patch fixes the problem by treating the slowest read entry
    > (the one causing the buffer to fill up) as a separate overflow entry,
    > allowing the lag tracker to continue operating correctly.
    
    Thank you for the patch. I have one comment.
    
    +       if (lag_tracker->overflowed[head].lsn > lsn)
    +           return now - lag_tracker->overflowed[head].time;
    
    Could this return a negative value if the clock somehow went
    backwards? The original code returns -1 in this case, so I'm curious
    about this.
    
    -- 
    Best regards,
    Shinya Kato
    NTT OSS Center
    
    
    
    
  6. Re: Fix lag columns in pg_stat_replication not advancing when replay LSN stalls

    Fujii Masao <masao.fujii@gmail.com> — 2025-10-20T01:12:07Z

    On Sat, Oct 18, 2025 at 9:16 AM Chao Li <li.evan.chao@gmail.com> wrote:
    > I think I put all concentration on the big picture yesterday, so I ignored this implementation detail:
    >
    > ```
    > +               if (lag_tracker->overflowed[head].lsn > lsn)
    > +                       return now - lag_tracker->overflowed[head].time;
    > ```
    >
    > Should this “>” be “>=“?
    
    I think either ">" or ">=" would work. However, the following loop
    in LagTrackerRead() advances the read head when the LSN
    in the current buffer entry is equal to the given LSN, so I followed
    the same logic for the overflow entry and used ">" there.
    
    Regards,
    
    -- 
    Fujii Masao
    
    
    
    
  7. Re: Fix lag columns in pg_stat_replication not advancing when replay LSN stalls

    Fujii Masao <masao.fujii@gmail.com> — 2025-10-20T01:17:21Z

    On Sun, Oct 19, 2025 at 2:04 AM Shinya Kato <shinya11.kato@gmail.com> wrote:
    > Thank you for the patch. I have one comment.
    >
    > +       if (lag_tracker->overflowed[head].lsn > lsn)
    > +           return now - lag_tracker->overflowed[head].time;
    >
    > Could this return a negative value if the clock somehow went
    > backwards? The original code returns -1 in this case, so I'm curious
    > about this.
    
    Thanks for the review!
    
    Yes, you're right. So I've updated the patch so that -1 is returned
    when the current time is earlier than the time in the overflow entry,
    treating it as "no new sample found".
    
    Regards,
    
    -- 
    Fujii Masao
    
  8. Re: Fix lag columns in pg_stat_replication not advancing when replay LSN stalls

    Fujii Masao <masao.fujii@gmail.com> — 2025-10-20T01:20:41Z

    On Mon, Oct 20, 2025 at 10:12 AM Fujii Masao <masao.fujii@gmail.com> wrote:
    >
    > On Sat, Oct 18, 2025 at 9:16 AM Chao Li <li.evan.chao@gmail.com> wrote:
    > > I think I put all concentration on the big picture yesterday, so I ignored this implementation detail:
    > >
    > > ```
    > > +               if (lag_tracker->overflowed[head].lsn > lsn)
    > > +                       return now - lag_tracker->overflowed[head].time;
    > > ```
    > >
    > > Should this “>” be “>=“?
    >
    > I think either ">" or ">=" would work. However, the following loop
    > in LagTrackerRead() advances the read head when the LSN
    > in the current buffer entry is equal to the given LSN
    
    I forgot to mention which loop actually advances the read head in that case.
    It's the following code in LagTrackerRead().
    
    ------------------------
    /* Read all unread samples up to this LSN or end of buffer. */
    while (lag_tracker->read_heads[head] != lag_tracker->write_head &&
       lag_tracker->buffer[lag_tracker->read_heads[head]].lsn <= lsn)
    {
    time = lag_tracker->buffer[lag_tracker->read_heads[head]].time;
    lag_tracker->last_read[head] =
    lag_tracker->buffer[lag_tracker->read_heads[head]];
    lag_tracker->read_heads[head] =
    (lag_tracker->read_heads[head] + 1) % LAG_TRACKER_BUFFER_SIZE;
    }
    ------------------------
    
    Regards,
    
    -- 
    Fujii Masao
    
    
    
    
  9. Re: Fix lag columns in pg_stat_replication not advancing when replay LSN stalls

    Shinya Kato <shinya11.kato@gmail.com> — 2025-10-20T04:45:01Z

    On Mon, Oct 20, 2025 at 10:17 AM Fujii Masao <masao.fujii@gmail.com> wrote:
    >
    > On Sun, Oct 19, 2025 at 2:04 AM Shinya Kato <shinya11.kato@gmail.com> wrote:
    > > Thank you for the patch. I have one comment.
    > >
    > > +       if (lag_tracker->overflowed[head].lsn > lsn)
    > > +           return now - lag_tracker->overflowed[head].time;
    > >
    > > Could this return a negative value if the clock somehow went
    > > backwards? The original code returns -1 in this case, so I'm curious
    > > about this.
    >
    > Thanks for the review!
    >
    > Yes, you're right. So I've updated the patch so that -1 is returned
    > when the current time is earlier than the time in the overflow entry,
    > treating it as "no new sample found".
    
    Thank you for updating the patch. It looks nice to me.
    
    -- 
    Best regards,
    Shinya Kato
    NTT OSS Center
    
    
    
    
  10. Re: Fix lag columns in pg_stat_replication not advancing when replay LSN stalls

    Fujii Masao <masao.fujii@gmail.com> — 2025-10-20T23:44:48Z

    On Mon, Oct 20, 2025 at 1:45 PM Shinya Kato <shinya11.kato@gmail.com> wrote:
    >
    > On Mon, Oct 20, 2025 at 10:17 AM Fujii Masao <masao.fujii@gmail.com> wrote:
    > >
    > > On Sun, Oct 19, 2025 at 2:04 AM Shinya Kato <shinya11.kato@gmail.com> wrote:
    > > > Thank you for the patch. I have one comment.
    > > >
    > > > +       if (lag_tracker->overflowed[head].lsn > lsn)
    > > > +           return now - lag_tracker->overflowed[head].time;
    > > >
    > > > Could this return a negative value if the clock somehow went
    > > > backwards? The original code returns -1 in this case, so I'm curious
    > > > about this.
    > >
    > > Thanks for the review!
    > >
    > > Yes, you're right. So I've updated the patch so that -1 is returned
    > > when the current time is earlier than the time in the overflow entry,
    > > treating it as "no new sample found".
    >
    > Thank you for updating the patch. It looks nice to me.
    
    Thanks for the review!
    Unless there are any objections, I'll commit the patch and
    backpatch it to all supported branches.
    
    Regards,
    
    -- 
    Fujii Masao
    
    
    
    
  11. Re: Fix lag columns in pg_stat_replication not advancing when replay LSN stalls

    Xuneng Zhou <xunengzhou@gmail.com> — 2025-10-21T14:52:39Z

    Hi Fujii-san,
    
    On Tue, Oct 21, 2025 at 7:45 AM Fujii Masao <masao.fujii@gmail.com> wrote:
    >
    > On Mon, Oct 20, 2025 at 1:45 PM Shinya Kato <shinya11.kato@gmail.com> wrote:
    > >
    > > On Mon, Oct 20, 2025 at 10:17 AM Fujii Masao <masao.fujii@gmail.com> wrote:
    > > >
    > > > On Sun, Oct 19, 2025 at 2:04 AM Shinya Kato <shinya11.kato@gmail.com> wrote:
    > > > > Thank you for the patch. I have one comment.
    > > > >
    > > > > +       if (lag_tracker->overflowed[head].lsn > lsn)
    > > > > +           return now - lag_tracker->overflowed[head].time;
    > > > >
    > > > > Could this return a negative value if the clock somehow went
    > > > > backwards? The original code returns -1 in this case, so I'm curious
    > > > > about this.
    > > >
    > > > Thanks for the review!
    > > >
    > > > Yes, you're right. So I've updated the patch so that -1 is returned
    > > > when the current time is earlier than the time in the overflow entry,
    > > > treating it as "no new sample found".
    > >
    > > Thank you for updating the patch. It looks nice to me.
    >
    > Thanks for the review!
    > Unless there are any objections, I'll commit the patch and
    > backpatch it to all supported branches.
    >
    
    Thanks for working on this.
    
    The patch LGTM.  I am wondering whether it is helpful to add some
    comments for this overflowed array
    WalTimeSample overflowed[NUM_SYNC_REP_WAIT_MODE];
    
    and replacing literal zeros with the constant InvalidXLogRecPtr for
    better readability.
    
    /* InvalidXLogRecPtr means no overflow yet */
    if (lag_tracker->overflowed[i].lsn == InvalidXLogRecPtr)
    
    Best,
    Xuneng
    
    
    
    
  12. Re: Fix lag columns in pg_stat_replication not advancing when replay LSN stalls

    Fujii Masao <masao.fujii@gmail.com> — 2025-10-22T02:34:13Z

    On Tue, Oct 21, 2025 at 11:52 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
    > The patch LGTM.
    
    Thanks for the review!
    I've pushed the patch and backpatched it to all supported versions.
    
    
    > I am wondering whether it is helpful to add some
    > comments for this overflowed array
    
    Yes, do you have any specific suggestions?
    
    
    > and replacing literal zeros with the constant InvalidXLogRecPtr for
    > better readability.
    >
    > /* InvalidXLogRecPtr means no overflow yet */
    > if (lag_tracker->overflowed[i].lsn == InvalidXLogRecPtr)
    
    I couldn't find any code like "lag_tracker->overflowed[i].lsn == 0",
    so I'm not sure which part should be replaced with InvalidXLogRecPtr.
    Could you point me to the exact location?
    
    Regards,
    
    -- 
    Fujii Masao
    
    
    
    
  13. Re: Fix lag columns in pg_stat_replication not advancing when replay LSN stalls

    Xuneng Zhou <xunengzhou@gmail.com> — 2025-10-22T07:49:18Z

    Hi,
    
    On Wed, Oct 22, 2025 at 10:34 AM Fujii Masao <masao.fujii@gmail.com> wrote:
    >
    > On Tue, Oct 21, 2025 at 11:52 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
    > > The patch LGTM.
    >
    > Thanks for the review!
    > I've pushed the patch and backpatched it to all supported versions.
    >
    >
    > > I am wondering whether it is helpful to add some
    > > comments for this overflowed array
    >
    > Yes, do you have any specific suggestions?
    >
    
    How about something like:
    
    /*
     * Overflow entries for read heads that collide with the write head.
     *
     * When the cyclic buffer fills (write head is about to collide with a read
     * head), we save that read head's current sample here and mark it as using
     * overflow (read_heads[i] = -1). This allows the write head to continue
     * advancing while the overflowed mode continues lag computation using the
     * saved sample.
     *
     * Once the standby's reported LSN advances past the overflow entry's LSN,
     * we transition back to normal buffer-based tracking.
     */
    
    
    > > and replacing literal zeros with the constant InvalidXLogRecPtr for
    > > better readability.
    > >
    > > /* InvalidXLogRecPtr means no overflow yet */
    > > if (lag_tracker->overflowed[i].lsn == InvalidXLogRecPtr)
    >
    > I couldn't find any code like "lag_tracker->overflowed[i].lsn == 0",
    > so I'm not sure which part should be replaced with InvalidXLogRecPtr.
    > Could you point me to the exact location?
    >
    
    Sorry for the noise here, I mean
    if (lag_tracker->read_heads[head] == InvalidXLogRecPtr) before.
    But with this change, position 0 would be treated as overflow mode,
    which is clearly wrong...
    
    Best,
    Xuneng
    
    
    
    
  14. Re: Fix lag columns in pg_stat_replication not advancing when replay LSN stalls

    Fujii Masao <masao.fujii@gmail.com> — 2025-10-22T14:45:39Z

    On Wed, Oct 22, 2025 at 4:49 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
    > How about something like:
    >
    > /*
    >  * Overflow entries for read heads that collide with the write head.
    >  *
    >  * When the cyclic buffer fills (write head is about to collide with a read
    >  * head), we save that read head's current sample here and mark it as using
    >  * overflow (read_heads[i] = -1). This allows the write head to continue
    >  * advancing while the overflowed mode continues lag computation using the
    >  * saved sample.
    >  *
    >  * Once the standby's reported LSN advances past the overflow entry's LSN,
    >  * we transition back to normal buffer-based tracking.
    >  */
    
    LGTM. Thanks!
    
    I've created a patch adding your suggested comments (attached).
    Since this is a follow-up to commit 883a95646a8, which was recently applied
    and backpatched to all supported branches, I think we should backpatch
    this one as well. Thought?
    
    Regards,
    
    -- 
    Fujii Masao
    
  15. Re: Fix lag columns in pg_stat_replication not advancing when replay LSN stalls

    Xuneng Zhou <xunengzhou@gmail.com> — 2025-10-22T15:25:51Z

    Hi,
    
    On Wed, Oct 22, 2025 at 10:45 PM Fujii Masao <masao.fujii@gmail.com> wrote:
    >
    > On Wed, Oct 22, 2025 at 4:49 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
    > > How about something like:
    > >
    > > /*
    > >  * Overflow entries for read heads that collide with the write head.
    > >  *
    > >  * When the cyclic buffer fills (write head is about to collide with a read
    > >  * head), we save that read head's current sample here and mark it as using
    > >  * overflow (read_heads[i] = -1). This allows the write head to continue
    > >  * advancing while the overflowed mode continues lag computation using the
    > >  * saved sample.
    > >  *
    > >  * Once the standby's reported LSN advances past the overflow entry's LSN,
    > >  * we transition back to normal buffer-based tracking.
    > >  */
    >
    > LGTM. Thanks!
    >
    > I've created a patch adding your suggested comments (attached).
    > Since this is a follow-up to commit 883a95646a8, which was recently applied
    > and backpatched to all supported branches, I think we should backpatch
    > this one as well. Thought?
    >
    
    LGTM. Thanks for the patch!
    
    Best,
    Xuneng
    
    
    
    
  16. Re: Fix lag columns in pg_stat_replication not advancing when replay LSN stalls

    Fujii Masao <masao.fujii@gmail.com> — 2025-10-23T04:30:21Z

    On Thu, Oct 23, 2025 at 12:26 AM Xuneng Zhou <xunengzhou@gmail.com> wrote:
    > > I've created a patch adding your suggested comments (attached).
    > > Since this is a follow-up to commit 883a95646a8, which was recently applied
    > > and backpatched to all supported branches, I think we should backpatch
    > > this one as well. Thought?
    > >
    >
    > LGTM. Thanks for the patch!
    
    I've pushed the patch. Thanks!
    
    Regards,
    
    -- 
    Fujii Masao