Michael Paquier wrote:
> On Fri, Sep 12, 2014 at 3:19 PM, Michael Paquier
> <michael.paquier@gmail.com> wrote:
> > On Mon, Feb 3, 2014 at 3:42 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> >> On Wed, Jan 15, 2014 at 2:43 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> >>> On 8 January 2014 08:33, Simon Riggs <simon@2ndquadrant.com> wrote:
> >>>
> >>> Patch attached, implemented to reduce writes by SELECTs only.
> >
> > This patch is registered in this CF. It does not apply anymore and
> > needs a rebase. Robert and Amit have provided as well some comments
> > but they have not been addressed. Is it fair to mark it as "returned
> > with feedback" even if it has not been reviewed within the last month?
> For the time being, status has been changed to "waiting on author".
As it happens, I was studying this patch yesterday on the flight back
home. I gave it a quick look; I noticed it was in the commitfest and
hadn't seen any review activity for many months, which seemed odd.
Anyway I first read the whole thread to know what to focus on, before
going over the patch itself. Once I finished reading the emails, I had
a vague idea of how I thought it would work: my thinking was that
heap/index scans would either call heap_page_prune_opt, or not,
depending on whether they were part of a read-only executor node. So if
you have a query that updates a certain table, and while doing so scans
another table in read-only mode, then the HOT updates would be enabled
for the table being written, but disabled for the one being read.
As it turns out, the patch as written is nothing at all like that, and
TBH I don't think I like it very much.
My idea is that we would have a new executor flag, say
EXEC_FLAG_READ_ONLY; we would set it on nodes that are known to be
read-only, and reset it on those that aren't, such as LockRows and
ModifyTable (obviously we need to pass it down correctly from parent to
children). Then in ExecInitSeqScan and ExecInitIndexScan, if we see the
flag set, we call heap/index_set_allow_prune(false) for the heap scan;
same thing in index scans. (I envisioned it as a boolean rather than
enabling a certain number of cleanups per scan.)
I tried to code this but I think it doesn't work correctly, and no time
for debug currently. Anyway let me know what you think of this general
idea.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
