Re: [PATCHES] pg_postmaster_reload_time() patch
"George Gensure" <[EMAIL PROTECTED]> writes: > The new function name is pg_conf_load_time() Applied with revisions. > I'm now using LWLocks only on the backend in order to protect the > PgReloadTime from mid copy reads. This may prove to be unnecessary, > since the code to handle HUPs seems to be executed synchronously on > the backend, but I'll let someone else tell me its safe before > removing it. The locking was not only entirely useless, but it didn't even compile, since you'd not supplied a definition for "ReloadTimeLock". I took it out. I moved the setting of PgReloadTime into ProcessConfigFile. The advantages are (1) only one place to do it, and (2) inside ProcessConfigFile, we know whether or not the reload actually "took". As committed, the definition of PgReloadTime is really "the time of the last *successful* load of postgresql.conf", which I think is more useful than "the last attempted load". I also improved the documentation a bit and added the copy step needed in restore_backend_variables(). regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] pg_postmaster_reload_time() patch
On Fri, May 2, 2008 at 10:31 AM, Alvaro Herrera <[EMAIL PROTECTED]> wrote: > George Gensure escribió: > > > > So if nobody's got any further objections, could this patch be applied? > > It's in the queue: > > http://wiki.postgresql.org/wiki/CommitFest:May > > -- > > > Alvaro Herrerahttp://www.CommandPrompt.com/ > The PostgreSQL Company - Command Prompt, Inc. > Thanks! -George -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] pg_postmaster_reload_time() patch
George Gensure escribió: > So if nobody's got any further objections, could this patch be applied? It's in the queue: http://wiki.postgresql.org/wiki/CommitFest:May -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] pg_postmaster_reload_time() patch
On Wed, Apr 30, 2008 at 12:58 PM, George Gensure <[EMAIL PROTECTED]> wrote: > On Wed, Apr 30, 2008 at 8:16 AM, Alvaro Herrera > <[EMAIL PROTECTED]> wrote: > > > > George Gensure escribió: > > > > > > > I've done a quick write up for reload time reporting from the > > > administration TODO. I was a little paranoid with the locking, but > > > didn't want problems to occur with signals on the postmaster and the > > > read side. > > > > I'd say too much -- postmaster runs with signals blocked all the time > > (except during select()) so this is not necessary there. > > > > Regarding the locking on backends, I admit I am not sure if this is > > really a problem enough that you need a spinlock for it. Anyway we tend > > not to use spinlocks too much -- probably an LWLock would be more > > apropos, if a lock is really needed. (A bigger question is whether the > > reload time should be local for each backend, or exposed globally > > through MyProc. I don't think it's interesting enough to warrant that, > > but perhaps others think differently.) > > > > Lastly, I didn't read the patch close enough to tell if it would work on > > both the EXEC_BACKEND case and the regular one. > > > > -- > > Alvaro Herrera > http://www.CommandPrompt.com/ > > The PostgreSQL Company - Command Prompt, Inc. > > > > I've reworked the patch in response to comments. > > The new function name is pg_conf_load_time() > I'm now using LWLocks only on the backend in order to protect the > PgReloadTime from mid copy reads. This may prove to be unnecessary, > since the code to handle HUPs seems to be executed synchronously on > the backend, but I'll let someone else tell me its safe before > removing it. > > -George > So if nobody's got any further objections, could this patch be applied? -George -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] pg_postmaster_reload_time() patch
On Wed, Apr 30, 2008 at 8:16 AM, Alvaro Herrera
<[EMAIL PROTECTED]> wrote:
> George Gensure escribió:
>
>
> > I've done a quick write up for reload time reporting from the
> > administration TODO. I was a little paranoid with the locking, but
> > didn't want problems to occur with signals on the postmaster and the
> > read side.
>
> I'd say too much -- postmaster runs with signals blocked all the time
> (except during select()) so this is not necessary there.
>
> Regarding the locking on backends, I admit I am not sure if this is
> really a problem enough that you need a spinlock for it. Anyway we tend
> not to use spinlocks too much -- probably an LWLock would be more
> apropos, if a lock is really needed. (A bigger question is whether the
> reload time should be local for each backend, or exposed globally
> through MyProc. I don't think it's interesting enough to warrant that,
> but perhaps others think differently.)
>
> Lastly, I didn't read the patch close enough to tell if it would work on
> both the EXEC_BACKEND case and the regular one.
>
> --
> Alvaro Herrerahttp://www.CommandPrompt.com/
> The PostgreSQL Company - Command Prompt, Inc.
>
I've reworked the patch in response to comments.
The new function name is pg_conf_load_time()
I'm now using LWLocks only on the backend in order to protect the
PgReloadTime from mid copy reads. This may prove to be unnecessary,
since the code to handle HUPs seems to be executed synchronously on
the backend, but I'll let someone else tell me its safe before
removing it.
-George
*** ./doc/src/sgml/func.sgml.orig 2008-04-29 23:47:39.378726574 -0400
--- ./doc/src/sgml/func.sgml 2008-04-30 11:31:38.434893712 -0400
***
*** 10892,10897
--- 10892,10903
+pg_conf_load_time()
+timestamp with time zone
+config load time
+
+
+
session_user
name
session user name
***
*** 11037,11042
--- 11043,11062
+ pg_conf_load_time
+
+
+
+ pg_conf_load_time returns the
+ timestamp with time zone when the
+ config was last loaded for this session.
+
+
+
+ version
+
+
+
version
*** ./src/backend/postmaster/postmaster.c.orig 2008-04-29 23:48:07.374455697 -0400
--- ./src/backend/postmaster/postmaster.c 2008-04-30 12:02:05.156215936 -0400
***
*** 390,395
--- 390,396
InheritableSocket pgStatSock;
pid_t PostmasterPid;
TimestampTz PgStartTime;
+ TimestampTz PgReloadTime;
bool redirection_done;
#ifdef WIN32
HANDLE PostmasterHandle;
***
*** 1008,1013
--- 1009,1015
* Remember postmaster startup time
*/
PgStartTime = GetCurrentTimestamp();
+ PgReloadTime = PgStartTime;
/* PostmasterRandom wants its own copy */
gettimeofday(&random_start_time, NULL);
***
*** 1931,1936
--- 1933,1943
/* Update the starting-point file for future children */
write_nondefault_variables(PGC_SIGHUP);
#endif
+
+ /*
+ * Remember config load time
+ */
+ PgReloadTime = GetCurrentTimestamp();
}
PG_SETMASK(&UnBlockSig);
***
*** 4263,4268
--- 4270,4276
param->PostmasterPid = PostmasterPid;
param->PgStartTime = PgStartTime;
+ param->PgReloadTime = PgReloadTime;
param->redirection_done = redirection_done;
*** ./src/backend/tcop/postgres.c.orig 2008-04-29 23:48:58.866600196 -0400
--- ./src/backend/tcop/postgres.c 2008-04-30 12:21:40.476913468 -0400
***
*** 58,63
--- 58,64
#include "storage/ipc.h"
#include "storage/proc.h"
#include "storage/sinval.h"
+ #include "storage/spin.h"
#include "tcop/fastpath.h"
#include "tcop/pquery.h"
#include "tcop/tcopprot.h"
***
*** 3375,3380
--- 3376,3382
*/
if (!IsUnderPostmaster)
PgStartTime = GetCurrentTimestamp();
+ PgReloadTime = PgStartTime;
/*
* POSTGRES main processing loop begins here
***
*** 3550,3555
--- 3552,3564
{
got_SIGHUP = false;
ProcessConfigFile(PGC_SIGHUP);
+
+ /*
+ * Remember config load time
+ */
+ LWLockAcquire(ReloadTimeLock, LW_EXCLUSIVE);
+ PgReloadTime = GetCurrentTimestamp();
+ LWLockRelease(ReloadTimeLock);
}
/*
*** ./src/backend/utils/adt/timestamp.c.orig 2008-04-29 23:49:46.359354923 -0400
--- ./src/backend/utils/adt/timestamp.c 2008-04-30 12:23:52.456779063 -0400
***
*** 42,47
--- 42,49
/* Set at postmaster start */
TimestampTz PgStartTime;
+ /* Set at configuration reload */
+ TimestampTz PgReloadTime;
static TimeOffset time2t(const int hour, const int min, const int sec, const fsec_t fsec);
***
*** 1162,1167
--- 1164,1179
PG_RETURN_TIMESTAMPTZ(PgStartTime);
}
+ Datum
+ pgsql_conf_load_time(PG_FUNCTION_ARGS)
+ {
+ TimestampTz ti
Re: [PATCHES] pg_postmaster_reload_time() patch
George Gensure escribió: > I've done a quick write up for reload time reporting from the > administration TODO. I was a little paranoid with the locking, but > didn't want problems to occur with signals on the postmaster and the > read side. I'd say too much -- postmaster runs with signals blocked all the time (except during select()) so this is not necessary there. Regarding the locking on backends, I admit I am not sure if this is really a problem enough that you need a spinlock for it. Anyway we tend not to use spinlocks too much -- probably an LWLock would be more apropos, if a lock is really needed. (A bigger question is whether the reload time should be local for each backend, or exposed globally through MyProc. I don't think it's interesting enough to warrant that, but perhaps others think differently.) Lastly, I didn't read the patch close enough to tell if it would work on both the EXEC_BACKEND case and the regular one. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] pg_postmaster_reload_time() patch
Gurjeet Singh wrote: On Wed, Apr 30, 2008 at 9:53 AM, George Gensure <[EMAIL PROTECTED]> wrote: I've done a quick write up for reload time reporting from the administration TODO. I was a little paranoid with the locking, but didn't want problems to occur with signals on the postmaster and the read side. IMHO, the function should return NULL if the postmaster never reloaded; that is, it was started, but never signaled to reload. I think it's useful to get the server startup time if the configuration has never been reloaded. That's when the configuration was loaded, if no reload has been triggered since. Perhaps the function should be named to reflect that better. pg_configuration_load_time() ? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] pg_postmaster_reload_time() patch
On Wed, Apr 30, 2008 at 9:53 AM, George Gensure <[EMAIL PROTECTED]> wrote: > I've done a quick write up for reload time reporting from the > administration TODO. I was a little paranoid with the locking, but > didn't want problems to occur with signals on the postmaster and the > read side. > > IMHO, the function should return NULL if the postmaster never reloaded; that is, it was started, but never signaled to reload. Best regards, -- [EMAIL PROTECTED] [EMAIL PROTECTED] gmail | hotmail | indiatimes | yahoo }.com EnterpriseDB http://www.enterprisedb.com Mail sent from my BlackLaptop device
[PATCHES] pg_postmaster_reload_time() patch
I've done a quick write up for reload time reporting from the
administration TODO. I was a little paranoid with the locking, but
didn't want problems to occur with signals on the postmaster and the
read side.
-George
*** ./doc/src/sgml/func.sgml.orig 2008-04-29 23:47:39.378726574 -0400
--- ./doc/src/sgml/func.sgml 2008-04-29 23:51:12.346237119 -0400
***
*** 10892,10897
--- 10892,10903
+pg_postmaster_reload_time()
+timestamp with time zone
+server last reload time
+
+
+
session_user
name
session user name
***
*** 11037,11042
--- 11043,11062
+ pg_postmaster_reload_time
+
+
+
+ pg_postmaster_reload_time returns the
+ timestamp with time zone when the
+ server was last reloaded.
+
+
+
+ version
+
+
+
version
*** ./src/backend/postmaster/postmaster.c.orig 2008-04-29 23:48:07.374455697 -0400
--- ./src/backend/postmaster/postmaster.c 2008-04-29 23:51:12.346237119 -0400
***
*** 112,117
--- 112,118
#include "storage/pg_shmem.h"
#include "storage/pmsignal.h"
#include "storage/proc.h"
+ #include "storage/spin.h"
#include "tcop/tcopprot.h"
#include "utils/builtins.h"
#include "utils/datetime.h"
***
*** 390,395
--- 391,398
InheritableSocket pgStatSock;
pid_t PostmasterPid;
TimestampTz PgStartTime;
+ TimestampTz PgReloadTime;
+ slock_t PgReloadTimeLock;
bool redirection_done;
#ifdef WIN32
HANDLE PostmasterHandle;
***
*** 1008,1013
--- 1011,1018
* Remember postmaster startup time
*/
PgStartTime = GetCurrentTimestamp();
+ PgReloadTime = PgStartTime;
+ SpinLockInit(&PgReloadTimeLock);
/* PostmasterRandom wants its own copy */
gettimeofday(&random_start_time, NULL);
***
*** 1931,1936
--- 1936,1948
/* Update the starting-point file for future children */
write_nondefault_variables(PGC_SIGHUP);
#endif
+
+ /*
+ * Remember postmaster reload time
+ */
+ SpinLockAcquire(&PgReloadTimeLock);
+ PgReloadTime = GetCurrentTimestamp();
+ SpinLockRelease(&PgReloadTimeLock);
}
PG_SETMASK(&UnBlockSig);
***
*** 4263,4268
--- 4275,4282
param->PostmasterPid = PostmasterPid;
param->PgStartTime = PgStartTime;
+ param->PgReloadTime = PgReloadTime;
+ param->PgReloadTimeLock = PgReloadTimeLock;
param->redirection_done = redirection_done;
*** ./src/backend/tcop/postgres.c.orig 2008-04-29 23:48:58.866600196 -0400
--- ./src/backend/tcop/postgres.c 2008-04-29 23:51:12.346237119 -0400
***
*** 58,63
--- 58,64
#include "storage/ipc.h"
#include "storage/proc.h"
#include "storage/sinval.h"
+ #include "storage/spin.h"
#include "tcop/fastpath.h"
#include "tcop/pquery.h"
#include "tcop/tcopprot.h"
***
*** 3373,3380
/*
* Remember stand-alone backend startup time
*/
! if (!IsUnderPostmaster)
PgStartTime = GetCurrentTimestamp();
/*
* POSTGRES main processing loop begins here
--- 3374,3384
/*
* Remember stand-alone backend startup time
*/
! if (!IsUnderPostmaster) {
PgStartTime = GetCurrentTimestamp();
+ PgReloadTime = PgStartTime;
+ SpinLockInit(&PgReloadTimeLock);
+ }
/*
* POSTGRES main processing loop begins here
***
*** 3550,3555
--- 3554,3566
{
got_SIGHUP = false;
ProcessConfigFile(PGC_SIGHUP);
+
+ /*
+ * Remember postmaster reload time
+ */
+ SpinLockAcquire(&PgReloadTimeLock);
+ PgReloadTime = GetCurrentTimestamp();
+ SpinLockRelease(&PgReloadTimeLock);
}
/*
*** ./src/backend/utils/adt/timestamp.c.orig 2008-04-29 23:49:46.359354923 -0400
--- ./src/backend/utils/adt/timestamp.c 2008-04-29 23:51:12.346237119 -0400
***
*** 27,32
--- 27,33
#include "libpq/pqformat.h"
#include "miscadmin.h"
#include "parser/scansup.h"
+ #include "storage/spin.h"
#include "utils/array.h"
#include "utils/builtins.h"
#include "utils/datetime.h"
***
*** 42,47
--- 43,50
/* Set at postmaster start */
TimestampTz PgStartTime;
+ TimestampTz PgReloadTime;
+ slock_t PgReloadTimeLock;
static TimeOffset time2t(const int hour, const int min, const int sec, const fsec_t fsec);
***
*** 1162,1167
--- 1165,1180
PG_RETURN_TIMESTAMPTZ(PgStartTime);
}
+ Datum
+ pgsql_postmaster_reload_time(PG_FUNCTION_ARGS)
+ {
+ TimestampTz timestamp;
+ SpinLockAcquire(&PgReloadTimeLock);
+ timestamp = PgReloadTime;
+ SpinLockRelease(&PgReloadTimeLock);
+ PG_RETURN_TIMESTAMPTZ(timestamp);
+ }
+
/*
* GetCurrentTimestamp -- get the current operating system time
*
*** ./src/include/catalog/pg_proc.h.orig 2008-04-29 23:50:10.355694154 -0400
--- ./src/include/catalog/pg_proc.h 2008-04-29 23:51:12.346237
