Horton

RE: BUG #18815: Logical replication worker Segmentation fault

Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>

View thread
From: "Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Sergey Belyashov <sergey.belyashov@gmail.com>, Tomas Vondra <tomas@vondra.me>, Nisha Moond <nisha.moond412@gmail.com>, "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Date: 2025-04-08T06:56:25Z
Lists: pgsql-bugs, pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix uninitialized index information access during apply.

  2. Assert that ExecOpenIndices and ExecCloseIndices are not repeated.

  3. Fix crash in brininsertcleanup during logical replication.

Attachments

On Tue, Apr 8, 2025 at 2:00 PM Amit Kapila wrote:
> 
> On Mon, Apr 7, 2025 at 3:28 PM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>
> wrote:
> >
> > > What is the
> > > behavior of conflict reporting code in case of exclusion constraints?
> >
> > Under logical replication context, since we do not detect conflicts
> > for exclusion constraints, it would simply report the original constraint
> violation ERROR.
> >
> 
> Fair enough. On considering it again, I find your idea of building conflict-related
> information when it is actually required sounds better, as it may also save us
> performance in some corner cases.

Thanks for reviewing.

After studying more on this alternative, I think it can be improved further.
Specifically, we can delay index info initialization until the
FindConflictTuple() function, where the index info is actually used. This
can minimize overhead associated with building index info when no conflict
exists.

I generated a profile on the HEAD to check the impact of
BuildSpeculativeIndexInfo() during bulk insert operations (without conflicts),
and the results were notable:

--14.68%--BuildSpeculativeIndexInfo
          |--5.34%--IndexAmTranslateCompareType 
          |           --5.05%--GetIndexAmRoutineByAmId
          |                     |--2.26%--GetIndexAmRoutine
          |                     |--1.68%--SearchSysCache1
          |                      --0.52%--ReleaseSysCache
          |--5.29%--get_opfamily_member
          |          |--4.06%--SearchSysCache4
          |           --0.50%--ReleaseSysCache
          |          
          |--2.66%--get_opcode
          |          |--1.74%--SearchSysCache1

And this function disappeared after delaying the init to FindConflictTuple().

To further analyze performance, I measured the time taken to apply a
transaction that inserts 1,000,000 rows into a table with three unique indexes:

HEAD: 6267 ms
Improved alternative: 5593 ms

It shows about 11% performance improvement with the refined approach.

I am attaching the V2 patch which implements this idea by
postponing index info initialization until the FindConflictTuple(). I confirmed
It can pass regression and pgindent check.

Best Regards,
Hou zj