Re: Adding a test for speculative insert abort case

2020-03-24 Thread Andres Freund
On 2020-03-24 18:03:57 +0530, Amit Kapila wrote: > Can we change the status of CF entry for this patch [1] to committed > or is there any work pending? Done!

Re: Adding a test for speculative insert abort case

2020-03-24 Thread Amit Kapila
On Wed, Feb 12, 2020 at 6:50 AM Melanie Plageman wrote: > > > On Tue, Feb 11, 2020 at 4:45 PM Andres Freund wrote: >> >> >> I additionally restricted the controller_print_speculative_locks step to >> the current database and made a bunch of debug output more >> precise. Survived ~150 runs

Re: Adding a test for speculative insert abort case

2020-02-11 Thread Melanie Plageman
On Tue, Feb 11, 2020 at 4:45 PM Andres Freund wrote: > > I additionally restricted the controller_print_speculative_locks step to > the current database and made a bunch of debug output more > precise. Survived ~150 runs locally. > > Lets see what the buildfarm says... > > Thanks so much for

Re: Adding a test for speculative insert abort case

2020-02-11 Thread Andres Freund
Hi, On 2020-02-07 16:40:46 -0800, Andres Freund wrote: > I'm currently fighting with a race I'm observing in about 1/4 of the > runs. [...] > I think the issue here is that determines whether s1 can finish its > check_exclusion_or_unique_constraint() check with one retry is whether > it reaches

Re: Adding a test for speculative insert abort case

2020-02-07 Thread Andres Freund
Hi, I'm currently fighting with a race I'm observing in about 1/4 of the runs. I get: @@ -361,16 +361,18 @@ locktype mode granted speculative tokenShareLock f speculative tokenExclusiveLock t step controller_unlock_2_4: SELECT pg_advisory_unlock(2, 4);

Re: Adding a test for speculative insert abort case

2020-02-06 Thread Andres Freund
Hi, On 2019-08-21 13:59:00 -0700, Melanie Plageman wrote: > >> > +step "controller_print_speculative_locks" { SELECT > >> locktype,classid,objid,mode,granted FROM pg_locks WHERE > >> locktype='speculative > >> > +token' ORDER BY granted; } > >> > >> I think showing the speculative locks is

Re: Adding a test for speculative insert abort case

2019-11-11 Thread Michael Paquier
On Wed, Aug 21, 2019 at 01:59:00PM -0700, Melanie Plageman wrote: > So, what should we do about this? Do you agree that the "controller_show" > step would be a problem? Andres, it seems to me that this is waiting some input from you. -- Michael signature.asc Description: PGP signature

Re: Adding a test for speculative insert abort case

2019-08-21 Thread Melanie Plageman
On Wed, Aug 7, 2019 at 1:47 PM Melanie Plageman wrote: > > > On Wed, Jul 24, 2019 at 11:48 AM Andres Freund wrote: > >> > diff --git a/src/test/isolation/specs/insert-conflict-specconflict.spec >> b/src/test/isolation/specs/insert-conflict-specconflict.spec >> > index 3a70484fc2..7f29fb9d02

Re: Adding a test for speculative insert abort case

2019-08-07 Thread Melanie Plageman
On Wed, Jul 24, 2019 at 11:48 AM Andres Freund wrote: > > diff --git a/src/test/isolation/specs/insert-conflict-specconflict.spec > b/src/test/isolation/specs/insert-conflict-specconflict.spec > > index 3a70484fc2..7f29fb9d02 100644 > > ---

Re: Adding a test for speculative insert abort case

2019-07-24 Thread Andres Freund
Hi, On 2019-06-05 15:49:47 -0700, Melanie Plageman wrote: > On Thu, May 16, 2019 at 8:46 PM Melanie Plageman > wrote: > > > > > Good idea. > > I squashed the changes I suggested in previous emails, Ashwin's patch, my > > suggested updates to that patch, and the index order check all into one >

Re: Adding a test for speculative insert abort case

2019-06-05 Thread Melanie Plageman
On Thu, May 16, 2019 at 8:46 PM Melanie Plageman wrote: > > Good idea. > I squashed the changes I suggested in previous emails, Ashwin's patch, my > suggested updates to that patch, and the index order check all into one > updated > patch attached. > > I've updated this patch to make it apply on

Re: Adding a test for speculative insert abort case

2019-05-17 Thread Alvaro Herrera
On 2019-May-17, Melanie Plageman wrote: > I realized that the numbers at the front probably indicate which patch > it is in a patch set and not the version, so, if that is the case, a > renamed patch -- second version but the only patch needed if you are > applying to master. Is this right?

Re: Adding a test for speculative insert abort case

2019-05-17 Thread Melanie Plageman
On Thu, May 16, 2019 at 8:46 PM Melanie Plageman wrote: > > I squashed the changes I suggested in previous emails, Ashwin's patch, my > suggested updates to that patch, and the index order check all into one > updated > patch attached. > > I realized that the numbers at the front probably

Re: Adding a test for speculative insert abort case

2019-05-16 Thread Melanie Plageman
On Thu, May 16, 2019 at 2:03 PM Andres Freund wrote: > Hi, > > On 2019-05-16 13:59:47 -0700, Melanie Plageman wrote: > > On Wed, May 15, 2019 at 10:32 PM Ashwin Agrawal > wrote: > > > > > > > > The second index would help to hold the session after inserting the > tuple > > > in unique index but

Re: Adding a test for speculative insert abort case

2019-05-16 Thread Andres Freund
Hi, On 2019-05-16 13:59:47 -0700, Melanie Plageman wrote: > On Wed, May 15, 2019 at 10:32 PM Ashwin Agrawal wrote: > > > > > The second index would help to hold the session after inserting the tuple > > in unique index but before completing the speculative insert. Hence, helps > > to create the

Re: Adding a test for speculative insert abort case

2019-05-16 Thread Melanie Plageman
On Wed, May 15, 2019 at 10:32 PM Ashwin Agrawal wrote: > > The second index would help to hold the session after inserting the tuple > in unique index but before completing the speculative insert. Hence, helps > to create the condition easily. I believe order of index insertion is > helping here

Re: Adding a test for speculative insert abort case

2019-05-15 Thread Ashwin Agrawal
On Wed, May 15, 2019 at 8:36 PM Melanie Plageman wrote: > > On Wed, May 15, 2019 at 6:50 PM Andres Freund wrote: > >> >> > I noticed that there is not a test case which would cover the >> speculative >> > wait >> > codepath. This seems much more challenging, however, it does seem like a >> >

Re: Adding a test for speculative insert abort case

2019-05-15 Thread Andres Freund
Hi, On 2019-05-15 20:35:49 -0700, Melanie Plageman wrote: > > > I noticed that there is not a test case which would cover the speculative > > > wait > > > codepath. This seems much more challenging, however, it does seem like a > > > worthwhile test to have. > > > > Shouldn't be that hard to

Re: Adding a test for speculative insert abort case

2019-05-15 Thread Melanie Plageman
On Wed, May 15, 2019 at 6:50 PM Andres Freund wrote: > > Also, it would make the test easier to understand for me if, for > instances > > of the > > word "lock" in the test description and comments, you specified locktype > -- > > e.g. > > advisory lock. > > I got confused between the

Re: Adding a test for speculative insert abort case

2019-05-15 Thread Andres Freund
Hi, On 2019-05-15 18:34:15 -0700, Melanie Plageman wrote: > So, I recognize this has already been merged. However, after reviewing the > test, > I believe there is a typo in the second permutation. > > # Test that speculative locks are correctly acquired and released, s2 > # inserts, s1 updates.

Re: Adding a test for speculative insert abort case

2019-05-15 Thread Melanie Plageman
On Tue, May 14, 2019 at 12:19 PM Andres Freund wrote: > Hi, > > On 2019-05-10 14:40:38 -0700, Andres Freund wrote: > > On 2019-05-01 11:41:48 -0700, Andres Freund wrote: > > > I'm imagining something like > > > > > > if (pg_try_advisory_xact_lock(1)) > > > pg_advisory_xact_lock(2); > > >

Re: Adding a test for speculative insert abort case

2019-05-14 Thread Andres Freund
Hi, On 2019-05-10 14:40:38 -0700, Andres Freund wrote: > On 2019-05-01 11:41:48 -0700, Andres Freund wrote: > > I'm imagining something like > > > > if (pg_try_advisory_xact_lock(1)) > > pg_advisory_xact_lock(2); > > else > > pg_advisory_xact_lock(1); > > > > in t1_lock_func. If you

Re: Adding a test for speculative insert abort case

2019-05-10 Thread Andres Freund
Hi, On 2019-05-01 11:41:48 -0700, Andres Freund wrote: > I'm imagining something like > > if (pg_try_advisory_xact_lock(1)) > pg_advisory_xact_lock(2); > else > pg_advisory_xact_lock(1); > > in t1_lock_func. If you then make the session something roughly like > > s1:

Re: Adding a test for speculative insert abort case

2019-05-01 Thread Andres Freund
Hi, On 2019-04-30 18:34:42 -0700, Melanie Plageman wrote: > On Tue, Apr 30, 2019 at 5:22 PM Andres Freund wrote: > > > > > Not easily so - that's why the ON CONFLICT patch didn't add code > > coverage for it :(. I wonder if you could whip something up by having > > another non-unique expression

Re: Adding a test for speculative insert abort case

2019-05-01 Thread Melanie Plageman
On Tue, Apr 30, 2019 at 7:14 PM Thomas Munro wrote: > I think it'd be nice to have a set of macros that can create wait > points in the C code that isolation tests can control, in a special > build. Perhaps there could be shm hash table of named wait points in > shared memory; if

Re: Adding a test for speculative insert abort case

2019-04-30 Thread Thomas Munro
On Wed, May 1, 2019 at 12:16 PM Melanie Plageman wrote: > s1: insert into t1 values(1, 'someval') on conflict(id) do update set val = > 'someotherval'; > s1: pause in ExecInsert before calling ExecInsertIndexTuples > s2: insert into t1 values(1, 'someval'); > s2: continue > > We don't know of a

Re: Adding a test for speculative insert abort case

2019-04-30 Thread Andres Freund
Hi, On April 30, 2019 6:43:08 PM PDT, Peter Geoghegan wrote: >On Tue, Apr 30, 2019 at 5:16 PM Melanie Plageman > wrote: >> Can anyone think of a good way to put this codepath under test? > >During the initial development of ON CONFLICT, speculative insertion >itself was tested using custom

Re: Adding a test for speculative insert abort case

2019-04-30 Thread Peter Geoghegan
On Tue, Apr 30, 2019 at 5:16 PM Melanie Plageman wrote: > Can anyone think of a good way to put this codepath under test? During the initial development of ON CONFLICT, speculative insertion itself was tested using custom stress testing that you can still get here:

Re: Adding a test for speculative insert abort case

2019-04-30 Thread Melanie Plageman
On Tue, Apr 30, 2019 at 5:22 PM Andres Freund wrote: > > Not easily so - that's why the ON CONFLICT patch didn't add code > coverage for it :(. I wonder if you could whip something up by having > another non-unique expression index, where the expression acquires a > advisory lock? If that

Re: Adding a test for speculative insert abort case

2019-04-30 Thread Andres Freund
Hi, On 2019-04-30 17:15:55 -0700, Melanie Plageman wrote: > Today, while poking around the table_complete_speculative code which Ashwin > mentioned in [1], we were trying to understand when exactly we would > complete a > speculative insert by aborting. (FWIW, it's on my todo queue to look at

Adding a test for speculative insert abort case

2019-04-30 Thread Melanie Plageman
Today, while poking around the table_complete_speculative code which Ashwin mentioned in [1], we were trying to understand when exactly we would complete a speculative insert by aborting. We added a logging message to heapam_tuple_complete_speculative before it calls heap_abort_speculative and