Thread: Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple

Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple

From
Alvaro Herrera
Date:
Peter Geoghegan wrote:
> On Thu, Sep 28, 2017 at 7:47 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> > Fix freezing of a dead HOT-updated tuple
> 
> If I run Dan Wood's test case again, the obvious symptom (spurious
> duplicates) goes away. However, the enhanced amcheck, and thus CREATE
> INDEX/REINDEX, still isn't happy about this:
> 
> postgres=# select bt_index_check('t_pkey', true);
> DEBUG:  00000: verifying presence of required tuples in index "t_pkey"
> LOCATION:  bt_check_every_level, verify_nbtree.c:424
> ERROR:  XX000: failed to find parent tuple for heap-only tuple at
> (0,6) in table "t"
> LOCATION:  IndexBuildHeapRangeScan, index.c:2597
> Time: 3.699 ms

... Rats, I obviously missed the message where you said that amcheck
detected this problem.

Odd that it's not fixed.  I guess there's still some more work to do
here ...

-- 
Álvaro Herrera                https://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] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updatedtuple

From
Alvaro Herrera
Date:
Alvaro Herrera wrote:

> Odd that it's not fixed.  I guess there's still some more work to do
> here ...

Maybe what this means is that we need to do both Dan's initially
proposed patch (or something related to it) apart from the fixes already
pushed.  IOW we need to put back some of the "tupkeep" business ...

-- 
Álvaro Herrera                https://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] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple

From
Peter Geoghegan
Date:
On Thu, Sep 28, 2017 at 1:20 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> Alvaro Herrera wrote:
>
>> Odd that it's not fixed.  I guess there's still some more work to do
>> here ...
>
> Maybe what this means is that we need to do both Dan's initially
> proposed patch (or something related to it) apart from the fixes already
> pushed.  IOW we need to put back some of the "tupkeep" business ...

We certainly do still see wrong answers to queries here:

postgres=# select ctid, xmin, xmax, * from t;ctid  | xmin  | xmax | id | name | x
-------+-------+------+----+------+---(0,1) | 21171 |    0 |  1 | 111  | 0(0,7) | 21177 |    0 |  3 | 333  | 5
(2 rows)

postgres=# select * from t where id = 3;id | name | x
----+------+--- 3 | 333  | 5
(1 row)

postgres=# set enable_seqscan = off;
SET
postgres=# select * from t where id = 3;id | name | x
----+------+---
(0 rows)

FWIW, I am reminded a little bit of the MultiXact/recovery bug I
reported way back in February of 2014 [1], which also had a HOT
interaction that caused index scans to give wrong answers, despite
more or less structurally sound indexes.

[1] https://www.postgresql.org/message-id/CAM3SWZTMQiCi5PV5OWHb+bYkUcnCk=O67w0cSswPvV7XfUcU5g@mail.gmail.com

-- 
Peter Geoghegan


-- 
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] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updatedtuple

From
Alvaro Herrera
Date:
Peter Geoghegan wrote:

> We certainly do still see wrong answers to queries here:
> 
> postgres=# select ctid, xmin, xmax, * from t;
>  ctid  | xmin  | xmax | id | name | x
> -------+-------+------+----+------+---
>  (0,1) | 21171 |    0 |  1 | 111  | 0
>  (0,7) | 21177 |    0 |  3 | 333  | 5
> (2 rows)
> 
> postgres=# select * from t where id = 3;
>  id | name | x
> ----+------+---
>   3 | 333  | 5
> (1 row)
> 
> postgres=# set enable_seqscan = off;
> SET
> postgres=# select * from t where id = 3;
>  id | name | x
> ----+------+---
> (0 rows)

Yeah, oops.

> FWIW, I am reminded a little bit of the MultiXact/recovery bug I
> reported way back in February of 2014 [1], which also had a HOT
> interaction that caused index scans to give wrong answers, despite
> more or less structurally sound indexes.
> 
> [1] https://www.postgresql.org/message-id/CAM3SWZTMQiCi5PV5OWHb+bYkUcnCk=O67w0cSswPvV7XfUcU5g@mail.gmail.com

Thanks for the reference.  I didn't remember this problem and it's not
(wasn't) in my list of things to look into.  Perhaps these are both the
same bug.

-- 
Álvaro Herrera                https://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] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple

From
Peter Geoghegan
Date:
On Thu, Sep 28, 2017 at 2:39 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>> FWIW, I am reminded a little bit of the MultiXact/recovery bug I
>> reported way back in February of 2014 [1], which also had a HOT
>> interaction that caused index scans to give wrong answers, despite
>> more or less structurally sound indexes.
>>
>> [1] https://www.postgresql.org/message-id/CAM3SWZTMQiCi5PV5OWHb+bYkUcnCk=O67w0cSswPvV7XfUcU5g@mail.gmail.com
>
> Thanks for the reference.  I didn't remember this problem and it's not
> (wasn't) in my list of things to look into.  Perhaps these are both the
> same bug.

I was reminded of that old bug because initially, at the time, it
looked very much like a corrupt index: sequential scans were fine, but
index scans gave wrong answers. This is what I saw today.

In the end, commit 6bfa88a fixed that old recovery bug by making sure
the recovery routine heap_xlog_lock() did the right thing. In both
cases (Feb 2014 and today), the index wasn't really corrupt -- it just
pointed to the root of a HOT chain when it should point to some child
tuple (or maybe a successor HOT chain).

-- 
Peter Geoghegan


-- 
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] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple

From
Peter Geoghegan
Date:
On Thu, Sep 28, 2017 at 2:47 PM, Peter Geoghegan <pg@bowt.ie> wrote:
> In the end, commit 6bfa88a fixed that old recovery bug by making sure
> the recovery routine heap_xlog_lock() did the right thing. In both
> cases (Feb 2014 and today), the index wasn't really corrupt -- it just
> pointed to the root of a HOT chain when it should point to some child
> tuple (or maybe a successor HOT chain).

Just to be clear, obviously I don't mean that the index should have
been magically updated to compensate for that 2014 heap_lock_tuple()
recovery bug (say, via an in-place IndexTuple update during recovery).
Certainly, the index AM was entitled to assume that the heap TID it
pointed to would continue to be the root of the same HOT chain, at
least until the next VACUUM. That was what I meant by "the index
wasn't actually corrupt".

All I meant here is that the index scan and sequential scan would have
been in agreement if something like that *had* happened artificially
(say, when the index tuple was edited with a hex editor). And, that
the actual high level problem was that we failed to treat a
HOT-updated tuple as a HOT-updated tuple, leading to consequences that
were mostly noticeable during index scans. Probably on both occasions
(back in 2014, and now).

-- 
Peter Geoghegan


-- 
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] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple

From
Robert Haas
Date:
On Thu, Sep 28, 2017 at 5:47 PM, Peter Geoghegan <pg@bowt.ie> wrote:
> In the end, commit 6bfa88a fixed that old recovery bug by making sure
> the recovery routine heap_xlog_lock() did the right thing. In both
> cases (Feb 2014 and today), the index wasn't really corrupt -- it just
> pointed to the root of a HOT chain when it should point to some child
> tuple (or maybe a successor HOT chain).

Unless I'm very confused, it's really not OK to point at a child tuple
rather than the root of the HOT chain.

Pointing to a successor HOT chain would be OK, as long as you point to
the root tuple thereof.

-- 
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] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple

From
Peter Geoghegan
Date:
On Thu, Sep 28, 2017 at 3:20 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, Sep 28, 2017 at 5:47 PM, Peter Geoghegan <pg@bowt.ie> wrote:
>> In the end, commit 6bfa88a fixed that old recovery bug by making sure
>> the recovery routine heap_xlog_lock() did the right thing. In both
>> cases (Feb 2014 and today), the index wasn't really corrupt -- it just
>> pointed to the root of a HOT chain when it should point to some child
>> tuple (or maybe a successor HOT chain).
>
> Unless I'm very confused, it's really not OK to point at a child tuple
> rather than the root of the HOT chain.

Please see my later clarification.

-- 
Peter Geoghegan


-- 
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] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple

From
Michael Paquier
Date:
On Fri, Sep 29, 2017 at 6:39 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> Peter Geoghegan wrote:
>
>> We certainly do still see wrong answers to queries here:
>>
>> postgres=# select ctid, xmin, xmax, * from t;
>>  ctid  | xmin  | xmax | id | name | x
>> -------+-------+------+----+------+---
>>  (0,1) | 21171 |    0 |  1 | 111  | 0
>>  (0,7) | 21177 |    0 |  3 | 333  | 5
>> (2 rows)
>>
>> postgres=# select * from t where id = 3;
>>  id | name | x
>> ----+------+---
>>   3 | 333  | 5
>> (1 row)
>>
>> postgres=# set enable_seqscan = off;
>> SET
>> postgres=# select * from t where id = 3;
>>  id | name | x
>> ----+------+---
>> (0 rows)
>
> Yeah, oops.

This really looks like a problem at heap-level with the parent
redirection not getting defined (?). Please note that a subsequent
REINDEX fails as well:
=# reindex index t_pkey ;
ERROR:  XX000: failed to find parent tuple for heap-only tuple at
(0,7) in table "t"
LOCATION:  IndexBuildHeapRangeScan, index.c:2597

VACUUM FREEZE also is not getting things right, but a VACUUM FULL does.

Also, dropping the constraint and attempting to recreate it is failing:
=# alter table t drop constraint t_pkey;
ALTER TABLE
=# create index t_pkey on t(id);
ERROR:  XX000: failed to find parent tuple for heap-only tuple at
(0,7) in table "t"
LOCATION:  IndexBuildHeapRangeScan, index.c:2597

A corrupted page clearly indicates that there are no tuple redirections:
=# select lp, t_ctid, lp_off, t_infomask, t_infomask2 from
heap_page_items(get_raw_page('t', 0));lp | t_ctid | lp_off | t_infomask | t_infomask2
----+--------+--------+------------+------------- 1 | (0,1)  |   8152 |       2818 |           3 2 | null   |      0 |
    null |        null 3 | (0,4)  |   8112 |       9986 |       49155 4 | (0,5)  |   8072 |       9986 |       49155 5
|(0,6)  |   8032 |       9986 |       49155 6 | (0,7)  |   7992 |       9986 |       49155 7 | (0,7)  |   7952 |
11010|       32771
 
(7 rows)

And a non-corrupted page clearly shows the redirection done with lp_off:lp | t_ctid | lp_off | t_infomask |
t_infomask2
----+--------+--------+------------+------------- 1 | (0,1)  |   8152 |       2818 |           3 2 | null   |      7 |
    null |        null 3 | null   |      0 |       null |        null 4 | null   |      0 |       null |        null 5
|null   |      0 |       null |        null 6 | null   |      0 |       null |        null 7 | (0,7)  |   8112 |
11010|       32771
 
(7 rows)
-- 
Michael


-- 
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] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple

From
Peter Geoghegan
Date:
On Thu, Sep 28, 2017 at 1:20 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> Maybe what this means is that we need to do both Dan's initially
> proposed patch (or something related to it) apart from the fixes already
> pushed.  IOW we need to put back some of the "tupkeep" business ...

I took the time to specifically check if that would fix the problem.
Unfortunately, it did not. We see exactly the same problem, or at
least amcheck/REINDEX produces exactly the same error. I checked both
Dan's original update_freeze.patch, and your revision that retained
some of the "tupkeep" stuff,
0002-Don-t-freeze-recently-dead-HOT-tuples, which you posted on
September 6th.

-- 
Peter Geoghegan


-- 
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] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple

From
Michael Paquier
Date:
On Sun, Oct 1, 2017 at 10:25 AM, Peter Geoghegan <pg@bowt.ie> wrote:
> On Thu, Sep 28, 2017 at 1:20 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>> Maybe what this means is that we need to do both Dan's initially
>> proposed patch (or something related to it) apart from the fixes already
>> pushed.  IOW we need to put back some of the "tupkeep" business ...
>
> I took the time to specifically check if that would fix the problem.
> Unfortunately, it did not. We see exactly the same problem, or at
> least amcheck/REINDEX produces exactly the same error. I checked both
> Dan's original update_freeze.patch, and your revision that retained
> some of the "tupkeep" stuff,
> 0002-Don-t-freeze-recently-dead-HOT-tuples, which you posted on
> September 6th.

I did not take the time to dig into that more than two hours, but my
first feeling is that some race condition is going on with the heap
pruning.
-- 
Michael


-- 
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] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updatedtuple

From
Alvaro Herrera
Date:
Michael Paquier wrote:
> On Sun, Oct 1, 2017 at 10:25 AM, Peter Geoghegan <pg@bowt.ie> wrote:
> > On Thu, Sep 28, 2017 at 1:20 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> >> Maybe what this means is that we need to do both Dan's initially
> >> proposed patch (or something related to it) apart from the fixes already
> >> pushed.  IOW we need to put back some of the "tupkeep" business ...
> >
> > I took the time to specifically check if that would fix the problem.
> > Unfortunately, it did not. We see exactly the same problem, or at
> > least amcheck/REINDEX produces exactly the same error. I checked both
> > Dan's original update_freeze.patch, and your revision that retained
> > some of the "tupkeep" stuff,
> > 0002-Don-t-freeze-recently-dead-HOT-tuples, which you posted on
> > September 6th.
> 
> I did not take the time to dig into that more than two hours, but my
> first feeling is that some race condition is going on with the heap
> pruning.

I'll look into this on Monday.  I found out yesterday that the
problematic case is when HTSV returns HEAPTUPLE_DEAD and the HOT tests
return true.  I haven't yet figured if it is one of those specifically,
but I suspect it is the IsHotUpdated case.

-- 
Álvaro Herrera                https://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