From: | Andres Freund <andres(at)anarazel(dot)de> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | Stephen Frost <sfrost(at)snowman(dot)net>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Reviewing freeze map code |
Date: | 2016-06-06 21:26:14 |
Message-ID: | 20160606212614.vq4v24vgtwysxudi@alap3.anarazel.de |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi,
On 2016-06-06 17:00:19 -0400, Robert Haas wrote:
> 1. I think it is pretty misleading to say that those checks aren't
> reachable any more. It's not like we freeze every page when we mark
> it all-visible.
True. What I mean is that you can't force the checks (and some that I
think should be added) to occur anymore. Once a page is frozen it'll be
kinda hard to predict whether vacuum touches it (due to the skip logic).
> 2. With the new pg_visibility extension, you can actually check the
> same thing that first warning checks like this:
>
> select * from pg_visibility('t1'::regclass) where all_visible and not
> pd_all_visible;
Right, but not the second.
> IMHO, that's a substantial improvement over running VACUUM and
> checking whether it spits out a WARNING.
I think it's a mixed bag. I do think that WARNINGS are a lot easier to
understand for a casual user/tester; rather than having to write/copy
queries which return results where you don't know what the expected
result is. I agree that it's better to have that in a non-modifying way
- although I'm afraid atm it's not really possible to do a
HeapTupleSatisfies* without modifications :(.
> 3. If you think there are analogous checks that I should add for the
> frozen case, or that you want to add yourself, please say what they
> are specifically. I *did* think about it when I wrote that code and I
> didn't see how to make it work. If I had, I would have added them.
> The whole point of review here is, hopefully, to illuminate what
> should have been done differently - if I'd known how to do it better,
> I would have done so. Provide an idea, or better yet, provide a
> patch. If you see how to do it, coding it up shouldn't be the hard
> part.
I think it's pretty important (and not hard) to add a check for
(all_frozen_according_to_vm && has_unfrozen_tuples). Checking for
VM_ALL_FROZEN && !VM_ALL_VISIBLE looks worthwhile as well, especially as
we could check that always, without a measurable overhead. But the
former primarily makes sense if we have a way to force the check to
occur in a way that's not dependant on the state of neighbouring pages.
Greetings,
Andres Freund
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2016-06-06 21:43:33 | Re: Reviewing freeze map code |
Previous Message | Robert Haas | 2016-06-06 21:22:38 | Re: Reviewing freeze map code |