On Sat, Oct 12, 2019 at 11:29 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > On Sat, Oct 12, 2019 at 12:33 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Fri, Oct 11, 2019 at 4:47 PM Mahendra Singh <mahi6...@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
fix_comments_amit_1.patch
Description: Binary data
