Обсуждение: Re: [HACKERS] [COMMITTERS] pgsql: Fix freezing of a dead HOT-updated tuple
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
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
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
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
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
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
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
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
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
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
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
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