Date: Tue, 2 Nov 2021 21:42:03 -0700
From: Noah Misch <noah@leadboat.com>
To: Sandeep Thakkar <sandeep.thakkar@enterprisedb.com>
Cc: Robert Haas <robertmhaas@gmail.com>,
	Andrey Borodin <x4mmm@yandex-team.ru>,
	Andres Freund <andres@anarazel.de>, CM Team <cm@enterprisedb.com>,
	PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>,
	Michael Paquier <michael@paquier.xyz>, Tom Lane <tgl@sss.pgh.pa.us>,
	Peter Geoghegan <pg@bowt.ie>,
	Semab Tariq <semab.tariq@enterprisedb.com>
Subject: Re: CREATE INDEX CONCURRENTLY does not index prepared xact's data
Message-ID: <20211103044203.GA570491@rfd.leadboat.com>
References: 
 <CANFyU95S6C2q=eByr=8ocd4BJkgesYUTd7n4OsFzJ_t611NdWQ@mail.gmail.com>
 <CANFyU94A1kO4TKB4PODGYUqDRvPnBmc=P=EJR6_VxoFLiRjEMw@mail.gmail.com>
 <20211030191919.GA355663@rfd.leadboat.com>
 <CANFyU95V9xW+0vrvF3A6v86u8E0zUkZVfWeFZe6KqWGgPeB=Jg@mail.gmail.com>
 <20211031191801.q2anckkqsbd7oow2@alap3.anarazel.de>
 <CANFyU97socOyfjyhS6ZYvguBdP=Fqqu-SK2z1_QT4t7-P5qzLQ@mail.gmail.com>
 <6A1F66DD-9A4E-4EB4-BF56-369FDBF30204@yandex-team.ru>
 <CANFyU94Xa8a5+4sZ7PxOiDLq+yN89g6y-9nNk-eLEvX6YUXbXA@mail.gmail.com>
 <CA+TgmobbOFpogLx6Qem1=WxGo9ineTSTwNE4_ucdgeBtyDCGrg@mail.gmail.com>
 <CANFyU96pdjUgGEopmU6KhaU782ff7HF4gvJ1RBqsbXiUWYQvMg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="llIrKcgUOe3dCx0c"
Content-Disposition: inline
In-Reply-To: 
 <CANFyU96pdjUgGEopmU6KhaU782ff7HF4gvJ1RBqsbXiUWYQvMg@mail.gmail.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Archived-At: 
 <https://www.postgresql.org/message-id/20211103044203.GA570491%40rfd.leadboat.com>
Precedence: bulk


--llIrKcgUOe3dCx0c
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Tue, Nov 02, 2021 at 06:20:42AM +0530, Sandeep Thakkar wrote:
> (gdb) frame 1
> 
> #1  0x40000000003fdc00:0 in equalTupleDescs (tupdesc1=0x60000000001f65e0,
> 
>     tupdesc2=0x60000000001fba08)
> 
...
> (gdb) p tupdesc2->attrs[2]
> 
> $6 = {attrelid = 27272, attname = {data = "b", '\000' <repeats 62 times>},
>   atttypid = 19, attstattarget = -1, attlen = 64, attnum = 3, attndims = 0,
>   attcacheoff = -1, atttypmod = -1, attbyval = false, attalign = 99 'c',
>   attstorage = 112 'p', attcompression = 0 '\000', attnotnull = false,
>   atthasdef = false, atthasmissing = false, attidentity = 0 '\000',
>   attgenerated = 0 '\000', attisdropped = false, attislocal = true,
>   attinhcount = 0, attcollation = 950}

That looks healthy.  Since gdb isn't giving line numbers, let's single-step
from the start of the function and see if that is informative.  Please apply
the attached patch, which just adds a test file.  Then run "make -C
src/test/subscription check PROVE_TESTS=t/080_step_equalTupleDescs.pl" and
attach
src/test/subscription/tmp_check/log/regress_log_080_step_equalTupleDescs in a
reply to this email.

On Mon, Nov 01, 2021 at 12:01:08AM +0500, Andrey Borodin wrote:
> So far I didn't come up with a clear understanding how this might happen.

Agreed.

> The only idea I have:
> 1. It seems equalTupleDescs() got two valid pointers, probably with broken data.
> 2. Maybe relation->rd_rel (alloceted just before relation->rd_att) was used incorrectly?
> 3. This could happen if CLASS_TUPLE_SIZE is calculated wrong. Don't we need to MAXALIGN everything due to added sizeof(relminmxid)?
> #define CLASS_TUPLE_SIZE \
>      (offsetof(FormData_pg_class,relminmxid) + sizeof(TransactionId))

See the comment at overread_tuplestruct_pg_cast for the reason why I think
that can't cause an actual malfunction.  Still, there's some possibility of
this being the explanation.

--llIrKcgUOe3dCx0c
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="step-to-crash-v0.patch"

diff --git a/src/test/subscription/t/080_step_equalTupleDescs.pl b/src/test/subscription/t/080_step_equalTupleDescs.pl
new file mode 100644
index 0000000..efa6dbd
--- /dev/null
+++ b/src/test/subscription/t/080_step_equalTupleDescs.pl
@@ -0,0 +1,94 @@
+# run with: make -C src/test/subscription check PROVE_TESTS=t/080_step_equalTupleDescs.pl
+# email src/test/subscription/tmp_check/log/regress_log_080_step_equalTupleDescs
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More tests => 1;
+use IPC::Run;
+
+my $node = PostgreSQL::Test::Cluster->new('main');
+$node->init;
+$node->start;
+
+my $in  = '';
+my $out = '';
+my $timer = IPC::Run::timeout(180);
+my $psql = $node->background_psql('postgres', \$in, \$out, $timer,
+	on_error_stop => 0);
+
+$in .= q{
+SELECT pg_backend_pid(), 'after_pid';
+};
+$psql->pump until $out =~ /^([0-9]*)\|after_pid/m;
+my $pid = $1;
+print "backend pid: $pid\n";
+
+my $gdb_cmd_file = $node->basedir . '/commands-gdb';
+PostgreSQL::Test::Utils::append_to_file($gdb_cmd_file, q{
+b equalTupleDescs
+ignore 1 9
+set logging file } . $node->basedir . q{/gdb_started.txt
+set logging on
+set logging off
+continue
+p *tupdesc1
+p *tupdesc2
+p tupdesc1->attrs[0]
+p tupdesc1->attrs[1]
+p tupdesc1->attrs[2]
+p tupdesc2->attrs[0]
+p tupdesc2->attrs[1]
+p tupdesc2->attrs[2]
+backtrace full
+set $loopvar = 25000
+while ($loopvar--)
+  stepi
+end
+quit
+});
+
+my $gdb = IPC::Run::start(
+	[qw(gdb --batch --nx --pid), $pid, '-x', $gdb_cmd_file],
+	$timer);
+sleep .1 while !-f $node->basedir.'/gdb_started.txt';
+
+# first 28 lines of src/test/regress/sql/alter_table.sql
+$in .= q{
+--
+-- ALTER_TABLE
+--
+
+-- Clean up in case a prior regression run failed
+SET client_min_messages TO 'warning';
+DROP ROLE IF EXISTS regress_alter_table_user1;
+RESET client_min_messages;
+
+CREATE USER regress_alter_table_user1;
+
+--
+-- add attribute
+--
+
+CREATE TABLE attmp (initial int4);
+
+COMMENT ON TABLE attmp_wrong IS 'table comment';
+COMMENT ON TABLE attmp IS 'table comment';
+COMMENT ON TABLE attmp IS NULL;
+
+ALTER TABLE attmp ADD COLUMN xmin integer; -- fails
+
+ALTER TABLE attmp ADD COLUMN a int4 default 3;
+
+ALTER TABLE attmp ADD COLUMN b name;
+
+ALTER TABLE attmp ADD COLUMN c text;
+};
+
+$in .= q{
+\q
+};
+$psql->finish;
+$gdb->finish;
+ok(1);

--llIrKcgUOe3dCx0c--