Re: [HACKERS] pg_background contrib module proposal

[Peter Eisentraut]

On 2/1/17 22:03, Michael Paquier wrote:
> On Fri, Jan 27, 2017 at 11:38 PM, Robert Haas  wrote:
>> On Fri, Jan 27, 2017 at 9:14 AM, Peter Eisentraut
>>  wrote:
>>> On 1/19/17 12:47 PM, Andrey Borodin wrote:
 4. There is some controversy on where implemented feature shall be: in 
 separate extension (as in this patch), in db_link, in some PL API, in FDW 
 or somewhere else. I think that new extension is an appropriate place for 
 the feature. But I’m not certain.
>>>
>>> I suppose we should decide first whether we want pg_background as a
>>> separate extension or rather pursue extending dblink as proposed elsewhere.
>>>
>>> I don't know if pg_background allows any use case that dblink can't
>>> handle (yet).
>>
>> For the record, I have no big problem with extending dblink to allow
>> this instead of adding pg_background.  But I think we should try to
>> get one or the other done in time for this release.
> 
> Moved to CF 2017-03 as the discussion is not over yet.

Set to returned with feedback, since the same was done to the background
sessions patch.

I would like to continue working on this for the next release.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Michael Paquier]

On Fri, Jan 27, 2017 at 11:38 PM, Robert Haas  wrote:
> On Fri, Jan 27, 2017 at 9:14 AM, Peter Eisentraut
>  wrote:
>> On 1/19/17 12:47 PM, Andrey Borodin wrote:
>>> 4. There is some controversy on where implemented feature shall be: in 
>>> separate extension (as in this patch), in db_link, in some PL API, in FDW 
>>> or somewhere else. I think that new extension is an appropriate place for 
>>> the feature. But I’m not certain.
>>
>> I suppose we should decide first whether we want pg_background as a
>> separate extension or rather pursue extending dblink as proposed elsewhere.
>>
>> I don't know if pg_background allows any use case that dblink can't
>> handle (yet).
>
> For the record, I have no big problem with extending dblink to allow
> this instead of adding pg_background.  But I think we should try to
> get one or the other done in time for this release.

Moved to CF 2017-03 as the discussion is not over yet.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Andrew Borodin]

2017-01-27 19:14 GMT+05:00 Peter Eisentraut :
> I suppose we should decide first whether we want pg_background as a
> separate extension or rather pursue extending dblink as proposed elsewhere.
>
> I don't know if pg_background allows any use case that dblink can't
> handle (yet).
pg_background in it's current version is just a start of a feature.
The question is: are they coherent in desired features? I do not know.
E.g. will it be possible to copy from stdin in dblink and possible
incarnations of pg_background functionality?)

2017-01-27 19:38 GMT+05:00 Robert Haas :
> For the record, I have no big problem with extending dblink to allow
> this instead of adding pg_background.  But I think we should try to
> get one or the other done in time for this release.
+1!
that's why I hesitate between not saying my points and making
controversy...need to settle it somehow.

Parallelism is a "selling" feature, everything has to be parallel for
a decade already (don't we have parallel sequential scan yet?).
It's fine to go with dblink, but dblink docs start with roughly "this
is an outdated substandard feature"(not a direct quote[0)].
What will we add there? "Do not use dblink for linking to databases.
This is the standard for doing concurrency." ?
Please excuse me for exaggeration. BTW, pg_background do not have docs at all.

[0] https://www.postgresql.org/docs/devel/static/dblink.html

Best regards, Andrey Borodin.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Robert Haas]

On Fri, Jan 27, 2017 at 9:14 AM, Peter Eisentraut
 wrote:
> On 1/19/17 12:47 PM, Andrey Borodin wrote:
>> 4. There is some controversy on where implemented feature shall be: in 
>> separate extension (as in this patch), in db_link, in some PL API, in FDW or 
>> somewhere else. I think that new extension is an appropriate place for the 
>> feature. But I’m not certain.
>
> I suppose we should decide first whether we want pg_background as a
> separate extension or rather pursue extending dblink as proposed elsewhere.
>
> I don't know if pg_background allows any use case that dblink can't
> handle (yet).

For the record, I have no big problem with extending dblink to allow
this instead of adding pg_background.  But I think we should try to
get one or the other done in time for this release.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Peter Eisentraut]

On 1/19/17 12:47 PM, Andrey Borodin wrote:
> 4. There is some controversy on where implemented feature shall be: in 
> separate extension (as in this patch), in db_link, in some PL API, in FDW or 
> somewhere else. I think that new extension is an appropriate place for the 
> feature. But I’m not certain.

I suppose we should decide first whether we want pg_background as a
separate extension or rather pursue extending dblink as proposed elsewhere.

I don't know if pg_background allows any use case that dblink can't
handle (yet).

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Andrey Borodin]

The following review has been posted through the commitfest application:
make installcheck-world:  tested, passed
Implements feature:   tested, passed
Spec compliant:   tested, passed
Documentation:tested, failed

I’ll summarize here my thoughts as a reviewer on the current state of the 
pg_background:
1. Current version of a code [1] is fine, from my point of view. I have no 
suggestions on improving it. There is no documentation, but code is commented.
2. Patch is dependent on background sessions from the same commitfest.
3. There can exist more features, but for v1 there is surely enough features.
4. There is some controversy on where implemented feature shall be: in separate 
extension (as in this patch), in db_link, in some PL API, in FDW or somewhere 
else. I think that new extension is an appropriate place for the feature. But 
I’m not certain.
Summarizing these points, appropriate statuses of the patch are ‘Ready for 
committer’ or ‘Rejected’. Between these two I choose ‘Ready for committer’, I 
think patch is committable (after bg sessions).

Best regards, Andrey Borodin.

The new status of this patch is: Ready for Committer

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[amul sul]

On Tue, Jan 10, 2017 at 7:09 AM, Jim Nasby  wrote:
> On 1/9/17 7:22 AM, amul sul wrote:
>>
>> On Sun, Jan 8, 2017 at 8:50 AM, Jim Nasby 
>> wrote:
>>>
>>>
>> [skipped...]
>>>
>>>
>>> Oh, hmm. So I guess if you do that when the background process is idle
>>> it's
>>> the same as a close?
>>>
>>> I think we need some way to safeguard against accidental forkbombs for
>>> cases
>>> where users aren't intending to leave something running in the
>>> background.
>>> There's other reasons to use this besides spawning long running
>>> processes,
>>> and I'd certainly want to be able to ensure the calling function wasn't
>>> accidentally leaving things running that it didn't mean to. (Maybe the
>>> patch
>>> already does this...)
>>>
>>
>> Current pg_background patch built to the top of BackgroundSession code
>> take care of that;
>> user need to call pg_background_close() to gracefully close previously
>> forked background
>> worker.  Even though if user session who forked this worker exited
>> without calling
>> pg_background_close(), this background worked force to exit with following
>> log:
>>
>> ERROR:  could not read from message queue: Other process has detached
>> queue
>> LOG:  could not send on message queue: Other process has detached queue
>> LOG:  worker process: background session by PID 61242 (PID 61358)
>> exited with exit code 1
>>
>> Does this make sense to you?
>
>
> I'm specifically thinking of autonomous transactions, where you do NOT want
> to run the command in the background, but you do want to run it in another
> connection.
>
> The other example is running a command in another database. Not the same as
> the autonomous transaction use case, but once again you likely don't want
> that command running in the background.
>
> Put another way, when you launch a new process in a shell script, you have
> to do something specific for that process to run in the background.
>
> My concern here is that AIUI the only way to launch a bgworker is with it
> already backgrounded. That makes it much more likely that a bug in your code
> produces an unintended fork-bomb (connection-bomb?). That would be
> especially true if the function was re-entrant.
>
> Since one of the major points of bgworkers is exactly to run stuff in the
> background, while the parent connection is doing something else, I can
> certainly understand the default case being to background something, but I
> think it'd be good to have a simple way to start a bgworker, have it do
> something, and wait until it's done.
>
> Make sense?
>
I am not sure that I've understand this completely, but note that in
current pg_background design we simply launch worker once and feed the
subsequent queries using pg_background_run() api.

Regards,
Amul


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Jim Nasby]

On 1/9/17 7:22 AM, amul sul wrote:

On Sun, Jan 8, 2017 at 8:50 AM, Jim Nasby  wrote:



[skipped...]


Oh, hmm. So I guess if you do that when the background process is idle it's
the same as a close?

I think we need some way to safeguard against accidental forkbombs for cases
where users aren't intending to leave something running in the background.
There's other reasons to use this besides spawning long running processes,
and I'd certainly want to be able to ensure the calling function wasn't
accidentally leaving things running that it didn't mean to. (Maybe the patch
already does this...)



Current pg_background patch built to the top of BackgroundSession code
take care of that;
user need to call pg_background_close() to gracefully close previously
forked background
worker.  Even though if user session who forked this worker exited
without calling
pg_background_close(), this background worked force to exit with following log:

ERROR:  could not read from message queue: Other process has detached queue
LOG:  could not send on message queue: Other process has detached queue
LOG:  worker process: background session by PID 61242 (PID 61358)
exited with exit code 1

Does this make sense to you?


I'm specifically thinking of autonomous transactions, where you do NOT 
want to run the command in the background, but you do want to run it in 
another connection.


The other example is running a command in another database. Not the same 
as the autonomous transaction use case, but once again you likely don't 
want that command running in the background.


Put another way, when you launch a new process in a shell script, you 
have to do something specific for that process to run in the background.


My concern here is that AIUI the only way to launch a bgworker is with 
it already backgrounded. That makes it much more likely that a bug in 
your code produces an unintended fork-bomb (connection-bomb?). That 
would be especially true if the function was re-entrant.


Since one of the major points of bgworkers is exactly to run stuff in 
the background, while the parent connection is doing something else, I 
can certainly understand the default case being to background something, 
but I think it'd be good to have a simple way to start a bgworker, have 
it do something, and wait until it's done.


Make sense?
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[amul sul]

On Sun, Jan 8, 2017 at 8:50 AM, Jim Nasby  wrote:
>
[skipped...]
>
> Oh, hmm. So I guess if you do that when the background process is idle it's
> the same as a close?
>
> I think we need some way to safeguard against accidental forkbombs for cases
> where users aren't intending to leave something running in the background.
> There's other reasons to use this besides spawning long running processes,
> and I'd certainly want to be able to ensure the calling function wasn't
> accidentally leaving things running that it didn't mean to. (Maybe the patch
> already does this...)
>

Current pg_background patch built to the top of BackgroundSession code
take care of that;
user need to call pg_background_close() to gracefully close previously
forked background
worker.  Even though if user session who forked this worker exited
without calling
pg_background_close(), this background worked force to exit with following log:

ERROR:  could not read from message queue: Other process has detached queue
LOG:  could not send on message queue: Other process has detached queue
LOG:  worker process: background session by PID 61242 (PID 61358)
exited with exit code 1

Does this make sense to you?


Regards,
Amul


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Jim Nasby]

On 12/22/16 4:30 PM, Robert Haas wrote:

On Thu, Dec 22, 2016 at 4:41 PM, Jim Nasby  wrote:

On 12/22/16 4:20 AM, amul sul wrote:

• pg_background_detach : This API takes the process id and detach the
background process. Stored worker's session is not dropped until this
called.


When I hear "detach" I think that whatever I'm detaching from is going to
stick around, which I don't think is the case here, right? I'd suggest
pg_background_close() instead.


Uh, I think it is.  At least in the original version of this patch,
pg_background_detach() leaves the spawned process running but says
that you don't care to read any results it may generate.



Oh, hmm. So I guess if you do that when the background process is idle 
it's the same as a close?


I think we need some way to safeguard against accidental forkbombs for 
cases where users aren't intending to leave something running in the 
background. There's other reasons to use this besides spawning long 
running processes, and I'd certainly want to be able to ensure the 
calling function wasn't accidentally leaving things running that it 
didn't mean to. (Maybe the patch already does this...)

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[amul sul]

On Sat, Jan 7, 2017 at 2:06 PM, Andrew Borodin  wrote:
> Hi!
>
> 2017-01-07 11:44 GMT+05:00 amul sul :
>
>> Changes:
>> 1. pg_background_launch renamed to pg_background_start
>> 2. pg_background_detach renamed to pg_background_close
>> 3. Added new api to display previously launch background worker & its
>> result count waiting to be read.
> Great work!
>
Thanks.
>
>> Note that attached patches need to apply to the top of the Peter
>> Eisentraut's v2 patch[1].
> I've looked a bit into that patch. I thought you would switch
> MemoryContext before calling BackgroundSessionStart() from
> pg_background? Have I got something wrong?
>
Hmm, understood your point, let pg_background extension do the
required context setup, attached version patch does that change.
Thanks.

Regards,
Amul


0001-Changes-to-Peter-Eisentraut-s-bgsession-v2-patch.patch
Description: Binary data


0002-pg_background-as-client-of-BackgroundSession-v2.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Andrew Borodin]

Hi!

2017-01-07 11:44 GMT+05:00 amul sul :

> Changes:
> 1. pg_background_launch renamed to pg_background_start
> 2. pg_background_detach renamed to pg_background_close
> 3. Added new api to display previously launch background worker & its
> result count waiting to be read.
Great work!


> Note that attached patches need to apply to the top of the Peter
> Eisentraut's v2 patch[1].
I've looked a bit into that patch. I thought you would switch
MemoryContext before calling BackgroundSessionStart() from
pg_background? Have I got something wrong?

Best regards, Andrey Borodin.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[amul sul]

Hi all,

Attaching latest pg_background patch for review as per design proposed
on 22 Dec '16 with following minor changes in the api.

Changes:
1. pg_background_launch renamed to pg_background_start
2. pg_background_detach renamed to pg_background_close
3. Added new api to display previously launch background worker & its
result count waiting to be read.

Todo:
1. Documentation.

Thanks to Andrew Borodin & David Fetter for regression test script,
added in the attached version patch.

Note that attached patches need to apply to the top of the Peter
Eisentraut's v2 patch[1].
Patch 0001 is does some changes in BackgroundSession code required by
pg_background, which we are currently discussing on BackgroundSession
thread.

References:
[1] 
https://www.postgresql.org/message-id/attachment/48403/v2-0001-Add-background-sessions.patch

Regards,
Amul


0001-Changes-to-Peter-Eisentraut-s-bgsession-v2-patch.patch
Description: Binary data


0002-pg_background-as-client-of-BackgroundSession-v1.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Robert Haas]

On Thu, Dec 22, 2016 at 4:41 PM, Jim Nasby  wrote:
> On 12/22/16 4:20 AM, amul sul wrote:
>> • pg_background_detach : This API takes the process id and detach the
>> background process. Stored worker's session is not dropped until this
>> called.
>
> When I hear "detach" I think that whatever I'm detaching from is going to
> stick around, which I don't think is the case here, right? I'd suggest
> pg_background_close() instead.

Uh, I think it is.  At least in the original version of this patch,
pg_background_detach() leaves the spawned process running but says
that you don't care to read any results it may generate.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Jim Nasby]

On 12/22/16 4:20 AM, amul sul wrote:

• pg_background_detach : This API takes the process id and detach the
background process. Stored worker's session is not dropped until this
called.


When I hear "detach" I think that whatever I'm detaching from is going 
to stick around, which I don't think is the case here, right? I'd 
suggest pg_background_close() instead.

--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[amul sul]

Hi all,

As we have discussed previously, we need to rework this patch as a client of
Peter Eisentraut's background sessions code[1].

Attaching trial version patch to discussed possible design and api.
We could have following APIs :

• pg_background_launch : This function start and stores new background
session, and returns the process id of background worker.

• pg_background_run : This API takes the process id and SQL command as
input parameter. Using this process id, stored worker's session is
retrieved and give SQL command is executed under it.

• pg_background_result : This API takes the process id as input
parameter and returns the result of command executed thought the
background worker session.  Same as it was before but now result can
be fetch in LIFO order i.e. result of last executed query using
pg_background_run will be fetched first.

• pg_background_detach : This API takes the process id and detach the
background process. Stored worker's session is not dropped until this
called.

• TBC : API to discard result of last query or discard altogether?

• TBC : How about having one more api to see all existing sessions ?


Kindly share your thoughts/suggestions.  Note that attach patch is WIP
version, code, comments & behaviour could be vague.

--
Quick demo:
--
Apply attach patch to the top of Peter Eisentraut's
0001-Add-background-sessions.patch[1]

postgres=# select pg_background_launch();
 pg_background_launch
--
21004
(1 row)

postgres=# select pg_background_run(21004, 'vacuum verbose foo');
 pg_background_run
---

(1 row)

postgres=# select * from pg_background_result(21004) as (x text);
INFO:  vacuuming "public.foo"
INFO:  "foo": found 0 removable, 5 nonremovable row versions in 1 out of 1 pages
DETAIL:  0 dead row versions cannot be removed yet.
There were 0 unused item pointers.
Skipped 0 pages due to buffer pins.
0 pages are entirely empty.
CPU: user: 0.00 s, system: 0.00 s, elapsed: 0.00 s.
   x

 VACUUM
(1 row)

postgres=# select pg_background_run(21004, 'select * from foo');
 pg_background_run
---

(1 row)

postgres=# select * from pg_background_result(21004) as (x int);
 x
---
 1
 2
 3
 4
 5
(5 rows)

postgres=# select pg_background_detach(21004);
 pg_background_detach
--

(1 row)


References :
[1] 
https://www.postgresql.org/message-id/e1c2d331-ee6a-432d-e9f5-dcf85cffaf29%402ndquadrant.com.


Regards,
Amul Sul


0002-pg_background_worker_as_client_of_bgsession_trial.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Andrew Borodin]

2016-12-21 20:42 GMT+05:00 Robert Haas :
> This whole subthread seems like a distraction to me.  I find it hard
> to believe that this test case would be stable enough to survive the
> buildfarm where, don't forget, we have things like
> CLOBBER_CACHE_ALWAYS machines where queries take 100x longer to run.
> But even if it is, surely we can pick a less contrived test case.  So
> why worry about this?

David Fetter's test is deterministic and shall pass no matter how slow
and unpredictable perfromance is on a server.

Best regards, Andrey Borodin.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[David Fetter]

On Wed, Dec 21, 2016 at 10:42:18AM -0500, Robert Haas wrote:
> On Wed, Dec 21, 2016 at 10:29 AM, David Fetter  wrote:
> > On Wed, Dec 21, 2016 at 06:31:52PM +0800, Craig Ringer wrote:
> >> On 21 December 2016 at 14:26, Andrew Borodin  wrote:
> >>
> >> > I'm not sure every platform supports microsecond sleeps
> >>
> >> Windows at least doesn't by default, unless that changed in Win2k12
> >> and Win8 with the same platform/kernel improvements that delivered
> >> https://msdn.microsoft.com/en-us/library/hh706895(v=vs.85).aspx . I'm
> >> not sure. On older systems sleeps are 1ms to 15ms.
> >
> > Apparently, as of 2011, there were ways to do this.  It's not crystal
> > clear to me just how reliable they are.
> >
> > http://stackoverflow.com/questions/9116618/cpp-windows-is-there-a-sleep-function-in-microseconds
> 
> This whole subthread seems like a distraction to me.  I find it hard
> to believe that this test case would be stable enough to survive the
> buildfarm where, don't forget, we have things like
> CLOBBER_CACHE_ALWAYS machines where queries take 100x longer to run.
> But even if it is, surely we can pick a less contrived test case.
> So why worry about this?

I wasn't super worried about the actual sleep times, but I was having
trouble puzzling out what the test was actually doing, so I rewrote it
with what I thought of as more clarity.

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Robert Haas]

On Wed, Dec 21, 2016 at 10:29 AM, David Fetter  wrote:
> On Wed, Dec 21, 2016 at 06:31:52PM +0800, Craig Ringer wrote:
>> On 21 December 2016 at 14:26, Andrew Borodin  wrote:
>>
>> > I'm not sure every platform supports microsecond sleeps
>>
>> Windows at least doesn't by default, unless that changed in Win2k12
>> and Win8 with the same platform/kernel improvements that delivered
>> https://msdn.microsoft.com/en-us/library/hh706895(v=vs.85).aspx . I'm
>> not sure. On older systems sleeps are 1ms to 15ms.
>
> Apparently, as of 2011, there were ways to do this.  It's not crystal
> clear to me just how reliable they are.
>
> http://stackoverflow.com/questions/9116618/cpp-windows-is-there-a-sleep-function-in-microseconds

This whole subthread seems like a distraction to me.  I find it hard
to believe that this test case would be stable enough to survive the
buildfarm where, don't forget, we have things like
CLOBBER_CACHE_ALWAYS machines where queries take 100x longer to run.
But even if it is, surely we can pick a less contrived test case.  So
why worry about this?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[David Fetter]

On Wed, Dec 21, 2016 at 06:31:52PM +0800, Craig Ringer wrote:
> On 21 December 2016 at 14:26, Andrew Borodin  wrote:
> 
> > I'm not sure every platform supports microsecond sleeps
> 
> Windows at least doesn't by default, unless that changed in Win2k12
> and Win8 with the same platform/kernel improvements that delivered
> https://msdn.microsoft.com/en-us/library/hh706895(v=vs.85).aspx . I'm
> not sure. On older systems sleeps are 1ms to 15ms.

Apparently, as of 2011, there were ways to do this.  It's not crystal
clear to me just how reliable they are.

http://stackoverflow.com/questions/9116618/cpp-windows-is-there-a-sleep-function-in-microseconds

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Craig Ringer]

On 21 December 2016 at 14:26, Andrew Borodin  wrote:

> I'm not sure every platform supports microsecond sleeps

Windows at least doesn't by default, unless that changed in Win2k12
and Win8 with the same platform/kernel improvements that delivered
https://msdn.microsoft.com/en-us/library/hh706895(v=vs.85).aspx . I'm
not sure. On older systems sleeps are 1ms to 15ms.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Andrew Borodin]

2016-12-21 4:55 GMT+05:00 David Fetter :
> I see.
>
> I find the following a little easier to follow, and the sleeps still
> work even when very short.

Now I know a little more SQL :) Thank you.

I'm not sure every platform supports microsecond sleeps. But it will
work anyway. This test is deterministic.
Without sequence here is no race condition. So it is not sleepsort, it
is deterministic. Though I agree that it is good thing for test, I'd
still add some miliseconds to test case when main query for sure have
to wait end of other sleeping query.
.
>
> CREATE TABLE input AS
> SELECT x, row_number() OVER (ORDER BY x) n
> FROM
> generate_series(0,.05,0.01) x
> ORDER BY x DESC;
>
> CREATE TABLE output(place int,value float);
>
> CREATE TABLE handles AS
> SELECT pg_background_launch(
> format($$
> SELECT pg_sleep(%s);
> INSERT INTO output VALUES (%s, %s)
> $$,
> x, n, x
> )
> ) h
> FROM input;
>
> --wait until everyone finishes
> SELECT * FROM handles JOIN LATERAL pg_background_result(handles.h) AS (x 
> TEXT) ON (true);
>
> --output results
> SELECT * FROM output ORDER BY place;

Best regrards, Andrey Borodin.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Robert Haas]

On Tue, Dec 20, 2016 at 7:32 PM, Simon Riggs  wrote:
> On 9 December 2016 at 13:00, Robert Haas  wrote:
>> That might be good, because then we wouldn't have to maintain two
>> copies of the code.
>
> So why are there two things at all? Why is this being worked on as
> well as Peter's patch? What will that give us?

A feature that can be accessed without writing C code.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Simon Riggs]

On 9 December 2016 at 13:00, Robert Haas  wrote:

> That might be good, because then we wouldn't have to maintain two
> copies of the code.

So why are there two things at all? Why is this being worked on as
well as Peter's patch? What will that give us?

-- 
Simon Riggshttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[David Fetter]

On Mon, Dec 19, 2016 at 09:30:32PM +0500, Andrew Borodin wrote:
> 2016-12-19 4:21 GMT+05:00 David Fetter :
> > Couldn't it sleep in increments smaller than a second?  Like maybe
> > milliseconds?  Also, it's probably cleaner (or at least more
> > comprehensible) to write something using format() and dollar quoting
> > than the line with the double 's.
> 
> Right. Here's version which waits for half a second. I do not see how
> to path doubles via dollar sign, pg_background_launch() gets a string
> as a parameter, it's not EXCEUTE USING.

I see.

I find the following a little easier to follow, and the sleeps still
work even when very short.

Best,
David.

CREATE TABLE input AS
SELECT x, row_number() OVER (ORDER BY x) n
FROM
generate_series(0,.05,0.01) x
ORDER BY x DESC;

CREATE TABLE output(place int,value float);

CREATE TABLE handles AS
SELECT pg_background_launch(
format($$
SELECT pg_sleep(%s);
INSERT INTO output VALUES (%s, %s)
$$,
x, n, x
)
) h
FROM input;

--wait until everyone finishes
SELECT * FROM handles JOIN LATERAL pg_background_result(handles.h) AS (x TEXT) 
ON (true);

--output results
SELECT * FROM output ORDER BY place;

-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[David Fetter]

On Tue, Dec 20, 2016 at 11:11:36AM +0530, amul sul wrote:
> On Tue, Dec 20, 2016 at 12:21 AM, David Fetter  wrote:
> > On Thu, Nov 24, 2016 at 09:16:53AM +0530, amul sul wrote:
> >> Hi All,
> >>
> >> I would like to take over pg_background patch and repost for
> >> discussion and review.
> >
> > This looks great.
> >
> > Sadly, it now breaks initdb when applied atop
> > dd728826c538f000220af98de025c00114ad0631 with:
> >
> > performing post-bootstrap initialization ... TRAP: 
> > FailedAssertion("!(const Node*)(rinfo))->type) == T_RestrictInfo))", 
> > File: "costsize.c", Line: 660)
> > sh: line 1:  2840 Aborted (core dumped) 
> > "/home/shackle/pggit/postgresql/tmp_install/home/shackle/10/bin/postgres" 
> > --single -F -O -j -c search_path=pg_catalog -c exit_on_error=true template1 
> > > /dev/null
> >
> 
> I've complied binary with --enable-cassert flag and pg_background
> patch to the top of described commit as well as on latest HEAD, but I
> am not able to reproduce this crash, see this:
> 
> [VM postgresql]$ /home/amul/Public/pg_inst/pg-master/bin/postgres
> --single -F -O -j -c search_path=pg_catalog -c exit_on_error=true
> template1

I haven't managed to reproduce it either.  Sorry about the noise.

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[amul sul]

On Tue, Dec 20, 2016 at 12:21 AM, David Fetter  wrote:
> On Thu, Nov 24, 2016 at 09:16:53AM +0530, amul sul wrote:
>> Hi All,
>>
>> I would like to take over pg_background patch and repost for
>> discussion and review.
>
> This looks great.
>
> Sadly, it now breaks initdb when applied atop
> dd728826c538f000220af98de025c00114ad0631 with:
>
> performing post-bootstrap initialization ... TRAP: 
> FailedAssertion("!(const Node*)(rinfo))->type) == T_RestrictInfo))", 
> File: "costsize.c", Line: 660)
> sh: line 1:  2840 Aborted (core dumped) 
> "/home/shackle/pggit/postgresql/tmp_install/home/shackle/10/bin/postgres" 
> --single -F -O -j -c search_path=pg_catalog -c exit_on_error=true template1 > 
> /dev/null
>

I've complied binary with --enable-cassert flag and pg_background
patch to the top of described commit as well as on latest HEAD, but I
am not able to reproduce this crash, see this:

[VM postgresql]$ /home/amul/Public/pg_inst/pg-master/bin/postgres
--single -F -O -j -c search_path=pg_catalog -c exit_on_error=true
template1

PostgreSQL stand-alone backend 10devel
backend> CREATE EXTENSION pg_background;

backend> select * from pg_extension;

1: extname (typeid = 19, len = 64, typmod = -1, byval = f)
2: extowner (typeid = 26, len = 4, typmod = -1, byval = t)
3: extnamespace (typeid = 26, len = 4, typmod = -1, byval = t)
4: extrelocatable (typeid = 16, len = 1, typmod = -1, byval = t)
5: extversion (typeid = 25, len = -1, typmod = -1, byval = f)
6: extconfig (typeid = 1028, len = -1, typmod = -1, byval = f)
7: extcondition (typeid = 1009, len = -1, typmod = -1, byval = f)

1: extname = "plpgsql" (typeid = 19, len = 64, typmod = -1, byval = f)
2: extowner = "10" (typeid = 26, len = 4, typmod = -1, byval = t)
3: extnamespace = "11" (typeid = 26, len = 4, typmod = -1, byval = t)
4: extrelocatable = "f" (typeid = 16, len = 1, typmod = -1, byval = t)
5: extversion = "1.0" (typeid = 25, len = -1, typmod = -1, byval = f)

1: extname = "pg_background" (typeid = 19, len = 64, typmod = -1, byval = f)
2: extowner = "10" (typeid = 26, len = 4, typmod = -1, byval = t)
3: extnamespace = "11" (typeid = 26, len = 4, typmod = -1, byval = t)
4: extrelocatable = "t" (typeid = 16, len = 1, typmod = -1, byval = t)
5: extversion = "1.0" (typeid = 25, len = -1, typmod = -1, byval = f)

backend>

I hope I am missing anything. Thanks !

Regards,
Amul


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[David Fetter]

On Thu, Nov 24, 2016 at 09:16:53AM +0530, amul sul wrote:
> Hi All,
> 
> I would like to take over pg_background patch and repost for
> discussion and review.

This looks great.

Sadly, it now breaks initdb when applied atop
dd728826c538f000220af98de025c00114ad0631 with:

performing post-bootstrap initialization ... TRAP: FailedAssertion("!(const 
Node*)(rinfo))->type) == T_RestrictInfo))", File: "costsize.c", Line: 660)
sh: line 1:  2840 Aborted (core dumped) 
"/home/shackle/pggit/postgresql/tmp_install/home/shackle/10/bin/postgres" 
--single -F -O -j -c search_path=pg_catalog -c exit_on_error=true template1 > 
/dev/null

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Andrew Borodin]

2016-12-19 4:21 GMT+05:00 David Fetter :
> Couldn't it sleep in increments smaller than a second?  Like maybe
> milliseconds?  Also, it's probably cleaner (or at least more
> comprehensible) to write something using format() and dollar quoting
> than the line with the double 's.

Right. Here's version which waits for half a second. I do not see how
to path doubles via dollar sign, pg_background_launch() gets a string
as a parameter, it's not EXCEUTE USING.

Best regards, Andrey Borodin.
diff --git a/contrib/pg_background/Makefile b/contrib/pg_background/Makefile
index c4e717d..085fbff 100644
--- a/contrib/pg_background/Makefile
+++ b/contrib/pg_background/Makefile
@@ -6,6 +6,8 @@ OBJS = pg_background.o
 EXTENSION = pg_background
 DATA = pg_background--1.0.sql
 
+REGRESS = pg_background
+
 ifdef USE_PGXS
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
diff --git a/contrib/pg_background/expected/pg_background.out 
b/contrib/pg_background/expected/pg_background.out
new file mode 100644
index 000..a6613ce
--- /dev/null
+++ b/contrib/pg_background/expected/pg_background.out
@@ -0,0 +1,31 @@
+CREATE EXTENSION pg_background;
+--run 6 workers which wait .0, .1, .2, .3, .4, .5 seconds respectively
+CREATE TABLE input as SELECT x FROM generate_series(0,.5,0.1) x ORDER BY x 
DESC;
+CREATE TABLE output(place int,value float);
+--sequence for indication of who's finished on which place
+CREATE sequence s start 1;
+CREATE TABLE handles as SELECT pg_background_launch(format('select 
pg_sleep(%s); insert into output values (nextval(''s''),%s);',x,x)) h FROM 
input;
+--wait until everyone finishes
+SELECT (SELECT * FROM pg_background_result(h) as (x text) limit 1) FROM 
handles;
+x 
+--
+ SELECT 1
+ SELECT 1
+ SELECT 1
+ SELECT 1
+ SELECT 1
+ SELECT 1
+(6 rows)
+
+--output results
+SELECT * FROM output ORDER BY place;
+ place | value 
+---+---
+ 1 | 0
+ 2 |   0.1
+ 3 |   0.2
+ 4 |   0.3
+ 5 |   0.4
+ 6 |   0.5
+(6 rows)
+
diff --git a/contrib/pg_background/sql/pg_background.sql 
b/contrib/pg_background/sql/pg_background.sql
new file mode 100644
index 000..770f0b8
--- /dev/null
+++ b/contrib/pg_background/sql/pg_background.sql
@@ -0,0 +1,12 @@
+CREATE EXTENSION pg_background;
+
+--run 6 workers which wait .0, .1, .2, .3, .4, .5 seconds respectively
+CREATE TABLE input as SELECT x FROM generate_series(0,.5,0.1) x ORDER BY x 
DESC;
+CREATE TABLE output(place int,value float);
+--sequence for indication of who's finished on which place
+CREATE sequence s start 1;
+CREATE TABLE handles as SELECT pg_background_launch(format('select 
pg_sleep(%s); insert into output values (nextval(''s''),%s);',x,x)) h FROM 
input;
+--wait until everyone finishes
+SELECT (SELECT * FROM pg_background_result(h) as (x text) limit 1) FROM 
handles;
+--output results
+SELECT * FROM output ORDER BY place;

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[David Fetter]

On Mon, Dec 12, 2016 at 10:17:24PM +0500, Andrew Borodin wrote:
> Hi!
> 
> Just in case you'd like to include sleepsort as a test, here it is
> wrapped as a regression test(see attachment). But it has serious
> downside: it runs no less than 5 seconds.

Couldn't it sleep in increments smaller than a second?  Like maybe
milliseconds?  Also, it's probably cleaner (or at least more
comprehensible) to write something using format() and dollar quoting
than the line with the double 's.

Best,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Craig Ringer]

On 13 Dec. 2016 20:54, "amul sul"  wrote:


postgres=> select * from pg_background_result(67069) as (x text);
ERROR:  terminating connection due to administrator command
CONTEXT:  background worker, pid 67069
postgres=>


It'll also want to handle cancellation due to conflict with recovery if you
intend it to be used on a standby, probably by making use
of procsignal_sigusr1_handler. The rest of the work is done by
CHECK_FOR_INTERRUPTS() .

This only matters if it's meant to work on standbys of course. I haven't
checked if you write to catalogs or otherwise do non-standby-friendly
things.

Re: [HACKERS] pg_background contrib module proposal

[amul sul]

On Tue, Dec 13, 2016 at 2:05 PM, Andrew Borodin  wrote:
> 2016-12-13 12:55 GMT+05:00 amul sul :
>> I think background-session code is not that much deviated from
>> pg_background code,
> It is not derived, though it is not much deviated. background sessions
> code do not have detailed exceptions and code comments, but it is
> doing somewhat similar things.
>>IIUC background-session patch we can manage to
>> reuse it, as suggested by Robert.  This will allow us to maintain
>> session in long run, we could reuse this session to run subsequent
>> queries instead of maintaining separate worker pool.  Thoughts?
>
> One API to deal with both features would be much better, sure.
> "Object" like sessions pool are much easier to implement on top of
> session "object" then on top of worker process, PIDs etc.
>
>>> 4. Query as a future (actually it is implemented already by
>>> pg_background_result)
>>> 5. Promised result. Declare that you are waiting for data of specific
>>> format, execute a query, someone (from another backend) will
>>> eventually place a data to promised result and your query continues
>>> execution.
>>
>> Could you please elaborate more?
>> Do you mean two way communication between foreground & background process?
>
> It is from C++11 threading: future, promise and async - these are
> related but different kinds of aquiring result between threads.
> Feature - is when caller Cl start thread T(or dequeue thread from
> pool) and Cl can wait until T finishes and provides result.
> Here waiting the result is just like selecting from pg_background_result().
> Promise - is when you declare a variable and caller do not know which
> thread will put the data to this variable. But any thread reading
> promise will wait until other thread put a data to promise.
> There are more parallelism patterns there, like async, deffered, lazy,
> but futures and promises from my point of view are most used.
>
Nice, thanks for detailed explanation.

We can use shm_mq infrastructure to share any kind of message between
two processes,
but perhaps we might end up with overestimating what originally pg_background
could used for - the user backend will launch workers and give them an
initial set
of instruction and then results will stream back from the workers to
the user backend.

>>> 6. Cancelation: a way to signal to background query that it's time to
>>> quit gracefully.
>> Let me check this too.
> I think Craig is right: any background query must be ready to be shut
> down. That's what databases are about, you can safely pull the plug at
> any moment.

SIGTERM is handled in current pg_background patch, user can terminate
backend execution using pg_cancel_backend() or pg_terminate_backend()
as shown below:

postgres=> select pg_background_launch('insert into foo
values(generate_series(1,1))');
 pg_background_launch
--
67069
(1 row)

postgres=> select pg_terminate_backend(67069);
 pg_terminate_backend
--
 t
(1 row)

postgres=> select * from pg_background_result(67069) as (x text);
ERROR:  terminating connection due to administrator command
CONTEXT:  background worker, pid 67069
postgres=>

Thanks & Regards,
Amul


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Andrew Borodin]

2016-12-13 12:55 GMT+05:00 amul sul :
> I think background-session code is not that much deviated from
> pg_background code,
It is not derived, though it is not much deviated. background sessions
code do not have detailed exceptions and code comments, but it is
doing somewhat similar things.
>IIUC background-session patch we can manage to
> reuse it, as suggested by Robert.  This will allow us to maintain
> session in long run, we could reuse this session to run subsequent
> queries instead of maintaining separate worker pool.  Thoughts?

One API to deal with both features would be much better, sure.
"Object" like sessions pool are much easier to implement on top of
session "object" then on top of worker process, PIDs etc.

>> 4. Query as a future (actually it is implemented already by
>> pg_background_result)
>> 5. Promised result. Declare that you are waiting for data of specific
>> format, execute a query, someone (from another backend) will
>> eventually place a data to promised result and your query continues
>> execution.
>
> Could you please elaborate more?
> Do you mean two way communication between foreground & background process?

It is from C++11 threading: future, promise and async - these are
related but different kinds of aquiring result between threads.
Feature - is when caller Cl start thread T(or dequeue thread from
pool) and Cl can wait until T finishes and provides result.
Here waiting the result is just like selecting from pg_background_result().
Promise - is when you declare a variable and caller do not know which
thread will put the data to this variable. But any thread reading
promise will wait until other thread put a data to promise.
There are more parallelism patterns there, like async, deffered, lazy,
but futures and promises from my point of view are most used.

>> 6. Cancelation: a way to signal to background query that it's time to
>> quit gracefully.
> Let me check this too.
I think Craig is right: any background query must be ready to be shut
down. That's what databases are about, you can safely pull the plug at
any moment.
I've remembered one more parallelism pattern: critical region. It's
when you ask the system "plz no TERM now, and, if you can, postpone
the vacuums, OOMKs and that kind of stuff" from user code. But I do
not think it's usable pattern here.

Thank you for your work.

Best regards, Andrey Borodin.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[amul sul]

On Mon, Dec 12, 2016 at 10:47 PM, Andrew Borodin  wrote:
> Hi!
>

Thanks a lot for your review.

> Just in case you'd like to include sleepsort as a test, here it is
> wrapped as a regression test(see attachment). But it has serious
> downside: it runs no less than 5 seconds.
>
> Also I'll list here every parallelism feature I managed to imagine. It
> is not to say that I suggest having some of these features at v1. We
> certainly have to start somewhere, this list is just for information
> purposes.
> 1. Poolable workers. Just to be sure you can reliably queue your task
> somewhere without having "increase max_connections" hint.
> 2. Inside one pool, you can have task chaining. After competition of
> task A (query A) start task B, in case of failure start task C. Task C
> is followed by task D.

I think background-session code is not that much deviated from
pg_background code, IIUC background-session patch we can manage to
reuse it, as suggested by Robert.  This will allow us to maintain
session in long run, we could reuse this session to run subsequent
queries instead of maintaining separate worker pool.  Thoughts?

> 3. Reliably read task status: running\awaiting\completed\errored\in a
> lock. Get a query plan of a task? (I know, there are reasons why it is
> not possible now)
+1, Let me check this.

> 4. Query as a future (actually it is implemented already by
> pg_background_result)
> 5. Promised result. Declare that you are waiting for data of specific
> format, execute a query, someone (from another backend) will
> eventually place a data to promised result and your query continues
> execution.

Could you please elaborate more?
Do you mean two way communication between foreground & background process?

> 6. Cancelation: a way to signal to background query that it's time to
> quit gracefully.
>
Let me check this too.

Thanks & Regards,
Amul


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Craig Ringer]

On 13 December 2016 at 01:17, Andrew Borodin  wrote:

> 6. Cancelation: a way to signal to background query that it's time to
> quit gracefully.

That at least should be fuss-free. SIGTERM it, and make sure the
worker does CHECK_FOR_INTERRUPTS() in regularly-hit places and loops.
Ensure the worker waits on latches rather than pg_usleep()ing.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Andrew Borodin]

Hi!

Just in case you'd like to include sleepsort as a test, here it is
wrapped as a regression test(see attachment). But it has serious
downside: it runs no less than 5 seconds.

Also I'll list here every parallelism feature I managed to imagine. It
is not to say that I suggest having some of these features at v1. We
certainly have to start somewhere, this list is just for information
purposes.
1. Poolable workers. Just to be sure you can reliably queue your task
somewhere without having "increase max_connections" hint.
2. Inside one pool, you can have task chaining. After competition of
task A (query A) start task B, in case of failure start task C. Task C
is followed by task D.
3. Reliably read task status: running\awaiting\completed\errored\in a
lock. Get a query plan of a task? (I know, there are reasons why it is
not possible now)
4. Query as a future (actually it is implemented already by
pg_background_result)
5. Promised result. Declare that you are waiting for data of specific
format, execute a query, someone (from another backend) will
eventually place a data to promised result and your query continues
execution.
6. Cancelation: a way to signal to background query that it's time to
quit gracefully.

Best regards, Andrey Borodin.
diff --git a/contrib/pg_background/Makefile b/contrib/pg_background/Makefile
index c4e717d..085fbff 100644
--- a/contrib/pg_background/Makefile
+++ b/contrib/pg_background/Makefile
@@ -6,6 +6,8 @@ OBJS = pg_background.o
 EXTENSION = pg_background
 DATA = pg_background--1.0.sql
 
+REGRESS = pg_background
+
 ifdef USE_PGXS
 PG_CONFIG = pg_config
 PGXS := $(shell $(PG_CONFIG) --pgxs)
diff --git a/contrib/pg_background/expected/pg_background.out 
b/contrib/pg_background/expected/pg_background.out
new file mode 100644
index 000..dbc344e
--- /dev/null
+++ b/contrib/pg_background/expected/pg_background.out
@@ -0,0 +1,26 @@
+CREATE EXTENSION pg_background;
+--run 5 workers which wait about 1 second
+CREATE TABLE input as SELECT x FROM generate_series(1,5,1) x ORDER BY x DESC;
+CREATE TABLE output(place int,value int);
+CREATE sequence s start 1;
+CREATE TABLE handles as SELECT pg_background_launch('select pg_sleep('||x||'); 
insert into output values (nextval(''s''),'||x||');') h FROM input;
+SELECT (SELECT * FROM pg_background_result(h) as (x text) limit 1) FROM 
handles;
+x 
+--
+ SELECT 1
+ SELECT 1
+ SELECT 1
+ SELECT 1
+ SELECT 1
+(5 rows)
+
+SELECT * FROM output ORDER BY place;
+ place | value 
+---+---
+ 1 | 1
+ 2 | 2
+ 3 | 3
+ 4 | 4
+ 5 | 5
+(5 rows)
+
diff --git a/contrib/pg_background/sql/pg_background.sql 
b/contrib/pg_background/sql/pg_background.sql
new file mode 100644
index 000..d7cbd44
--- /dev/null
+++ b/contrib/pg_background/sql/pg_background.sql
@@ -0,0 +1,9 @@
+CREATE EXTENSION pg_background;
+
+--run 5 workers which wait about 1 second
+CREATE TABLE input as SELECT x FROM generate_series(1,5,1) x ORDER BY x DESC;
+CREATE TABLE output(place int,value int);
+CREATE sequence s start 1;
+CREATE TABLE handles as SELECT pg_background_launch('select pg_sleep('||x||'); 
insert into output values (nextval(''s''),'||x||');') h FROM input;
+SELECT (SELECT * FROM pg_background_result(h) as (x text) limit 1) FROM 
handles;
+SELECT * FROM output ORDER BY place;

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Andrew Borodin]

2016-12-09 18:00 GMT+05:00 Robert Haas :
> It looks like this could be reworked as a client of Peter Eisentraut's
> background sessions code, which I think is also derived from
> pg_background:
>
> http://archives.postgresql.org/message-id/e1c2d331-ee6a-432d-e9f5-dcf85cffa...@2ndquadrant.com
>
> That might be good, because then we wouldn't have to maintain two
> copies of the code.

Code looks quite different. I mean bgsession.c code and pg_background.c code.
Definitly, there is possibility to refactor both patches to have
common subset of base routines, they operate with similar concepts.
But to start, it's better to choose which patch goes first, or merge
them.

There is no possibility to make one on base of other since they both
require some work.
Personally, I like C code from pg_background more. It is far better
commented and has more exceptions info for user. But interface of
bgsessions is crispier. Finally, they solve different problems.

I signed up for review there too (in background sessions patch). I
hope I'll have enough resources to provide decent review for both in
december, before commitfest.

Best regards, Andrey Borodin.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Robert Haas]

On Wed, Nov 23, 2016 at 10:46 PM, amul sul  wrote:
> I would like to take over pg_background patch and repost for
> discussion and review.
>
> Initially Robert Haas has share this for parallelism demonstration[1]
> and abandoned later with
> summary of open issue[2] with this pg_background patch need to be
> fixed, most of them seems to be
> addressed in core except handling of type exists without binary
> send/recv functions and documentation.
> I have added handling for types that don't have binary send/recv
> functions in the attach patch and will
> work on documentation at the end.
>
> One concern with this patch is code duplication with
> exec_simple_query(), we could
> consider Jim Nasby’s patch[3] to overcome this,  but  certainly we
> will end up by complicating
> exec_simple_query() to make pg_background happy.
>
> As discussed previously[1] pg_background is a contrib module that lets
> you launch
> arbitrary command in a background worker.

It looks like this could be reworked as a client of Peter Eisentraut's
background sessions code, which I think is also derived from
pg_background:

http://archives.postgresql.org/message-id/e1c2d331-ee6a-432d-e9f5-dcf85cffa...@2ndquadrant.com

That might be good, because then we wouldn't have to maintain two
copies of the code.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Pavel Stehule]

2016-12-09 13:48 GMT+01:00 Andrew Borodin :

> I've read the code and now I have more suggestions.
>
> 1. Easy one. The case of different natts of expected and acutal result
> throws same errmsg as the case of wrong types.
> I think we should assist the user more here
>
> if (natts != tupdesc->natts)
>ereport(ERROR,
>  (errcode(ERRCODE_DATATYPE_MISMATCH),
>   errmsg("remote query result rowtype does not match the
> specified FROM clause rowtype")));
>
> and here
>
>if (type_id != tupdesc->attrs[i]->atttypid)
>   ereport(ERROR,
> (errcode(ERRCODE_DATATYPE_MISMATCH),
>  errmsg("remote query result rowtype does not match the
> specified FROM clause rowtype")));
>
> 2. This code
> errhint("You may need to increase max_worker_processes.")
> Is technically hinting absolutley right.
> But this extension requires something like pg_bouncer or what is
> called "thread pool".
> You just cannot safely fire a background task because you do not know
> how many workes can be spawned. Maybe it'd be better to create 'pool'
> of workers and form a queue of queries for them?
> This will make possible start a millions of subtasks.
>

This is not easy, because pgworker is related to database, not to server.
There are servers where you can have thousands databases.

Regards

Pavel


>
> 3. About getting it to the core, not as an extension. IMO this is too
> sharp thing to place it into core, I think that it's best place is in
> contribs.
>
> Thanks for the work on such a cool and fun extension.
>
> Best regards, Andrey Borodin.
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>

Re: [HACKERS] pg_background contrib module proposal

[Andrew Borodin]

I've read the code and now I have more suggestions.

1. Easy one. The case of different natts of expected and acutal result
throws same errmsg as the case of wrong types.
I think we should assist the user more here

if (natts != tupdesc->natts)
   ereport(ERROR,
 (errcode(ERRCODE_DATATYPE_MISMATCH),
  errmsg("remote query result rowtype does not match the
specified FROM clause rowtype")));

and here

   if (type_id != tupdesc->attrs[i]->atttypid)
  ereport(ERROR,
(errcode(ERRCODE_DATATYPE_MISMATCH),
 errmsg("remote query result rowtype does not match the
specified FROM clause rowtype")));

2. This code
errhint("You may need to increase max_worker_processes.")
Is technically hinting absolutley right.
But this extension requires something like pg_bouncer or what is
called "thread pool".
You just cannot safely fire a background task because you do not know
how many workes can be spawned. Maybe it'd be better to create 'pool'
of workers and form a queue of queries for them?
This will make possible start a millions of subtasks.

3. About getting it to the core, not as an extension. IMO this is too
sharp thing to place it into core, I think that it's best place is in
contribs.

Thanks for the work on such a cool and fun extension.

Best regards, Andrey Borodin.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] pg_background contrib module proposal

[Andrew Borodin]

This is simply wonderful!
Finaly, I can implement my favorite sleepsort in postgres:

create table input as select round(random()*20) x from generate_series(1,5,1);
create table output(place int,value int);
create sequence s start 1;
create table handles as select pg_background_launch('select
pg_sleep('||x||'); insert into output values
(nextval(''s''),'||x||');') h from input;
select (select * from pg_background_result(h) as (x text) limit 1) from handles;
select * from output;

Works like a charm. It has perfrect running time O(1) where n is
number of items to sort, but it cannot sort more than
max_worker_processes-1.
Next step of user cuncurrency mechanics is future indexes (indexes
under cunstruction are taken into plans, query is executed when index
is actualy constructed) and promised tables (query waits untial there
is actually data in the table).


I like the idea and implementation and I'm planning to post review by
the end of december.
Right now I have just one useful idea: I do not see comfortable way to
check up state of task (finished\failed) without possibility of haning
for long or catching fire.

Best regards, Andrey Borodin.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] pg_background contrib module proposal

[amul sul]

Hi All,

I would like to take over pg_background patch and repost for
discussion and review.

Initially Robert Haas has share this for parallelism demonstration[1]
and abandoned later with
summary of open issue[2] with this pg_background patch need to be
fixed, most of them seems to be
addressed in core except handling of type exists without binary
send/recv functions and documentation.
I have added handling for types that don't have binary send/recv
functions in the attach patch and will
work on documentation at the end.

One concern with this patch is code duplication with
exec_simple_query(), we could
consider Jim Nasby’s patch[3] to overcome this,  but  certainly we
will end up by complicating
exec_simple_query() to make pg_background happy.

As discussed previously[1] pg_background is a contrib module that lets
you launch
arbitrary command in a background worker.

• VACUUM in background
• Autonomous transaction implementation better than dblink way (i.e.
no separate authentication required).
• Allows to perform task like CREATE INDEX CONCURRENTLY from a
procedural language.

This module comes with following SQL APIs:

• pg_background_launch : This API takes SQL command, which user wants
to execute, and size of queue buffer.
  This function returns the process id of background worker.
• pg_background_result   : This API takes the process id as input
parameter and returns the result of command
  executed thought the background worker.
• pg_background_detach : This API takes the process id and detach the
background process which is waiting for
 user to read its results.


Here's an example of running vacuum and then fetching the results.
Notice that the
notices from the original session are propagated to our session; if an
error had occurred,
it would be re-thrown locally when we try to read the results.

postgres=# create table foo (a int);
CREATE TABLE
postgres=# insert into foo values(generate_series(1,5));
INSERT 0 5

postgres=# select pg_background_launch('vacuum verbose foo');
pg_background_launch
--
  65427
(1 row)

postgres=# select * from pg_background_result(65427) as (x text);
INFO:  vacuuming "public.foo"
INFO:  "foo": found 0 removable, 5 nonremovable row versions in 1 out of 1 pages
DETAIL:  0 dead row versions cannot be removed yet.
There were 0 unused item pointers.
Skipped 0 pages due to buffer pins.
0 pages are entirely empty.
CPU: user: 0.00 s, system: 0.00 s, elapsed: 0.00 s.
 x

VACUUM
(1 row)


Thanks to Vibhor kumar, Rushabh Lathia and Robert Haas for feedback.

Please let me know your thoughts, and thanks for reading.

[1]. 
https://www.postgresql.org/message-id/CA%2BTgmoam66dTzCP8N2cRcS6S6dBMFX%2BJMba%2BmDf68H%3DKAkNjPQ%40mail.gmail.com
[2]. 
https://www.postgresql.org/message-id/CA%2BTgmobPiT_3Qgjeh3_v%2B8Cq2nMczkPyAYernF_7_W9a-6T1PA%40mail.gmail.com
[3]. https://www.postgresql.org/message-id/54541779.1010906%40BlueTreble.com

Regards,
Amul


0001-pg_background_v7.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers