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. -- Regrads, Japin Li. ChengDu WenWu Information Technology Co.,Ltd.
