From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
---|---|
To: | Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com> |
Cc: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, David Steele <david(at)pgmasters(dot)net>, Claudio Freire <klaussfreire(at)gmail(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [HACKERS] Block level parallel vacuum |
Date: | 2019-01-24 02:14:12 |
Message-ID: | CAD21AoDo=4vHkTRV2KnzZ9fDtanhcwXukf=Qz7L8bCgthZqMog@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Jan 22, 2019 at 9:59 PM Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com> wrote:
>
>
> Thanks for the latest patch. I have some more minor comments.
Thank you for reviewing the patch.
>
> + Execute index vacuum and cleanup index in parallel with
>
> Better to use vacuum index and cleanup index? This is in same with
> the description of vacuum phases. It is better to follow same notation
> in the patch.
Agreed. I've changed it to "Vacuum index and cleanup index in parallel
with ...".
>
>
> + dead_tuples = lazy_space_alloc(lvstate, nblocks, parallel_workers);
>
> With the change, the lazy_space_alloc takes care of initializing the
> parallel vacuum, can we write something related to that in the comments.
>
Agreed.
>
> + initprog_val[2] = dead_tuples->max_dead_tuples;
>
> dead_tuples variable may need rename for better reading?
>
I might not get your comment correctly but I've tried to fix it.
Please review it.
>
>
> + if (lvshared->indstats[idx].updated)
> + result = &(lvshared->indstats[idx].stats);
> + else
> + copy_result = true;
>
>
> I don't see a need for copy_result variable, how about directly using
> the updated flag to decide whether to copy or not? Once the result is
> copied update the flag.
>
You're right. Fixed.
>
> +use Test::More tests => 34;
>
> I don't find any new tetst are added in this patch.
Fixed.
>
> I am thinking of performance penalty if we use the parallel option of
> vacuum on a small sized table?
Hm, unlike other parallel operations other than ParallelAppend the
parallel vacuum executes multiple index vacuum simultaneously.
Therefore this can avoid contension. I think there is a performance
penalty but it would not be big.
Attached the latest patches.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
Attachment | Content-Type | Size |
---|---|---|
v13-0001-Add-parallel-option-to-VACUUM-command.patch | application/x-patch | 76.8 KB |
v13-0002-Add-P-option-to-vacuumdb-command.patch | application/x-patch | 6.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2019-01-24 02:39:18 | Re: Allowing extensions to supply operator-/function-specific info |
Previous Message | David Rowley | 2019-01-24 01:59:50 | Re: [HACKERS] PATCH: multivariate histograms and MCV lists |