Re: pg_receivewal starting position

On Thu, Sep 02, 2021 at 02:45:54PM +0900, Kyotaro Horiguchi wrote:
> At Wed, 1 Sep 2021 10:30:05 +0530, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote in
> If I read the patch correctly the situation above is warned by the
> following message then continue to the next step giving up to identify
> start position from slot data.

Better to fallback to the past behavior if attempting to use a
pg_receivewal >= 15 with a PG instance older than 14.

>> "server does not suport fetching the slot's position, resuming from the current server position instead"
>
> (The message looks a bit too long, though..)

Agreed. Falling back to a warning is not the best answer we can have
here, as there could be various failure types, and for some of them a
hard failure is adapted;
- Failure in the backend while running READ_REPLICATION_SLOT. This
should imply a hard failure, no?
- Slot that does not exist. In this case, we could fall back to the
current write position of the server.

by default if the slot information cannot be retrieved.
Something that's disturbing me in patch 0002 is that we would ignore
the results of GetSlotInformation() if any error happens, even if
there is a problem in the backend, like an OOM. We should be careful
about the semantics here.

> However, if the operator doesn't know the server is old, pg_receivewal
> starts streaming from unexpected position, which is a kind of
> disaster. So I'm inclined to agree to Bharath, but rather I imagine of
> an option to explicitly specify how to determine the start position.
>
> --start-source=[server,wal,slot] specify starting-LSN source, default is
> trying all of them in the order of wal, slot, server.
>
> I don't think the option doesn't need to accept multiple values at once.

What is the difference between "wal" and "server"? "wal" stands for
the start position of the set of files stored in the location
directory, and "server" is the location that we'd receive from the
server? I don't think that we need that because, when using a slot,
we know that we can rely on the LSN that the slot retains for
pg_receivewal as that should be the same point as what has been
streamed last. Could there be an argument instead for changing the
default and rely on the slot information rather than scanning the
local WAL archive path for the start point when using --slot? When
using pg_receivewal as a service, relying on a scan of the WAL archive
directory if there is no slot and fallback to an invalid LSN if there
is nothing is fine by me, but I think that just relying on the slot
information is saner as the backend makes sure that nothing is
missing. That's also more useful when streaming changes from a single
slot from multiple locations (stream to location 1 with a slot, stop
pg_receivewal, stream to location 2 that completes 1 with the same
slot).

+ pg_log_error("Slot \"%s\" is not a physical replication slot",
+ replication_slot);
In 0003, the format of this error is not really project-like.
Something like that perhaps would be more adapted:
"cannot use the slot provided, physical slot expected."

I am not really convinced about the need of getting the active state
and the PID used in the backend when fetcing the slot data,
particularly if that's just for some frontend-side checks. The
backend has safeguards already for all that.

While looking at that, I have applied de1d4fe to add
PostgresNode::command_fails_like(), coming from 0003, and put my hands
on 0001 as per the attached, as the starting point. That basically
comes down to all the points raised upthread, plus some tweaks for
things I bumped into to get the semantics of the command to what looks
like the right shape.
--
Michael