Re: [HACKERS] Pluggable storage

On Fri, Jan 5, 2018 at 9:55 AM, Alexander Korotkov <
a(dot)korotkov(at)postgrespro(dot)ru> wrote:

> On Thu, Jan 4, 2018 at 8:03 AM, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
> wrote:
>
>> On Thu, Jan 4, 2018 at 10:00 AM, Alexander Korotkov <
>> a(dot)korotkov(at)postgrespro(dot)ru> wrote:
>>
>>> On Wed, Jan 3, 2018 at 10:08 AM, Haribabu Kommi <
>>> kommi(dot)haribabu(at)gmail(dot)com> wrote:
>>>
>>>> Apart from rebase, Added storage shared memory API, currently this API
>>>> is used
>>>>
>>> only by the syncscan. And also all the exposed functions of syncscan
>>>> usage is
>>>> removed outside the heap.
>>>>
>>>
>>> This makes me uneasy. You introduce two new hooks for size estimation
>>> and initialization
>>> of shared memory needed by storage am's. But if storage am is
>>> implemented in shared library,
>>> then this shared library can use our generic method for allocation of
>>> shared memory
>>> (including memory needed by storage am). If storage am is builtin, then
>>> hooks are also not
>>> needed, because we know all our builtin storage am's in advance. For
>>> me, it would be
>>> nice to encapsulate heap am requirements in shared memory into functions
>>> like
>>> HeapAmShmemSize() and HeapAmShmemInit(), and don't explicitly show
>>> outside that
>>> this memory is needed for synchronized scan. But separate hooks don't
>>> look justified for me.
>>>
>>
>> Yes, I agree that for the builtin storage's there is no need of hooks.
>> But in future,
>> if we want to support multiple storage's in an instance, we may need
>> hooks for shared memory
>> registration. I am fine to change it.
>>
>
> Yes, but we already have hooks for shared memory registration in shared
> modules. I don't see the point for another hooks for the same purpose.
>

Oh, yes, I missed it. I will update the patch and share it later.

Regards,
Hari Babu
Fujitsu Australia