On Thu, Apr 6, 2023 at 6:32 PM Drouvot, Bertrand
<bertranddrouvot...@gmail.com> wrote:
>
> Hi,
>
> On 4/6/23 11:55 AM, Amit Kapila wrote:
> > On Thu, Apr 6, 2023 at 12:10 PM Amit Kapila <amit.kapil...@gmail.com> wrote:
> >>
> >> On Wed, Apr 5, 2023 at 9:27 PM Drouvot, Bertrand
> >> <bertranddrouvot...@gmail.com> wrote:
> >>>
> >>
> >> Another comment on 0001.
> >> extern void CheckSlotRequirements(void);
> >> extern void CheckSlotPermissions(void);
> >> +extern void ResolveRecoveryConflictWithLogicalSlots(Oid dboid,
> >> TransactionId xid, char *reason);
> >>
> >> This doesn't seem to be called from anywhere.
> >>
> >
> > Few other comments:
> > ==================
> > 0004
> > 1.
> > + * - physical walsenders in case of new time line and cascade
> > + * replication is allowed.
> > + * - logical walsenders in case of new time line or recovery is in
> > progress
> > + * (logical decoding on standby).
> > + */
> > + WalSndWakeup(switchedTLI && AllowCascadeReplication(),
> > + switchedTLI || RecoveryInProgress());
> >
> > Do we need AllowCascadeReplication() check specifically for physical
> > walsenders? I think this should be true for both physical and logical
> > walsenders.
> >
>
> I don't think it could be possible to create logical walsenders on a standby
> if
> AllowCascadeReplication() is not true, or am I missing something?
>
Right, so why to even traverse walsenders for that case? What I was
imagining a code is like:
if (AllowCascadeReplication())
WalSndWakeup(switchedTLI, true);
Do you see any problem with this change?
Few more minor comments on 0005
=============================
0005
1.
+ <para>
+ Take a snapshot of running transactions and write this to WAL without
+ having to wait bgwriter or checkpointer to log one.
/wait bgwriter/wait for bgwriter
2.
+use Test::More tests => 67;
We no more use the number of tests. Please refer to other similar tests.
--
With Regards,
Amit Kapila.
