Date: Sun, 7 Jul 2019 10:00:35 -0700
From: Noah Misch <noah@leadboat.com>
To: Andres Freund <andres@anarazel.de>
Cc: pgsql-hackers@postgresql.org
Subject: Re: [RFC] Removing "magic" oids
Message-ID: <20190707170035.GA1485546@rfd.leadboat.com>
References: <20180930034810.ywp2c7awz7opzcfr@alap3.anarazel.de>
 <20181114080152.xkk6coduvfrh4ntr@alap3.anarazel.de>
 <20181115045728.GA1121738@rfd.leadboat.com>
 <20181115050241.exereob7wk3xxewj@alap3.anarazel.de>
 <20181120092004.f3j2znfodallkksn@alap3.anarazel.de>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="IS0zKkzwUGydFO0o"
Content-Disposition: inline
In-Reply-To: <20181120092004.f3j2znfodallkksn@alap3.anarazel.de>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk


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

On Tue, Nov 20, 2018 at 01:20:04AM -0800, Andres Freund wrote:
> On 2018-11-14 21:02:41 -0800, Andres Freund wrote:
> > On 2018-11-15 04:57:28 +0000, Noah Misch wrote:
> > > On Wed, Nov 14, 2018 at 12:01:52AM -0800, Andres Freund wrote:
> > > > - one pgbench test tested concurrent insertions into a table with
> > > >   oids, as some sort of stress test for lwlocks and spinlocks. I *think*
> > > >   this doesn't really have to be a system oid column, and this was just
> > > >   because that's how we triggered a bug on some machine. Noah, do I get
> > > >   this right?
> > > 
> > > The point of the test is to exercise OidGenLock by issuing many parallel
> > > GetNewOidWithIndex() and verifying absence of duplicates.  There's nothing
> > > special about OidGenLock, but it is important to use an operation that takes a
> > > particular LWLock many times, quickly.  If the test query spends too much time
> > > on things other than taking locks, it will catch locking races too rarely.
> > 
> > Sequences ought to do that, too. And if it's borked, we'd hopefully see
> > unique violations. But it's definitely not a 1:1 replacement.

> I've tested this on ppc.  Neither the old version nor the new version
> stress test spinlocks sufficiently to error out with weakened spinlocks
> (not that surprising, there are no spinlocks in any hot path of either
> workload). Both versions very reliably trigger on weakened lwlocks. So I
> think we're comparatively good on that front.

I tested this on xlc, the compiler that motivated the OID test, and the v12+
version of the test didn't catch the bug[1] with xlc 13.1.3.  CREATE TYPE
... AS ENUM generates an OID for each label, so the attached patch makes the
v12+ test have locking behavior similar to its v11 ancestor.

[1] https://postgr.es/m/flat/a72cfcb0-37d0-de2f-b3ec-f38ad8d6a8cc@postgrespro.ru

--IS0zKkzwUGydFO0o
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="lwlock-test-v1.patch"

diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl
index dc2c72f..3b097a9 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -58,27 +58,20 @@ sub pgbench
 	return;
 }
 
-# Test concurrent insertion into table with serial column.  This
-# indirectly exercises LWLock and spinlock concurrency.  This test
-# makes a 5-MiB table.
-
-$node->safe_psql('postgres',
-	'CREATE UNLOGGED TABLE insert_tbl (id serial primary key); ');
-
+# Test concurrent OID generation via pg_enum_oid_index.  This indirectly
+# exercises LWLock and spinlock concurrency.
+my $labels = join ',', map { "'l$_'" } 1 .. 1000;
 pgbench(
 	'--no-vacuum --client=5 --protocol=prepared --transactions=25',
 	0,
 	[qr{processed: 125/125}],
 	[qr{^$}],
-	'concurrent insert workload',
+	'concurrent OID generation',
 	{
 		'001_pgbench_concurrent_insert' =>
-		  'INSERT INTO insert_tbl SELECT FROM generate_series(1,1000);'
+		  "CREATE TYPE pg_temp.e AS ENUM ($labels); DROP TYPE pg_temp.e;"
 	});
 
-# cleanup
-$node->safe_psql('postgres', 'DROP TABLE insert_tbl;');
-
 # Trigger various connection errors
 pgbench(
 	'no-such-database',

--IS0zKkzwUGydFO0o--