Re: Adding a test for speculative insert abort case

On Wed, May 15, 2019 at 6:50 PM Andres Freund <andres(at)anarazel(dot)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