Re: foreign key locks, 2nd attempt - Mailing list pgsql-hackers

On Mon, Mar 5, 2012 at 7:53 PM, Alvaro Herrera
<alvherre@commandprompt.com> wrote:

>> My other comments so far are
>>
>> * some permutations commented out - no comments as to why
>> Something of a fault with the isolation tester that it just shows
>> output, there's no way to record expected output in the spec
>
> The reason they are commented out is that they are "invalid", that is,
> it requires running a command on a session that's blocked in the
> previous command.  Obviously, that cannot happen in real life.
>
> isolationtester now has support for detecting such conditions; if the
> spec specifies running a command in a locked session, the permutation is
> killed with an error message "invalid permutation" and just continues
> with the next permutation.  It used to simply die, aborting the test.
> Maybe we could just modify the specs so that all permutations are there
> (this can be done by simply removing the permutation lines), and the
> "invalid permutation" messages are part of the expected file.  Would
> that be better?

It would be better to have an isolation tester mode that checks to see
it was invalid and if not, report that.

At the moment we can't say why you commented something out. There's no
comment or explanation, and we need something, otherwise 3 years from
now we'll be completely in the dark.


>> Comments required for these points
>>
>> * Why do we need multixact to be persistent? Do we need every page of
>> multixact to be persistent, or just particular pages in certain
>> circumstances?
>
> Any page that contains at least one multi with an update as a member
> must persist.  It's possible that some pages contain no update (and this
> is even likely in some workloads, if updates are rare), but I'm not sure
> it's worth complicating the code to cater for early removal of some
> pages.

If the multixact contains an xid and that is being persisted then you
need to set an LSN to ensure that a page writes causes an XLogFlush()
before the multixact write. And you need to set do_fsync, no? Or
explain why not in comments...

I was really thinking we could skip the fsync of a page if we've not
persisted anything important on that page, since that was one of
Robert's performance points.


>> * Why do we need to expand multixact with flags? Can we avoid that in
>> some cases?
>
> Did you read my blog post?
> http://www.commandprompt.com/blogs/alvaro_herrera/2011/08/fixing_foreign_key_deadlocks_part_three/
> This explains the reason -- the point is that we need to distinguish the
> lock strength acquired by each locker.

Thanks, I will, but it all belongs in a README please.


>> * Why do we need to store just single xids in multixact members?
>> Didn't understand comments, no explanation
>
> This is just for SELECT FOR SHARE.  We don't have a hint bit to indicate
> "this tuple has a for-share lock", so we need to create a multi for it.
> Since FOR SHARE is probably going to be very uncommon, this isn't likely
> to be a problem.  We're mainly catering for users of SELECT FOR SHARE so
> that it continues to work, i.e. maintain backwards compatibility.

Good, thanks.

Are we actively recommending people use FOR KEY SHARE rather than FOR
SHARE, in explicit use?

> (Maybe I misunderstood your question -- what I think you're asking is,
> "why are there some multixacts that have a single member?")
>
> I'll try to come up with a good place to add some paragraphs about all
> this.  Please let me know if answers here are unclear and/or you have
> further questions.

Thanks

I think we need to define some test workloads to measure the
performance impact of this patch. We need to be certain that it has a
good impact in target cases, plus a known impact in other cases.

Suggest

* basic pgbench - no RI

* inserts into large table, RI checks to small table, no activity on small table

* large table parent, large table: child
20 child rows per parent, fk from child to parent
updates of multiple children at same time
low/medium/heavy locking

* large table parent, large table: child
20 child rows per parent,fk from child to parent
updates of parent and child at same time
low/medium/heavy locking

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services