From: Dmitry Dolgov <9erthalion6@gmail.com>
To: Peter Geoghegan <pg@bowt.ie>
Cc: Jesper Pedersen <jesper.pedersen@redhat.com>
Cc: Floris Van Nee <florisvannee@optiver.com>
Cc: David Rowley <david.rowley@2ndquadrant.com>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>
Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Thomas Munro <thomas.munro@gmail.com>
Cc: James Coleman <jtc331@gmail.com>
Cc: Rafia Sabih <rafia.pghackers@gmail.com>
Cc: Jeff Janes <jeff.janes@gmail.com>
Cc: Bhushan Uparkar <bhushan.uparkar@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Cc: Alexander Korotkov <a.korotkov@postgrespro.ru>
Subject: Re: Index Skip Scan
Date: Tue, 21 Jan 2020 17:09:42 +0100
Message-ID: <20200121160942.eu4bvbu46nt223ug@localhost> (raw)
In-Reply-To: <CAH2-Wzk6hvXvZ+1q4JBRunZkxmJwdXxnNsWuKJBp+F+mJuZYZw@mail.gmail.com>
References: <1e59d800b38141c5af2ad3a64cf9758e@opammb0562.comp.optiver.com>
<802e46fa-dc2c-4c32-e55b-884e04aa7f12@redhat.com>
<CAH2-Wzk6hvXvZ+1q4JBRunZkxmJwdXxnNsWuKJBp+F+mJuZYZw@mail.gmail.com>
> On Mon, Jan 20, 2020 at 01:19:30PM -0800, Peter Geoghegan wrote:
Thanks for the commentaries. I'm trying to clarify your conclusions for
myself, so couple of questions.
> > > - nbtsearch.c in general
> > > Most of the code seems to rely quite heavily on the fact that xs_want_itup forces _bt_drop_lock_and_maybe_pin to never release the buffer pin. Have you considered that compacting of a page may still happen even if you hold the pin? [1] I've been trying to come up with cases in which this may break the patch, but I haven't able to produce such a scenario - so it may be fine.
>
> Try making _bt_findinsertloc() call _bt_vacuum_one_page() whenever the
> page is P_HAS_GARBAGE(), regardless of whether or not the page is
> about to split. That will still be correct, while having a much better
> chance of breaking the patch during stress-testing.
>
> Relying on a buffer pin to prevent the B-Tree structure itself from
> changing in any important way seems likely to be broken already. Even
> if it isn't, it sounds fragile.
Except for checking low/high key (which should be done with a lock), I
believe the current implementation follows the same pattern I see quite
often, namely
* get a lock on a page of interest and test it's values (if we can find
next distinct value right on the next one without goind down the tree).
* if not, unlock the current page, search within the tree with
_bt_search (which locks a resuling new page) and examine values on a
new page, when necessary do _bt_steppage
Is there an obvious problem with this approach, when it comes to the
page structure modification?
> A leaf page doesn't really have anything called a low key. It usually
> has a current first "data item"/non-pivot tuple, which is an
> inherently unstable thing.
Would this inherent instability be resolved for this particular case by
having a lock on a page while checking a first data item, or there is
something else I need to take into account?
> > There is a BT_READ lock in place when finding the correct leaf page, or
> > searching within the leaf page itself. _bt_vacuum_one_page deletes only
> > LP_DEAD tuples, but those are already ignored in _bt_readpage. Peter, do
> > you have some feedback for this ?
>
> It sounds like the design of the patch relies on doing something other
> than stopping a scan "between" pages, in the sense that is outlined in
> the commit message of commit 09cb5c0e. If so, then that's a serious
> flaw in its design.
Could you please elaborate why does it sound like that? If I understand
correctly, to stop a scan only "between" pages one need to use only
_bt_readpage/_bt_steppage? Other than that there is no magic with scan
position in the patch, so I'm not sure if I'm missing something here.
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: 9erthalion6@gmail.com, pg@bowt.ie, jesper.pedersen@redhat.com, florisvannee@optiver.com, david.rowley@2ndquadrant.com, tomas.vondra@2ndquadrant.com, horikyota.ntt@gmail.com, alvherre@2ndquadrant.com, thomas.munro@gmail.com, jtc331@gmail.com, rafia.pghackers@gmail.com, jeff.janes@gmail.com, bhushan.uparkar@gmail.com, a.korotkov@postgrespro.ru
Subject: Re: Index Skip Scan
In-Reply-To: <20200121160942.eu4bvbu46nt223ug@localhost>
* 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