Re: Corrupted btree index on HEAD because of covering indexes

Teodor, are you caught up to the point where it'd be okay to run pgindent, or are there still patches waiting? There is a gin predicate lock patch from Heikki, I will work on it. But I have not objection to run pgindent, I believe gin patch will be easy to change. -- Teodor Sigaev

Re: Corrupted btree index on HEAD because of covering indexes

On Wed, Apr 25, 2018 at 2:09 PM, Alexander Korotkov wrote: > Great. Thank you very much for your efforts on this feature! You're welcome. I also appreciate the efforts of all Postgres Pro people. I especially appreciate Anastasia's work on this project. I hope that

Re: Corrupted btree index on HEAD because of covering indexes

On Wed, Apr 25, 2018 at 11:48 PM, Peter Geoghegan wrote: > On Wed, Apr 25, 2018 at 1:45 PM, Tom Lane wrote: > > Teodor, are you caught up to the point where it'd be okay to run > pgindent, > > or are there still patches waiting? > > I can't speak for Teodor,

Re: Corrupted btree index on HEAD because of covering indexes

On Wed, Apr 25, 2018 at 1:45 PM, Tom Lane wrote: > Teodor, are you caught up to the point where it'd be okay to run pgindent, > or are there still patches waiting? I can't speak for Teodor, but fwiw I am not aware of any more patches waiting. -- Peter Geoghegan

Re: Corrupted btree index on HEAD because of covering indexes

Teodor Sigaev writes: >> It looks like you pushed v1, which didn't have the tests and other >> changes you asked for. Attached patch adds those back. > Oops, I missed, I don't know how... Thank you very much for your quick eye! Teodor, are you caught up to the point where it'd

Re: Corrupted btree index on HEAD because of covering indexes

On Wed, Apr 25, 2018 at 12:06 PM, Teodor Sigaev wrote: > Oops, I missed, I don't know how... Thank you very much for your quick eye! Thanks Teodor. -- Peter Geoghegan

Re: Corrupted btree index on HEAD because of covering indexes

It looks like you pushed v1, which didn't have the tests and other changes you asked for. Attached patch adds those back. Oops, I missed, I don't know how... Thank you very much for your quick eye! -- Teodor Sigaev E-mail: teo...@sigaev.ru

Re: Corrupted btree index on HEAD because of covering indexes

On Wed, Apr 25, 2018 at 8:06 AM, Teodor Sigaev wrote: > Leave it in both places. In ideal world, in amcheck test we need to create a > broken index, but I don't know a correct way to do it :) There are many ways to create a broken index, but they're hard to make work with

Re: Corrupted btree index on HEAD because of covering indexes

2) Pls, add to test DELETE as it done in 6db4b49986be3fe59a1f6ba6fabf9852864efc3e Done. I will leave it to you to decide whether or not the original create_index.sql test can now be removed. Leave it in both places. In ideal world, in amcheck test we need to create a broken index, but I don't

Re: Corrupted btree index on HEAD because of covering indexes

Thank you, pushed. Peter Geoghegan wrote: On Tue, Apr 24, 2018 at 9:06 AM, Teodor Sigaev wrote: Perfect! -- Teodor Sigaev E-mail: teo...@sigaev.ru WWW: http://www.sigaev.ru/

Re: Corrupted btree index on HEAD because of covering indexes

On Tue, Apr 24, 2018 at 9:06 AM, Teodor Sigaev wrote: > Perfect! Thanks! > I would like to commit it but have some suggestions: I attach a revised version, which has changes based on your feedback. > to improve test stability it would be better to disable autovacuum: > ALTER

Re: Corrupted btree index on HEAD because of covering indexes

Perfect! I would like to commit it but have some suggestions: 1) TRUNCATE bttest_multi; INSERT INTO bttest_multi SELECT i, i%2 FROM generate_series(1, 10) as i; SELECT bt_index_parent_check('bttest_multi_idx', true); to improve test stability it would be better to disable autovacuum: ALTER

Re: Corrupted btree index on HEAD because of covering indexes

On Sat, Apr 21, 2018 at 6:02 PM, Peter Geoghegan wrote: > I refined the amcheck enhancement quite a bit today. It will not just > check that a downlink is not missing; It will also confirm that it > wasn't a legitimately interrupted multi-level deletion, by descending > to the leaf

Re: Corrupted btree index on HEAD because of covering indexes

Thank you, pushed I see your point. Maybe don't have the newline between the get and set, though, to match the existing style. And, the note about the assertion seems unnecessary. Removed newline and note "Get/set leaf page highkey's link. During the second phase of deletion, the target leaf

Re: Corrupted btree index on HEAD because of covering indexes

On Thu, Apr 19, 2018 at 9:42 AM, Teodor Sigaev wrote: > But some later, in _bt_unlink_halfdead_page() we check ItemPointer high key > with ItemPointerIsValid macro - and it returns false, because offset is > actually InvalidOffsetNumber - i.e. 0 which was set by

Re: Corrupted btree index on HEAD because of covering indexes

On Fri, Apr 20, 2018 at 7:18 AM, Teodor Sigaev wrote: > After close look I change my opinion. To have a clean code it's much better > to have new pair get/set macroses specialy to manage link to top pare during > page deletion. This removes last naked usage of >

Re: Corrupted btree index on HEAD because of covering indexes

On Thu, Apr 19, 2018 at 11:41 PM, Teodor Sigaev wrote: >> heard of people using bt_index_parent_check() in production, but only >> when they already knew that their database was corrupt, and wanted to >> isolate the problem. I imagine that people that use >>

Re: Corrupted btree index on HEAD because of covering indexes

Should we use BTreeInnerTupleGetDownLink() as soon as we use BTreeInnerTupleSetDownLink() for setting this? Or even invent BTreeInnerTupleDownLinkIsValid() macro? I am not sure. Here we actually store UP link - to top parent to remove. I'm afraid using

Re: Corrupted btree index on HEAD because of covering indexes

heard of people using bt_index_parent_check() in production, but only when they already knew that their database was corrupt, and wanted to isolate the problem. I imagine that people that use bt_index_parent_check() in production do so because they want as much information as possible, and are

Re: Corrupted btree index on HEAD because of covering indexes

I tried to minimize Michael's test case and add it to patch. -if (ItemPointerIsValid(leafhikey)) +if (ItemPointerGetBlockNumberNoCheck(leafhikey) != InvalidBlockNumber) Should we use BTreeInnerTupleGetDownLink() as soon as we use BTreeInnerTupleSetDownLink() for setting this? Or even

Re: Corrupted btree index on HEAD because of covering indexes

On Thu, Apr 19, 2018 at 10:47:02PM +0300, Teodor Sigaev wrote: > I tried to minimize Michael's test case and add it to patch. I cannot comment on the actual fix as I lack background in the area, but having a test case and even more having pg_upgrade do some work on those pages are good things.

Re: Corrupted btree index on HEAD because of covering indexes

On Thu, Apr 19, 2018 at 11:59 AM, Teodor Sigaev wrote: > Agree. Hope, nobody found a way how to use amcheck module in production to > serve user requests. But, it should be implemented before BETA stage, in > opposite case we will get a lot of objections. It shouldn't take that

Re: Corrupted btree index on HEAD because of covering indexes

On Thu, Apr 19, 2018 at 2:00 PM, Alexander Korotkov wrote: > - if (ItemPointerIsValid(leafhikey)) > + if (ItemPointerGetBlockNumberNoCheck(leafhikey) != InvalidBlockNumber) > > Should we use BTreeInnerTupleGetDownLink() as soon as we use > BTreeInnerTupleSetDownLink()

Re: Corrupted btree index on HEAD because of covering indexes

I also think that we could have better conventional regression test coverage here. I tried to minimize Michael's test case and add it to patch. -- Teodor Sigaev E-mail: teo...@sigaev.ru WWW: http://www.sigaev.ru/ diff --git

Re: Corrupted btree index on HEAD because of covering indexes

I would like to go and implement this check now, and if all goes well I may propose that you commit it to the master branch for v11. I don't see this as a new feature. I see it as essential testing infrastructure. What do you think about adding this new check soon? Agree. Hope, nobody found a way

Re: Corrupted btree index on HEAD because of covering indexes

On Thu, Apr 19, 2018 at 9:42 AM, Teodor Sigaev wrote: > Interesting, contrib/amcheck doesn't find any error in index. Seems, it's > subject for further improvement. I think that you're right that this should be detectable by bt_index_parent_check(). I have an idea about how we

Re: Corrupted btree index on HEAD because of covering indexes

I'll take a look tomorrow. Interesting, contrib/amcheck doesn't find any error in index. Seems, it's subject for further improvement. Nevertheless, seems, I found. In _bt_mark_page_halfdead() we use truncated high key IndexTuple as a storage of blocknumber of top parent to remove. And sets

Re: Corrupted btree index on HEAD because of covering indexes

On Wed, Apr 18, 2018 at 10:29 PM, Teodor Sigaev wrote: > Will see... I'll take a look tomorrow. -- Peter Geoghegan

Re: Corrupted btree index on HEAD because of covering indexes

Will see... Michael Paquier wrote: Hi all, I was just testing the VACUUM truncation logic, and bumped into what looks like a corrupted btree index. Here is a reproducer: create table aa (a int primary key, b bool); insert into aa values (generate_series(1,100), false); checkpoint; update

Corrupted btree index on HEAD because of covering indexes

Hi all, I was just testing the VACUUM truncation logic, and bumped into what looks like a corrupted btree index. Here is a reproducer: create table aa (a int primary key, b bool); insert into aa values (generate_series(1,100), false); checkpoint; update aa set b = false where a > 50; --