On Wednesday, April 14, 2021 11:38 AM Japin Li <japi...@hotmail.com> wrote: > On Tue, 13 Apr 2021 at 21:54, osumi.takami...@fujitsu.com > <osumi.takami...@fujitsu.com> wrote: > > On Monday, April 12, 2021 3:58 PM Amit Kapila <amit.kapil...@gmail.com> > wrote: > >> On Mon, Apr 12, 2021 at 10:03 AM osumi.takami...@fujitsu.com > >> <osumi.takami...@fujitsu.com> wrote: > >> > but if we take a measure to fix the doc, we have to be careful for > >> > the description, because when we remove the primary keys of 'test' > >> > tables on the > >> scenario in [1], we don't have this issue. > >> > It means TRUNCATE in synchronous logical replication is not always > >> blocked. > >> > > >> > >> The problem happens only when we try to fetch IDENTITY_KEY attributes > >> because pgoutput uses RelationGetIndexAttrBitmap() to get that > >> information which locks the required indexes. Now, because TRUNCATE > >> has already acquired an exclusive lock on the index, it seems to > >> create a sort of deadlock where the actual Truncate operation waits > >> for logical replication of operation to complete and logical replication > >> waits > for actual Truncate operation to finish. > >> > >> Do we really need to use RelationGetIndexAttrBitmap() to build > >> IDENTITY_KEY attributes? During decoding, we don't even lock the main > >> relation, we just scan the system table and build that information > >> using a historic snapshot. Can't we do something similar here? > > I think we can build the IDENTITY_KEY attributes with NoLock instead > > of calling RelationGetIndexAttrBitmap(). > > > > When we trace back the caller side of logicalrep_write_attrs(), doing > > the thing equivalent to RelationGetIndexAttrBitmap() for > > INDEX_ATTR_BITMAP_IDENTITY_KEY impacts only pgoutput_truncate. > > > > OTOH, I can't find codes similar to RelationGetIndexAttrBitmap() in > > pgoutput_* functions and in the file of relcache.c. > > Therefore, I'd like to discuss how to address the hang. > > > > My first idea is to extract some parts of RelationGetIndexAttrBitmap() > > only for INDEX_ATTR_BITMAP_IDENTITY_KEY and implement those either > in > > a logicalrep_write_attrs() or as a new function. > > RelationGetIndexAttrBitmap() has 'restart' label for goto statement in > > order to ensure to return up-to-date attribute bitmaps, so I prefer > > having a new function when we choose this direction. > > Having that goto in logicalrep_write_attrs() makes it a little bit messy, I > > felt. > > > > The other direction might be to extend RelationGetIndexAttrBitmap's > > function definition to accept lockmode to give NoLock from > logicalrep_write_attrs(). > > But, this change impacts on other several callers so is not as good as the > > first > direction above, I think. > > > > If someone has any better idea, please let me know. > > > > I think the first idea is better than the second. OTOH, can we release the > locks > before SyncRepWaitForLSN(), since it already flush to local WAL files. Thank you for your comments. I didn't mean to change and touch TRUNCATE side to release the locks, because I expected that its AccessExclusiveLock protects any other operation (e.g. DROP INDEX) to the table which affects IDENTITY KEY building. But, now as I said in another e-mail, both ideas above can't work. Really sorry for making noises.
Best Regards, Takamichi Osumi