Re: [RFC] Removing "magic" oids

Hi,

On 2019-07-07 10:00:35 -0700, Noah Misch wrote:
> On Tue, Nov 20, 2018 at 01:20:04AM -0800, Andres Freund wrote:
> > On 2018-11-14 21:02:41 -0800, Andres Freund wrote:
> > > > 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.

Interesting. It's not obvious to me why that works meaningfully better.

> [1] https://postgr.es/m/flat/a72cfcb0-37d0-de2f-b3ec-f38ad8d6a8cc(at)postgrespro(dot)ru

> 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;"
> });

Hm, perhaps we should just do something stupid an insert into a catalog
table, determining the oid to insert with pg_nextoid? That ought to be a
lot faster and thus more "stress testing" than going through a full
blown DDL statement? But perhaps that's just too ugly.

Greetings,

Andres Freund