Re: [HACKERS] Block level parallel vacuum - Mailing list pgsql-hackers

From Masahiko Sawada
Subject Re: [HACKERS] Block level parallel vacuum
Date
Msg-id CAD21AoCxZOU6Y3F1Fi-ea1EHWUozYKxr-tcqqRMLaFH-=iWR6A@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] Block level parallel vacuum  (Amit Kapila <amit.kapila16@gmail.com>)
Responses Re: [HACKERS] Block level parallel vacuum  (Amit Kapila <amit.kapila16@gmail.com>)
List pgsql-hackers
On Sat, Oct 12, 2019 at 12:33 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Fri, Oct 11, 2019 at 4:47 PM Mahendra Singh <mahi6run@gmail.com> wrote:
> >
> >
> > I did some analysis and found that we are trying to free some already freed memory. Or we are freeing palloced
memoryin 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.
 
> >
>
> I don't think the problem is in vac_update_relstats as we are not even
> passing stats to it, so it won't be able to free it.  I think the real
> problem is in the way we copy the stats from shared memory to local
> memory in the function end_parallel_vacuum().  Basically, it allocates
> the memory for all the index stats together and then in function
> update_index_statistics,  it is trying to free memory of individual
> array elements, that won't work.  I have tried to fix the allocation
> in end_parallel_vacuum, see if this fixes the problem for you.   You
> need to apply the attached patch atop
> v28-0001-Add-parallel-option-to-VACUUM-command posted above by
> Sawada-San.

Thank you for reviewing and creating the patch!

I think the patch fixes this issue correctly. Attached the updated
version patch.

Regards,

--
Masahiko Sawada

Attachment

pgsql-hackers by date:

Previous
From: Peter Geoghegan
Date:
Subject: Re: BTP_DELETED leaf still in tree
Next
From: Michael Paquier
Date:
Subject: Re: dropping column prevented due to inherited index