Re: COPY FREEZE and setting PD_ALL_VISIBLE/visibility map bits

Anastasia Lubennikova <a.lubennikova@postgrespro.ru>

From: Anastasia Lubennikova <a.lubennikova@postgrespro.ru>
To: Ibrar Ahmed <ibrar.ahmad@gmail.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>, Robert Haas <robertmhaas@gmail.com>, Daniel Gustafsson <daniel@yesql.se>, Justin Pryzby <pryzby@telsasoft.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, Andres Freund <andres@anarazel.de>
Date: 2020-08-26T21:14:34Z
Lists: pgsql-hackers

Attachments

On 21.08.2020 19:43, Ibrar Ahmed wrote:
>
>
> On Wed, Aug 19, 2020 at 6:15 PM Anastasia Lubennikova 
> <a.lubennikova@postgrespro.ru <mailto:a.lubennikova@postgrespro.ru>> 
> wrote:
>
>     On 18.08.2020 02:54, Alvaro Herrera wrote:
>     > On 2020-Aug-14, Ibrar Ahmed wrote:
>     >
>     >> The table used for the test contains three columns (integer, text,
>     >> varchar).
>     >> The total number of rows is 10000000 in total.
>     >>
>     >> Unpatched (Master: 92c12e46d5f1e25fc85608a6d6a19b8f5ea02600)
>     >> COPY: 9069.432 ms vacuum; 2567.961ms
>     >> COPY: 9004.533 ms vacuum: 2553.075ms
>     >> COPY: 8832.422 ms vacuum: 2540.742ms
>     >>
>     >> Patched (Master: 92c12e46d5f1e25fc85608a6d6a19b8f5ea02600)
>     >> COPY: 10031.723 ms vacuum: 127.524 ms
>     >> COPY: 9985.109  ms vacuum: 39.953 ms
>     >> COPY: 9283.373  ms vacuum: 37.137 ms
>     >>
>     >> Time to take the copy slightly increased but the vacuum time
>     significantly
>     >> decrease.
>     > "Slightly"?  It seems quite a large performance drop to me --
>     more than
>     > 10%.  Where is that time being spent?  Andres said in [1] that he
>     > thought the performance shouldn't be affected noticeably, but this
>     > doesn't seem to hold true.  As I understand, the idea was that there
>     > would be little or no additional WAL records .. only flags in the
>     > existing record.  So what is happening?
>     >
>     > [1]
>     https://postgr.es/m/20190408010427.4l63qr7h2fjcyp77@alap3.anarazel.de
>
>     I agree that 10% performance drop is not what we expect with this
>     patch.
>     Ibrar, can you share more info about your tests? I'd like to
>     reproduce
>     this slowdown and fix it, if necessary.
>
>
> Here is my test;
>
> postgres=# BEGIN;
>
> BEGIN
>
>
> postgres=*# TRUNCATE foo;
>
> TRUNCATE TABLE
>
>
> postgres=*# COPY foo(id, name, address) FROM '/home/ibrar/bar.csv' 
> DELIMITER ',' FREEZE;
>
> COPY 10000000
>
>
>
> -- 
> Ibrar Ahmed


I've repeated the test and didn't notice any slowdown for COPY FREEZE.
Test data is here [1].

The numbers do fluctuate a bit, but there is no dramatic difference 
between master and patched version. So I assume that the performance 
drop in your test has something to do with the measurement error. 
Unless, you have some non-default configuration that could affect it.

patched:

COPY: 12327,090 ms vacuum: 37,555 ms
COPY: 12939,540 ms vacuum: 35,703 ms
COPY: 12245,819 ms vacuum: 36,273 ms

master:
COPY
COPY: 13253,605 ms vacuum: 3592,849 ms
COPY: 12619,428 ms vacuum: 4253,836 ms
COPY: 12512,940 ms vacuum: 4009,847 ms

I also slightly cleaned up comments, so the new version of the patch is 
attached. As this is just a performance optimization documentation is 
not needed. It would be great, if other reviewers could run some 
independent performance tests, as I believe that this patch is ready for 
committer.

[1] https://drive.google.com/file/d/11r19NX6yyPjvxdDub8Ce-kmApRurp4Nx/view

-- 
Anastasia Lubennikova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Companyt

Commits

  1. Fix COPY FREEZE with CLOBBER_CACHE_ALWAYS

  2. Set PD_ALL_VISIBLE and visibility map bits in COPY FREEZE

  3. docs: client certificates are always sent to the server

  4. Mark commit and abort WAL records with XLR_SPECIAL_REL_UPDATE.

  5. docs: land height is "elevation", not "altitude"

  6. Improve the internal implementation of ereport().