On Fri, Jan 5, 2018 at 9:55 AM, Alexander Korotkov < a.korot...@postgrespro.ru> wrote:
> On Thu, Jan 4, 2018 at 8:03 AM, Haribabu Kommi <kommi.harib...@gmail.com> > wrote: > >> On Thu, Jan 4, 2018 at 10:00 AM, Alexander Korotkov < >> a.korot...@postgrespro.ru> wrote: >> >>> On Wed, Jan 3, 2018 at 10:08 AM, Haribabu Kommi < >>> kommi.harib...@gmail.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
