Re: SSI patch version 14

From: "Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov>
To: <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Cc: <simon(at)2ndQuadrant(dot)com>,<markus(at)bluegap(dot)ch>, <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: SSI patch version 14
Date: 2011-02-05 17:58:13
Message-ID: 4D4D3B56020000250003A466@gw.wicourts.gov
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Heikki Linnakangas wrote:
> On 02.02.2011 19:36, Kevin Grittner wrote:
>
>> Then there's one still lurking outside the new predicate* files,
>> in heapam.c. It's about 475 lines into the heap_update function
>> (25 lines from the bottom). In reviewing where we needed to
>> acquire predicate locks, this struck me as a place where we might
>> need to duplicate the predicate lock from one tuple to another,
>> but I wasn't entirely sure. I tried for a few hours one day to
>> construct a scenario which would show a serialization anomaly if I
>> didn't do this, and failed create one. If someone who understands
>> both the patch and heapam.c wants to look at this and offer an
>> opinion, I'd be most grateful. I'll take another look and see if I
>> can get my head around it better, but failing that, I'm
>> disinclined to either add locking I'm not sure we need or to
>> remove the comment that says we *might* need it there.
>
> Have you convinced yourself that there's nothing to do? If so, we
> should replace the todo comment with a comment explaining why.

It turns out that nagging doubt from my right-brain was on target.
Here's the simplest example I was able to construct of a false
negative due to the lack of some code there:

-- setup
create table t (id int not null, txt text) with (fillfactor=50);
insert into t (id)
select x from (select * from generate_series(1, 1000000)) a(x);
alter table t add primary key (id);

-- session 1
-- T1
begin transaction isolation level serializable;
select * from t where id = 1000000;

-- session 2
-- T2
begin transaction isolation level serializable;
update t set txt = 'x' where id = 1000000;
commit;
-- T3
begin transaction isolation level serializable;
update t set txt = 'y' where id = 1000000;
select * from t where id = 500000;

-- session 3
-- T4
begin transaction isolation level serializable;
update t set txt = 'z' where id = 500000;
select * from t where id = 1;
commit;

-- session 2
commit;

-- session 1
update t set txt = 'a' where id = 1;
commit;

Based on visibility of work, there's a cycle in the apparent order of
execution:

T1 -> T2 -> T3 -> T4 -> T1

So now that I'm sure we actually do need code there, I'll add it.
And I'll add the new test to the isolation suite.

-Kevin

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2011-02-05 18:05:01 Re: OCLASS_FOREIGN_TABLE support is incomplete
Previous Message Cédric Villemain 2011-02-05 17:54:28 Re: We need to log aborted autovacuums