Re: Freeze avoidance of very large table.

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Jim Nasby <Jim(dot)Nasby(at)bluetreble(dot)com>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Bruce Momjian <bruce(at)momjian(dot)us>, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Josh Berkus <josh(at)agliodbs(dot)com>, Masao Fujii <masao(dot)fujii(at)gmail(dot)com>, Petr Jelinek <petr(at)2ndquadrant(dot)com>, Greg S <stark(at)mit(dot)edu>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Freeze avoidance of very large table.
Date: 2016-02-14 05:19:38
Message-ID: CAD21AoBRRxhab0PE6MrEtc=WMFFPW=tfVR2PqPVJTEBPLzZDAQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Fri, Feb 12, 2016 at 4:46 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Wed, Feb 3, 2016 at 12:32 PM, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>> I've divided the main patch into two patches; add frozen bit patch and
>> pg_upgrade support patch.
>> 000 patch is almost same as previous code. (includes small fix)
>> 001 patch provides rewriting visibility map as a pageConverter routine.
>> 002 patch is for enhancement debug message in visibilitymap.c
>
> I'd like to suggest splitting 000 into two patches. The first one
> would change the format of the visibility map, and the second one
> would change VACUUM to optimize scans based on the new format. I
> think that would make it easier to get this reviewed and committed.
>
> I think this patch churns a bunch of things that don't really need to
> be churned. For example, consider this hunk:
>
> /*
> * If we didn't pin the visibility map page and the page has become all
> - * visible while we were busy locking the buffer, we'll have to unlock and
> - * re-lock, to avoid holding the buffer lock across an I/O. That's a bit
> - * unfortunate, but hopefully shouldn't happen often.
> + * visible or all frozen while we were busy locking the buffer, we'll
> + * have to unlock and re-lock, to avoid holding the buffer lock across an
> + * I/O. That's a bit unfortunate, but hopefully shouldn't happen often.
> */
>
> Since the page can't become all-frozen without also becoming
> all-visible, the original text is still 100% accurate, and the change
> doesn't seem to add any useful clarity. Let's think about which
> things really need to be changed and not just mechanically change
> everything.
>
> - Assert(PageIsAllVisible(heapPage));
> + /*
> + * Caller is expected to set PD_ALL_VISIBLE or
> + * PD_ALL_FROZEN first.
> + */
> + Assert(((flags | VISIBILITYMAP_ALL_VISIBLE) &&
> PageIsAllVisible(heapPage)) ||
> + ((flags | VISIBILITYMAP_ALL_FROZEN) &&
> PageIsAllFrozen(heapPage)));
>
> I think this would be more clear as two separate assertions.
>
> Your 000 patch has a little bit of whitespace damage:
>
> [rhaas pgsql]$ git diff --check
> src/backend/commands/vacuumlazy.c:1951: indent with spaces.
> + bool *all_visible, bool
> *all_frozen)
> src/include/access/heapam_xlog.h:393: indent with spaces.
> + Buffer vm_buffer, TransactionId
> cutoff_xid, uint8 flags);
>

Thank you for reviewing this patch.
I've divided 000 patch into two patches, and attached latest 4 patches in total.

I changed pg_upgrade plugin logic so that all kind of type suffix has
one convert plugin.
The type suffix which doesn't need to be converted has pg_copy_file()
function as plugin function.

Regards,

--
Masahiko Sawada

Attachment Content-Type Size
000_add_frozen_bit_into_visibilitymap_v35.patch application/octet-stream 39.8 KB
001_optimize_vacuum_scan_based_on_freezemap_v35.patch application/octet-stream 35.2 KB
002_freezemap_support_for_pg_upgrade_v35.patch application/octet-stream 31.0 KB
003_enhance_visibilitymap_debug_messages_v35.patch application/octet-stream 1.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2016-02-14 05:59:26 Re: extend pgbench expressions with functions
Previous Message Pavel Stehule 2016-02-14 03:28:53 Re: proposal: function parse_ident