Message-ID: <0d8fe359-e6aa-4997-7e61-19d3dba360e0@amazon.com>
Date: Thu, 30 Jun 2022 10:49:29 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.11.0
From: "Drouvot, Bertrand" <bdrouvot@amazon.com>
Subject: Re: Minimal logical decoding on standbys
To: Andres Freund <andres@anarazel.de>, Robert Haas <robertmhaas@gmail.com>
CC: Alvaro Herrera <alvherre@2ndquadrant.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: <202107281526.o74ieuj6sj7z@alvherre.pgsql>
 <ae1f2199-65e7-a431-d79e-c32b9379aa6e@amazon.com>
 <b8cfc33a-36f0-a247-1c61-13500ef5ab0a@amazon.com>
 <2b14d595-1e8b-e5b6-6098-6a11d19fa1c7@amazon.com>
 <688cbedf-a172-f0d8-8352-d4ea223bceeb@amazon.com>
 <CA+TgmobgOLH-JpBoBSdu4i+sjRdgwmDEZGECkmowXqQgQL6WhQ@mail.gmail.com>
 <20211028210755.afmwcvpo6ajwdx6n@alap3.anarazel.de>
 <efb77c85-9263-1889-4593-0988ad23fd8c@amazon.com>
Content-Language: en-US
In-Reply-To: <efb77c85-9263-1889-4593-0988ad23fd8c@amazon.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/0d8fe359-e6aa-4997-7e61-19d3dba360e0%40amazon.com>
Precedence: bulk

Hi,

On 2/25/22 10:34 AM, Drouvot, Bertrand wrote:
> Hi,
>
> On 10/28/21 11:07 PM, Andres Freund wrote:
>> Hi,
>>
>> On 2021-10-28 16:24:22 -0400, Robert Haas wrote:
>>> On Wed, Oct 27, 2021 at 2:56 AM Drouvot, Bertrand 
>>> <bdrouvot@amazon.com> wrote:
>>>> So you have in mind to check for XLogLogicalInfoActive() first, and 
>>>> if true, then open the relation and call
>>>> RelationIsAccessibleInLogicalDecoding()?
>>> I think 0001 is utterly unacceptable. We cannot add calls to
>>> table_open() in low-level functions like this. Suppose for example
>>> that _bt_getbuf() calls _bt_log_reuse_page() which with 0001 applied
>>> would call get_rel_logical_catalog(). _bt_getbuf() will have acquired
>>> a buffer lock on the page. The idea that it's safe to call
>>> table_open() while holding a buffer lock cannot be taken seriously.
>> Yes - that's pretty clearly a deadlock hazard. It shouldn't too hard 
>> to fix, I
>> think. Possibly a bit more verbose than nice, but...
>>
>> Alternatively we could propagate the information whether a relcache 
>> entry is
>> for a catalog from the table to the index. Then we'd not need to 
>> change the
>> btree code to pass the table down.
>
> +1 for the idea of propagating to the index. If that sounds good to 
> you too, I can try to have a look at it.
>
> Thanks Robert and Andres for the feedbacks you have done on the 
> various sub-patches.
>
> I've now in mind to work sub patch by sub patch (starting with 0001 
> then) and move to the next one once we agree that the current one is 
> "ready".
>
> I think that could help us to get this new feature moving forward more 
> "easily", what do you think?
>
> Thanks
>
> Bertrand
>
I'm going to re-create a CF entry for it, as:

- It seems there is a clear interest for the feature (given the time 
already spend on it and the number of people that worked on)

- I've in mind to resume working on it

- It would give more visibility in case others want to jump in

Hope that makes sense,

Thanks,

Bertrand