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?