Feedback-ID: ie3de48e3:Fastmail
Date: Wed, 26 Nov 2025 19:34:16 +0100
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de>
To: Mihail Nikalayeu <mihailnikalayeu@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Noah Misch <noah@leadboat.com>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Issues with ON CONFLICT UPDATE and REINDEX CONCURRENTLY
Message-ID: <202511261817.fyixgtt3hqdr@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="ejvp2k77j565hcot"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CADzfLwV5oQq-Vg_VmG_o4SdL6yHjDoNO4T4pMtgJLzYGmYf74g@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202511261817.fyixgtt3hqdr%40alvherre.pgsql>
Precedence: bulk


--ejvp2k77j565hcot
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

Hello,

We ran into one more problem with the new test, evidenced by timeouts by
buildfarm member prion.  For CATCACHE_FORCE_RELEASE builds on two of the
tests, we get a few invalidations of the catalog snapshot ahead of what
we expect, and because we have an injection point to sleep there, those
tests get stuck.

Here's one possible fix.  I had to take the attach operation on
invalidate-catalog-snapshot-end to a new step of s1, instead of
occurring in the setup block.  I understand that this is because no step
can run until the setup of all steps completes, so if one setup gets
stuck, we're out of luck.  And then, session s4 can do a conditional
wakeup of session s1.

Patch attached.  Thoughts?

Maybe there's some other way to go about this -- for instance I
considered the idea of moving the injection point somewhere else from
InvalidateCatalogSnapshot().  I don't have any ideas about that though,
but I'm willing to listen if anybody has any.


The output in both cases is different (we get more notices in the weird
build and also s1 goes to sleep in different order), so I had to add an
alternative expected file.  The diffs look okay to me -- essentially
they say, in the normal case, the injection_points_attach() returns
immediately, but in the other case, s1 goes to sleep and is awakened
when it sees the 'case' expression by session 4:

--- expected/index-concurrently-upsert-predicate.out	2025-11-26 19:13:58.702213673 +0100
+++ expected/index-concurrently-upsert-predicate_1.out	2025-11-26 19:04:16.745666956 +0100
@@ -1,8 +1,9 @@
 Parsed test spec with 5 sessions
 
 starting permutation: s1_attach_invalidate_catalog_snapshot s4_wakeup_s1_setup s5_noop s3_start_create_index s1_start_upsert s4_wakeup_define_index_before_set_valid s2_start_upsert s5_wakeup_s1_from_invalidate_catalog_snapshot s4_wakeup_s2 s4_wakeup_s1
+s1: NOTICE:  notice triggered for injection point pre-invalidate-catalog-snapshot-end
 injection_points_attach
 -----------------------
                        
 (1 row)
 
@@ -14,18 +15,15 @@
 injection_points_attach
 -----------------------
                        
 (1 row)
 
+s1: NOTICE:  notice triggered for injection point pre-invalidate-catalog-snapshot-end
+s1: NOTICE:  notice triggered for injection point pre-invalidate-catalog-snapshot-end
 step s1_attach_invalidate_catalog_snapshot: 
 	SELECT injection_points_attach('invalidate-catalog-snapshot-end', 'wait');
-
-injection_points_attach
------------------------
-                       
-(1 row)
-
+ <waiting ...>
 step s4_wakeup_s1_setup: 
 	select case when
 			(select pid from pg_stat_activity
 			      where wait_event_type = 'InjectionPoint' and
 			      wait_event = 'invalidate-catalog-snapshot-end') is not null
@@ -35,10 +33,16 @@
 case
 ----
     
 (1 row)
 
+step s1_attach_invalidate_catalog_snapshot: <... completed>
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
 step s5_noop: 
  <waiting ...>
 step s3_start_create_index: 
 	CREATE UNIQUE INDEX CONCURRENTLY tbl_pkey_special_duplicate ON test.tbl(abs(i)) WHERE i < 10000;
  <waiting ...>


-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"Los minutos y los segundos son mercadería de la ciudad, donde un infeliz se
afana por no perder ni siquiera un segundo y no advierto que obrando de ese
modo pierde una vida."              ("La vuelta de Don Camilo", G. Guareschi)

--ejvp2k77j565hcot
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
	filename="0001-Fix-new-test-for-CATCACHE_FORCE_RELEASE-builds.patch"

From 795427760facc8b3dcbeaa8a19bf0d237d3e630e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=81lvaro=20Herrera?= <alvherre@kurilemu.de>
Date: Wed, 26 Nov 2025 19:15:12 +0100
Subject: [PATCH] Fix new test for CATCACHE_FORCE_RELEASE builds

---
 .../index-concurrently-upsert-predicate.out   |  23 +++-
 .../index-concurrently-upsert-predicate_1.out | 116 ++++++++++++++++++
 .../expected/index-concurrently-upsert.out    |  23 +++-
 .../expected/index-concurrently-upsert_1.out  | 116 ++++++++++++++++++
 .../index-concurrently-upsert-predicate.spec  |  18 +++
 .../specs/index-concurrently-upsert.spec      |  18 +++
 6 files changed, 312 insertions(+), 2 deletions(-)
 create mode 100644 src/test/modules/injection_points/expected/index-concurrently-upsert-predicate_1.out
 create mode 100644 src/test/modules/injection_points/expected/index-concurrently-upsert_1.out

diff --git a/src/test/modules/injection_points/expected/index-concurrently-upsert-predicate.out b/src/test/modules/injection_points/expected/index-concurrently-upsert-predicate.out
index 3d9512c8fc7..7adbab1c78e 100644
--- a/src/test/modules/injection_points/expected/index-concurrently-upsert-predicate.out
+++ b/src/test/modules/injection_points/expected/index-concurrently-upsert-predicate.out
@@ -1,6 +1,6 @@
 Parsed test spec with 5 sessions
 
-starting permutation: s5_noop s3_start_create_index s1_start_upsert s4_wakeup_define_index_before_set_valid s2_start_upsert s5_wakeup_s1_from_invalidate_catalog_snapshot s4_wakeup_s2 s4_wakeup_s1
+starting permutation: s1_attach_invalidate_catalog_snapshot s4_wakeup_s1_setup s5_noop s3_start_create_index s1_start_upsert s4_wakeup_define_index_before_set_valid s2_start_upsert s5_wakeup_s1_from_invalidate_catalog_snapshot s4_wakeup_s2 s4_wakeup_s1
 injection_points_attach
 -----------------------
                        
@@ -16,6 +16,27 @@ injection_points_attach
                        
 (1 row)
 
+step s1_attach_invalidate_catalog_snapshot: 
+	SELECT injection_points_attach('invalidate-catalog-snapshot-end', 'wait');
+
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+step s4_wakeup_s1_setup: 
+	select case when
+			(select pid from pg_stat_activity
+			      where wait_event_type = 'InjectionPoint' and
+			      wait_event = 'invalidate-catalog-snapshot-end') is not null
+			then injection_points_wakeup('invalidate-catalog-snapshot-end')
+		end;
+
+case
+----
+    
+(1 row)
+
 step s5_noop: 
  <waiting ...>
 step s3_start_create_index: 
diff --git a/src/test/modules/injection_points/expected/index-concurrently-upsert-predicate_1.out b/src/test/modules/injection_points/expected/index-concurrently-upsert-predicate_1.out
new file mode 100644
index 00000000000..affc256ff82
--- /dev/null
+++ b/src/test/modules/injection_points/expected/index-concurrently-upsert-predicate_1.out
@@ -0,0 +1,116 @@
+Parsed test spec with 5 sessions
+
+starting permutation: s1_attach_invalidate_catalog_snapshot s4_wakeup_s1_setup s5_noop s3_start_create_index s1_start_upsert s4_wakeup_define_index_before_set_valid s2_start_upsert s5_wakeup_s1_from_invalidate_catalog_snapshot s4_wakeup_s2 s4_wakeup_s1
+s1: NOTICE:  notice triggered for injection point pre-invalidate-catalog-snapshot-end
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+s1: NOTICE:  notice triggered for injection point pre-invalidate-catalog-snapshot-end
+s1: NOTICE:  notice triggered for injection point pre-invalidate-catalog-snapshot-end
+step s1_attach_invalidate_catalog_snapshot: 
+	SELECT injection_points_attach('invalidate-catalog-snapshot-end', 'wait');
+ <waiting ...>
+step s4_wakeup_s1_setup: 
+	select case when
+			(select pid from pg_stat_activity
+			      where wait_event_type = 'InjectionPoint' and
+			      wait_event = 'invalidate-catalog-snapshot-end') is not null
+			then injection_points_wakeup('invalidate-catalog-snapshot-end')
+		end;
+
+case
+----
+    
+(1 row)
+
+step s1_attach_invalidate_catalog_snapshot: <... completed>
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+step s5_noop: 
+ <waiting ...>
+step s3_start_create_index: 
+	CREATE UNIQUE INDEX CONCURRENTLY tbl_pkey_special_duplicate ON test.tbl(abs(i)) WHERE i < 10000;
+ <waiting ...>
+s1: NOTICE:  notice triggered for injection point pre-invalidate-catalog-snapshot-end
+step s1_start_upsert: 
+	INSERT INTO test.tbl VALUES(13,now()) ON CONFLICT (abs(i)) WHERE i < 100 DO UPDATE SET updated_at = now();
+ <waiting ...>
+step s5_noop: <... completed>
+step s4_wakeup_define_index_before_set_valid: 
+	SELECT injection_points_detach('define-index-before-set-valid');
+	SELECT injection_points_wakeup('define-index-before-set-valid');
+
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+step s2_start_upsert: 
+	INSERT INTO test.tbl VALUES(13,now()) ON CONFLICT (abs(i)) WHERE i < 100 DO UPDATE SET updated_at = now();
+ <waiting ...>
+step s5_wakeup_s1_from_invalidate_catalog_snapshot: 
+	SELECT injection_points_detach('invalidate-catalog-snapshot-end');
+	SELECT injection_points_wakeup('invalidate-catalog-snapshot-end');
+
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+step s4_wakeup_s2: 
+	SELECT injection_points_detach('exec-insert-before-insert-speculative');
+	SELECT injection_points_wakeup('exec-insert-before-insert-speculative');
+
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+step s4_wakeup_s1: 
+	SELECT injection_points_detach('check-exclusion-or-unique-constraint-no-conflict');
+	SELECT injection_points_wakeup('check-exclusion-or-unique-constraint-no-conflict');
+
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+s1: NOTICE:  notice triggered for injection point pre-invalidate-catalog-snapshot-end
+step s1_start_upsert: <... completed>
+step s2_start_upsert: <... completed>
+step s3_start_create_index: <... completed>
diff --git a/src/test/modules/injection_points/expected/index-concurrently-upsert.out b/src/test/modules/injection_points/expected/index-concurrently-upsert.out
index a9e8bb5d00e..cf961378942 100644
--- a/src/test/modules/injection_points/expected/index-concurrently-upsert.out
+++ b/src/test/modules/injection_points/expected/index-concurrently-upsert.out
@@ -1,6 +1,6 @@
 Parsed test spec with 5 sessions
 
-starting permutation: s5_noop s3_start_create_index s1_start_upsert s4_wakeup_define_index_before_set_valid s2_start_upsert s5_wakeup_s1_from_invalidate_catalog_snapshot s4_wakeup_s2 s4_wakeup_s1
+starting permutation: s1_attach_invalidate_catalog_snapshot s4_wakeup_s1_setup s5_noop s3_start_create_index s1_start_upsert s4_wakeup_define_index_before_set_valid s2_start_upsert s5_wakeup_s1_from_invalidate_catalog_snapshot s4_wakeup_s2 s4_wakeup_s1
 injection_points_attach
 -----------------------
                        
@@ -16,6 +16,27 @@ injection_points_attach
                        
 (1 row)
 
+step s1_attach_invalidate_catalog_snapshot: 
+	SELECT injection_points_attach('invalidate-catalog-snapshot-end', 'wait');
+
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+step s4_wakeup_s1_setup: 
+	select case when
+			(select pid from pg_stat_activity
+			      where wait_event_type = 'InjectionPoint' and
+			      wait_event = 'invalidate-catalog-snapshot-end') is not null
+			then injection_points_wakeup('invalidate-catalog-snapshot-end')
+		end;
+
+case
+----
+    
+(1 row)
+
 step s5_noop: 
  <waiting ...>
 step s3_start_create_index: 
diff --git a/src/test/modules/injection_points/expected/index-concurrently-upsert_1.out b/src/test/modules/injection_points/expected/index-concurrently-upsert_1.out
new file mode 100644
index 00000000000..7a77830d56c
--- /dev/null
+++ b/src/test/modules/injection_points/expected/index-concurrently-upsert_1.out
@@ -0,0 +1,116 @@
+Parsed test spec with 5 sessions
+
+starting permutation: s1_attach_invalidate_catalog_snapshot s4_wakeup_s1_setup s5_noop s3_start_create_index s1_start_upsert s4_wakeup_define_index_before_set_valid s2_start_upsert s5_wakeup_s1_from_invalidate_catalog_snapshot s4_wakeup_s2 s4_wakeup_s1
+s1: NOTICE:  notice triggered for injection point pre-invalidate-catalog-snapshot-end
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+s1: NOTICE:  notice triggered for injection point pre-invalidate-catalog-snapshot-end
+s1: NOTICE:  notice triggered for injection point pre-invalidate-catalog-snapshot-end
+step s1_attach_invalidate_catalog_snapshot: 
+	SELECT injection_points_attach('invalidate-catalog-snapshot-end', 'wait');
+ <waiting ...>
+step s4_wakeup_s1_setup: 
+	select case when
+			(select pid from pg_stat_activity
+			      where wait_event_type = 'InjectionPoint' and
+			      wait_event = 'invalidate-catalog-snapshot-end') is not null
+			then injection_points_wakeup('invalidate-catalog-snapshot-end')
+		end;
+
+case
+----
+    
+(1 row)
+
+step s1_attach_invalidate_catalog_snapshot: <... completed>
+injection_points_attach
+-----------------------
+                       
+(1 row)
+
+step s5_noop: 
+ <waiting ...>
+step s3_start_create_index: 
+	CREATE UNIQUE INDEX CONCURRENTLY tbl_pkey_duplicate ON test.tbl(i);
+ <waiting ...>
+s1: NOTICE:  notice triggered for injection point pre-invalidate-catalog-snapshot-end
+step s1_start_upsert: 
+    INSERT INTO test.tbl VALUES (13,now()) ON CONFLICT (i) DO UPDATE SET updated_at = now();
+ <waiting ...>
+step s5_noop: <... completed>
+step s4_wakeup_define_index_before_set_valid: 
+	SELECT injection_points_detach('define-index-before-set-valid');
+	SELECT injection_points_wakeup('define-index-before-set-valid');
+
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+step s2_start_upsert: 
+	INSERT INTO test.tbl VALUES (13,now()) ON CONFLICT (i) DO UPDATE SET updated_at = now();
+ <waiting ...>
+step s5_wakeup_s1_from_invalidate_catalog_snapshot: 
+	SELECT injection_points_detach('invalidate-catalog-snapshot-end');
+	SELECT injection_points_wakeup('invalidate-catalog-snapshot-end');
+
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+step s4_wakeup_s2: 
+	SELECT injection_points_detach('exec-insert-before-insert-speculative');
+	SELECT injection_points_wakeup('exec-insert-before-insert-speculative');
+
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+step s4_wakeup_s1: 
+	SELECT injection_points_detach('check-exclusion-or-unique-constraint-no-conflict');
+	SELECT injection_points_wakeup('check-exclusion-or-unique-constraint-no-conflict');
+
+injection_points_detach
+-----------------------
+                       
+(1 row)
+
+injection_points_wakeup
+-----------------------
+                       
+(1 row)
+
+s1: NOTICE:  notice triggered for injection point pre-invalidate-catalog-snapshot-end
+step s1_start_upsert: <... completed>
+step s2_start_upsert: <... completed>
+step s3_start_create_index: <... completed>
diff --git a/src/test/modules/injection_points/specs/index-concurrently-upsert-predicate.spec b/src/test/modules/injection_points/specs/index-concurrently-upsert-predicate.spec
index 725f6f22295..e65e4a9120c 100644
--- a/src/test/modules/injection_points/specs/index-concurrently-upsert-predicate.spec
+++ b/src/test/modules/injection_points/specs/index-concurrently-upsert-predicate.spec
@@ -28,6 +28,9 @@ setup
 	SELECT injection_points_set_local();
 	SELECT injection_points_attach('check-exclusion-or-unique-constraint-no-conflict', 'wait');
 	SELECT injection_points_attach('pre-invalidate-catalog-snapshot-end', 'notice');
+}
+step s1_attach_invalidate_catalog_snapshot
+{
 	SELECT injection_points_attach('invalidate-catalog-snapshot-end', 'wait');
 }
 step s1_start_upsert
@@ -58,6 +61,19 @@ step s3_start_create_index
 }
 
 session s4
+# Step s1_attach_invalidate_catalog_snapshot sleeps or not depending on
+# build conditions (CATCACHE_FORCE_RELEASE). Here we send a wakeup signal if
+# it's sleeping or do nothing otherwise, and print a null value in either
+# case.
+step s4_wakeup_s1_setup
+{
+	select case when
+			(select pid from pg_stat_activity
+			      where wait_event_type = 'InjectionPoint' and
+			      wait_event = 'invalidate-catalog-snapshot-end') is not null
+			then injection_points_wakeup('invalidate-catalog-snapshot-end')
+		end;
+}
 step s4_wakeup_s1
 {
 	SELECT injection_points_detach('check-exclusion-or-unique-constraint-no-conflict');
@@ -85,6 +101,8 @@ step s5_wakeup_s1_from_invalidate_catalog_snapshot
 }
 
 permutation
+	s1_attach_invalidate_catalog_snapshot
+	s4_wakeup_s1_setup
 	s5_noop(s1_start_upsert notices 1)
 	s3_start_create_index(s1_start_upsert, s2_start_upsert)
 	s1_start_upsert
diff --git a/src/test/modules/injection_points/specs/index-concurrently-upsert.spec b/src/test/modules/injection_points/specs/index-concurrently-upsert.spec
index 4487834aa8e..f78ed16c4f3 100644
--- a/src/test/modules/injection_points/specs/index-concurrently-upsert.spec
+++ b/src/test/modules/injection_points/specs/index-concurrently-upsert.spec
@@ -27,6 +27,9 @@ setup
 	SELECT injection_points_set_local();
 	SELECT injection_points_attach('check-exclusion-or-unique-constraint-no-conflict', 'wait');
 	SELECT injection_points_attach('pre-invalidate-catalog-snapshot-end', 'notice');
+}
+step s1_attach_invalidate_catalog_snapshot
+{
 	SELECT injection_points_attach('invalidate-catalog-snapshot-end', 'wait');
 }
 step s1_start_upsert
@@ -57,6 +60,19 @@ step s3_start_create_index
 }
 
 session s4
+# Step s1_attach_invalidate_catalog_snapshot sleeps or not depending on
+# build conditions (CATCACHE_FORCE_RELEASE). Here we send a wakeup signal if
+# it's sleeping or do nothing otherwise, and print a null value in either
+# case.
+step s4_wakeup_s1_setup
+{
+	select case when
+			(select pid from pg_stat_activity
+			      where wait_event_type = 'InjectionPoint' and
+			      wait_event = 'invalidate-catalog-snapshot-end') is not null
+			then injection_points_wakeup('invalidate-catalog-snapshot-end')
+		end;
+}
 step s4_wakeup_s1
 {
 	SELECT injection_points_detach('check-exclusion-or-unique-constraint-no-conflict');
@@ -84,6 +100,8 @@ step s5_wakeup_s1_from_invalidate_catalog_snapshot
 }
 
 permutation
+	s1_attach_invalidate_catalog_snapshot
+	s4_wakeup_s1_setup
 	s5_noop(s1_start_upsert notices 1)
 	s3_start_create_index(s1_start_upsert, s2_start_upsert)
 	s1_start_upsert
-- 
2.47.3


--ejvp2k77j565hcot--