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-f38ad8d6a...@postgrespro.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
