Message-ID: <1903ca9b-1dc9-20a6-404a-d4c460c7bdcc@gmail.com>
Date: Thu, 6 Apr 2023 15:02:00 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.9.1
Subject: Re: Minimal logical decoding on standbys
Content-Language: en-US
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Jeff Davis <pgsql@j-davis.com>,
 Andres Freund <andres@anarazel.de>, Robert Haas <robertmhaas@gmail.com>,
 Thomas Munro <thomas.munro@gmail.com>, Ibrar Ahmed <ibrar.ahmad@gmail.com>,
 Amit Khandekar <amitdkhan.pg@gmail.com>, fabriziomello@gmail.com,
 tushar <tushar.ahuja@enterprisedb.com>,
 Rahila Syed <rahila.syed@2ndquadrant.com>,
 pgsql-hackers <pgsql-hackers@postgresql.org>
References: <ba5aa952-ef8c-b862-69df-0ad9e5d19d23@gmail.com>
 <20230330095449.47yz2q4mn27t6xgl@alvherre.pgsql>
 <CAA4eK1JX-HAmm3TuY=wCSgom+hmiKtkf_xaJLRX9JBo-iXUxHw@mail.gmail.com>
 <CAA4eK1+Tj3B+ODmdRhz7F3fnhang9qEUQatknMuZdi91e=QXeA@mail.gmail.com>
 <68a47e4c-f4bb-45ae-28be-938af60b203c@gmail.com>
 <CAA4eK1L_Zc+7cjm0OhktiPT5xn3=yXn6oenLuEyvJY_CM6T6Mw@mail.gmail.com>
 <e34ab1df-573b-20c1-664a-697851fc736d@gmail.com>
 <CAA4eK1+aUMLu3Q-y6A7nX6kfg_0_6zcRSevWK6Sx6kY1VegCBQ@mail.gmail.com>
 <8c6e7670-3914-35d1-7977-34f60e598477@gmail.com>
 <CAA4eK1+v27a+LONLuRwnzevkp5RuPgsey2VkFB1cfOq2R3XZrA@mail.gmail.com>
 <CAA4eK1+-pdrx3rRD1QCdKcQRaY+B+T3njEXgC3VC3eRAKmDhog@mail.gmail.com>
From: "Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>
In-Reply-To: 
 <CAA4eK1+-pdrx3rRD1QCdKcQRaY+B+T3njEXgC3VC3eRAKmDhog@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Archived-At: 
 <https://www.postgresql.org/message-id/1903ca9b-1dc9-20a6-404a-d4c460c7bdcc%40gmail.com>
Precedence: bulk

Hi,

On 4/6/23 11:55 AM, Amit Kapila wrote:
> On Thu, Apr 6, 2023 at 12:10 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>>
>> On Wed, Apr 5, 2023 at 9:27 PM Drouvot, Bertrand
>> <bertranddrouvot.pg@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?

If so, I think it has to be set to true for the logical walsenders in all the case (like
done in V62 posted up-thread).

Andres, made the point up-thread that RecoveryInProgress() is always true, and
as we don't want to be woken up only when there is a time line change then I think
it has to be always true for logical walsenders.

> 0005
> 2.
> --- a/src/backend/access/transam/xlogfuncs.c
> +++ b/src/backend/access/transam/xlogfuncs.c
> @@ -38,6 +38,7 @@
>   #include "utils/pg_lsn.h"
>   #include "utils/timestamp.h"
>   #include "utils/tuplestore.h"
> +#include "storage/standby.h"
> 
> The header includes should be in alphabetical order.
> 

Good catch, thanks! Done in V62.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com