Re: COPY FREEZE and setting PD_ALL_VISIBLE/visibility map bits

From: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
To: Tatsuo Ishii <ishii(at)sraoss(dot)co(dot)jp>
Cc: lubennikovaav(at)gmail(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org, pavan(dot)deolasee(at)gmail(dot)com, ibrar(dot)ahmad(at)gmail(dot)com
Subject: Re: COPY FREEZE and setting PD_ALL_VISIBLE/visibility map bits
Date: 2020-10-30 00:42:52
Message-ID: 20201030004252.d63rwclys4bniluu@development
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


I might be somewhat late to the party, but I've done a bit of
benchmarking too ;-) I used TPC-H data from a 100GB test, and tried
different combinations of COPY [FREEZE] and VACUUM [FREEZE], both on
current master and with the patch.

The results look like this (the columns say what combination of COPY and
VACUUM was used - e.g. -/FREEZE means plain COPY and VACUUM FREEZE)


COPY 2471 2477 2486 2484
VACUUM 228 209 453 206


COPY 2459 2445 2458 2446
VACUUM 227 0 467 0

So I don't really observe any measurable slowdowns in the COPY part (in
fact there seems to be a tiny speedup, but it might be just noise). In
the VACUUM part, there's clear speedup when the data was already frozen
by COPY (Yes, those are zeroes, because it took less than 1 second.)

So that looks pretty awesome, I guess.

For the record, these tests were run on a server with NVMe SSD, so
hopefully reliable and representative numbers.

A couple minor comments about the code:

1) Maybe add a comment before the block setting xlrec->flags in
heap_multi_insert. It's not very complex, but it used to be a bit
simpler, and all the other pieces around have comments, so it won't

2) I find the "if (all_frozen_set)" block a bit strange. It's a matter
of personal preference, but I'd just use a single level of nesting, i.e.
something like this:

/* if everything frozen, the whole page has to be visible */
Assert(!(all_frozen_set && !PageIsAllVisible(page)));

* If we've frozen everything on the page, and if we're already
* holding pin on the vmbuffer, record that in the visibilitymap.
* If we're not holding the pin, it's OK to skip this - it's just
* an optimization.
* It's fine to use InvalidTransactionId here - this is only used
* when HEAP_INSERT_FROZEN is specified, which intentionally
* violates visibility rules.
if (all_frozen_set &&
visibilitymap_pin_ok(BufferGetBlockNumber(buffer), vmbuffer))

IMHO it's easier to read, but YMMV. I've also reworded the comment a bit
to say what we're doing etc. And I've moved the comment from inside the
if block into the main comment - that was making it harder to read too.

3) I see RelationGetBufferForTuple does this:

* This is for COPY FREEZE needs. If page is empty,
* pin vmbuffer to set all_frozen bit
if ((options & HEAP_INSERT_FROZEN) &&
(PageGetMaxOffsetNumber(BufferGetPage(buffer)) == 0))
visibilitymap_pin(relation, BufferGetBlockNumber(buffer), vmbuffer);

so is it actually possible to get into the (all_frozen_set) without
holding a pin on the visibilitymap? I haven't investigated this so
maybe there are other ways to get into that code block. But the new
additions to hio.c get the pin too.

4) In heap_xlog_multi_insert we now have this:

if (xlrec->flags & XLH_INSERT_ALL_FROZEN_SET)

IIUC it makes no sense to have both flags at the same time, right? So
what about adding


to check that?

5) Not sure we need to explicitly say this is for COPY FREE in all the
blocks added to hio.c. IMO it's sufficient to use HEAP_INSERT_FROZEN in
the condition, at this level of abstraction.

I wonder what to do about the heap_insert - I know there are concerns it
would negatively impact regular insert, but is it really true? I suppose
this optimization would be valuable even for cases where multi-insert is
not possible.


Tomas Vondra
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Sharma 2020-10-30 00:43:53 MINUS SIGN (U+2212) in EUC-JP encoding is mapped to FULLWIDTH HYPHEN-MINUS (U+FF0D) in UTF-8
Previous Message Greg Nancarrow 2020-10-30 00:38:49 Re: Parallel INSERT (INTO ... SELECT ...)