Re: Minimal logical decoding on standbys - Mailing list pgsql-hackers
From | Drouvot, Bertrand |
---|---|
Subject | Re: Minimal logical decoding on standbys |
Date | |
Msg-id | ad9e7cb2-92ac-04b8-537c-c325b358e2c6@gmail.com Whole thread Raw |
In response to | Re: Minimal logical decoding on standbys ("Drouvot, Bertrand" <bertranddrouvot.pg@gmail.com>) |
Responses |
Re: Minimal logical decoding on standbys
|
List | pgsql-hackers |
Hi, On 9/30/22 2:11 PM, Drouvot, Bertrand wrote: > Hi, > > On 7/6/22 3:30 PM, 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. >> >> Looking closer at RelationIsAccessibleInLogicalDecoding() It seems to me that the missing part to be able to tell whetheror not an index is for a catalog is the rd_options->user_catalog_table value of its related heap relation. >> >> Then, a way to achieve that could be to: >> >> - Add to Relation a new "heap_rd_options" representing the rd_options of the related heap relation when appropriate >> >> - Trigger the related indexes relcache invalidations when an ATExecSetRelOptions() is triggered on a heap relation >> >> - Write an equivalent of RelationIsUsedAsCatalogTable() for indexes that would make use of the heap_rd_options instead >> >> Does that sound like a valid option to you or do you have another idea in mind to propagate the information whether arelcache entry is for a catalog from the table to the index? >> > > I ended up with the attached proposal to propagate the catalog information to the indexes. > > The attached adds a new field "isusercatalog" in pg_index to indicate whether or not the index is linked to a table thathas the storage parameter user_catalog_table set to true. > > Then it defines new macros, including "IndexIsAccessibleInLogicalDecoding" making use of this new field. > > This new macro replaces get_rel_logical_catalog() that was part of the previous patch version. > > What do you think about this approach and the attached? > > If that sounds reasonable, then I'll add tap tests for it and try to improve the way isusercatalog is propagated to theindex(es) in case a reset is done on user_catalog_table on the table (currently in this POC patch, it's hardcoded to "false"which is the default value for user_catalog_table in boolRelOpts[]) (A better approach would be probably to retrievethe value from the table once the reset is done and then propagate it to the index(es).) Please find attached a rebase to propagate the catalog information to the indexes. It also takes care of the RESET on user_catalog_table (adding a new Macro "HEAP_DEFAULT_USER_CATALOG_TABLE") and adds a fewtests in contrib/test_decoding/sql/ddl.sql. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
Attachment
pgsql-hackers by date: