On Tue, Jan 3, 2017 at 11:36 PM, Andrew Borodin <borodin@octonica.com> wrote:
> 2017-01-03 19:39 GMT+05:00 Peter Eisentraut
> <peter.eisentraut@2ndquadrant.com>:
>>
>> On 1/3/17 1:26 AM, amul sul wrote:
>> > One more requirement for pg_background is session, command_qh,
>> > response_qh and worker_handle should be last longer than current
>> > memory context, for that we might need to allocate these in
>> > TopMemoryContext. Please find attach patch does the same change in
>> > BackgroundSessionStart().
>>
>> I had pondered this issue extensively. The standard coding convention
>> in postgres is that the caller sets the memory context. See the dblink
>> and plpython patches that make this happen in their own way.
>>
>> I agree it would make sense that you either pass in a memory context or
>> always use TopMemoryContext. I'm open to these ideas, but they did not
>> seem to match any existing usage.
>
> +1
> Please excuse me if I'm not getting something obvious, but seems like
> BackgroundSessionNew() caller from pg_background_launch() can pick up
> TopMemoryCtx. I think that context setup should be done by extension, not by
> bg_session API.
>
Agree, will do this changes for pg_background.
One more query, can we modify
BackgroundSessionStart()/BackgroundSession struct to get background
worker PID as well?
I am asking because of BackgroundSessionNew() only returns session pointer,
but pg_background_launch() requires this PID to pass to user, which
will be further
used as session identifier at fetching result and/or executing further
queries as well as
to send query cancelation signal to background worker.
I can understand this requirement could be sound useless for now,
because it only for the benefit of pg_background contrib module only.
Thoughts?
Thanks & Regards,
Amul