BUG #1707: statistics collector starts with stats_start_collector = false

"Federico Di Gregorio" <fog@initd.org> writes:

If the following combinatio of parameters is used:

stats_start_collector = false
stats_row_level = true

the collector process is started even if the documentation says that "The
parameter stats_start_collector must be set to true for the statistics
collector to be launched at all."

I believe stats_start_collector is forced on if any of the subsidiary
parameters is set. Isn't that what you want?

regards, tom lane

Tom Lane wrote:

"Federico Di Gregorio" <fog@initd.org> writes:

If the following combinatio of parameters is used:

stats_start_collector = false
stats_row_level = true

the collector process is started even if the documentation says that "The
parameter stats_start_collector must be set to true for the statistics
collector to be launched at all."

I believe stats_start_collector is forced on if any of the subsidiary
parameters is set. Isn't that what you want?

If that is the logic, why do we have stats_start_collector at all?
Seems we either should honor it, or remove it.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

If that is the logic, why do we have stats_start_collector at all?

Ask Jan ;-)

I can see some advantage to it as a way of finding out whether the
collector was started or not ... but if that's the intent, we
should make it non user-writable.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

If that is the logic, why do we have stats_start_collector at all?

Ask Jan ;-)

I can see some advantage to it as a way of finding out whether the
collector was started or not ... but if that's the intent, we
should make it non user-writable.

I can see someone turning it off but leaving other options on because
they will want to turn them on later. I think our current behavior is
broken.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I can see someone turning it off but leaving other options on because
they will want to turn them on later.

"later" when? It's not useful to try to turn on stats_start_collector
after postmaster start, because the UDP socket won't exist.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I can see someone turning it off but leaving other options on because
they will want to turn them on later.

"later" when? It's not useful to try to turn on stats_start_collector
after postmaster start, because the UDP socket won't exist.

Later such as in a later postmaster start, but personally I would just
remove the option completely.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Later such as in a later postmaster start, but personally I would just
remove the option completely.

I don't have a problem with removing it as a writable option ... but
I'm thinking we should leave it as a read-only GUC parameter (like
the several others we have already). Otherwise we'll need to add some
other method of finding out whether the collector is running.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Later such as in a later postmaster start, but personally I would just
remove the option completely.

I don't have a problem with removing it as a writable option ... but
I'm thinking we should leave it as a read-only GUC parameter (like
the several others we have already). Otherwise we'll need to add some
other method of finding out whether the collector is running.

Why would a user care? They either are gathering such statistics, or
they are not. I think the only issue is that you can't turn on certain
options after postmaster start unless the stats collector is running ---
is that the issue?

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Tom Lane wrote:

I don't have a problem with removing it as a writable option ... but
I'm thinking we should leave it as a read-only GUC parameter (like
the several others we have already). Otherwise we'll need to add some
other method of finding out whether the collector is running.

Why would a user care?

luser> It's not collecting statistics!

us> Is the collector running?

luser> How should I know?

us> er ...

If we don't have a way to check this, we'll regret it soon enough...
now maybe a GUC setting isn't the optimal way, but I think we need
*some* way besides ps. ps doesn't work remotely and I think there's
no simple equivalent under Windows either.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Tom Lane wrote:

I don't have a problem with removing it as a writable option ... but
I'm thinking we should leave it as a read-only GUC parameter (like
the several others we have already). Otherwise we'll need to add some
other method of finding out whether the collector is running.

Why would a user care?

luser> It's not collecting statistics!

us> Is the collector running?

luser> How should I know?

us> er ...

If we don't have a way to check this, we'll regret it soon enough...
now maybe a GUC setting isn't the optimal way, but I think we need
*some* way besides ps. ps doesn't work remotely and I think there's
no simple equivalent under Windows either.

Sure, but the GUC only reports that it thinks the stats collector
started, not whether it is running or not. I think 'ps' is a fine way
to tell.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian wrote:

If we don't have a way to check this, we'll regret it soon enough...
now maybe a GUC setting isn't the optimal way, but I think we need
*some* way besides ps. ps doesn't work remotely and I think there's
no simple equivalent under Windows either.

Sure, but the GUC only reports that it thinks the stats collector
started, not whether it is running or not. I think 'ps' is a fine way
to tell.

Um, no such beast under win32. You can only see some postgres processes,
but can't distinguish them. We'd need some functions that examine the
pids known to the postmaster (pg_get_collector_pid, pg_get_logger_pid, ...)

Regards,
Andreas

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Later such as in a later postmaster start, but personally I would just
remove the option completely.

I don't have a problem with removing it as a writable option ... but
I'm thinking we should leave it as a read-only GUC parameter (like
the several others we have already). Otherwise we'll need to add some
other method of finding out whether the collector is running.

I assume the collector should always be running, and if it isn't, it is
mentioned in the server logs. That seems sufficient.

The following patch removes the GUC stats_start_collector. The stats
process is now always started.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

The following patch removes the GUC stats_start_collector. The stats
process is now always started.

WAIT a second!!!

That was *not* the agenda; the collector is only supposed to start if
you turn on at least one of the collection options.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

The following patch removes the GUC stats_start_collector. The stats
process is now always started.

WAIT a second!!!

That was *not* the agenda; the collector is only supposed to start if
you turn on at least one of the collection options.

Really? I thought the process always started and it just sits there if
nothing is fed to it, no? In fact, if it wasn't started from the
beginning, we couldn't enable query strings without restarting the
server.

The current default is true, meaning it always started.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

The current default is true, meaning it always started.

*By default* it starts. But you can turn it off, and there was no
discussion of removing that option.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

The current default is true, meaning it always started.

*By default* it starts. But you can turn it off, and there was no
discussion of removing that option.

I thought we agreed that there was little value in allowing someone to
prevent the collector from starting. It is just a process slot, right?

You were suggesting we make it read-only in the email I quoted
originally. I am confused.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

On Mon, Jun 27, 2005 at 08:51:46PM -0400, Bruce Momjian wrote:

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

The following patch removes the GUC stats_start_collector. The stats
process is now always started.

WAIT a second!!!

That was *not* the agenda; the collector is only supposed to start if
you turn on at least one of the collection options.

Really? I thought the process always started and it just sits there if
nothing is fed to it, no?

No, the process is only started if you ask for it (which is true by
default).

In fact, if it wasn't started from the beginning, we couldn't enable
query strings without restarting the server.

That's true -- if you start with the collector disabled, you can't
enable query strings.

The current default is true, meaning it always started.

Yes, but you can turn it off.

--
Alvaro Herrera (<alvherre[a]surnet.cl>)
"La libertad es como el dinero; el que no la sabe emplear la pierde" (Alvarez)

Alvaro Herrera wrote:

On Mon, Jun 27, 2005 at 08:51:46PM -0400, Bruce Momjian wrote:

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

The following patch removes the GUC stats_start_collector. The stats
process is now always started.

WAIT a second!!!

That was *not* the agenda; the collector is only supposed to start if
you turn on at least one of the collection options.

Really? I thought the process always started and it just sits there if
nothing is fed to it, no?

No, the process is only started if you ask for it (which is true by
default).

In fact, if it wasn't started from the beginning, we couldn't enable
query strings without restarting the server.

That's true -- if you start with the collector disabled, you can't
enable query strings.

The current default is true, meaning it always started.

Yes, but you can turn it off.

OK, but if all the stats* variables are off, what overhead is there in
having the stats process run? I would think very little, and if we turn
it always on, we make configuration easier and remove a GUC variable.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

On Mon, Jun 27, 2005 at 09:13:20PM -0400, Bruce Momjian wrote:

Alvaro Herrera wrote:

In fact, if it wasn't started from the beginning, we couldn't enable
query strings without restarting the server.

That's true -- if you start with the collector disabled, you can't
enable query strings.

OK, but if all the stats* variables are off, what overhead is there in
having the stats process run? I would think very little, and if we turn
it always on, we make configuration easier and remove a GUC variable.

What about arranging postmaster's main loop so that it starts the stats
collector process if one of the options is activated (assuming they were
all off at start)? Right now you have to restart the server, but I
don't see why it has to be like that.

That has even less overhead. We will have to keep the GUC var, but I
don't think that's too onerous, is it?

--
Alvaro Herrera (<alvherre[a]surnet.cl>)
"Granting software the freedom to evolve guarantees only different results,
not better ones." (Zygo Blaxell)