| From: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> |
|---|---|
| To: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Add tests for concurrent DML retry paths in logical replication apply |
| Date: | 2026-04-29 01:45:00 |
| Message-ID: | CALj2ACWLpxQ=z-oO6QNKW6PiT9A-dS6mYKga+m8=+sko=ggr+w@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Tue, Apr 28, 2026 at 12:47 AM Hayato Kuroda (Fujitsu)
<kuroda(dot)hayato(at)fujitsu(dot)com> wrote:
>
> > While reading the logical replication apply code in execReplication.c, I noticed
> > that the retry paths in RelationFindReplTupleByIndex and RelationFindReplTupleSeq
> > for concurrent updates and deletes have no test coverage [1]. Specifically,
> > when the same tuple is being updated/deleted on the publisher and subscriber at
> > the same time, the dirty snapshot finds the tuple being modified by another
> > transaction, the apply worker waits and retries the index/sequential scan.
>
> Good catch.
>
> > The attached patch adds an injection point before table_tuple_lock and a TAP test
> > exercising these retry paths, hitting both TM_Updated and TM_Deleted.
>
> I read the code briefly. Here are questions:
Thank you for reviewing!
> 1.
> The test looks like to add the test for retry acquiring the lock. But there is
> another retry path, which waits till xwait finishes. Do you have a reason to
> skip testing?
There are two retry paths, and I added TAP tests covering both. The
XactLockTableWait path is tested by holding an in-progress transaction
on the subscriber, which naturally blocks the apply worker until the
transaction finishes, then it retries the scan. The table_tuple_lock
retry path (TM_Updated / TM_Deleted) is tested using an injection
point that pauses the worker between finding and locking the tuple,
allowing concurrent DML to intervene.
> 2.
> Is it OK to use the same injection point name twice? I cannot find the existing
> case.
Good catch. I used separate injection points for each path.
Please find the attached v2 patch for further review. Thank you!
--
Bharath Rupireddy
Amazon Web Services: https://aws.amazon.com
| Attachment | Content-Type | Size |
|---|---|---|
| v2-0001-Add-tests-for-concurrent-DML-retry-paths-in-logic.patch | application/x-patch | 13.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nathan Bossart | 2026-04-29 02:29:12 | Re: New vacuum config to avoid anti wraparound vacuums |
| Previous Message | Ajin Cherian | 2026-04-29 01:42:23 | Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE |