public inbox for [email protected]
help / color / mirror / Atom feedFrom: [email protected] <[email protected]>
To: Önder Kalacı <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Marco Slot <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Amit Kapila <[email protected]>
Subject: RE: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
Date: Fri, 3 Mar 2023 03:40:31 +0000
Message-ID: <OS0PR01MB5716BE4954A99EAF14F4D1F294B39@OS0PR01MB5716.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CACawEhVk9_eSfbyEB-ej2N4FH2ZS7zc4mwXRGQWxA5LbKD+xww@mail.gmail.com>
References: <OSZPR01MB63105BD4B03B3C1FBFCC7E6CFD249@OSZPR01MB6310.jpnprd01.prod.outlook.com>
<CACawEhWdoiaXwKjn5ZfAKF_Toz=t1aqT1ewHmJLRnXmKOQKwyg@mail.gmail.com>
<OS3PR01MB62754E945BD95A9B2A1164789E289@OS3PR01MB6275.jpnprd01.prod.outlook.com>
<CACawEhXBtt9aMoU0j6funj-s+CW+e8HMFCGz30gyEwLazXB_1w@mail.gmail.com>
<OSZPR01MB63109FF52120649EECACC7AEFD2A9@OSZPR01MB6310.jpnprd01.prod.outlook.com>
<CACawEhUUHcSVhp-bODSAnYNUQHY=4gvY29DXM=yuphgrkcqqmQ@mail.gmail.com>
<CACawEhX6TvX+j8EpcpCKvnMGao8Gcp8W43Sgc87pg9o6-Xbf2Q@mail.gmail.com>
<[email protected]>
<CACawEhV68Ry7BGJV150xsUskJPa88rpOf3zo_OcA_MpSWGQobQ@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<CAFMSG9E7_JgnaBj1f_jU4dEmOnL=K6UyBAPR7eZ0NukUcR2L9g@mail.gmail.com>
<CACawEhUxEfVr64ywC0sj7nDwFg=vR30qehNoqSDcx6VV2=B_Lw@mail.gmail.com>
<CAA4eK1L6P8hM+17fNyWogSnueTJebvZUX7YseL54HSFpX_0m_A@mail.gmail.com>
<CACawEhUN=+vjY0+4q416-rAYx6pw-nZMHQYsJZCftf9MjoPN3w@mail.gmail.com>
<CAA4eK1LhQWmFDvFHpO6yR-E4iL4ekUuSRwTw44dH9rMvpdu_hA@mail.gmail.com>
<OSZPR01MB63103A4AFBBA56BAF8AE7FAAFDA39@OSZPR01MB6310.jpnprd01.prod.outlook.com>
<CAA4eK1JOq0_r=LwYcqZ+8wufyg9NAeJBSCPbavmGES512pOeAQ@mail.gmail.com>
<CACawEhV8X88dDxL+LYs9X=qGh+en1nVjNSevXcQPVuSnN+d2Pg@mail.gmail.com>
<CACawEhUU8MgaCCLeLmcZe_XkOYnLEg9myWbsRZTogp_R8Jx02Q@mail.gmail.com>
<CAA4eK1LRqn=-X2rDS4T_-UyiwdEnfSjz+2qRLoBJ6sMTaYga3A@mail.gmail.com>
<CACawEhUQjxYU=UAamdnfaJU0uGJFXzhxrOT-km343kZYpeCGfw@mail.gmail.com>
<CAA4eK1LPVqhuUkbHbCXDuQAvCvLA_qeCmg3WkOA3ZKBx1sh8dQ@mail.gmail.com>
<CACawEhVk9_eSfbyEB-ej2N4FH2ZS7zc4mwXRGQWxA5LbKD+xww@mail.gmail.com>
On Thursday, March 2, 2023 11:23 PM Önder Kalacı <[email protected]> wrote:
> Both the patches are numbered 0001. It would be better to number them
> as 0001 and 0002.
>
> Alright, attached v27_0001_use_index_on_subs_when_pub_rep_ident_full.patch and
> v27_0002_use_index_on_subs_when_pub_rep_ident_full.patch.
>
> I also added one more test which Andres asked me on a private chat
> (Testcase start: SUBSCRIPTION USES INDEX WITH PUB/SUB different data).
Thanks for updating the patch. I think this patch can bring noticeable
performance improvements in some use cases.
And here are few comments after reading the patch.
1.
+ usableIndexContext = AllocSetContextCreate(CurrentMemoryContext,
+ "usableIndexContext",
+ ALLOCSET_DEFAULT_SIZES);
+ oldctx = MemoryContextSwitchTo(usableIndexContext);
+
+ /* Get index list of the local relation */
+ indexlist = RelationGetIndexList(localrel);
+ Assert(indexlist != NIL);
+
+ foreach(lc, indexlist)
Is it necessary to create a memory context here ? I thought the memory will be
freed after this apply action soon.
2.
+ /*
+ * Furthermore, because primary key and unique key indexes can't
+ * include expressions we also sanity check the index is neither
+ * of those kinds.
+ */
+ Assert(!IdxIsRelationIdentityOrPK(rel, idxrel->rd_id));
It seems you mean "replica identity key" instead of "unique key" in the comments.
3.
--- a/src/include/replication/logicalrelation.h
+++ b/src/include/replication/logicalrelation.h
...
+extern bool IsIndexOnlyOnExpression(IndexInfo *indexInfo);
The definition function seems better to be placed in execReplication.c
4.
+extern Oid GetRelationIdentityOrPK(Relation rel);
The function is only used in relation.c, so we can make it a static
function.
5.
+ /*
+ * If index scans are disabled, use a sequential scan.
+ *
+ * Note that we do not use index scans below when enable_indexscan is
+ * false. Allowing primary key or replica identity even when index scan is
+ * disabled is the legacy behaviour. So we hesitate to move the below
+ * enable_indexscan check to be done earlier in this function.
+ */
+ if (!enable_indexscan)
+ return InvalidOid;
Since the document of enable_indexscan says "Enables or disables the query
planner's use of index-scan plan types. The default is on.", and we don't use
planner here, so I am not sure should we allow/disallow index scan in apply
worker based on this GUC.
Best Regards,
Hou zj
view thread (49+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: RE: [PATCH] Use indexes on the subscriber when REPLICA IDENTITY is full on the publisher
In-Reply-To: <OS0PR01MB5716BE4954A99EAF14F4D1F294B39@OS0PR01MB5716.jpnprd01.prod.outlook.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox