Re: [HACKERS] Predicate Locks for writes?

[Alexander Korotkov]

On Wed, Oct 11, 2017 at 7:27 PM, Robert Haas  wrote:

> On Wed, Oct 11, 2017 at 11:51 AM, Simon Riggs 
> wrote:
> > I'm inclined to believe Robert's hypothesis that there is some race
> > condition there.
> >
> > The question is whether that still constitutes a serializability
> > problem since we haven't done anything with the data, just passed it
> > upwards to be modified.
>
> Well, the question is whether passing it upwards constitutes reading
> it.  I kind of suspect it does.  The plan tree isn't just blindly
> propagating values upward but stuff with them.  There could be quite a
> bit between the ModifyTable node and the underlying scan if DELETE ..
> FROM or UPDATE .. USING is in use.
>

Right.  In general we can't skip SIReadLock just basing on the fact that
we're under ModifyTable node.
It seems still possible for me to skip SIReadLock in some very limited (but
probably typical) cases.
But before analyzing this deeper, it would be nice to estimate possible
benefits.
We can try some broken version which skip all SIReadLock's under
ModifyTable node and benchmark it.
If it wouldn't have significant performance benefit, then there is no
reason to do something further...

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Re: [HACKERS] Predicate Locks for writes?

[Robert Haas]

On Wed, Oct 11, 2017 at 11:51 AM, Simon Riggs  wrote:
> I'm inclined to believe Robert's hypothesis that there is some race
> condition there.
>
> The question is whether that still constitutes a serializability
> problem since we haven't done anything with the data, just passed it
> upwards to be modified.

Well, the question is whether passing it upwards constitutes reading
it.  I kind of suspect it does.  The plan tree isn't just blindly
propagating values upward but stuff with them.  There could be quite a
bit between the ModifyTable node and the underlying scan if DELETE ..
FROM or UPDATE .. USING is in use.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Predicate Locks for writes?

[Simon Riggs]

On 11 October 2017 at 15:33, Robert Haas  wrote:
> On Sat, Oct 7, 2017 at 7:26 AM, Simon Riggs  wrote:
>> PredicateLockTuple() specifically avoids adding an SIRead lock if the
>> tuple already has a write lock on it, so surely it must also be
>> correct to skip the SIRead lock if we are just about to update the
>> row?
>
> I wonder if there's a race condition.  Can someone else read/update
> the tuple between the time when we would've taken the SIRead lock and
> the time when we would have gotten the actual tuple lock?

On 9 October 2017 at 13:23, Alexander Korotkov
 wrote:

>> PredicateLockTuple() specifically avoids adding an SIRead lock if the
>> tuple already has a write lock on it, so surely it must also be
>> correct to skip the SIRead lock if we are just about to update the
>> row?
>>
>> I am suggesting that we ignore PredicateLockTuple() for cases where we
>> are about to update or delete the found tuple.
>
>
> If my thoughts above are correct, than it would be a reasonable
> optimization.  We would skip cycle of getting SIReadLock on tuple and then
> releasing it, without any change of behavior.

I'm inclined to believe Robert's hypothesis that there is some race
condition there.

The question is whether that still constitutes a serializability
problem since we haven't done anything with the data, just passed it
upwards to be modified.

If not we can just skip the SIread lock.

If it is an issue that still leaves the optimization in the case where
we choose to locate the row using an exclusive lock and just skip the
SIread lock altogether.

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Predicate Locks for writes?

[Robert Haas]

On Sat, Oct 7, 2017 at 7:26 AM, Simon Riggs  wrote:
> PredicateLockTuple() specifically avoids adding an SIRead lock if the
> tuple already has a write lock on it, so surely it must also be
> correct to skip the SIRead lock if we are just about to update the
> row?

I wonder if there's a race condition.  Can someone else read/update
the tuple between the time when we would've taken the SIRead lock and
the time when we would have gotten the actual tuple lock?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Predicate Locks for writes?

[Alexander Korotkov]

On Sat, Oct 7, 2017 at 2:26 PM, Simon Riggs  wrote:

> SERIALIZABLE looks for chains of rw cases.
>
> When we perform UPDATEs and DELETEs we search for rows and then modify
> them. The current implementation views that as a read followed by a
> write because we issue PredicateLockTuple() during the index fetch.
>
> Is it correct that a statement that only changes data will add
> predicate locks for the rows that it modifies?
>

I'm not very aware of how this SI code exactly works.  But it seems that
once we update row, read SIReadLock on that tuple is released, because
we're already holding stronger lock.  I made simple experiment.

# begin;
BEGIN
Time: 0,456 ms
smagen@postgres=*# select * from test where id = 1;
 id │ val
┼─
  1 │ xyz
(1 row)

*# select locktype, database, relation, page, tuple, mode, granted from
pg_locks where pid = pg_backend_pid();
  locktype  │ database │ relation │ page │ tuple │  mode   │ granted
┼──┼──┼──┼───┼─┼─
 relation   │12558 │11577 │ NULL │  NULL │ AccessShareLock │ t
 relation   │12558 │65545 │ NULL │  NULL │ AccessShareLock │ t
 relation   │12558 │65538 │ NULL │  NULL │ AccessShareLock │ t
 virtualxid │ NULL │ NULL │ NULL │  NULL │ ExclusiveLock   │ t
 page   │12558 │65545 │1 │  NULL │ SIReadLock  │ t
 tuple  │12558 │65538 │0 │ 3 │ SIReadLock  │ t
(6 rows)

*# update test set val = 'xyz' where id = 1;

*# select locktype, database, relation, page, tuple, mode, granted from
pg_locks where pid = pg_backend_pid();
   locktype│ database │ relation │ page │ tuple │   mode   │
granted
───┼──┼──┼──┼───┼──┼─
 relation  │12558 │11577 │ NULL │  NULL │ AccessShareLock  │ t
 relation  │12558 │65545 │ NULL │  NULL │ AccessShareLock  │ t
 relation  │12558 │65545 │ NULL │  NULL │ RowExclusiveLock │ t
 relation  │12558 │65538 │ NULL │  NULL │ AccessShareLock  │ t
 relation  │12558 │65538 │ NULL │  NULL │ RowExclusiveLock │ t
 virtualxid│ NULL │ NULL │ NULL │  NULL │ ExclusiveLock│ t
 transactionid │ NULL │ NULL │ NULL │  NULL │ ExclusiveLock│ t
 page  │12558 │65545 │1 │  NULL │ SIReadLock   │ t
(8 rows)

Once we update the same tuple that we read before, SIReadLock on that tuple
disappears.

PredicateLockTuple() specifically avoids adding an SIRead lock if the
> tuple already has a write lock on it, so surely it must also be
> correct to skip the SIRead lock if we are just about to update the
> row?
>
> I am suggesting that we ignore PredicateLockTuple() for cases where we
> are about to update or delete the found tuple.
>

If my thoughts above are correct, than it would be a reasonable
optimization.  We would skip cycle of getting SIReadLock on tuple and then
releasing it, without any change of behavior.


> ISTM that a Before Row Trigger would need to add an SIRead lock since
> that is clearly a read.
>

Yes, because depending on result of "Before Row Trigger" update might not
really happen.  That SIReadLock on tuple is necessary.
However, ISTM that we could place SIReadLock on tuple after Before Row
Trigger and only in the case when trigger has cancelled an update.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

[HACKERS] Predicate Locks for writes?

[Simon Riggs]

SERIALIZABLE looks for chains of rw cases.

When we perform UPDATEs and DELETEs we search for rows and then modify
them. The current implementation views that as a read followed by a
write because we issue PredicateLockTuple() during the index fetch.

Is it correct that a statement that only changes data will add
predicate locks for the rows that it modifies?

PredicateLockTuple() specifically avoids adding an SIRead lock if the
tuple already has a write lock on it, so surely it must also be
correct to skip the SIRead lock if we are just about to update the
row?

I am suggesting that we ignore PredicateLockTuple() for cases where we
are about to update or delete the found tuple.

ISTM that a Before Row Trigger would need to add an SIRead lock since
that is clearly a read.

Thoughts?

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers