Re: [HACKERS] Block level parallel vacuum

Hi
I applied all 3 patches and ran regression test. I was getting one
regression failure.

diff -U3
> /home/mahendra/postgres_base_rp/postgres/src/test/regress/expected/vacuum.out
> /home/mahendra/postgres_base_rp/postgres/src/test/regress/results/vacuum.out
> ---
> /home/mahendra/postgres_base_rp/postgres/src/test/regress/expected/vacuum.out
> 2019-10-17 10:01:58.138863802 +0530
> +++
> /home/mahendra/postgres_base_rp/postgres/src/test/regress/results/vacuum.out
> 2019-10-17 11:41:20.930699926 +0530
> @@ -105,7 +105,7 @@
> CREATE TEMPORARY TABLE tmp (a int PRIMARY KEY);
> CREATE INDEX tmp_idx1 ON tmp (a);
> VACUUM (PARALLEL 1) tmp; -- error, cannot parallel vacuum temporary tables
> -WARNING: skipping "tmp" --- cannot parallel vacuum temporary tables
> +WARNING: skipping vacuum on "tmp" --- cannot vacuum temporary tables in
> parallel
> -- INDEX_CLEANUP option
> CREATE TABLE no_index_cleanup (i INT PRIMARY KEY, t TEXT);
> -- Use uncompressed data stored in toast.
>

It look likes that you changed warning message for temp table, but haven't
updated expected out file.

Thanks and Regards
Mahendra Thalor
EnterpriseDB: http://www.enterprisedb.com

On Wed, 16 Oct 2019 at 06:50, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:

> On Tue, Oct 15, 2019 at 6:33 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> >
> > On Tue, Oct 15, 2019 at 1:26 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
> wrote:
> > >
> > > On Tue, Oct 15, 2019 at 4:15 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
> wrote:
> > > >
> > > > > > > If we avoid postponing deleting empty pages till the cleanup
> phase,
> > > > > > > then we don't have the problem for gist indexes.
> > > > > >
> > > > > > Yes. But considering your pointing out I guess that there might
> be
> > > > > > other index AMs use the stats returned from bulkdelete in the
> similar
> > > > > > way to gist index (i.e. using more larger structure of which
> > > > > > IndexBulkDeleteResult is just the first field). If we have the
> same
> > > > > > concern the parallel vacuum still needs to deal with that as you
> > > > > > mentioned.
> > > > > >
> > > > >
> > > > > Right, apart from some functions for memory allocation/estimation
> and
> > > > > stats copy, we might need something like amcanparallelvacuum, so
> that
> > > > > index methods can have the option to not participate in parallel
> > > > > vacuum due to reasons similar to gist or something else. I think
> we
> > > > > can work towards this direction as this anyway seems to be required
> > > > > and till we reach any conclusion for gist indexes, you can mark
> > > > > amcanparallelvacuum for gist indexes as false.
> > > >
> > > > Agreed. I'll create a separate patch to add this callback and change
> > > > parallel vacuum patch so that it checks the participation of indexes
> > > > and then vacuums on un-participated indexes after parallel vacuum.
> > >
> > > amcanparallelvacuum is not necessary to be a callback, it can be a
> > > boolean field of IndexAmRoutine.
> > >
> >
> > Yes, it will be a boolean. Note that for parallel-index scans, we
> > already have amcanparallel.
> >
>
> Attached updated patch set. 0001 patch introduces new index AM field
> amcanparallelvacuum. All index AMs except for gist sets true for now.
> 0002 patch incorporated the all comments I got so far.
>
> Regards,
>
> --
> Masahiko Sawada
>