Re: [HACKERS] Block level parallel vacuum

Hi

On Thu, 10 Oct 2019 at 13:18, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:

> On Thu, Oct 10, 2019 at 2:19 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> >
> > On Fri, Oct 4, 2019 at 4:18 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
> wrote:
> > >
> > > On Wed, Oct 2, 2019 at 7:29 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
> wrote:
> > >>
> >
> > Few more comments:
>
> Thank you for reviewing the patch!
>
> > ---------------------------------
> > 1. Caurrently parallel vacuum is allowed for temporary relations
> > which is wrong. It leads to below error:
> >
> > postgres=# create temporary table tmp_t1(c1 int, c2 char(10));
> > CREATE TABLE
> > postgres=# create index idx_tmp_t1 on tmp_t1(c1);
> > CREATE INDEX
> > postgres=# create index idx1_tmp_t1 on tmp_t1(c2);
> > CREATE INDEX
> > postgres=# insert into tmp_t1 values(generate_series(1,10000),'aaaa');
> > INSERT 0 10000
> > postgres=# delete from tmp_t1 where c1 > 5000;
> > DELETE 5000
> > postgres=# vacuum (parallel 2) tmp_t1;
> > ERROR: cannot access temporary tables during a parallel operation
> > CONTEXT: parallel worker
> >
> > The parallel vacuum shouldn't be allowed for temporary relations.
>
> Fixed.
>
> >
> > 2.
> > --- a/doc/src/sgml/ref/vacuum.sgml
> > +++ b/doc/src/sgml/ref/vacuum.sgml
> > @@ -34,6 +34,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [
> > <replaceable class="paramet
> > SKIP_LOCKED [ <replaceable class="parameter">boolean</replaceable> ]
> > INDEX_CLEANUP [ <replaceable
> > class="parameter">boolean</replaceable> ]
> > TRUNCATE [ <replaceable class="parameter">boolean</replaceable> ]
> > + PARALLEL [ <replaceable
> > class="parameter">integer</replaceable> ]
> >
> > Now, if the user gives a command like Vacuum (analyze, parallel)
> > <table_name>; it is not very obvious that a parallel option will be
> > only used for vacuum purposes but not for analyze. I think we can add
> > a note in the docs to mention this explicitly. This can avoid any
> > confusion.
>
> Agreed.
>
> Attached the latest version patch although the memory usage problem is
> under discussion. I'll update the patches according to the result of
> that discussion.
>
>
I applied both patches on HEAD and did some testing. I am getting one crash
in freeing memory. (pfree(stats[i]))

*Steps to reproduc*e:
*Step 1) *Apply both the patches and configure with below command.
./configure --with-zlib --enable-debug --prefix=$PWD/inst/
--with-openssl CFLAGS="-ggdb3" > war && make -j 8 install > war

*Step 2) Now start the server.*

*Step 3) Fire below commands:*

> create table tmp_t1(c1 int, c2 char(10));
> create index idx_tmp_t1 on tmp_t1(c1);
> create index idx1_tmp_t1 on tmp_t1(c2);
> insert into tmp_t1 values(generate_series(1,10000),'aaaa');
> insert into tmp_t1 values(generate_series(1,10000),'aaaa');
> insert into tmp_t1 values(generate_series(1,10000),'aaaa');
> insert into tmp_t1 values(generate_series(1,10000),'aaaa');
> insert into tmp_t1 values(generate_series(1,10000),'aaaa');
> insert into tmp_t1 values(generate_series(1,10000),'aaaa');
> delete from tmp_t1 where c1 > 5000;
> vacuum (parallel 2) tmp_t1;
>

*Call stack:*

> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib64/libthread_db.so.1".
> Core was generated by `postgres: mahendra postgres [local] VACUUM
> '.
> Program terminated with signal 11, Segmentation fault.
> #0 0x0000000000a4f97a in pfree (pointer=0x10baa68) at mcxt.c:1060
> 1060 context->methods->free_p(context, pointer);
> Missing separate debuginfos, use: debuginfo-install
> keyutils-libs-1.5.8-3.el7.x86_64 krb5-libs-1.15.1-19.el7.x86_64
> libcom_err-1.42.9-12.el7_5.x86_64 libselinux-2.5-12.el7.x86_64
> openssl-libs-1.0.2k-12.el7.x86_64 pcre-8.32-17.el7.x86_64
> zlib-1.2.7-17.el7.x86_64
> (gdb) bt
> #0 0x0000000000a4f97a in pfree (pointer=0x10baa68) at mcxt.c:1060
> #1 0x00000000004e7d13 in update_index_statistics (Irel=0x10b9808,
> stats=0x10b9828, nindexes=2) at vacuumlazy.c:2277
> #2 0x00000000004e693f in lazy_scan_heap (onerel=0x7f8d99610d08,
> params=0x7ffeeaddb7f0, vacrelstats=0x10b9728, Irel=0x10b9808, nindexes=2,
> aggressive=false) at vacuumlazy.c:1659
> '#3 0x00000000004e4d25 in heap_vacuum_rel (onerel=0x7f8d99610d08,
> params=0x7ffeeaddb7f0, bstrategy=0x1117528) at vacuumlazy.c:431
> #4 0x00000000006a71a7 in table_relation_vacuum (rel=0x7f8d99610d08,
> params=0x7ffeeaddb7f0, bstrategy=0x1117528) at
> ../../../src/include/access/tableam.h:1432
> #5 0x00000000006a9899 in vacuum_rel (relid=16384, relation=0x103b308,
> params=0x7ffeeaddb7f0) at vacuum.c:1870
> #6 0x00000000006a7c22 in vacuum (relations=0x11176b8,
> params=0x7ffeeaddb7f0, bstrategy=0x1117528, isTopLevel=true) at vacuum.c:425
> #7 0x00000000006a77e6 in ExecVacuum (pstate=0x105f578, vacstmt=0x103b3d8,
> isTopLevel=true) at vacuum.c:228
> #8 0x00000000008af401 in standard_ProcessUtility (pstmt=0x103b6f8,
> queryString=0x103a808 "vacuum (parallel 2) tmp_t1;",
> context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0,
> dest=0x103b7d8, completionTag=0x7ffeeaddbc50 "") at utility.c:670
> #9 0x00000000008aec40 in ProcessUtility (pstmt=0x103b6f8,
> queryString=0x103a808 "vacuum (parallel 2) tmp_t1;",
> context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0,
> dest=0x103b7d8, completionTag=0x7ffeeaddbc50 "") at utility.c:360
> #10 0x00000000008addbb in PortalRunUtility (portal=0x10a1a28,
> pstmt=0x103b6f8, isTopLevel=true, setHoldSnapshot=false, dest=0x103b7d8,
> completionTag=0x7ffeeaddbc50 "") at pquery.c:1175
> #11 0x00000000008adf9f in PortalRunMulti (portal=0x10a1a28,
> isTopLevel=true, setHoldSnapshot=false, dest=0x103b7d8, altdest=0x103b7d8,
> completionTag=0x7ffeeaddbc50 "") at pquery.c:1321
> #12 0x00000000008ad55d in PortalRun (portal=0x10a1a28,
> count=9223372036854775807, isTopLevel=true, run_once=true, dest=0x103b7d8,
> altdest=0x103b7d8, completionTag=0x7ffeeaddbc50 "")
> at pquery.c:796
> #13 0x00000000008a7789 in exec_simple_query (query_string=0x103a808
> "vacuum (parallel 2) tmp_t1;") at postgres.c:1231
> #14 0x00000000008ab8f2 in PostgresMain (argc=1, argv=0x1065b00,
> dbname=0x1065a28 "postgres", username=0x1065a08 "mahendra") at
> postgres.c:4256
> #15 0x0000000000811a42 in BackendRun (port=0x105d9c0) at postmaster.c:4465
> #16 0x0000000000811241 in BackendStartup (port=0x105d9c0) at
> postmaster.c:4156
> #17 0x000000000080d7d6 in ServerLoop () at postmaster.c:1718
> #18 0x000000000080d096 in PostmasterMain (argc=3, argv=0x1035270) at
> postmaster.c:1391
> #19 0x000000000072accb in main (argc=3, argv=0x1035270) at main.c:210
>

I did some analysis and found that we are trying to free some already freed
memory. Or we are freeing palloced memory in vac_update_relstats.
for (i = 0; i < nindexes; i++)
{
if (stats[i] == NULL || stats[i]->estimated_count)
continue;

/* Update index statistics */
vac_update_relstats(Irel[i],
stats[i]->num_pages,
stats[i]->num_index_tuples,
0,
false,
InvalidTransactionId,
InvalidMultiXactId,
false);
pfree(stats[i]);
}

As my table have 2 indexes, so we have to free both stats. When i = 0, it
is freeing propery but when i = 1, then vac_update_relstats is freeing
memory.

> (gdb) p *stats[i]
> $1 = {num_pages = 218, pages_removed = 0, estimated_count = false,
> num_index_tuples = 30000, tuples_removed = 30000, pages_deleted = 102,
> pages_free = 0}
> (gdb) p *stats[i]
> $2 = {num_pages = 0, pages_removed = 65536, estimated_count = false,
> num_index_tuples = 0, tuples_removed = 0, pages_deleted = 0, pages_free = 0}
> (gdb)
>

From above data, it looks like, somewhere inside vac_update_relstats, we
are freeing all palloced memory. I don't know, why is it.

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