Re: [HACKERS] Review: Extra Daemons / bgworker

2012-12-05 Thread Alvaro Herrera
Here's a first attempt at a new documentation chapter. This goes in part Server Programming, just after the SPI chapter. I just noticed that worker_spi could use some more sample code, for example auth_counter was getting its own LWLock and also its own shmem area, which would be helpful to

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-12-05 Thread Andres Freund
On 2012-12-05 12:09:58 -0300, Alvaro Herrera wrote: Here's a first attempt at a new documentation chapter. This goes in part Server Programming, just after the SPI chapter. I just noticed that worker_spi could use some more sample code, for example auth_counter was getting its own LWLock and

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-12-05 Thread Alvaro Herrera
Andres Freund wrote: On 2012-12-05 12:09:58 -0300, Alvaro Herrera wrote: Here's a first attempt at a new documentation chapter. This goes in part Server Programming, just after the SPI chapter. I just noticed that worker_spi could use some more sample code, for example auth_counter was

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-12-05 Thread Simon Riggs
On 5 December 2012 15:09, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Here's a first attempt at a new documentation chapter. This goes in part Server Programming, just after the SPI chapter. I just noticed that worker_spi could use some more sample code, for example auth_counter was

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-12-05 Thread Alvaro Herrera
Simon Riggs wrote: On 5 December 2012 15:09, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Here's a first attempt at a new documentation chapter. This goes in part Server Programming, just after the SPI chapter. I just noticed that worker_spi could use some more sample code, for

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-12-05 Thread Andres Freund
On 2012-12-05 19:03:44 -0300, Alvaro Herrera wrote: Simon Riggs wrote: Prefer BgWorkerStart_ConsistentState to be renamed to BgWorkerRun_InHotStandby BgWorkerStart_RecoveryFinished to be renamed to BgWorkerRun_InNormalMode presumably a process will shutdown if (BgWorkerRun_InHotStandby

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-12-05 Thread Andres Freund
On 2012-12-05 18:42:42 -0300, Alvaro Herrera wrote: para structfieldbgw_sighup/structfield and structfieldbgw_sigterm/ are pointers to functions that will be installed as signal handlers for the new - process. + process. XXX: Can they be NULL? /para Hm. The

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-12-05 Thread Simon Riggs
On 5 December 2012 22:22, Andres Freund and...@2ndquadrant.com wrote: On 2012-12-05 19:03:44 -0300, Alvaro Herrera wrote: Simon Riggs wrote: Prefer BgWorkerStart_ConsistentState to be renamed to BgWorkerRun_InHotStandby BgWorkerStart_RecoveryFinished to be renamed to

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-12-04 Thread Markus Wanner
On 12/03/2012 10:35 PM, Alvaro Herrera wrote: So here's version 8. This fixes a couple of bugs and most notably creates a separate PGPROC list for bgworkers, so that they don't interfere with client-connected backends. Nice, thanks. I'll try to review this again, shortly. Regards Markus

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-12-04 Thread Alvaro Herrera
Alvaro Herrera wrote: One notable thing is that I had to introduce this in the postmaster startup sequence: /* * process any libraries that should be preloaded at postmaster start */ process_shared_preload_libraries(); /* * If loadable modules have added

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-12-03 Thread Alvaro Herrera
Markus Wanner wrote: On 11/28/2012 03:51 PM, Alvaro Herrera wrote: I remember your patchset. I didn't look at it for this round, for no particular reason. I did look at KaiGai's submission from two commitfests ago, and also at a patch from Simon which AFAIK was never published openly.

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-12-03 Thread Markus Wanner
On 12/03/2012 03:28 PM, Alvaro Herrera wrote: I'm not really that interested in it currently ... and there are enough other patches to review. I would like bgworkers to mature a bit more and get some heavy real world testing before we change autovacuum. Understood. Just like av_launcher

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-12-03 Thread Simon Riggs
On 3 December 2012 15:17, Markus Wanner mar...@bluegap.ch wrote: Just like av_launcher does it now: set a flag in shared memory and signal the postmaster (PMSIGNAL_START_AUTOVAC_WORKER). I'm not sure how this works. What process is in charge of setting such a flag? The only process that

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-12-03 Thread Alvaro Herrera
Markus Wanner wrote: On 12/03/2012 03:28 PM, Alvaro Herrera wrote: Just like av_launcher does it now: set a flag in shared memory and signal the postmaster (PMSIGNAL_START_AUTOVAC_WORKER). I'm not sure how this works. What process is in charge of setting such a flag? The only

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-12-03 Thread Alvaro Herrera
Simon Riggs wrote: On 3 December 2012 15:17, Markus Wanner mar...@bluegap.ch wrote: The only process that currently starts background workers ... ehm ... autovacuum workers is the autovacuum launcher. It uses the above Postmaster Signal in autovacuum.c:do_start_autovacuum_worker() to have

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-12-03 Thread Markus Wanner
On 12/03/2012 04:27 PM, Simon Riggs wrote: My understanding was that the patch keep autovac workers and background workers separate at this point. I agree to that, for this patch. However, that might not keep me from trying to (re-)sumbit some of the bgworker patches in my queue. :-) Regards

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-12-03 Thread Markus Wanner
On 12/03/2012 04:44 PM, Alvaro Herrera wrote: Simon Riggs wrote: Is there anything to be gained *now* from merging those two concepts? I saw that as refactoring that can occur once we are happy it should take place, but isn't necessary. IMO it's a net loss in robustness of the autovac

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-12-03 Thread Markus Wanner
Alvaro, in general, please keep in mind that there are two aspects that I tend to mix and confuse: one is what's implemented and working for Postgres-R. The other this is how I envision (parts of it) to be merged back into Postgres, itself. Sorry if that causes confusion. On 12/03/2012 04:43 PM,

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-11-30 Thread Dimitri Fontaine
Markus Wanner mar...@bluegap.ch writes: However, as you say, maybe we need more coding examples. Maybe a minimally usable extra daemon example? Showing how to avoid common pitfalls? Use cases, anybody? :-) What about the PGQ ticker, pgqd?

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-11-30 Thread Pavel Stehule
2012/11/30 Dimitri Fontaine dimi...@2ndquadrant.fr: Markus Wanner mar...@bluegap.ch writes: However, as you say, maybe we need more coding examples. Maybe a minimally usable extra daemon example? Showing how to avoid common pitfalls? Use cases, anybody? :-) What about the PGQ ticker, pgqd?

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-11-30 Thread Markus Wanner
On 11/30/2012 11:09 AM, Dimitri Fontaine wrote: Markus Wanner mar...@bluegap.ch writes: However, as you say, maybe we need more coding examples. Maybe a minimally usable extra daemon example? Showing how to avoid common pitfalls? Use cases, anybody? :-) What about the PGQ ticker, pgqd?

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-11-30 Thread Dimitri Fontaine
Markus Wanner mar...@bluegap.ch writes: AFAICS pgqd currently uses libpq, so I think it would rather turn into what I call a background worker, with a connection to Postgres shared memory. I perfectly well see use cases (plural!) for those. What I'm questioning is the use for what I rather

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-11-30 Thread Alvaro Herrera
Dimitri Fontaine wrote: Markus Wanner mar...@bluegap.ch writes: AFAICS pgqd currently uses libpq, so I think it would rather turn into what I call a background worker, with a connection to Postgres shared memory. I perfectly well see use cases (plural!) for those. What I'm questioning

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-11-30 Thread Andres Freund
On 2012-11-30 09:57:20 -0300, Alvaro Herrera wrote: Dimitri Fontaine wrote: Markus Wanner mar...@bluegap.ch writes: AFAICS pgqd currently uses libpq, so I think it would rather turn into what I call a background worker, with a connection to Postgres shared memory. I perfectly well see

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-11-30 Thread Dimitri Fontaine
Andres Freund and...@2ndquadrant.com writes: One of the uses for bgworkers that don't have shmem connection is to have them use libpq connections instead. I don't really see the point of forcing everyone to use backend connections when libpq connections are enough. In particular, they are

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-11-30 Thread Kohei KaiGai
2012/11/30 Dimitri Fontaine dimi...@2ndquadrant.fr: Andres Freund and...@2ndquadrant.com writes: One of the uses for bgworkers that don't have shmem connection is to have them use libpq connections instead. I don't really see the point of forcing everyone to use backend connections when libpq

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-11-30 Thread Markus Wanner
On 11/30/2012 01:40 PM, Dimitri Fontaine wrote: I totally missed the need to connect to shared memory to be able to connect to a database and query it. Can't we just link the bgworkder code to the client libpq library, just as plproxy is doing I believe? Well, sure you can use libpq. But so

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-11-30 Thread Markus Wanner
On 11/30/2012 01:59 PM, Andres Freund wrote: On 2012-11-30 09:57:20 -0300, Alvaro Herrera wrote: One of the uses for bgworkers that don't have shmem connection is to have them use libpq connections instead. I don't really see the point of forcing everyone to use backend connections when libpq

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-11-30 Thread Dimitri Fontaine
Kohei KaiGai kai...@kaigai.gr.jp writes: One thing we have to pay attention is, the backend code cannot distinguish connection from pgworker via libpq from other regular connections, from perspective of access control. Even if we implement major portion with C-function, do we have a reliable

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-11-30 Thread Alvaro Herrera
Markus Wanner wrote: On 11/30/2012 01:40 PM, Dimitri Fontaine wrote: I totally missed the need to connect to shared memory to be able to connect to a database and query it. Can't we just link the bgworkder code to the client libpq library, just as plproxy is doing I believe? Well, sure

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-11-30 Thread Kohei KaiGai
2012/11/30 Dimitri Fontaine dimi...@2ndquadrant.fr: Kohei KaiGai kai...@kaigai.gr.jp writes: One thing we have to pay attention is, the backend code cannot distinguish connection from pgworker via libpq from other regular connections, from perspective of access control. Even if we implement

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-11-30 Thread Markus Wanner
Alvaro, On 11/30/2012 02:44 PM, Alvaro Herrera wrote: So it makes easier to have processes that need to run alongside postmaster. That's where we are in respectful disagreement, then. As I don't think that's easier, overall, but in my eye, this looks like a foot gun. As long as things like

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-11-30 Thread Markus Wanner
On 11/30/2012 03:16 PM, Kohei KaiGai wrote: This feature does not enforce them to implement with this new framework. If they can perform as separate daemons, it is fine enough. I'm not clear on what exactly you envision, but if a process needs access to shared buffers, it sounds like it should

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-11-30 Thread Kohei KaiGai
2012/11/30 Markus Wanner mar...@bluegap.ch: On 11/30/2012 03:16 PM, Kohei KaiGai wrote: This feature does not enforce them to implement with this new framework. If they can perform as separate daemons, it is fine enough. I'm not clear on what exactly you envision, but if a process needs

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-11-28 Thread Alvaro Herrera
Markus Wanner wrote: Hi Markus, Many thanks for your review. first of all, thank you for bringing this up again and providing a patch. My first attempt on that was more than two years ago [1]. As the author of a former bgworker patch, I'd like to provide an additional review - KaiGai was

Re: [HACKERS] Review: Extra Daemons / bgworker

2012-11-28 Thread Markus Wanner
On 11/28/2012 03:51 PM, Alvaro Herrera wrote: I remember your patchset. I didn't look at it for this round, for no particular reason. I did look at KaiGai's submission from two commitfests ago, and also at a patch from Simon which AFAIK was never published openly. Simon's patch merged the