On Thu, Nov 21, 2019 at 3:25 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Thu, 21 Nov 2019, 14:15 Masahiko Sawada, <masahiko.sawada@2ndquadrant.com> wrote:
>>
>> On Thu, 21 Nov 2019 at 14:32, Dilip Kumar <dilipbalaut@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
beable to copy directly outside the AM. So I thought that if we have a call back for the copy then the AM can flatten
thestats such that IndexBulkDeleteResult, followed by AM specific stats. Yeah but someone may argue that we might
forcethe 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
