On Thu, Nov 21, 2019 at 3:25 PM Dilip Kumar <dilipbal...@gmail.com> wrote: > > On Thu, 21 Nov 2019, 14:15 Masahiko Sawada, <masahiko.saw...@2ndquadrant.com> > wrote: >> >> On Thu, 21 Nov 2019 at 14:32, Dilip Kumar <dilipbal...@gmail.com> wrote: >> > >> > >> > In v33-0001-Add-index-AM-field-and-callback-for-parallel-ind patch, I >> > am a bit doubtful about this kind of arrangement, where the code in >> > the "if" is always unreachable with the current AMs. I am not sure >> > what is the best way to handle this, should we just drop the >> > amestimateparallelvacuum altogether? >> >> IIUC the motivation of amestimateparallelvacuum is for third party >> index AM. If it allocates memory more than IndexBulkDeleteResult like >> the current gist indexes (although we'll change it) it will break >> index statistics of other indexes or even can be cause of crash. I'm >> not sure there is such third party index AMs and it's true that all >> index AMs in postgres code will not use this callback as you >> mentioned, but I think we need to take care of it because such usage >> is still possible. >> >> > Because currently, we are just >> > providing a size estimate function without a copy function, even if >> > the in future some Am give an estimate about the size of the stats, we >> > can not directly memcpy the stat from the local memory to the shared >> > memory, we might then need a copy function also from the AM so that it >> > can flatten the stats and store in proper format? >> >> I might be missing something but why can't we copy the stats from the >> local memory to the DSM without the callback for copying stats? The >> lazy vacuum code will get the pointer of the stats that are allocated >> by index AM and the code can know the size of it. So I think we can >> just memcpy to DSM. > > > Oh sure. But, what I meant is that if AM may keep pointers in its stats as > GistBulkDeleteResult do so we might not be able to copy directly outside the > AM. So I thought that if we have a call back for the copy then the AM can > flatten the stats such that IndexBulkDeleteResult, followed by AM specific > stats. Yeah but someone may argue that we might force the AM to return the > stats in a form that it can be memcpy directly. So I think I am fine with > the way it is. >
I think we have discussed this point earlier as well and the conclusion was to provide an API if there is a need for the same. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
