Date: Thu, 15 Nov 2018 04:57:28 +0000
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: <20181115045728.GA1121738@rfd.leadboat.com>
References: <20180930034810.ywp2c7awz7opzcfr@alap3.anarazel.de>
 <20181114080152.xkk6coduvfrh4ntr@alap3.anarazel.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20181114080152.xkk6coduvfrh4ntr@alap3.anarazel.de>
User-Agent: Mutt/1.5.24 (2015-08-30)
Precedence: bulk

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.

> --- a/src/bin/pgbench/t/001_pgbench_with_server.pl
> +++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
> @@ -48,28 +48,26 @@ sub pgbench
>  	return;
>  }
>  
> -# Test concurrent insertion into table with UNIQUE oid column.  DDL expects
> -# GetNewOidWithIndex() to successfully avoid violating uniqueness for indexes
> -# like pg_class_oid_index and pg_proc_oid_index.  This indirectly exercises
> -# LWLock and spinlock concurrency.  This test makes a 5-MiB table.
> +# 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 oid_tbl () WITH OIDS; '
> -	  . 'ALTER TABLE oid_tbl ADD UNIQUE (oid);');
> +	    'CREATE UNLOGGED TABLE insert_tbl (id serial primary key); ');
>  
>  pgbench(
>  	'--no-vacuum --client=5 --protocol=prepared --transactions=25',
>  	0,
>  	[qr{processed: 125/125}],
>  	[qr{^$}],
> -	'concurrency OID generation',
> +	'concurrent insert generation',
>  	{
> -		'001_pgbench_concurrent_oid_generation' =>
> -		  'INSERT INTO oid_tbl SELECT FROM generate_series(1,1000);'
> +		'001_pgbench_concurrent_insert' =>
> +		  'INSERT INTO insert_tbl SELECT FROM generate_series(1,1000);'

The code for sequences is quite different, so this may or may not be an
effective replacement.  To study that, you could remove a few barriers from
lwlock.c, measure how many iterations today's test needs to catch the
mutation, and then measure the same for this proposal.