From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Andrey Borodin <x4mmm@yandex-team.ru>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Cc: Костя Кузнецов <chapaev28@ya.ru>
Subject: Re: GiST VACUUM
Date: Mon, 4 Mar 2019 15:04:37 +0200
Message-ID: <96ec7ebd-42b9-4df5-18a4-42181c8a5a41@iki.fi> (raw)
In-Reply-To: <EB87A69B-EE5E-4259-9EEB-DA9DC1F7E265@yandex-team.ru>
References: <479336FE-B713-4DFB-9661-9BFCCA70F4EF@yandex-team.ru>
<51675201-3F1C-4796-ADB4-28D0B35E9CF0@yandex-team.ru>
<651572F0-65C3-4285-85FC-50E81B1199F1@yandex-team.ru>
<CAEepm=1vznVTZf01f4KNVJEa4m4tCgtBuumgmY8q=g+S=Gdt5A@mail.gmail.com>
<2AF4B7F1-ABCF-4602-8AF8-06EA45191BFC@yandex-team.ru>
<844f3373-486e-a34d-d5a1-6a048e19355c@iki.fi>
<4A1FF988-A69D-4607-8A6E-B3EC0FD6C2CF@yandex-team.ru>
<43f15ab5-dd95-97d4-658b-e9954ca986a6@iki.fi>
<DABC2BB9-154A-47E2-B1D9-B58A4023E6D7@yandex-team.ru>
<E8FDC7F0-0516-4A84-B754-4CC92801E5F5@yandex-team.ru>
<20181002011404.GU11712@paquier.xyz>
<C1D00194-9A4D-4DE1-AAFE-6DA478A15207@yandex-team.ru>
<d7e89e1b-efd4-1464-f03d-4ba4ea9e1835@iki.fi>
<EB87A69B-EE5E-4259-9EEB-DA9DC1F7E265@yandex-team.ru>
On 04/01/2019 02:47, Andrey Borodin wrote:
>> 2 янв. 2019 г., в 20:35, Heikki Linnakangas <hlinnaka@iki.fi> написал(а):
>>
>> In patch #1, to do the vacuum scan in physical order:
>> ...
>> I think this is ready to be committed, except that I didn't do any testing. We discussed the need for testing earlier. Did you write some test scripts for this, or how have you been testing?
> Please see test I used to check left jumps for v18:
> 0001-Physical-GiST-scan-in-VACUUM-v18-with-test-modificat.patch
> 0002-Test-left-jumps-v18.patch
>
> To trigger FollowRight GiST sometimes forget to clear follow-right marker simulating crash of an insert. This fills logs with "fixing incomplete split" messages. Search for "REMOVE THIS" to disable these ill-behavior triggers.
> To trigger NSN jump GiST allocate empty page after every real allocation.
>
> In log output I see
> 2019-01-03 22:27:30.028 +05 [54596] WARNING: RESCAN TRIGGERED BY NSN
> WARNING: RESCAN TRIGGERED BY NSN
> 2019-01-03 22:27:30.104 +05 [54596] WARNING: RESCAN TRIGGERED BY FollowRight
> This means that code paths were really executed (for v18).
Thanks! As I noted at
https://www.postgresql.org/message-id/2ff57b1f-01b4-eacf-36a2-485a12017f6e%40iki.fi,
the test patch left the index corrupt. I fixed it so that it leaves
behind incompletely split pages, without the corruption, see attached.
It's similar to yours, but let me recap what it does:
* Hacks gistbuild(), create 100 empty pages immediately after the root
pages. They are leaked, so they won't be reused until the a VACUUM sees
them and puts them to the FSM
* Hacks gistinserttuples(), to leave the split incompleted with 50%
probability
* In vacuum, print a line to the log whenever it needs to "jump left"
I used this patch, with the attached test script that's similar to
yours, but it also tries to verify that the index returns correct
results. It prints a result set like this:
sum
---------
-364450
364450
(2 rows)
If the two numbers add up to 0, the index seems valid. And you should
see "RESCAN" lines in the log, to show that jumping left happened.
Because the behavior is random and racy, you may need to run the script
many times to see both "RESCAN TRIGGERED BY NSN" and "RESCAN TRIGGERED
BY FollowRight" cases. Especially the "FollowRight" case happens less
frequently than the NSN case, you might need to run the script > 10
times to see it.
I also tried your amcheck tool with this. It did not report any errors.
Attached is also latest version of the patch itself. It is the same as
your latest patch v19, except for some tiny comment kibitzing. I'll mark
this as Ready for Committer in the commitfest app, and will try to
commit it in the next couple of days.
- Heikki
Attachments:
gist-vacuum-test.sh (application/x-shellscript, 1.7K) downloadTest-left-jumps-2.patch (text/x-patch, 2.6K) downloaddiff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.cindex 3f52b8f4dc..cad4a2a46e 100644--- a/src/backend/access/gist/gist.c+++ b/src/backend/access/gist/gist.c@@ -1187,6 +1187,8 @@ gistinserttuple(GISTInsertState *state, GISTInsertStack *stack,
InvalidBuffer, InvalidBuffer, false, false);
}
+static bool HACK = false;+
/* ----------------
* An extended workhorse version of gistinserttuple(). This version allows
* inserting multiple tuples, or replacing a single tuple with multiple tuples.
@@ -1230,6 +1232,14 @@ gistinserttuples(GISTInsertState *state, GISTInsertStack *stack,
CheckForSerializableConflictIn(state->r, NULL, stack->buffer);
/* Insert the tuple(s) to the page, splitting the page if necessary */
+ if (BufferIsValid(leftchild) && HACK)+ {+ /* skip actually inserting */+ splitinfo = NULL;+ is_split = false;+ }+ else+ {
is_split = gistplacetopage(state->r, state->freespace, giststate,
stack->buffer,
tuples, ntup,
@@ -1238,6 +1248,7 @@ gistinserttuples(GISTInsertState *state, GISTInsertStack *stack,
&splitinfo,
true,
state->heapRel);
+ }
/*
* Before recursing up in case the page was split, release locks on the
@@ -1256,7 +1267,12 @@ gistinserttuples(GISTInsertState *state, GISTInsertStack *stack,
* didn't have to split, release it ourselves.
*/
if (splitinfo)
+ {+ if (random() % 2 == 0)+ HACK = true;
gistfinishsplit(state, stack, giststate, splitinfo, unlockbuf);
+ HACK = false;+ }
else if (unlockbuf)
LockBuffer(stack->buffer, GIST_UNLOCK);
diff --git a/src/backend/access/gist/gistbuild.c b/src/backend/access/gist/gistbuild.cindex bd142a3560..fdfc54b009 100644--- a/src/backend/access/gist/gistbuild.c+++ b/src/backend/access/gist/gistbuild.c@@ -201,6 +201,9 @@ gistbuild(Relation heap, Relation index, IndexInfo *indexInfo)
buildstate.indtuples = 0;
buildstate.indtuplesSize = 0;
+ for (int i = 0; i < 100; i++)+ ReleaseBuffer(ReadBuffer(index, P_NEW));+
/*
* Do the heap scan.
*/
diff --git a/src/backend/access/gist/gistvacuum.c b/src/backend/access/gist/gistvacuum.cindex 1cbcb038f7..c306621e1e 100644--- a/src/backend/access/gist/gistvacuum.c+++ b/src/backend/access/gist/gistvacuum.c@@ -254,6 +254,11 @@ restart:
(opaque->rightlink != InvalidBlockNumber) &&
(opaque->rightlink < orig_blkno))
{
+ if (GistFollowRight(page))+ elog(LOG, "RESCAN %u->%u TRIGGERED BY FollowRight", blkno, opaque->rightlink);+ if (vstate->startNSN < GistPageGetNSN(page))+ elog(LOG,"RESCAN %u->%u TRIGGERED BY NSN", blkno, opaque->rightlink);+
recurse_to = opaque->rightlink;
}
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: hlinnaka@iki.fi, x4mmm@yandex-team.ru, michael@paquier.xyz, thomas.munro@enterprisedb.com, robertmhaas@gmail.com, chapaev28@ya.ru
Subject: Re: GiST VACUUM
In-Reply-To: <96ec7ebd-42b9-4df5-18a4-42181c8a5a41@iki.fi>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox