public inbox for [email protected]  

help / color / mirror / Atom feed

From: Michail Nikolaev <[email protected]>
To: PostgreSQL Hackers <[email protected]>
Subject: bt_index_parent_check and concurrently build indexes
Date: Mon, 9 Dec 2024 21:51:24 +0100
Message-ID: <CANtu0ojmVd27fEhfpST7RG2KZvwkX=dMyKUqg0KM87FkOSdz8Q@mail.gmail.com> (raw)

Hello, everyone!

While working on [0], I encountered an issue involving a missing tuple in
an index that was built concurrently. The problem only occurred once, but
it caused me a significant amount of frustration. :)

After some time, I managed to find a way to reproduce the issue. It turns
out that bt_index_parent_check is not suitable for validating indexes built
concurrently. The reason is that bt_index_parent_check uses SnapshotAny
during the heap scan, whereas an MVCC snapshot is used for the index build.

I’ve attached a patch that reproduces the issue (it incorrectly reports the
index as invalid, even though it is actually valid).

I’m unsure of the best way to address this issue, but here are some
possible options:
* Simply update the documentation.
* Issue a WARNING if !tupleIsAlive.
* Modify bt_index_parent_check to use an MVCC snapshot for the heap scan

Best regards,
Mikhail.

[0]: https://commitfest.postgresql.org/51/4971/


Attachments:

  [application/x-patch] v1-0001-test-to-reproduce-issue-with-bt_index_parent_chec.patch (3.0K, ../CANtu0ojmVd27fEhfpST7RG2KZvwkX=dMyKUqg0KM87FkOSdz8Q@mail.gmail.com/3-v1-0001-test-to-reproduce-issue-with-bt_index_parent_chec.patch)
  download | inline diff:
From 60d17152f47e8d85fd3bad961d1965c335493c94 Mon Sep 17 00:00:00 2001
From: nkey <[email protected]>
Date: Sun, 8 Dec 2024 17:32:15 +0100
Subject: [PATCH v1] test to reproduce issue with bt_index_parent_check and
 CREATE INDEX CONCURRENTLY

---
 contrib/amcheck/meson.build                   |  1 +
 .../t/006_cic_bt_index_parent_check.pl        | 59 +++++++++++++++++++
 2 files changed, 60 insertions(+)
 create mode 100644 contrib/amcheck/t/006_cic_bt_index_parent_check.pl

diff --git a/contrib/amcheck/meson.build b/contrib/amcheck/meson.build
index fc08e32539a..2eb7ff11bd7 100644
--- a/contrib/amcheck/meson.build
+++ b/contrib/amcheck/meson.build
@@ -45,6 +45,7 @@ tests += {
       't/003_cic_2pc.pl',
       't/004_verify_nbtree_unique.pl',
       't/005_pitr.pl',
+      't/006_cic_bt_index_parent_check.pl',
     ],
   },
 }
diff --git a/contrib/amcheck/t/006_cic_bt_index_parent_check.pl b/contrib/amcheck/t/006_cic_bt_index_parent_check.pl
new file mode 100644
index 00000000000..d4bb9c2a99d
--- /dev/null
+++ b/contrib/amcheck/t/006_cic_bt_index_parent_check.pl
@@ -0,0 +1,59 @@
+# Copyright (c) 2021-2024, PostgreSQL Global Development Group
+
+# Test CREATE INDEX CONCURRENTLY with concurrent prepared-xact modifications
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+
+use Test::More;
+
+Test::More->builder->todo_start('filesystem bug')
+  if PostgreSQL::Test::Utils::has_wal_read_bug;
+
+my ($node, $result);
+
+#
+# Test set-up
+#
+$node = PostgreSQL::Test::Cluster->new('CIC_HOT_test');
+$node->init;
+$node->append_conf('postgresql.conf', 'fsync = off');
+$node->append_conf('postgresql.conf',
+	'lock_timeout = ' . (1000 * $PostgreSQL::Test::Utils::timeout_default));
+$node->start;
+$node->safe_psql('postgres', q(CREATE EXTENSION amcheck));
+$node->safe_psql('postgres', q(CREATE TABLE tbl(i int primary key, updated_at timestamp)));
+$node->safe_psql('postgres', q(INSERT INTO tbl SELECT i,now() FROM generate_series(1, 10000) s(i);));
+
+# Run pgbench.
+$node->pgbench(
+	'--no-vacuum --client=30 --jobs=5 --exit-on-abort --transactions=500',
+	0,
+	[qr{actually processed}],
+	[qr{^$}],
+	'concurrent UPDATES w/ and CIC',
+	{
+		'pgbench_concurrent_cic' => q(
+			SELECT pg_try_advisory_lock(42)::integer AS gotlock \gset
+			\if :gotlock
+				CREATE INDEX CONCURRENTLY idx ON tbl(i, updated_at);
+				SELECT bt_index_parent_check('idx', heapallindexed => true, rootdescend => true, checkunique => true);
+				DROP INDEX CONCURRENTLY idx;
+				SELECT pg_advisory_unlock(42);
+			\else
+					BEGIN;
+					UPDATE tbl SET updated_at = now() WHERE i = floor(random()*10000);
+					UPDATE tbl SET updated_at = now() WHERE i = floor(random()*10000);
+					UPDATE tbl SET updated_at = now() WHERE i = floor(random()*10000);
+					UPDATE tbl SET updated_at = now() WHERE i = floor(random()*10000);
+					UPDATE tbl SET updated_at = now() WHERE i = floor(random()*10000);
+					UPDATE tbl SET updated_at = now() WHERE i = floor(random()*10000);
+					COMMIT;
+			\endif
+		  )
+	});
+
+$node->stop;
+done_testing();
-- 
2.43.0

view thread (3+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected]
  Subject: Re: bt_index_parent_check and concurrently build indexes
  In-Reply-To: <CANtu0ojmVd27fEhfpST7RG2KZvwkX=dMyKUqg0KM87FkOSdz8Q@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox