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.m...@gmail.com> wrote: > On Tue, Oct 15, 2019 at 6:33 PM Amit Kapila <amit.kapil...@gmail.com> > wrote: > > > > On Tue, Oct 15, 2019 at 1:26 PM Masahiko Sawada <sawada.m...@gmail.com> > wrote: > > > > > > On Tue, Oct 15, 2019 at 4:15 PM Masahiko Sawada <sawada.m...@gmail.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 >
