Обсуждение: Add WALRCV_CONNECTING state to walreceiver
Hi Hackers, Bug #19093 [1] reported that pg_stat_wal_receiver.status = 'streaming' does not accurately reflect streaming health. In that discussion, Noah noted that even before the reported regression, status = 'streaming' was unreliable because walreceiver sets it during early startup, before attempting a connection. He suggested: "Long-term, in master only, perhaps we should introduce another status like 'connecting'. Perhaps enact the connecting->streaming status transition just before tendering the first byte of streamed WAL to the startup process. Alternatively, enact that transition when the startup process accepts the first streamed byte." Michael and I also thought this could be a useful addition. This patch implements that suggestion by adding a new WALRCV_CONNECTING state. == Background == Currently, walreceiver transitions directly from STARTING to STREAMING early in WalReceiverMain(), before any WAL data has been received. This means status = 'streaming' can be observed even when: - The connection to the primary has not been established - No WAL data has actually been received or flushed This makes it difficult for monitoring tools to distinguish between a healthy streaming replica and one that is merely attempting to stream. == Proposal == Introduce WALRCV_CONNECTING as an intermediate state between STARTING and STREAMING: - When walreceiver starts, it enters CONNECTING (instead of going directly to STREAMING). - The transition to STREAMING occurs in XLogWalRcvFlush(), inside the existing spinlock-protected block that updates flushedUpto. Feedbacks welcome. [1] https://www.postgresql.org/message-id/flat/19093-c4fff49a608f82a0%40postgresql.org -- Best, Xuneng
Вложения
On Fri, Dec 12, 2025 at 12:51:00PM +0800, Xuneng Zhou wrote: > Bug #19093 [1] reported that pg_stat_wal_receiver.status = 'streaming' > does not accurately reflect streaming health. In that discussion, > Noah noted that even before the reported regression, status = > 'streaming' was unreliable because walreceiver sets it during early > startup, before attempting a connection. He suggested: > > "Long-term, in master only, perhaps we should introduce another status > like 'connecting'. Perhaps enact the connecting->streaming status > transition just before tendering the first byte of streamed WAL to the > startup process. Alternatively, enact that transition when the startup > process accepts the > first streamed byte." > == Proposal == > > Introduce WALRCV_CONNECTING as an intermediate state between STARTING > and STREAMING: > > - When walreceiver starts, it enters CONNECTING (instead of going > directly to STREAMING). > - The transition to STREAMING occurs in XLogWalRcvFlush(), inside the > existing spinlock-protected block that updates flushedUpto. I think this has the drawback that if the primary's WAL is incompatible, e.g. unacceptable timeline, the walreceiver will still briefly enter STREAMING. That could trick monitoring. Waiting for applyPtr to advance would avoid the short-lived STREAMING. What's the feasibility of that?
Hi Noah, On Fri, Dec 12, 2025 at 1:05 PM Noah Misch <noah@leadboat.com> wrote: > > On Fri, Dec 12, 2025 at 12:51:00PM +0800, Xuneng Zhou wrote: > > Bug #19093 [1] reported that pg_stat_wal_receiver.status = 'streaming' > > does not accurately reflect streaming health. In that discussion, > > Noah noted that even before the reported regression, status = > > 'streaming' was unreliable because walreceiver sets it during early > > startup, before attempting a connection. He suggested: > > > > "Long-term, in master only, perhaps we should introduce another status > > like 'connecting'. Perhaps enact the connecting->streaming status > > transition just before tendering the first byte of streamed WAL to the > > startup process. Alternatively, enact that transition when the startup > > process accepts the > > first streamed byte." > > > == Proposal == > > > > Introduce WALRCV_CONNECTING as an intermediate state between STARTING > > and STREAMING: > > > > - When walreceiver starts, it enters CONNECTING (instead of going > > directly to STREAMING). > > - The transition to STREAMING occurs in XLogWalRcvFlush(), inside the > > existing spinlock-protected block that updates flushedUpto. > > I think this has the drawback that if the primary's WAL is incompatible, > e.g. unacceptable timeline, the walreceiver will still briefly enter > STREAMING. That could trick monitoring. Thanks for pointing this out. Waiting for applyPtr to advance > would avoid the short-lived STREAMING. What's the feasibility of that? I think this could work, but with complications. If replay latency is high or replay is paused with pg_wal_replay_pause, the WalReceiver would stay in the CONNECTING state longer than expected. Whether this is ok depends on the definition of the 'connecting' state. For the implementation, deciding where and when to check applyPtr against LSNs like receiveStart is more difficult—the WalReceiver doesn't know when applyPtr advances. While the WalReceiver can read applyPtr from shared memory, it isn't automatically notified when that pointer advances. This leads to latency between checking and replay if this is done in the WalReceiver part unless we let the startup process set the state, which would couple the two components. Am I missing something here? -- Best, Xuneng
Hi, On Fri, Dec 12, 2025 at 4:45 PM Xuneng Zhou <xunengzhou@gmail.com> wrote: > > Hi Noah, > > On Fri, Dec 12, 2025 at 1:05 PM Noah Misch <noah@leadboat.com> wrote: > > > > On Fri, Dec 12, 2025 at 12:51:00PM +0800, Xuneng Zhou wrote: > > > Bug #19093 [1] reported that pg_stat_wal_receiver.status = 'streaming' > > > does not accurately reflect streaming health. In that discussion, > > > Noah noted that even before the reported regression, status = > > > 'streaming' was unreliable because walreceiver sets it during early > > > startup, before attempting a connection. He suggested: > > > > > > "Long-term, in master only, perhaps we should introduce another status > > > like 'connecting'. Perhaps enact the connecting->streaming status > > > transition just before tendering the first byte of streamed WAL to the > > > startup process. Alternatively, enact that transition when the startup > > > process accepts the > > > first streamed byte." > > > > > == Proposal == > > > > > > Introduce WALRCV_CONNECTING as an intermediate state between STARTING > > > and STREAMING: > > > > > > - When walreceiver starts, it enters CONNECTING (instead of going > > > directly to STREAMING). > > > - The transition to STREAMING occurs in XLogWalRcvFlush(), inside the > > > existing spinlock-protected block that updates flushedUpto. > > > > I think this has the drawback that if the primary's WAL is incompatible, > > e.g. unacceptable timeline, the walreceiver will still briefly enter > > STREAMING. That could trick monitoring. > > Thanks for pointing this out. > > Waiting for applyPtr to advance > > would avoid the short-lived STREAMING. What's the feasibility of that? > > I think this could work, but with complications. If replay latency is > high or replay is paused with pg_wal_replay_pause, the WalReceiver > would stay in the CONNECTING state longer than expected. Whether this > is ok depends on the definition of the 'connecting' state. For the > implementation, deciding where and when to check applyPtr against LSNs > like receiveStart is more difficult—the WalReceiver doesn't know when > applyPtr advances. While the WalReceiver can read applyPtr from shared > memory, it isn't automatically notified when that pointer advances. > This leads to latency between checking and replay if this is done in > the WalReceiver part unless we let the startup process set the state, > which would couple the two components. Am I missing something here? > After some thoughts, a potential approach could be to expose a new function in the WAL receiver that transitions the state from CONNECTING to STREAMING. This function can then be invoked directly from WaitForWALToBecomeAvailable in the startup process, ensuring the state change aligns with the actual acceptance of the WAL stream. -- Best, Xuneng
Hi, On Fri, Dec 12, 2025 at 9:52 PM Xuneng Zhou <xunengzhou@gmail.com> wrote: > > Hi, > > On Fri, Dec 12, 2025 at 4:45 PM Xuneng Zhou <xunengzhou@gmail.com> wrote: > > > > Hi Noah, > > > > On Fri, Dec 12, 2025 at 1:05 PM Noah Misch <noah@leadboat.com> wrote: > > > > > > On Fri, Dec 12, 2025 at 12:51:00PM +0800, Xuneng Zhou wrote: > > > > Bug #19093 [1] reported that pg_stat_wal_receiver.status = 'streaming' > > > > does not accurately reflect streaming health. In that discussion, > > > > Noah noted that even before the reported regression, status = > > > > 'streaming' was unreliable because walreceiver sets it during early > > > > startup, before attempting a connection. He suggested: > > > > > > > > "Long-term, in master only, perhaps we should introduce another status > > > > like 'connecting'. Perhaps enact the connecting->streaming status > > > > transition just before tendering the first byte of streamed WAL to the > > > > startup process. Alternatively, enact that transition when the startup > > > > process accepts the > > > > first streamed byte." > > > > > > > == Proposal == > > > > > > > > Introduce WALRCV_CONNECTING as an intermediate state between STARTING > > > > and STREAMING: > > > > > > > > - When walreceiver starts, it enters CONNECTING (instead of going > > > > directly to STREAMING). > > > > - The transition to STREAMING occurs in XLogWalRcvFlush(), inside the > > > > existing spinlock-protected block that updates flushedUpto. > > > > > > I think this has the drawback that if the primary's WAL is incompatible, > > > e.g. unacceptable timeline, the walreceiver will still briefly enter > > > STREAMING. That could trick monitoring. > > > > Thanks for pointing this out. > > > > Waiting for applyPtr to advance > > > would avoid the short-lived STREAMING. What's the feasibility of that? > > > > I think this could work, but with complications. If replay latency is > > high or replay is paused with pg_wal_replay_pause, the WalReceiver > > would stay in the CONNECTING state longer than expected. Whether this > > is ok depends on the definition of the 'connecting' state. For the > > implementation, deciding where and when to check applyPtr against LSNs > > like receiveStart is more difficult—the WalReceiver doesn't know when > > applyPtr advances. While the WalReceiver can read applyPtr from shared > > memory, it isn't automatically notified when that pointer advances. > > This leads to latency between checking and replay if this is done in > > the WalReceiver part unless we let the startup process set the state, > > which would couple the two components. Am I missing something here? > > > > After some thoughts, a potential approach could be to expose a new > function in the WAL receiver that transitions the state from > CONNECTING to STREAMING. This function can then be invoked directly > from WaitForWALToBecomeAvailable in the startup process, ensuring the > state change aligns with the actual acceptance of the WAL stream. > V2 makes the transition from WALRCV_CONNECTING to STREAMING only when the first valid WAL record is processed by the startup process. A new function WalRcvSetStreaming is introduced to enable the transition. -- Best, Xuneng
Вложения
On Sun, Dec 14, 2025 at 12:45:46PM +0800, Xuneng Zhou wrote: > On Fri, Dec 12, 2025 at 9:52 PM Xuneng Zhou <xunengzhou@gmail.com> wrote: > > On Fri, Dec 12, 2025 at 4:45 PM Xuneng Zhou <xunengzhou@gmail.com> wrote: > > > On Fri, Dec 12, 2025 at 1:05 PM Noah Misch <noah@leadboat.com> wrote: > > > > Waiting for applyPtr to advance > > > > would avoid the short-lived STREAMING. What's the feasibility of that? > > > > > > I think this could work, but with complications. If replay latency is > > > high or replay is paused with pg_wal_replay_pause, the WalReceiver > > > would stay in the CONNECTING state longer than expected. Whether this > > > is ok depends on the definition of the 'connecting' state. For the > > > implementation, deciding where and when to check applyPtr against LSNs > > > like receiveStart is more difficult—the WalReceiver doesn't know when > > > applyPtr advances. While the WalReceiver can read applyPtr from shared > > > memory, it isn't automatically notified when that pointer advances. > > > This leads to latency between checking and replay if this is done in > > > the WalReceiver part unless we let the startup process set the state, > > > which would couple the two components. Am I missing something here? > > > > After some thoughts, a potential approach could be to expose a new > > function in the WAL receiver that transitions the state from > > CONNECTING to STREAMING. This function can then be invoked directly > > from WaitForWALToBecomeAvailable in the startup process, ensuring the > > state change aligns with the actual acceptance of the WAL stream. > > V2 makes the transition from WALRCV_CONNECTING to STREAMING only when > the first valid WAL record is processed by the startup process. A new > function WalRcvSetStreaming is introduced to enable the transition. The original patch set STREAMING in XLogWalRcvFlush(). XLogWalRcvFlush() callee XLogWalRcvSendReply() already fetches applyPtr to send a status message. So I would try the following before involving the startup process like v2 does: 1. store the applyPtr when we enter CONNECTING 2. force a status message as long as we remain in CONNECTING 3. become STREAMING when applyPtr differs from the one stored at (1) A possible issue with all patch versions: when the primary is writing no WAL and the standby was caught up before this walreceiver started, CONNECTING could persist for an unbounded amount of time. Only actual primary WAL generation would move the walreceiver to STREAMING. This relates to your above point about high latency. If that's a concern, perhaps this change deserves a total of two new states, CONNECTING and a state that represents "connection exists, no WAL yet applied"?
Hi,
On Sun, Dec 14, 2025 at 1:14 PM Noah Misch <noah@leadboat.com> wrote:
>
> On Sun, Dec 14, 2025 at 12:45:46PM +0800, Xuneng Zhou wrote:
> > On Fri, Dec 12, 2025 at 9:52 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> > > On Fri, Dec 12, 2025 at 4:45 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> > > > On Fri, Dec 12, 2025 at 1:05 PM Noah Misch <noah@leadboat.com> wrote:
> > > > > Waiting for applyPtr to advance
> > > > > would avoid the short-lived STREAMING. What's the feasibility of that?
> > > >
> > > > I think this could work, but with complications. If replay latency is
> > > > high or replay is paused with pg_wal_replay_pause, the WalReceiver
> > > > would stay in the CONNECTING state longer than expected. Whether this
> > > > is ok depends on the definition of the 'connecting' state. For the
> > > > implementation, deciding where and when to check applyPtr against LSNs
> > > > like receiveStart is more difficult—the WalReceiver doesn't know when
> > > > applyPtr advances. While the WalReceiver can read applyPtr from shared
> > > > memory, it isn't automatically notified when that pointer advances.
> > > > This leads to latency between checking and replay if this is done in
> > > > the WalReceiver part unless we let the startup process set the state,
> > > > which would couple the two components. Am I missing something here?
> > >
> > > After some thoughts, a potential approach could be to expose a new
> > > function in the WAL receiver that transitions the state from
> > > CONNECTING to STREAMING. This function can then be invoked directly
> > > from WaitForWALToBecomeAvailable in the startup process, ensuring the
> > > state change aligns with the actual acceptance of the WAL stream.
> >
> > V2 makes the transition from WALRCV_CONNECTING to STREAMING only when
> > the first valid WAL record is processed by the startup process. A new
> > function WalRcvSetStreaming is introduced to enable the transition.
>
> The original patch set STREAMING in XLogWalRcvFlush(). XLogWalRcvFlush()
> callee XLogWalRcvSendReply() already fetches applyPtr to send a status
> message. So I would try the following before involving the startup process
> like v2 does:
>
> 1. store the applyPtr when we enter CONNECTING
> 2. force a status message as long as we remain in CONNECTING
> 3. become STREAMING when applyPtr differs from the one stored at (1)
Thanks for the suggestion. Using XLogWalRcvSendReply() for the
transition could make sense. My concern before is about latency in a
rare case: if the first flush completes but applyPtr hasn't advanced
yet at the time of check and then the flush stalls after that, we
might wait up to wal_receiver_status_interval (default 10s) before the
next check or indefinitely if (wal_receiver_status_interval <= 0).
This could be mitigated by shortening the wakeup interval while in
CONNECTING (step 2), which reduces worst-case latency to ~1 second.
Given that monitoring typically doesn't require sub-second precision,
this approach could be feasible.
case WALRCV_WAKEUP_REPLY:
if (WalRcv->walRcvState == WALRCV_CONNECTING)
{
/* Poll frequently while CONNECTING to avoid long latency */
wakeup[reason] = TimestampTzPlusMilliseconds(now, 1000);
}
> A possible issue with all patch versions: when the primary is writing no WAL
> and the standby was caught up before this walreceiver started, CONNECTING
> could persist for an unbounded amount of time. Only actual primary WAL
> generation would move the walreceiver to STREAMING. This relates to your
> above point about high latency. If that's a concern, perhaps this change
> deserves a total of two new states, CONNECTING and a state that represents
> "connection exists, no WAL yet applied"?
Yes, this could be an issue. Using two states would help address it.
That said, when the primary is idle in this case, we might end up
repeatedly polling the apply status in the state before streaming if
we implement the 1s short-interval checking like above, which could be
costful. However, If we do not implement it &&
wal_receiver_status_interval is set to < 0 && flush stalls, the
walreceiver could stay in the pre-streaming state indefinitely even if
streaming did occur, which violates the semantics. Do you think this
is a valid concern or just an artificial edge case?
--
Best,
Xuneng
Hi,
On Sun, Dec 14, 2025 at 4:55 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
>
> Hi,
>
> On Sun, Dec 14, 2025 at 1:14 PM Noah Misch <noah@leadboat.com> wrote:
> >
> > On Sun, Dec 14, 2025 at 12:45:46PM +0800, Xuneng Zhou wrote:
> > > On Fri, Dec 12, 2025 at 9:52 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> > > > On Fri, Dec 12, 2025 at 4:45 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> > > > > On Fri, Dec 12, 2025 at 1:05 PM Noah Misch <noah@leadboat.com> wrote:
> > > > > > Waiting for applyPtr to advance
> > > > > > would avoid the short-lived STREAMING. What's the feasibility of that?
> > > > >
> > > > > I think this could work, but with complications. If replay latency is
> > > > > high or replay is paused with pg_wal_replay_pause, the WalReceiver
> > > > > would stay in the CONNECTING state longer than expected. Whether this
> > > > > is ok depends on the definition of the 'connecting' state. For the
> > > > > implementation, deciding where and when to check applyPtr against LSNs
> > > > > like receiveStart is more difficult—the WalReceiver doesn't know when
> > > > > applyPtr advances. While the WalReceiver can read applyPtr from shared
> > > > > memory, it isn't automatically notified when that pointer advances.
> > > > > This leads to latency between checking and replay if this is done in
> > > > > the WalReceiver part unless we let the startup process set the state,
> > > > > which would couple the two components. Am I missing something here?
> > > >
> > > > After some thoughts, a potential approach could be to expose a new
> > > > function in the WAL receiver that transitions the state from
> > > > CONNECTING to STREAMING. This function can then be invoked directly
> > > > from WaitForWALToBecomeAvailable in the startup process, ensuring the
> > > > state change aligns with the actual acceptance of the WAL stream.
> > >
> > > V2 makes the transition from WALRCV_CONNECTING to STREAMING only when
> > > the first valid WAL record is processed by the startup process. A new
> > > function WalRcvSetStreaming is introduced to enable the transition.
> >
> > The original patch set STREAMING in XLogWalRcvFlush(). XLogWalRcvFlush()
> > callee XLogWalRcvSendReply() already fetches applyPtr to send a status
> > message. So I would try the following before involving the startup process
> > like v2 does:
> >
> > 1. store the applyPtr when we enter CONNECTING
> > 2. force a status message as long as we remain in CONNECTING
> > 3. become STREAMING when applyPtr differs from the one stored at (1)
>
> Thanks for the suggestion. Using XLogWalRcvSendReply() for the
> transition could make sense. My concern before is about latency in a
> rare case: if the first flush completes but applyPtr hasn't advanced
> yet at the time of check and then the flush stalls after that, we
> might wait up to wal_receiver_status_interval (default 10s) before the
> next check or indefinitely if (wal_receiver_status_interval <= 0).
> This could be mitigated by shortening the wakeup interval while in
> CONNECTING (step 2), which reduces worst-case latency to ~1 second.
> Given that monitoring typically doesn't require sub-second precision,
> this approach could be feasible.
>
> case WALRCV_WAKEUP_REPLY:
> if (WalRcv->walRcvState == WALRCV_CONNECTING)
> {
> /* Poll frequently while CONNECTING to avoid long latency */
> wakeup[reason] = TimestampTzPlusMilliseconds(now, 1000);
> }
>
> > A possible issue with all patch versions: when the primary is writing no WAL
> > and the standby was caught up before this walreceiver started, CONNECTING
> > could persist for an unbounded amount of time. Only actual primary WAL
> > generation would move the walreceiver to STREAMING. This relates to your
> > above point about high latency. If that's a concern, perhaps this change
> > deserves a total of two new states, CONNECTING and a state that represents
> > "connection exists, no WAL yet applied"?
>
> Yes, this could be an issue. Using two states would help address it.
> That said, when the primary is idle in this case, we might end up
> repeatedly polling the apply status in the state before streaming if
> we implement the 1s short-interval checking like above, which could be
> costful. However, If we do not implement it &&
> wal_receiver_status_interval is set to < 0 && flush stalls, the
> walreceiver could stay in the pre-streaming state indefinitely even if
> streaming did occur, which violates the semantics. Do you think this
> is a valid concern or just an artificial edge case?
After looking more closely, I found that true indefinite waiting
requires ALL of:
wal_receiver_status_interval <= 0 (disables status updates)
wal_receiver_timeout <= 0
Primary sends no keepalives
No more WAL arrives after the first failed-check flush
Startup never sets force_reply
which is quite impossible and artificial, sorry for the noise here.
The worst-case latency of state-transition in the scenario described
above would be max(Primary keepalive, REPLY timeout, PING timeout),
which might be ok without the short-interval mitigation, given this
case is pretty rare. I plan to implement the following approach with
two new states like you suggested as v3.
1. enter CONNECTING
2. transite the state to CONNECTED/IDLE when START_REPLICATION
succeeds, store the applyPtr
2. force a status message in XLogWalRcvFlush as long as we remain in
CONNECTED/IDLE
3. become STREAMING when applyPtr differs from the one stored at (2)
--
Best,
Xuneng
On Sun, Dec 14, 2025 at 06:17:34PM +0800, Xuneng Zhou wrote:
> On Sun, Dec 14, 2025 at 4:55 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> > On Sun, Dec 14, 2025 at 1:14 PM Noah Misch <noah@leadboat.com> wrote:
> > > > V2 makes the transition from WALRCV_CONNECTING to STREAMING only when
> > > > the first valid WAL record is processed by the startup process. A new
> > > > function WalRcvSetStreaming is introduced to enable the transition.
> > >
> > > The original patch set STREAMING in XLogWalRcvFlush(). XLogWalRcvFlush()
> > > callee XLogWalRcvSendReply() already fetches applyPtr to send a status
> > > message. So I would try the following before involving the startup process
> > > like v2 does:
> > >
> > > 1. store the applyPtr when we enter CONNECTING
> > > 2. force a status message as long as we remain in CONNECTING
> > > 3. become STREAMING when applyPtr differs from the one stored at (1)
> >
> > Thanks for the suggestion. Using XLogWalRcvSendReply() for the
> > transition could make sense. My concern before is about latency in a
> > rare case: if the first flush completes but applyPtr hasn't advanced
> > yet at the time of check and then the flush stalls after that, we
> > might wait up to wal_receiver_status_interval (default 10s) before the
> > next check or indefinitely if (wal_receiver_status_interval <= 0).
> > This could be mitigated by shortening the wakeup interval while in
> > CONNECTING (step 2), which reduces worst-case latency to ~1 second.
> > Given that monitoring typically doesn't require sub-second precision,
> > this approach could be feasible.
> >
> > case WALRCV_WAKEUP_REPLY:
> > if (WalRcv->walRcvState == WALRCV_CONNECTING)
> > {
> > /* Poll frequently while CONNECTING to avoid long latency */
> > wakeup[reason] = TimestampTzPlusMilliseconds(now, 1000);
> > }
> >
> > > A possible issue with all patch versions: when the primary is writing no WAL
> > > and the standby was caught up before this walreceiver started, CONNECTING
> > > could persist for an unbounded amount of time. Only actual primary WAL
> > > generation would move the walreceiver to STREAMING. This relates to your
> > > above point about high latency. If that's a concern, perhaps this change
> > > deserves a total of two new states, CONNECTING and a state that represents
> > > "connection exists, no WAL yet applied"?
> >
> > Yes, this could be an issue. Using two states would help address it.
> > That said, when the primary is idle in this case, we might end up
> > repeatedly polling the apply status in the state before streaming if
> > we implement the 1s short-interval checking like above, which could be
> > costful. However, If we do not implement it &&
> > wal_receiver_status_interval is set to < 0 && flush stalls, the
> > walreceiver could stay in the pre-streaming state indefinitely even if
> > streaming did occur, which violates the semantics. Do you think this
> > is a valid concern or just an artificial edge case?
>
> After looking more closely, I found that true indefinite waiting
> requires ALL of:
>
> wal_receiver_status_interval <= 0 (disables status updates)
> wal_receiver_timeout <= 0
> Primary sends no keepalives
> No more WAL arrives after the first failed-check flush
> Startup never sets force_reply
>
> which is quite impossible and artificial, sorry for the noise here.
Even if indefinite wait is a negligible concern, you identified a lot of
intricacy that I hadn't pictured. That makes your startup-process-driven
version potentially more attractive. Forcing status messages like I was
thinking may also yield an unwanted flurry of them if the startup process is
slow. Let's see what the patch reviewer thinks.
Hi,
On Mon, Dec 15, 2025 at 12:14 PM Noah Misch <noah@leadboat.com> wrote:
>
> On Sun, Dec 14, 2025 at 06:17:34PM +0800, Xuneng Zhou wrote:
> > On Sun, Dec 14, 2025 at 4:55 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> > > On Sun, Dec 14, 2025 at 1:14 PM Noah Misch <noah@leadboat.com> wrote:
> > > > > V2 makes the transition from WALRCV_CONNECTING to STREAMING only when
> > > > > the first valid WAL record is processed by the startup process. A new
> > > > > function WalRcvSetStreaming is introduced to enable the transition.
> > > >
> > > > The original patch set STREAMING in XLogWalRcvFlush(). XLogWalRcvFlush()
> > > > callee XLogWalRcvSendReply() already fetches applyPtr to send a status
> > > > message. So I would try the following before involving the startup process
> > > > like v2 does:
> > > >
> > > > 1. store the applyPtr when we enter CONNECTING
> > > > 2. force a status message as long as we remain in CONNECTING
> > > > 3. become STREAMING when applyPtr differs from the one stored at (1)
> > >
> > > Thanks for the suggestion. Using XLogWalRcvSendReply() for the
> > > transition could make sense. My concern before is about latency in a
> > > rare case: if the first flush completes but applyPtr hasn't advanced
> > > yet at the time of check and then the flush stalls after that, we
> > > might wait up to wal_receiver_status_interval (default 10s) before the
> > > next check or indefinitely if (wal_receiver_status_interval <= 0).
> > > This could be mitigated by shortening the wakeup interval while in
> > > CONNECTING (step 2), which reduces worst-case latency to ~1 second.
> > > Given that monitoring typically doesn't require sub-second precision,
> > > this approach could be feasible.
> > >
> > > case WALRCV_WAKEUP_REPLY:
> > > if (WalRcv->walRcvState == WALRCV_CONNECTING)
> > > {
> > > /* Poll frequently while CONNECTING to avoid long latency */
> > > wakeup[reason] = TimestampTzPlusMilliseconds(now, 1000);
> > > }
> > >
> > > > A possible issue with all patch versions: when the primary is writing no WAL
> > > > and the standby was caught up before this walreceiver started, CONNECTING
> > > > could persist for an unbounded amount of time. Only actual primary WAL
> > > > generation would move the walreceiver to STREAMING. This relates to your
> > > > above point about high latency. If that's a concern, perhaps this change
> > > > deserves a total of two new states, CONNECTING and a state that represents
> > > > "connection exists, no WAL yet applied"?
> > >
> > > Yes, this could be an issue. Using two states would help address it.
> > > That said, when the primary is idle in this case, we might end up
> > > repeatedly polling the apply status in the state before streaming if
> > > we implement the 1s short-interval checking like above, which could be
> > > costful. However, If we do not implement it &&
> > > wal_receiver_status_interval is set to < 0 && flush stalls, the
> > > walreceiver could stay in the pre-streaming state indefinitely even if
> > > streaming did occur, which violates the semantics. Do you think this
> > > is a valid concern or just an artificial edge case?
> >
> > After looking more closely, I found that true indefinite waiting
> > requires ALL of:
> >
> > wal_receiver_status_interval <= 0 (disables status updates)
> > wal_receiver_timeout <= 0
> > Primary sends no keepalives
> > No more WAL arrives after the first failed-check flush
> > Startup never sets force_reply
> >
> > which is quite impossible and artificial, sorry for the noise here.
>
> Even if indefinite wait is a negligible concern, you identified a lot of
> intricacy that I hadn't pictured. That makes your startup-process-driven
> version potentially more attractive. Forcing status messages like I was
> thinking may also yield an unwanted flurry of them if the startup process is
> slow. Let's see what the patch reviewer thinks.
OK, both approaches are presented for review. Adding two states to
avoid the confusion of the status caused by the stall you depicted
earlier seems reasonable to me. So, I adapted it in v3.
--
Best,
Xuneng
Вложения
On Mon, Dec 15, 2025 at 9:44 AM Noah Misch <noah@leadboat.com> wrote:
On Sun, Dec 14, 2025 at 06:17:34PM +0800, Xuneng Zhou wrote:
> On Sun, Dec 14, 2025 at 4:55 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> > On Sun, Dec 14, 2025 at 1:14 PM Noah Misch <noah@leadboat.com> wrote:
> > > > V2 makes the transition from WALRCV_CONNECTING to STREAMING only when
> > > > the first valid WAL record is processed by the startup process. A new
> > > > function WalRcvSetStreaming is introduced to enable the transition.
> > >
> > > The original patch set STREAMING in XLogWalRcvFlush(). XLogWalRcvFlush()
> > > callee XLogWalRcvSendReply() already fetches applyPtr to send a status
> > > message. So I would try the following before involving the startup process
> > > like v2 does:
> > >
> > > 1. store the applyPtr when we enter CONNECTING
> > > 2. force a status message as long as we remain in CONNECTING
> > > 3. become STREAMING when applyPtr differs from the one stored at (1)
> >
> > Thanks for the suggestion. Using XLogWalRcvSendReply() for the
> > transition could make sense. My concern before is about latency in a
> > rare case: if the first flush completes but applyPtr hasn't advanced
> > yet at the time of check and then the flush stalls after that, we
> > might wait up to wal_receiver_status_interval (default 10s) before the
> > next check or indefinitely if (wal_receiver_status_interval <= 0).
> > This could be mitigated by shortening the wakeup interval while in
> > CONNECTING (step 2), which reduces worst-case latency to ~1 second.
> > Given that monitoring typically doesn't require sub-second precision,
> > this approach could be feasible.
> >
> > case WALRCV_WAKEUP_REPLY:
> > if (WalRcv->walRcvState == WALRCV_CONNECTING)
> > {
> > /* Poll frequently while CONNECTING to avoid long latency */
> > wakeup[reason] = TimestampTzPlusMilliseconds(now, 1000);
> > }
> >
> > > A possible issue with all patch versions: when the primary is writing no WAL
> > > and the standby was caught up before this walreceiver started, CONNECTING
> > > could persist for an unbounded amount of time. Only actual primary WAL
> > > generation would move the walreceiver to STREAMING. This relates to your
> > > above point about high latency. If that's a concern, perhaps this change
> > > deserves a total of two new states, CONNECTING and a state that represents
> > > "connection exists, no WAL yet applied"?
> >
> > Yes, this could be an issue. Using two states would help address it.
> > That said, when the primary is idle in this case, we might end up
> > repeatedly polling the apply status in the state before streaming if
> > we implement the 1s short-interval checking like above, which could be
> > costful. However, If we do not implement it &&
> > wal_receiver_status_interval is set to < 0 && flush stalls, the
> > walreceiver could stay in the pre-streaming state indefinitely even if
> > streaming did occur, which violates the semantics. Do you think this
> > is a valid concern or just an artificial edge case?
>
> After looking more closely, I found that true indefinite waiting
> requires ALL of:
>
> wal_receiver_status_interval <= 0 (disables status updates)
> wal_receiver_timeout <= 0
> Primary sends no keepalives
> No more WAL arrives after the first failed-check flush
> Startup never sets force_reply
>
> which is quite impossible and artificial, sorry for the noise here.
Even if indefinite wait is a negligible concern, you identified a lot of
intricacy that I hadn't pictured. That makes your startup-process-driven
version potentially more attractive. Forcing status messages like I was
thinking may also yield an unwanted flurry of them if the startup process is
slow. Let's see what the patch reviewer thinks.
FWIW, I think doing it in startup might be slightly better.
It seems more logical to make the state change near the point where the status
is updated, as this helps prevent reading the status from shared memory and
reduces related delays.
is updated, as this helps prevent reading the status from shared memory and
reduces related delays.
The current proposal is to advance the state to STREAMING after applyPtr has
been updated.
IIUC, the rationale is to avoid having a short-lived streaming state if applying WAL fails.
However, this approach can be confusing because the receiver may already be receiving
WAL from the primary, yet its state remains CONNECTING until the WAL is flushed.
if (walrcv_startstreaming(wrconn, &options))
{
if (first_stream)
ereport(LOG,
errmsg("started streaming WAL from primary at %X/%08X on timeline %u",
LSN_FORMAT_ARGS(startpoint), startpointTLI));
Thank you,
been updated.
IIUC, the rationale is to avoid having a short-lived streaming state if applying WAL fails.
However, this approach can be confusing because the receiver may already be receiving
WAL from the primary, yet its state remains CONNECTING until the WAL is flushed.
Would it be better to advance the state to streaming after the connection
is successfully established and the following LOG message is emitted?
is successfully established and the following LOG message is emitted?
if (walrcv_startstreaming(wrconn, &options))
{
if (first_stream)
ereport(LOG,
errmsg("started streaming WAL from primary at %X/%08X on timeline %u",
LSN_FORMAT_ARGS(startpoint), startpointTLI));
Thank you,
Rahila Syed
Hi Rahila,
Thanks for looking into this.
On Mon, Dec 15, 2025 at 9:48 PM Rahila Syed <rahilasyed90@gmail.com> wrote:
>
>
> Hi,
>
> On Mon, Dec 15, 2025 at 9:44 AM Noah Misch <noah@leadboat.com> wrote:
>>
>> On Sun, Dec 14, 2025 at 06:17:34PM +0800, Xuneng Zhou wrote:
>> > On Sun, Dec 14, 2025 at 4:55 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
>> > > On Sun, Dec 14, 2025 at 1:14 PM Noah Misch <noah@leadboat.com> wrote:
>> > > > > V2 makes the transition from WALRCV_CONNECTING to STREAMING only when
>> > > > > the first valid WAL record is processed by the startup process. A new
>> > > > > function WalRcvSetStreaming is introduced to enable the transition.
>> > > >
>> > > > The original patch set STREAMING in XLogWalRcvFlush(). XLogWalRcvFlush()
>> > > > callee XLogWalRcvSendReply() already fetches applyPtr to send a status
>> > > > message. So I would try the following before involving the startup process
>> > > > like v2 does:
>> > > >
>> > > > 1. store the applyPtr when we enter CONNECTING
>> > > > 2. force a status message as long as we remain in CONNECTING
>> > > > 3. become STREAMING when applyPtr differs from the one stored at (1)
>> > >
>> > > Thanks for the suggestion. Using XLogWalRcvSendReply() for the
>> > > transition could make sense. My concern before is about latency in a
>> > > rare case: if the first flush completes but applyPtr hasn't advanced
>> > > yet at the time of check and then the flush stalls after that, we
>> > > might wait up to wal_receiver_status_interval (default 10s) before the
>> > > next check or indefinitely if (wal_receiver_status_interval <= 0).
>> > > This could be mitigated by shortening the wakeup interval while in
>> > > CONNECTING (step 2), which reduces worst-case latency to ~1 second.
>> > > Given that monitoring typically doesn't require sub-second precision,
>> > > this approach could be feasible.
>> > >
>> > > case WALRCV_WAKEUP_REPLY:
>> > > if (WalRcv->walRcvState == WALRCV_CONNECTING)
>> > > {
>> > > /* Poll frequently while CONNECTING to avoid long latency */
>> > > wakeup[reason] = TimestampTzPlusMilliseconds(now, 1000);
>> > > }
>> > >
>> > > > A possible issue with all patch versions: when the primary is writing no WAL
>> > > > and the standby was caught up before this walreceiver started, CONNECTING
>> > > > could persist for an unbounded amount of time. Only actual primary WAL
>> > > > generation would move the walreceiver to STREAMING. This relates to your
>> > > > above point about high latency. If that's a concern, perhaps this change
>> > > > deserves a total of two new states, CONNECTING and a state that represents
>> > > > "connection exists, no WAL yet applied"?
>> > >
>> > > Yes, this could be an issue. Using two states would help address it.
>> > > That said, when the primary is idle in this case, we might end up
>> > > repeatedly polling the apply status in the state before streaming if
>> > > we implement the 1s short-interval checking like above, which could be
>> > > costful. However, If we do not implement it &&
>> > > wal_receiver_status_interval is set to < 0 && flush stalls, the
>> > > walreceiver could stay in the pre-streaming state indefinitely even if
>> > > streaming did occur, which violates the semantics. Do you think this
>> > > is a valid concern or just an artificial edge case?
>> >
>> > After looking more closely, I found that true indefinite waiting
>> > requires ALL of:
>> >
>> > wal_receiver_status_interval <= 0 (disables status updates)
>> > wal_receiver_timeout <= 0
>> > Primary sends no keepalives
>> > No more WAL arrives after the first failed-check flush
>> > Startup never sets force_reply
>> >
>> > which is quite impossible and artificial, sorry for the noise here.
>>
>> Even if indefinite wait is a negligible concern, you identified a lot of
>> intricacy that I hadn't pictured. That makes your startup-process-driven
>> version potentially more attractive. Forcing status messages like I was
>> thinking may also yield an unwanted flurry of them if the startup process is
>> slow. Let's see what the patch reviewer thinks.
>
>
> FWIW, I think doing it in startup might be slightly better.
> It seems more logical to make the state change near the point where the status
> is updated, as this helps prevent reading the status from shared memory and
> reduces related delays.
>
> The current proposal is to advance the state to STREAMING after applyPtr has
> been updated.
> IIUC, the rationale is to avoid having a short-lived streaming state if applying WAL fails.
> However, this approach can be confusing because the receiver may already be receiving
> WAL from the primary, yet its state remains CONNECTING until the WAL is flushed.
>
> Would it be better to advance the state to streaming after the connection
> is successfully established and the following LOG message is emitted?
>
> if (walrcv_startstreaming(wrconn, &options))
> {
> if (first_stream)
> ereport(LOG,
> errmsg("started streaming WAL from primary at %X/%08X on timeline %u",
> LSN_FORMAT_ARGS(startpoint), startpointTLI));
AFAICS, this may depend on how we define the streaming status. If
streaming is defined simply as “the connection has been established
and walreceiver is ready to operate,” then this approach fits well and
keeps the model simple. However, if streaming is meant to indicate
that WAL has actually started flowing and replay is in progress, then
this approach could fall short, particularly for the short-lived
streaming cases you mentioned. Introducing finer-grained states can
handle these edge cases more accurately, but it also makes the state
transitions more complex. That said, I’m not well positioned to fully
evaluate the trade-offs here, as I’m not a day-to-day end user.
--
Best,
Xuneng
Hi,
On Mon, Dec 15, 2025 at 11:38 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
>
> Hi Rahila,
>
> Thanks for looking into this.
>
> On Mon, Dec 15, 2025 at 9:48 PM Rahila Syed <rahilasyed90@gmail.com> wrote:
> >
> >
> > Hi,
> >
> > On Mon, Dec 15, 2025 at 9:44 AM Noah Misch <noah@leadboat.com> wrote:
> >>
> >> On Sun, Dec 14, 2025 at 06:17:34PM +0800, Xuneng Zhou wrote:
> >> > On Sun, Dec 14, 2025 at 4:55 PM Xuneng Zhou <xunengzhou@gmail.com> wrote:
> >> > > On Sun, Dec 14, 2025 at 1:14 PM Noah Misch <noah@leadboat.com> wrote:
> >> > > > > V2 makes the transition from WALRCV_CONNECTING to STREAMING only when
> >> > > > > the first valid WAL record is processed by the startup process. A new
> >> > > > > function WalRcvSetStreaming is introduced to enable the transition.
> >> > > >
> >> > > > The original patch set STREAMING in XLogWalRcvFlush(). XLogWalRcvFlush()
> >> > > > callee XLogWalRcvSendReply() already fetches applyPtr to send a status
> >> > > > message. So I would try the following before involving the startup process
> >> > > > like v2 does:
> >> > > >
> >> > > > 1. store the applyPtr when we enter CONNECTING
> >> > > > 2. force a status message as long as we remain in CONNECTING
> >> > > > 3. become STREAMING when applyPtr differs from the one stored at (1)
> >> > >
> >> > > Thanks for the suggestion. Using XLogWalRcvSendReply() for the
> >> > > transition could make sense. My concern before is about latency in a
> >> > > rare case: if the first flush completes but applyPtr hasn't advanced
> >> > > yet at the time of check and then the flush stalls after that, we
> >> > > might wait up to wal_receiver_status_interval (default 10s) before the
> >> > > next check or indefinitely if (wal_receiver_status_interval <= 0).
> >> > > This could be mitigated by shortening the wakeup interval while in
> >> > > CONNECTING (step 2), which reduces worst-case latency to ~1 second.
> >> > > Given that monitoring typically doesn't require sub-second precision,
> >> > > this approach could be feasible.
> >> > >
> >> > > case WALRCV_WAKEUP_REPLY:
> >> > > if (WalRcv->walRcvState == WALRCV_CONNECTING)
> >> > > {
> >> > > /* Poll frequently while CONNECTING to avoid long latency */
> >> > > wakeup[reason] = TimestampTzPlusMilliseconds(now, 1000);
> >> > > }
> >> > >
> >> > > > A possible issue with all patch versions: when the primary is writing no WAL
> >> > > > and the standby was caught up before this walreceiver started, CONNECTING
> >> > > > could persist for an unbounded amount of time. Only actual primary WAL
> >> > > > generation would move the walreceiver to STREAMING. This relates to your
> >> > > > above point about high latency. If that's a concern, perhaps this change
> >> > > > deserves a total of two new states, CONNECTING and a state that represents
> >> > > > "connection exists, no WAL yet applied"?
> >> > >
> >> > > Yes, this could be an issue. Using two states would help address it.
> >> > > That said, when the primary is idle in this case, we might end up
> >> > > repeatedly polling the apply status in the state before streaming if
> >> > > we implement the 1s short-interval checking like above, which could be
> >> > > costful. However, If we do not implement it &&
> >> > > wal_receiver_status_interval is set to < 0 && flush stalls, the
> >> > > walreceiver could stay in the pre-streaming state indefinitely even if
> >> > > streaming did occur, which violates the semantics. Do you think this
> >> > > is a valid concern or just an artificial edge case?
> >> >
> >> > After looking more closely, I found that true indefinite waiting
> >> > requires ALL of:
> >> >
> >> > wal_receiver_status_interval <= 0 (disables status updates)
> >> > wal_receiver_timeout <= 0
> >> > Primary sends no keepalives
> >> > No more WAL arrives after the first failed-check flush
> >> > Startup never sets force_reply
> >> >
> >> > which is quite impossible and artificial, sorry for the noise here.
> >>
> >> Even if indefinite wait is a negligible concern, you identified a lot of
> >> intricacy that I hadn't pictured. That makes your startup-process-driven
> >> version potentially more attractive. Forcing status messages like I was
> >> thinking may also yield an unwanted flurry of them if the startup process is
> >> slow. Let's see what the patch reviewer thinks.
> >
> >
> > FWIW, I think doing it in startup might be slightly better.
> > It seems more logical to make the state change near the point where the status
> > is updated, as this helps prevent reading the status from shared memory and
> > reduces related delays.
> >
> > The current proposal is to advance the state to STREAMING after applyPtr has
> > been updated.
> > IIUC, the rationale is to avoid having a short-lived streaming state if applying WAL fails.
> > However, this approach can be confusing because the receiver may already be receiving
> > WAL from the primary, yet its state remains CONNECTING until the WAL is flushed.
> >
> > Would it be better to advance the state to streaming after the connection
> > is successfully established and the following LOG message is emitted?
> >
> > if (walrcv_startstreaming(wrconn, &options))
> > {
> > if (first_stream)
> > ereport(LOG,
> > errmsg("started streaming WAL from primary at %X/%08X on timeline %u",
> > LSN_FORMAT_ARGS(startpoint), startpointTLI));
>
> AFAICS, this may depend on how we define the streaming status. If
> streaming is defined simply as “the connection has been established
> and walreceiver is ready to operate,” then this approach fits well and
> keeps the model simple. However, if streaming is meant to indicate
> that WAL has actually started flowing and replay is in progress, then
> this approach could fall short, particularly for the short-lived
> streaming cases you mentioned. Introducing finer-grained states can
> handle these edge cases more accurately, but it also makes the state
> transitions more complex. That said, I’m not well positioned to fully
> evaluate the trade-offs here, as I’m not a day-to-day end user.
>
After some thoughts, I’m more inclined toward a startup-process–driven
approach. Marking the status as streaming immediately after the
connection is established seems not provide sufficient accuracy for
monitoring purposes. Introducing an intermediate state, such as
connected, would help reduce confusion when the startup process is
stalled and would make it easier for users to detect and diagnose
anomalies.
V4 whitelisted CONNECTED and CONNECTING in WalRcvWaitForStartPosition
to handle valid stream termination scenarios without triggering a
FATAL error.
Specifically, the walreceiver may need to transition to WAITING (idle) if:
1. 'CONNECTED': The handshake succeeded (COPY_BOTH started), but
the stream ended before any WAL was applied (e.g., timeline divergence
detected mid-stream).
2. 'CONNECTING': The handshake completed (START_REPLICATION
acknowledged), but the primary declined to stream (e.g., no WAL
available on the requested timeline).
In both cases, the receiver should pause and await a new timeline or
restart position from the startup process.
Both approaches have been updated.
--
Best,
Xuneng
Вложения
On Sun, Jan 11, 2026 at 08:56:57PM +0800, Xuneng Zhou wrote: > After some thoughts, I’m more inclined toward a startup-process–driven > approach. Marking the status as streaming immediately after the > connection is established seems not provide sufficient accuracy for > monitoring purposes. Introducing an intermediate state, such as > connected, would help reduce confusion when the startup process is > stalled and would make it easier for users to detect and diagnose > anomalies. > > V4 whitelisted CONNECTED and CONNECTING in WalRcvWaitForStartPosition > to handle valid stream termination scenarios without triggering a > FATAL error. > > Specifically, the walreceiver may need to transition to WAITING (idle) if: > 1. 'CONNECTED': The handshake succeeded (COPY_BOTH started), but > the stream ended before any WAL was applied (e.g., timeline divergence > detected mid-stream). > 2. 'CONNECTING': The handshake completed (START_REPLICATION > acknowledged), but the primary declined to stream (e.g., no WAL > available on the requested timeline). > > In both cases, the receiver should pause and await a new timeline or > restart position from the startup process. This stuff depends on the philosophical difference you want to put behind "connecting" and "streaming". My own opinion is that it is a non-starter to introduce more states that can be set by the startup process, and that a new state should reflect what we do in the code. We already have some of that for in the start and stop phases because we want some ordering when the WAL receiver process is spawned and at shutdown. That's just a simple way to say that we should not rely on more static variables to control how to set one or more states, and I don't see why that's actually required here? initialApplyPtr and force_reply are what I see as potential recipes for more bugs in the long term, as showed in the first approach. The second patch, introducing a similar new complexity with walrcv_streaming_set, is no better in terms of complexity added. The main take that I can retrieve from this thread is that it may take time between the moment we begin a WAL receiver in WalReceiverMain(), where walRcvState is switched to WALRCV_STREAMING, and the moment we actually have established a connection, location where "first_stream = false" (which is just to track if a WAL receiver is restarting, actually) after walrcv_startstreaming() has returned true, so as far as I can see you would be happy enough with the addition of a single state called CONNECTING, set at the beginning of WalReceiverMain() instead of where STREAMING is set now. The same would sound kind of true for WalRcvWaitForStartPosition(), because we are not actively streaming yet, still we are marking the WAL receiver as streaming, so the current code feels like we are cheating as if we define "streaming" as a WAL receiver that has already done an active connection. We also want the WAL receiver to be killable by the startup process while in "connecting" or "streaming" more. Hence I would suggest something like the following guidelines: - Add only a CONNECTING state. Set this state where we switch the state to "streaming" now, aka the two locations in the tree now. - Switch to STREAMING once the connection has been established, as returned by walrcv_startstreaming(), because we are acknowledging *in the code* that we have started streaming successfully. - Update the docs to reflect the new state, because this state can show up in the system view pg_stat_wal_receiver. - I am not convinved by what we gain with a CONNECTED state, either. Drop it. - The fact that we'd want to switch the state once the startup process has acknowleged the reception of the first byte from the stream is already something we track in the WAL receiver, AFAIK. I think that there would be a point in expanding the SQL functions to report more states of the startup process, including the data received by the startup process, but we should not link that to the state of the WAL receiver. An extra reason to not do that: WAL receivers are not the only source feeding data to the startup process, we could have data pushed to pg_wal/, or archive commands/modules doing this job. -- Michael
Вложения
Hi Michael, On Mon, Jan 19, 2026 at 8:13 AM Michael Paquier <michael@paquier.xyz> wrote: > > On Sun, Jan 11, 2026 at 08:56:57PM +0800, Xuneng Zhou wrote: > > After some thoughts, I’m more inclined toward a startup-process–driven > > approach. Marking the status as streaming immediately after the > > connection is established seems not provide sufficient accuracy for > > monitoring purposes. Introducing an intermediate state, such as > > connected, would help reduce confusion when the startup process is > > stalled and would make it easier for users to detect and diagnose > > anomalies. > > > > V4 whitelisted CONNECTED and CONNECTING in WalRcvWaitForStartPosition > > to handle valid stream termination scenarios without triggering a > > FATAL error. > > > > Specifically, the walreceiver may need to transition to WAITING (idle) if: > > 1. 'CONNECTED': The handshake succeeded (COPY_BOTH started), but > > the stream ended before any WAL was applied (e.g., timeline divergence > > detected mid-stream). > > 2. 'CONNECTING': The handshake completed (START_REPLICATION > > acknowledged), but the primary declined to stream (e.g., no WAL > > available on the requested timeline). > > > > In both cases, the receiver should pause and await a new timeline or > > restart position from the startup process. > > This stuff depends on the philosophical difference you want to put > behind "connecting" and "streaming". My own opinion is that it is a > non-starter to introduce more states that can be set by the startup > process, and that a new state should reflect what we do in the code. > We already have some of that for in the start and stop phases because > we want some ordering when the WAL receiver process is spawned and at > shutdown. That's just a simple way to say that we should not rely on > more static variables to control how to set one or more states, and I > don't see why that's actually required here? initialApplyPtr and > force_reply are what I see as potential recipes for more bugs in the > long term, as showed in the first approach. The second patch, > introducing a similar new complexity with walrcv_streaming_set, is no > better in terms of complexity added. > The main take that I can retrieve from this thread is that it may take > time between the moment we begin a WAL receiver in WalReceiverMain(), > where walRcvState is switched to WALRCV_STREAMING, and the moment we > actually have established a connection, location where "first_stream = > false" (which is just to track if a WAL receiver is restarting, > actually) after walrcv_startstreaming() has returned true, so as far > as I can see you would be happy enough with the addition of a single > state called CONNECTING, set at the beginning of WalReceiverMain() > instead of where STREAMING is set now. The same would sound kind of > true for WalRcvWaitForStartPosition(), because we are not actively > streaming yet, still we are marking the WAL receiver as streaming, so > the current code feels like we are cheating as if we define > "streaming" as a WAL receiver that has already done an active > connection. We also want the WAL receiver to be killable by the > startup process while in "connecting" or "streaming" more. > > Hence I would suggest something like the following guidelines: > - Add only a CONNECTING state. Set this state where we switch the > state to "streaming" now, aka the two locations in the tree now. > - Switch to STREAMING once the connection has been established, as > returned by walrcv_startstreaming(), because we are acknowledging *in > the code* that we have started streaming successfully. > - Update the docs to reflect the new state, because this state can > show up in the system view pg_stat_wal_receiver. > - I am not convinved by what we gain with a CONNECTED state, either. > Drop it. > - The fact that we'd want to switch the state once the startup process > has acknowleged the reception of the first byte from the stream is > already something we track in the WAL receiver, AFAIK. Thank you for the detailed feedback. I agree with your analysis — the simpler approach seems preferable and should be sufficient in most cases. Tightly coupling the startup process with the WAL receiver to set state is not very ideal. I'll post v5 with the simplified walreceiver changes as you suggested shortly. > I think that > there would be a point in expanding the SQL functions to report more > states of the startup process, including the data received by the > startup process, but we should not link that to the state of the WAL > receiver. An extra reason to not do that: WAL receivers are not the > only source feeding data to the startup process, we could have data > pushed to pg_wal/, or archive commands/modules doing this job. +1. I'll prepare a separate patch to expose startup process metrics like pg_stat_get_wal_receiver does. This would complement pg_stat_wal_receiver without coupling the two subsystems. -- Best, Xuneng
On Mon, Jan 19, 2026 at 11:35:28PM +0800, Xuneng Zhou wrote: > On Mon, Jan 19, 2026 at 8:13 AM Michael Paquier <michael@paquier.xyz> wrote: >> I think that >> there would be a point in expanding the SQL functions to report more >> states of the startup process, including the data received by the >> startup process, but we should not link that to the state of the WAL >> receiver. An extra reason to not do that: WAL receivers are not the >> only source feeding data to the startup process, we could have data >> pushed to pg_wal/, or archive commands/modules doing this job. > > +1. I'll prepare a separate patch to expose startup process metrics > like pg_stat_get_wal_receiver does. This would complement > pg_stat_wal_receiver without coupling the two subsystems. In this area, I mean to expose the contents of XLogRecoveryCtlData at SQL level. It may be better to move this structure to a header, and have the new SQL function in xlogfuncs.c. That's at least how I would shape such a change. -- Michael
Вложения
Hi, On Tue, Jan 20, 2026 at 7:21 AM Michael Paquier <michael@paquier.xyz> wrote: > > On Mon, Jan 19, 2026 at 11:35:28PM +0800, Xuneng Zhou wrote: > > On Mon, Jan 19, 2026 at 8:13 AM Michael Paquier <michael@paquier.xyz> wrote: > >> I think that > >> there would be a point in expanding the SQL functions to report more > >> states of the startup process, including the data received by the > >> startup process, but we should not link that to the state of the WAL > >> receiver. An extra reason to not do that: WAL receivers are not the > >> only source feeding data to the startup process, we could have data > >> pushed to pg_wal/, or archive commands/modules doing this job. > > > > +1. I'll prepare a separate patch to expose startup process metrics > > like pg_stat_get_wal_receiver does. This would complement > > pg_stat_wal_receiver without coupling the two subsystems. > > In this area, I mean to expose the contents of XLogRecoveryCtlData at > SQL level. It may be better to move this structure to a header, and > have the new SQL function in xlogfuncs.c. That's at least how I would > shape such a change. > -- > Michael Thanks for the suggestion! It makes sense to me. -- Best, Xuneng
Hi, On Mon, Jan 19, 2026 at 11:35 PM Xuneng Zhou <xunengzhou@gmail.com> wrote: > > Hi Michael, > > On Mon, Jan 19, 2026 at 8:13 AM Michael Paquier <michael@paquier.xyz> wrote: > > > > On Sun, Jan 11, 2026 at 08:56:57PM +0800, Xuneng Zhou wrote: > > > After some thoughts, I’m more inclined toward a startup-process–driven > > > approach. Marking the status as streaming immediately after the > > > connection is established seems not provide sufficient accuracy for > > > monitoring purposes. Introducing an intermediate state, such as > > > connected, would help reduce confusion when the startup process is > > > stalled and would make it easier for users to detect and diagnose > > > anomalies. > > > > > > V4 whitelisted CONNECTED and CONNECTING in WalRcvWaitForStartPosition > > > to handle valid stream termination scenarios without triggering a > > > FATAL error. > > > > > > Specifically, the walreceiver may need to transition to WAITING (idle) if: > > > 1. 'CONNECTED': The handshake succeeded (COPY_BOTH started), but > > > the stream ended before any WAL was applied (e.g., timeline divergence > > > detected mid-stream). > > > 2. 'CONNECTING': The handshake completed (START_REPLICATION > > > acknowledged), but the primary declined to stream (e.g., no WAL > > > available on the requested timeline). > > > > > > In both cases, the receiver should pause and await a new timeline or > > > restart position from the startup process. > > > > This stuff depends on the philosophical difference you want to put > > behind "connecting" and "streaming". My own opinion is that it is a > > non-starter to introduce more states that can be set by the startup > > process, and that a new state should reflect what we do in the code. > > We already have some of that for in the start and stop phases because > > we want some ordering when the WAL receiver process is spawned and at > > shutdown. That's just a simple way to say that we should not rely on > > more static variables to control how to set one or more states, and I > > don't see why that's actually required here? initialApplyPtr and > > force_reply are what I see as potential recipes for more bugs in the > > long term, as showed in the first approach. The second patch, > > introducing a similar new complexity with walrcv_streaming_set, is no > > better in terms of complexity added. > > The main take that I can retrieve from this thread is that it may take > > time between the moment we begin a WAL receiver in WalReceiverMain(), > > where walRcvState is switched to WALRCV_STREAMING, and the moment we > > actually have established a connection, location where "first_stream = > > false" (which is just to track if a WAL receiver is restarting, > > actually) after walrcv_startstreaming() has returned true, so as far > > as I can see you would be happy enough with the addition of a single > > state called CONNECTING, set at the beginning of WalReceiverMain() > > instead of where STREAMING is set now. The same would sound kind of > > true for WalRcvWaitForStartPosition(), because we are not actively > > streaming yet, still we are marking the WAL receiver as streaming, so > > the current code feels like we are cheating as if we define > > "streaming" as a WAL receiver that has already done an active > > connection. We also want the WAL receiver to be killable by the > > startup process while in "connecting" or "streaming" more. > > > > Hence I would suggest something like the following guidelines: > > - Add only a CONNECTING state. Set this state where we switch the > > state to "streaming" now, aka the two locations in the tree now. > > - Switch to STREAMING once the connection has been established, as > > returned by walrcv_startstreaming(), because we are acknowledging *in > > the code* that we have started streaming successfully. > > - Update the docs to reflect the new state, because this state can > > show up in the system view pg_stat_wal_receiver. > > - I am not convinved by what we gain with a CONNECTED state, either. > > Drop it. > > - The fact that we'd want to switch the state once the startup process > > has acknowleged the reception of the first byte from the stream is > > already something we track in the WAL receiver, AFAIK. > > Thank you for the detailed feedback. I agree with your analysis — the > simpler approach seems preferable and should be sufficient in most > cases. Tightly coupling the startup process with the WAL receiver to > set state is not very ideal. I'll post v5 with the simplified > walreceiver changes as you suggested shortly. Please see v5 of the updated patch. -- Best, Xuneng
Вложения
On Wed, Jan 21, 2026 at 08:40:16PM +0800, Xuneng Zhou wrote: > Please see v5 of the updated patch. This seems acceptable here, cool. Something worth a note: I thought that the states of the WAL sender were documented already, and I was wrong in thinking that it was the case. Sorry. :) By the way, the list of state values you are specifying in the patch is not complete. In theory, we allow all state values like "stopped" to show up in a single function call of pg_stat_get_wal_receiver(), but you are not documenting all of them. Using a list (like with the <itemizedlist> markup) would be better. The documentation improvement can be treated as a separate change, worth its own before adding the new "connecting" state. Could you split that into two patches please? That would make one patch for the documentation changes with the list of state values that can be reported, and a second patch for the new "connecting" state. -- Michael
Вложения
Hi,
Thanks for the feedbacks.
On Thu, Jan 22, 2026 at 8:20 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Jan 21, 2026 at 08:40:16PM +0800, Xuneng Zhou wrote:
> > Please see v5 of the updated patch.
>
> This seems acceptable here, cool.
>
> Something worth a note: I thought that the states of the WAL sender
> were documented already, and I was wrong in thinking that it was the
> case. Sorry. :)
>
> By the way, the list of state values you are specifying in the patch
> is not complete. In theory, we allow all state values like "stopped"
> to show up in a single function call of pg_stat_get_wal_receiver(),
> but you are not documenting all of them.
My concern for not adding it before is that this status could be
invisible to users.
Looking at pg_stat_get_wal_receiver():
if (pid == 0 || !ready_to_display)
PG_RETURN_NULL();
The function returns NULL (no row) when pid == 0.
When the WAL receiver is in WALRCV_STOPPED state (WalRcvDie), pid is set to 0:
walrcv->walRcvState = WALRCV_STOPPED;
walrcv->pid = 0;
So the view returns no row rather than a row with status = 'stopped'.
But for completeness, maybe we should add it.
> Using a list (like with the
> <itemizedlist> markup) would be better.
The states are now wrapped in an itemizedlist.
> The documentation improvement can be treated as a separate change,
> worth its own before adding the new "connecting" state. Could you
> split that into two patches please? That would make one patch for
> the documentation changes with the list of state values that can be
> reported, and a second patch for the new "connecting" state.
The changes have been splitted into two patches as suggested.
--
Best,
Xuneng
Вложения
> On Jan 22, 2026, at 12:37, Xuneng Zhou <xunengzhou@gmail.com> wrote:
>
> Hi,
>
> Thanks for the feedbacks.
>
> On Thu, Jan 22, 2026 at 8:20 AM Michael Paquier <michael@paquier.xyz> wrote:
>>
>> On Wed, Jan 21, 2026 at 08:40:16PM +0800, Xuneng Zhou wrote:
>>> Please see v5 of the updated patch.
>>
>> This seems acceptable here, cool.
>>
>> Something worth a note: I thought that the states of the WAL sender
>> were documented already, and I was wrong in thinking that it was the
>> case. Sorry. :)
>>
>> By the way, the list of state values you are specifying in the patch
>> is not complete. In theory, we allow all state values like "stopped"
>> to show up in a single function call of pg_stat_get_wal_receiver(),
>> but you are not documenting all of them.
>
> My concern for not adding it before is that this status could be
> invisible to users.
> Looking at pg_stat_get_wal_receiver():
>
> if (pid == 0 || !ready_to_display)
> PG_RETURN_NULL();
>
> The function returns NULL (no row) when pid == 0.
>
> When the WAL receiver is in WALRCV_STOPPED state (WalRcvDie), pid is set to 0:
> walrcv->walRcvState = WALRCV_STOPPED;
> walrcv->pid = 0;
>
> So the view returns no row rather than a row with status = 'stopped'.
> But for completeness, maybe we should add it.
>
>> Using a list (like with the
>> <itemizedlist> markup) would be better.
>
> The states are now wrapped in an itemizedlist.
>
>> The documentation improvement can be treated as a separate change,
>> worth its own before adding the new "connecting" state. Could you
>> split that into two patches please? That would make one patch for
>> the documentation changes with the list of state values that can be
>> reported, and a second patch for the new "connecting" state.
>
> The changes have been splitted into two patches as suggested.
>
> --
> Best,
> Xuneng
>
<v6-0002-Add-WALRCV_CONNECTING-state-to-walreceiver.patch><v6-0001-Doc-document-all-pg_stat_wal_receiver-status-valu.patch>
Hi Xuneng,
I just reviewed the patch. It splits the current STREAMING state into CONNECTING and STREAMING, where CONNECTING
representsthe connection establishing phase, which makes sense. The patch is solid, I have just a few small comments:
1 - 0001
```
+ <para>
+ <literal>stopped</literal>: WAL receiver process is not running.
+ This state is normally not visible because the view returns no row
+ when the WAL receiver is not running.
+ </para>
```
“Is not running” appears twice in this short paragraph, looks redundant. Suggestion:
```
<literal>stopped</literal>: WAL receiver process is not running.
This state is normally not visible because the view returns no row in this case.
```
2 - 0002
```
+ WALRCV_CONNECTING, /* connecting to primary, replication not yet
+ * started */
```
I think “primary” is inaccurate. A connection destination could be a primary, or the other standby, so maybe change
“primary”to “upstream server”.
3 - 0002
```
* Mark walreceiver as running in shared memory.
```
I think we should update this comment, changing “running” to “connecting”. There is not a “running” state, before this
patch,“streaming” can roughly equal to “running”, after this patch, “running” is kinda unclear.
4 - 0002
```
+ /* Connection established, switch to streaming state */
+ SpinLockAcquire(&walrcv->mutex);
+ Assert(walrcv->walRcvState == WALRCV_CONNECTING);
+ walrcv->walRcvState = WALRCV_STREAMING;
+ SpinLockRelease(&walrcv->mutex);
```
I don’t feel good with this Assert(). Looking at ShutdownWalRcv(), the startup process might set the state to STOPPING,
sothere is a race.
Before this patch, this is no state change here, thus rest logic can handle STOPPING. After this patch, if the race
occurs,in dev mode, the Assert is fired; in production mode, STOPPING is overwritten by STREAMING, which is wrong.
So, instead of Assert(), I think we should check if current state is CONNECTING, if not, it should not proceed.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
On Thu, Jan 22, 2026 at 12:37:42PM +0800, Xuneng Zhou wrote: > So the view returns no row rather than a row with status = 'stopped'. > But for completeness, maybe we should add it. Yeah, you are making me doubt here, "stopped" is the only state that would never show up. After pondering a bit on this one, I have removed this part, and applied the rest of 0001 on HEAD after some reordering of the items and fixing a <para> markup which was at an incorrect location. That's one. -- Michael
Вложения
On Thu, Jan 22, 2026 at 03:34:32PM +0800, Chao Li wrote: > “Is not running” appears twice in this short paragraph, looks redundant. Suggestion: > ``` > <literal>stopped</literal>: WAL receiver process is not running. > This state is normally not visible because the view returns no row in this case. > ``` I have removed this part at the end, see 1a1e733b6231. Let's see later about the second episode of this saga. -- Michael
Вложения
Hi Chao, Thanks for looking into this. > > Hi Xuneng, > > I just reviewed the patch. It splits the current STREAMING state into CONNECTING and STREAMING, where CONNECTING representsthe connection establishing phase, which makes sense. The patch is solid, I have just a few small comments: > > 1 - 0001 > ``` > + <para> > + <literal>stopped</literal>: WAL receiver process is not running. > + This state is normally not visible because the view returns no row > + when the WAL receiver is not running. > + </para> > ``` > > “Is not running” appears twice in this short paragraph, looks redundant. Suggestion: > ``` > <literal>stopped</literal>: WAL receiver process is not running. > This state is normally not visible because the view returns no row in this case. > ``` > > 2 - 0002 > ``` > + WALRCV_CONNECTING, /* connecting to primary, replication not yet > + * started */ > ``` > > I think “primary” is inaccurate. A connection destination could be a primary, or the other standby, so maybe change “primary”to “upstream server”. Yeah, "upstream server" is more precise. There are multiple user-facing log messages in this file that also use “primary”. I’m wondering whether we should update those as well. > > > 3 - 0002 > ``` > * Mark walreceiver as running in shared memory. > ``` > > I think we should update this comment, changing “running” to “connecting”. There is not a “running” state, before thispatch, “streaming” can roughly equal to “running”, after this patch, “running” is kinda unclear. > It has been changed. > 4 - 0002 > ``` > + /* Connection established, switch to streaming state */ > + SpinLockAcquire(&walrcv->mutex); > + Assert(walrcv->walRcvState == WALRCV_CONNECTING); > + walrcv->walRcvState = WALRCV_STREAMING; > + SpinLockRelease(&walrcv->mutex); > ``` > > I don’t feel good with this Assert(). Looking at ShutdownWalRcv(), the startup process might set the state to STOPPING,so there is a race. > > Before this patch, this is no state change here, thus rest logic can handle STOPPING. After this patch, if the race occurs,in dev mode, the Assert is fired; in production mode, STOPPING is overwritten by STREAMING, which is wrong. > > So, instead of Assert(), I think we should check if current state is CONNECTING, if not, it should not proceed. It makes sense to me. Please check the updated patch. -- Best, Xuneng
Вложения
Hi, On Thu, Jan 22, 2026 at 4:08 PM Michael Paquier <michael@paquier.xyz> wrote: > > On Thu, Jan 22, 2026 at 12:37:42PM +0800, Xuneng Zhou wrote: > > So the view returns no row rather than a row with status = 'stopped'. > > But for completeness, maybe we should add it. > > Yeah, you are making me doubt here, "stopped" is the only state that > would never show up. After pondering a bit on this one, I have > removed this part, and applied the rest of 0001 on HEAD after some > reordering of the items and fixing a <para> markup which was at an > incorrect location. That's one. > -- > Michael Thanks for updating and pushing it. -- Best, Xuneng
On Thu, Jan 22, 2026 at 06:02:54PM +0800, Xuneng Zhou wrote:
>> Before this patch, this is no state change here, thus rest logic
>> can handle STOPPING. After this patch, if the race occurs, in dev
>> mode, the Assert is fired; in production mode, STOPPING is overwritten
>> by STREAMING, which is wrong.
>>
>> So, instead of Assert(), I think we should check if current state
>> is CONNECTING, if not, it should not proceed.
>
> It makes sense to me. Please check the updated patch.
+ if (state != WALRCV_CONNECTING)
+ {
+ if (state == WALRCV_STOPPING)
+ proc_exit(0);
+ elog(FATAL, "unexpected walreceiver state");
+ }
Sorry, but this addition does not make sense to me. Before this
patch, if the startup process decides to mark a WAL receiver as
stopping after it has been started in WalReceiverMain(), then we would
run at least one loop until we reach WalRcvWaitForStartPosition(),
which is the path where the WAL receiver exits. Aka, I don't think we
should increase the number of paths where we decide if a WAL receiver
should fast-exit or not (I'd rather try to reduce them, but I don't
think we can do that currently based on the ping-pong game between the
startup process and WAL receiver process). Saying that, you are right
about the fact that the assertion is wrong, and that we should not
upgrade the status to STREAMING if the WAL receiver is not CONNECTING.
So it seems to me that this code should remain as simple as followed,
documenting that we switch to STREAMING when the first connection has
been established after the WAL receiver has started, or when the WAL
receiver is switched after WalRcvWaitForStartPosition() once
startstreaming() has acknowledged that streaming is happening (I would
add a comment saying that):
if (state == WALRCV_CONNECTING)
walrcv->walRcvState = WALRCV_STREAMING;
--
Michael
Вложения
> On Jan 23, 2026, at 06:54, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Thu, Jan 22, 2026 at 06:02:54PM +0800, Xuneng Zhou wrote:
>>> Before this patch, this is no state change here, thus rest logic
>>> can handle STOPPING. After this patch, if the race occurs, in dev
>>> mode, the Assert is fired; in production mode, STOPPING is overwritten
>>> by STREAMING, which is wrong.
>>>
>>> So, instead of Assert(), I think we should check if current state
>>> is CONNECTING, if not, it should not proceed.
>>
>> It makes sense to me. Please check the updated patch.
>
> + if (state != WALRCV_CONNECTING)
> + {
> + if (state == WALRCV_STOPPING)
> + proc_exit(0);
> + elog(FATAL, "unexpected walreceiver state");
> + }
>
> Sorry, but this addition does not make sense to me. Before this
> patch, if the startup process decides to mark a WAL receiver as
> stopping after it has been started in WalReceiverMain(), then we would
> run at least one loop until we reach WalRcvWaitForStartPosition(),
> which is the path where the WAL receiver exits. Aka, I don't think we
> should increase the number of paths where we decide if a WAL receiver
> should fast-exit or not (I'd rather try to reduce them, but I don't
> think we can do that currently based on the ping-pong game between the
> startup process and WAL receiver process). Saying that, you are right
> about the fact that the assertion is wrong, and that we should not
> upgrade the status to STREAMING if the WAL receiver is not CONNECTING.
>
> So it seems to me that this code should remain as simple as followed,
> documenting that we switch to STREAMING when the first connection has
> been established after the WAL receiver has started, or when the WAL
> receiver is switched after WalRcvWaitForStartPosition() once
> startstreaming() has acknowledged that streaming is happening (I would
> add a comment saying that):
> if (state == WALRCV_CONNECTING)
> walrcv->walRcvState = WALRCV_STREAMING;
> --
> Michael
+1.
If current state is not CONNECTING, then leave it as is, which will then follow the original logic. Unless we find some
issuein future, I think we can take this approach for now.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Hi,
On Fri, Jan 23, 2026 at 6:54 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Thu, Jan 22, 2026 at 06:02:54PM +0800, Xuneng Zhou wrote:
> >> Before this patch, this is no state change here, thus rest logic
> >> can handle STOPPING. After this patch, if the race occurs, in dev
> >> mode, the Assert is fired; in production mode, STOPPING is overwritten
> >> by STREAMING, which is wrong.
> >>
> >> So, instead of Assert(), I think we should check if current state
> >> is CONNECTING, if not, it should not proceed.
> >
> > It makes sense to me. Please check the updated patch.
>
> + if (state != WALRCV_CONNECTING)
> + {
> + if (state == WALRCV_STOPPING)
> + proc_exit(0);
> + elog(FATAL, "unexpected walreceiver state");
> + }
>
> Sorry, but this addition does not make sense to me. Before this
> patch, if the startup process decides to mark a WAL receiver as
> stopping after it has been started in WalReceiverMain(), then we would
> run at least one loop until we reach WalRcvWaitForStartPosition(),
> which is the path where the WAL receiver exits. Aka, I don't think we
> should increase the number of paths where we decide if a WAL receiver
> should fast-exit or not (I'd rather try to reduce them, but I don't
> think we can do that currently based on the ping-pong game between the
> startup process and WAL receiver process). Saying that, you are right
> about the fact that the assertion is wrong, and that we should not
> upgrade the status to STREAMING if the WAL receiver is not CONNECTING.
>
> So it seems to me that this code should remain as simple as followed,
> documenting that we switch to STREAMING when the first connection has
> been established after the WAL receiver has started, or when the WAL
> receiver is switched after WalRcvWaitForStartPosition() once
> startstreaming() has acknowledged that streaming is happening (I would
> add a comment saying that):
> if (state == WALRCV_CONNECTING)
> walrcv->walRcvState = WALRCV_STREAMING;
> --
> Michael
Thanks Michael — agreed. Patch v7 dropped the extra fast‑exit path and
kept the change minimal: only switch to STREAMING if the state is
still CONNECTING, otherwise leave it unchanged.
--
Best,
Xuneng
Вложения
On Fri, Jan 23, 2026 at 12:07:36PM +0800, Xuneng Zhou wrote: > Thanks Michael — agreed. Patch v7 dropped the extra fast‑exit path and > kept the change minimal: only switch to STREAMING if the state is > still CONNECTING, otherwise leave it unchanged. Thanks for the newer version of the patch. I have been playing with it today, even before you have sent this v7, with a set of injection points to make the WAL receiver wait at some of its steps, then double-checked that the startup process was able to control the WAL receiver as it should, flipping primary_conninfo with reloads. AFAIK as I can see, that felt OK, so applied as a36164e7465f. -- Michael
Вложения
Hi, On Fri, Jan 23, 2026 at 1:31 PM Michael Paquier <michael@paquier.xyz> wrote: > > On Fri, Jan 23, 2026 at 12:07:36PM +0800, Xuneng Zhou wrote: > > Thanks Michael — agreed. Patch v7 dropped the extra fast‑exit path and > > kept the change minimal: only switch to STREAMING if the state is > > still CONNECTING, otherwise leave it unchanged. > > Thanks for the newer version of the patch. I have been playing with > it today, even before you have sent this v7, with a set of injection > points to make the WAL receiver wait at some of its steps, then > double-checked that the startup process was able to control the WAL > receiver as it should, flipping primary_conninfo with reloads. AFAIK > as I can see, that felt OK, so applied as a36164e7465f. > -- > Michael Thanks for checking and applying it. I’m playing with exposing XLogRecoveryCtlData metrics at the SQL level, following your input. I’ll post the patches and possibly start a new thread for discussion. -- Best, Xuneng
On Fri, Jan 23, 2026 at 04:47:43PM +0800, Xuneng Zhou wrote: > I’ll post the patches and possibly start a new thread for discussion. Thanks for all that. I'd suggest posting the startup proposal into an entirely new thread for clarity. -- Michael
Вложения
Hi Michael, On Sun, Jan 25, 2026 at 9:26 AM Michael Paquier <michael@paquier.xyz> wrote: > > On Fri, Jan 23, 2026 at 04:47:43PM +0800, Xuneng Zhou wrote: > > I’ll post the patches and possibly start a new thread for discussion. > > Thanks for all that. I'd suggest posting the startup proposal into an > entirely new thread for clarity. > -- > Michael I have started a new thread [1] and posted the patches there, as you suggested. [1] https://www.postgresql.org/message-id/CABPTF7W+Nody-+P9y4PNk37-QWuLpfUrEonHuEhrX+Vx9Kq+Kw@mail.gmail.com -- Best, Xuneng