On Sat, Dec 24, 2022 at 12:58 AM David Christensen
<david.christensen@crunchydata.com> wrote:
>
> On Fri, Dec 23, 2022 at 12:57 PM David Christensen
> <david.christensen@crunchydata.com> wrote:
> >
> > On Wed, Dec 21, 2022 at 5:47 AM Bharath Rupireddy
> > <bharath.rupireddyforpostgres@gmail.com> wrote:
>
> > > 2. +$node->init(extra => ['-k'], allows_streaming => 1);
> > > When enabled with allows_streaming, there are a bunch of things that
> > > happen to the node while initializing, I don't think we need all of
> > > them for this.
> >
> > I think the "allows_streaming" was required to ensure the WAL files
> > were preserved properly, and was the approach we ended up taking
> > rather than trying to fail the archive_command or other approaches I'd
> > taken earlier. I'd rather keep this if we can, unless you can propose
> > a different approach that would continue to work in the same way.
>
> Confirmed that we needed this in order to create the replication slot,
> so this /is/ required for the test to work.
The added test needs wal_level to be replica, but the TAP tests set it
to minimal if allows_streaming isn't passed. However, if passed
allows_streaming, it sets a bunch of other parameters which are not
required for this test (see note->init function in cluster.pm), hence
we could just set the required parameters wal_level = replica and
max_wal_senders for the replication slot to be created.
> Enclosing v11 with yours and Michael's latest feedback.
Thanks for the patch. I've made the above change as well as renamed
the test file name to be save_fpi.pl, everything else remains the same
as v11. Here's the v12 patch which LGTM. I'll mark it as RfC -
https://commitfest.postgresql.org/41/3628/.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
