Re: Adding a test for speculative insert abort case - Mailing list pgsql-hackers

From Melanie Plageman
Subject Re: Adding a test for speculative insert abort case
Date
Msg-id CAAKRu_aCwN-KZAa0B+xjm20F7k4Ki-_mVn=e4xe1cOcUW7SRoQ@mail.gmail.com
Whole thread Raw
In response to Re: Adding a test for speculative insert abort case  (Andres Freund <andres@anarazel.de>)
Responses Re: Adding a test for speculative insert abort case  (Andres Freund <andres@anarazel.de>)
Re: Adding a test for speculative insert abort case  (Ashwin Agrawal <aagrawal@pivotal.io>)
List pgsql-hackers


On Wed, May 15, 2019 at 6:50 PM Andres Freund <andres@anarazel.de> 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 speculative lock, the object locks on the index
> which
> are required for probing or inserting into the index, and the advisory
> locks.
>
> Below is a potential re-wording of one of the permutations that is more
> explicit
> and more clear to me as a reader.

Minor gripe: For the future, it's easier to such changes as a patch as
well - otherwise others need to move it to the file and diff it to
comment on the changes.

 
Will do--attached, though the wording is a rough suggestion.

> I was also wondering: Is it possible that one of the
> "controller_unlock_*" functions will get called before the session
> with the upsert has had a chance to move forward in its progress and
> be waiting on that lock?  That is, given that we don't check that the
> sessions are waiting on the locks before unlocking them, is there a
> race condition?

Isolationtester only switches between commands when either the command
finished, or once it's know to be waiting for a lock. Therefore I don't
think this race exists?  That logic is in the if (flags & STEP_NONBLOCK)
block in isolationtester.c:try_complete_step().

Does that make sense? Or did I misunderstand your concern?


I see. I didn't know what the blocking/waiting logic was in the isolation
framework. Nevermind, then.
 

> 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 create, I think. I think acquiring another
lock in a second, non-unique, expression index, ought to do the trick?
It probably has to be created after the unique index (so it's later in
the


I would think that the sequence would be s1 and s2 probe the index, s1 and s2
insert into the table, s1 updates the index but does not complete the
speculative insert and clear the token (pause before
table_complete_speculative). s2 is in speculative wait when attempting to update
the index.

Something like

permutation
   "controller_locks"
   "controller_show"
   "s1_upsert" "s2_upsert"
   "controller_show"
   "controller_unlock_1_1" "controller_unlock_2_1"
   "controller_unlock_1_3" "controller_unlock_2_3"
   "controller_unlock_1_2"
   "s1_magically_pause_before_complete_speculative"
   # put s2 in speculative wait
   "controller_unlock_2_2"
   "s1_magically_unpause_before_complete_speculative"

So, how would another lock on another index keep s1 from clearing the
speculative token after it has updated the index?

--
Melanie Plageman
Attachment

pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: Parallel Foreign Scans - need advice
Next
From: Andres Freund
Date:
Subject: Re: Adding a test for speculative insert abort case