On Sat, Oct 12, 2019 at 11:29 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> 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:
> > >
>
> Thank you for reviewing and creating the patch!
>
> I think the patch fixes this issue correctly. Attached the updated
> version patch.
>
I see a much bigger problem with the way this patch collects the index
stats in shared memory. IIUC, it allocates the shared memory (DSM)
for all the index stats, in the same way, considering its size as
IndexBulkDeleteResult. For the first time, it gets the stats from
local memory as returned by ambulkdelete/amvacuumcleanup call and then
copies it in shared memory space. There onwards, it always updates
the stats in shared memory by pointing each index stats to that
memory. In this scheme, you overlooked the point that an index AM
could choose to return a larger structure of which
IndexBulkDeleteResult is just the first field. This generally
provides a way for ambulkdelete to communicate additional private data
to amvacuumcleanup. We use this idea in the gist index, see how
gistbulkdelete and gistvacuumcleanup works. The current design won't
work for such cases.
One idea is to change the design such that each index method provides
a method to estimate/allocate the shared memory required for stats of
ambulkdelete/amvacuumscan and then later we also need to use index
method-specific function which copies the stats from local memory to
shared memory. I think this needs further investigation.
I have also made a few other changes in the attached delta patch. The
main point that fixed by attached patch is that even if we don't allow
a parallel vacuum on temporary tables, the analyze should be able to
work if the user has asked for it. I have changed an error message
and few other cosmetic changes related to comments. Kindly include
this in the next version if you don't find any problem with the
changes.
--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
