| From: | "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com> |
|---|---|
| To: | 'Alexander Lakhin' <exclusion(at)gmail(dot)com> |
| Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Srinath Reddy Sadipiralla <srinath2133(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
| Subject: | RE: Fix "unexpected logical decoding status change" error; from concurrent logical decoding activation |
| Date: | 2026-10-30 12:37:41 |
| Message-ID: | OS7PR01MB183172A425382C8A781DB153FF56D2@OS7PR01MB18317.jpnprd01.prod.outlook.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
> `check-world` with this sleep reveals also other tests suffering from the
> same disease:
> src/test/modules/test_misc/t/010_index_concurrently_upsert.pl
> repack_toast
> repack_missingval
> repack_temporal
> repack_temporal_multirange
I analyzed other issues and attaches patch set. 0001 fixes the
Regarding the isolation tests for REPACK, I observed that the requested injection
point for repack_missingval had already been registered:
```
--- /home/hayato/postgres/src/test/modules/injection_points/expected/repack_missingval.out 2026-10-30 17:48:34.651557191 +0900
+++ /home/hayato/builddir/testrun/injection_points/isolation/results/repack_missingval.out 2026-10-30 19:56:22.486024917 +0900
@@ -1,38 +1,4 @@
Parsed test spec with 2 sessions
starting permutation: s1_wait_before_lock s2_update s2_wakeup_before_lock s1_check
-injection_points_attach
------------------------
...
+setup of session s1 failed: ERROR: injection point "repack-concurrently-before-lock" already defined
```
I think the root cause is in the previous test repack_dropped.pl. It attaches
the injection point but does not detach. In general, it can work because the shmem
exit callback removes the point from the shared memory. But if the cleanup is
delayed, the isolationtester starts a new test and the backend tries to attach
the same injection point, causes the ERROR. Attached 0002 patch adds a teardown
code for the test, it expressly removes the injection point.
===
As for the 010_update_concurrently_upsert, there is a race condition in the
injection point module. When the backend calls injection_points_set_local(),
it appends all attached points in inj_list_local and removes listed points at exit.
Since inj_list_local is the process-local, it cannot be maintained when other
backends detach the injection point. This can cause the below:
1. a backend calls injection_points_set_local() and attaches an injection point #1.
2. another backend detaches #1.
3. yet another process attaches the injection point #1.
4. initial backend exits, at that time the #1 can be removed on the shared memory.
I think above race can happen in 010. 010 repeats the test that three background
psql sessions are created, they attach injection points after the set_local(),
run some SQL commands and ensures sessions can hit the injection point.
Background sessions are quit every case but test does not wait till backends
are actually exit. It's possible that registered injection points are immediately
removed by exiting backends if they are delayed. This can stuck the test at
ok_injection_point().
A debug log [1] allows printing whether the shmem exit callback actually detaches
the injection point or not. For 010 test, every lines should be "not found" because
points are expressly detached. But you can find that some injection points are
detached by the cleanup function then the test can stuck because backends won't
wait the injection point anymore.
If we try to fix only for this test, it's enough to ensure backend processes used
for the tests are exit. Attached 0003 patch implements that. But I feel it might
be a fundamental issue for the injection point module. Shouldn't backends try to
clean up points which is only registered by them? We can separate it from the
thread based on the needs.
[1]
```
--- a/src/test/modules/injection_points/injection_points.c
+++ b/src/test/modules/injection_points/injection_points.c
@@ -180,12 +180,15 @@ injection_points_cleanup(int code, Datum arg)
if (!injection_point_local)
return;
+ pg_usleep(1000000);
+
/* Detach all the local points */
foreach(lc, inj_list_local)
{
char *name = strVal(lfirst(lc));
- (void) InjectionPointDetach(name);
+ elog(LOG, "detaching injection point \"%s\": %s",
+ name, InjectionPointDetach(name) ? "detached" : "not found");
}
```
Best regards,
Hayato Kuroda
FUJITSU LIMITED
| Attachment | Content-Type | Size |
|---|---|---|
| 0001-Stabilize-051_effective_wal_level.patch | application/octet-stream | 2.2 KB |
| 0002-Ensure-injection-point-repack-concurrently-before-lo.patch | application/octet-stream | 1.3 KB |
| 0003-Stabilize-010_index_concurrently_upsert.patch | application/octet-stream | 6.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Greg Burd | 2026-10-30 12:48:56 | Re: Reapply graceful socket shutdown on Windows (revert 29992a6a509) |
| Previous Message | Amit Kapila | 2026-10-30 12:19:25 | Re: Proposal: Conflict log history table for Logical Replication |