Re: Changeset Extraction v7.7 - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Changeset Extraction v7.7
Date
Msg-id 20140226172919.GD14104@awork2.anarazel.de
Whole thread Raw
In response to Re: Changeset Extraction v7.7  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: Changeset Extraction v7.7
Re: Changeset Extraction v7.7
List pgsql-hackers
On 2014-02-24 17:06:53 -0500, Robert Haas wrote:
> -       heap_page_prune_opt(scan->rs_rd, buffer, RecentGlobalXmin);
> +       if (IsSystemRelation(scan->rs_rd)
> +               || RelationIsAccessibleInLogicalDecoding(scan->rs_rd))
> +               heap_page_prune_opt(scan->rs_rd, buffer, RecentGlobalXmin);
> +       else
> +               heap_page_prune_opt(scan->rs_rd, buffer, RecentGlobalDataXmin);
>
> Instead of changing the callers of heap_page_prune_opt() in this way,
> I think it might be better to change heap_page_prune_opt() to take
> only the first two of its current three parameters; everybody's just
> passing RecentGlobalXmin right now anyway.

I've changed stuff this way, and it indeed looks better.

I am wondering about the related situation of GetOldestXmin()
callers. There's a fair bit of duplicated logic in the callers, before
but especially after this patchset. What about adding 'Relation rel'
parameter instead of `allDbs' and `systable'? That keeps the logic
centralized and there's been a fair amount of talk about vacuum
optimizations that could also use it.
It's a bit sad that that requires including rel.h from procarray.h...

What do you think? Isolated patch attached.

Greetings,

Andres Freund

--
 Andres Freund                       http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Attachment

pgsql-hackers by date:

Previous
From: Alvaro Herrera
Date:
Subject: Re: [PATCH] Use MAP_HUGETLB where supported (v3)
Next
From: Josh Berkus
Date:
Subject: Re: jsonb and nested hstore