Re: [HACKERS] system views for walsender activity

2011-01-13 Thread Magnus Hagander
On Wed, Jan 12, 2011 at 03:03, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Jan 11, 2011 at 7:24 AM, Magnus Hagander mag...@hagander.net wrote:
 No, do this at top

 if (walsnd-state == state)
  return;

 Keep spinlocks when actually setting it.

 I think this is safe...

 Aha. Thanks for the pointers, pfa a new version.

 ...but I think you also need to take the spinlock when reading the value.

Even when it can only ever be set by one process (the owning
walsender), and the variable is atomic (as it should be, since it's a
single enum/int)?

Anyway, it should be as simple as copying it out to a local variable
when it's already in the spinlock and then use that, right?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] system views for walsender activity

2011-01-13 Thread Robert Haas
On Thu, Jan 13, 2011 at 11:08 AM, Magnus Hagander mag...@hagander.net wrote:
 On Wed, Jan 12, 2011 at 03:03, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Jan 11, 2011 at 7:24 AM, Magnus Hagander mag...@hagander.net wrote:
 No, do this at top

 if (walsnd-state == state)
  return;

 Keep spinlocks when actually setting it.

 I think this is safe...

 Aha. Thanks for the pointers, pfa a new version.

 ...but I think you also need to take the spinlock when reading the value.

 Even when it can only ever be set by one process (the owning
 walsender), and the variable is atomic (as it should be, since it's a
 single enum/int)?

The fact that it can only be modified by one process makes it safe for
*that process* to read it without taking the lock, but another process
that wants to read it still needs the lock, I believe - otherwise you
might get a slightly stale value.  That's probably not a *huge* deal
in this case, but I think it'd be better to get it right because
people tend to copy these sorts of things elsewhere, and it'd be bad
if it got copied into some place more critical.

-- 
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] system views for walsender activity

2011-01-13 Thread Magnus Hagander
On Thu, Jan 13, 2011 at 18:43, Robert Haas robertmh...@gmail.com wrote:
 On Thu, Jan 13, 2011 at 11:08 AM, Magnus Hagander mag...@hagander.net wrote:
 On Wed, Jan 12, 2011 at 03:03, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Jan 11, 2011 at 7:24 AM, Magnus Hagander mag...@hagander.net 
 wrote:
 No, do this at top

 if (walsnd-state == state)
  return;

 Keep spinlocks when actually setting it.

 I think this is safe...

 Aha. Thanks for the pointers, pfa a new version.

 ...but I think you also need to take the spinlock when reading the value.

 Even when it can only ever be set by one process (the owning
 walsender), and the variable is atomic (as it should be, since it's a
 single enum/int)?

 The fact that it can only be modified by one process makes it safe for
 *that process* to read it without taking the lock, but another process
 that wants to read it still needs the lock, I believe - otherwise you
 might get a slightly stale value.  That's probably not a *huge* deal
 in this case, but I think it'd be better to get it right because
 people tend to copy these sorts of things elsewhere, and it'd be bad
 if it got copied into some place more critical.

ok, thanks for the pointers - fix applied.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] system views for walsender activity

2011-01-11 Thread Magnus Hagander
On Tue, Jan 11, 2011 at 02:24, Robert Haas robertmh...@gmail.com wrote:
 On Mon, Jan 10, 2011 at 10:41 AM, Simon Riggs si...@2ndquadrant.com wrote:
  I think we need a status enum. ('BACKUP', 'CATCHUP', 'STREAM') for the 3
  phases of replication.
 
  That seems reasonable. But if we keep BACKUP in there, should we
  really have it called pg_stat_replication? (yeah, I know, I'm not
  giving up :P)
 
  (You'd need a 4th mode for WAITING or so, to indicate it's waiting for
  a command)
 
  That's something different.
 
  The 3 phases are more concrete.
 
  BACKUP --  CATCHUP---  STREAM
 
  When you connect you either do BACKUP or CATCHUP. Once in CATCHUP mode
  you never issue a BACKUP. Once we have caught up we move to STREAM. That
  has nothing to do with idle/active.

 So how does a walsender that's waiting for a command from the client
 show up? Surely it's not in catchup mode yet?

 There is a trivial state between connect and first command. If you think
 that is worth publishing, feel free. STARTING?

 I think it's worth publishing.  STARTING would be OK, or maybe STARTUP
 to parallel the other two -UP states.

Here's a patch for this. I chose IDLE, because that's what we call
other backends that are waiting for commands...

Does this seem correct?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/
*** a/doc/src/sgml/monitoring.sgml
--- b/doc/src/sgml/monitoring.sgml
***
*** 298,305  postgres: replaceableuser/ replaceabledatabase/ replaceablehost/ re
entrystructnamepg_stat_replication/indextermprimarypg_stat_replication/primary/indexterm/entry
entryOne row per WAL sender process, showing process acronymID/,
user OID, user name, application name, client's address and port number,
!   time at which the server process began execution, and transaction log
!   location.
   /entry
   /row
  
--- 298,305 
entrystructnamepg_stat_replication/indextermprimarypg_stat_replication/primary/indexterm/entry
entryOne row per WAL sender process, showing process acronymID/,
user OID, user name, application name, client's address and port number,
!   time at which the server process began execution, current WAL sender
!   state and transaction log location.
   /entry
   /row
  
*** a/src/backend/catalog/system_views.sql
--- b/src/backend/catalog/system_views.sql
***
*** 501,506  CREATE VIEW pg_stat_replication AS
--- 501,507 
  S.client_addr,
  S.client_port,
  S.backend_start,
+ W.state,
  W.sent_location
  FROM pg_stat_get_activity(NULL) AS S, pg_authid U,
  pg_stat_get_wal_senders() AS W
*** a/src/backend/replication/basebackup.c
--- b/src/backend/replication/basebackup.c
***
*** 24,29 
--- 24,30 
  #include libpq/pqformat.h
  #include nodes/pg_list.h
  #include replication/basebackup.h
+ #include replication/walsender.h
  #include storage/fd.h
  #include storage/ipc.h
  #include utils/builtins.h
***
*** 83,88  SendBaseBackup(const char *options)
--- 84,91 
  		   ALLOCSET_DEFAULT_MAXSIZE);
  	old_context = MemoryContextSwitchTo(backup_context);
  
+ 	WalSndSetState(WalSndState_BACKUP);
+ 
  	if (backup_label == NULL)
  		ereport(FATAL,
  (errcode(ERRCODE_PROTOCOL_VIOLATION),
*** a/src/backend/replication/walsender.c
--- b/src/backend/replication/walsender.c
***
*** 179,184  WalSndHandshake(void)
--- 179,185 
  	{
  		int			firstchar;
  
+ 		WalSndSetState(WalSndState_IDLE);
  		set_ps_display(idle, false);
  
  		/* Wait for a command to arrive */
***
*** 482,487  WalSndLoop(void)
--- 483,491 
  			if (!XLogSend(output_message, caughtup))
  break;
  		}
+ 
+ 		/* Update our state to indicate if we're behind or not */
+ 		WalSndSetState(caughtup ? WalSndState_STREAMING : WalSndState_CATCHUP);
  	}
  
  	/*
***
*** 533,538  InitWalSnd(void)
--- 537,543 
  			 */
  			walsnd-pid = MyProcPid;
  			MemSet(walsnd-sentPtr, 0, sizeof(XLogRecPtr));
+ 			walsnd-state = WalSndState_IDLE;
  			SpinLockRelease(walsnd-mutex);
  			/* don't need the lock anymore */
  			OwnLatch((Latch *) walsnd-latch);
***
*** 960,965  WalSndWakeup(void)
--- 965,999 
  		SetLatch(WalSndCtl-walsnds[i].latch);
  }
  
+ /* Set state for current walsender (only called in walsender) */
+ void WalSndSetState(WalSndState state)
+ {
+ 	/* use volatile pointer to prevent code rearrangement */
+ 	volatile WalSnd *walsnd = MyWalSnd;
+ 
+ 	SpinLockAcquire(walsnd-mutex);
+ 	walsnd-state = state;
+ 	SpinLockRelease(walsnd-mutex);
+ }
+ 
+ /*
+  * Return a string constant representing the state. This is used
+  * in system views, and should *not* be translated.
+  */
+ static const char *
+ WalSndGetStateString(WalSndState state)
+ {
+ 	switch (state)
+ 	{
+ 		case WalSndState_IDLE: return IDLE;
+ 	

Re: [HACKERS] system views for walsender activity

2011-01-11 Thread Robert Haas
On Tue, Jan 11, 2011 at 5:28 AM, Magnus Hagander mag...@hagander.net wrote:
 Does this seem correct?

It looks reasonable, except that I the way you've chosen to capitalize
the wal sender states makes me want to shoot myself.

-- 
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] system views for walsender activity

2011-01-11 Thread Simon Riggs
On Tue, 2011-01-11 at 11:28 +0100, Magnus Hagander wrote:
  
   (You'd need a 4th mode for WAITING or so, to indicate it's waiting for
   a command)
  
   That's something different.
  
   The 3 phases are more concrete.
  
   BACKUP --  CATCHUP---  STREAM
  
   When you connect you either do BACKUP or CATCHUP. Once in CATCHUP mode
   you never issue a BACKUP. Once we have caught up we move to STREAM. That
   has nothing to do with idle/active.
 
  So how does a walsender that's waiting for a command from the client
  show up? Surely it's not in catchup mode yet?
 
  There is a trivial state between connect and first command. If you think
  that is worth publishing, feel free. STARTING?
 
  I think it's worth publishing.  STARTING would be OK, or maybe STARTUP
  to parallel the other two -UP states.
 
 Here's a patch for this. I chose IDLE, because that's what we call
 other backends that are waiting for commands...
 
 Does this seem correct?

No

You can be idle yet in STREAMING mode. What mode we are in has nothing
to do with idle/active. Either STARTING/STARTUP/NULL but not IDLE.

If you want that as well, then we need a second column, but personally
it's too much information and its too hard to say what it actually
means. For example, with sync rep, the WALSender might be idle, yet
there might yet be backends waiting for a reply.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and 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] system views for walsender activity

2011-01-11 Thread Magnus Hagander
On Tue, Jan 11, 2011 at 12:17, Robert Haas robertmh...@gmail.com wrote:
 On Tue, Jan 11, 2011 at 5:28 AM, Magnus Hagander mag...@hagander.net wrote:
 Does this seem correct?

 It looks reasonable, except that I the way you've chosen to capitalize
 the wal sender states makes me want to shoot myself.

Hah, I was waiting for that. I can certainly change that :-)


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] system views for walsender activity

2011-01-11 Thread Magnus Hagander
On Tue, Jan 11, 2011 at 12:23, Simon Riggs si...@2ndquadrant.com wrote:
 On Tue, 2011-01-11 at 11:28 +0100, Magnus Hagander wrote:
  
   (You'd need a 4th mode for WAITING or so, to indicate it's waiting for
   a command)
  
   That's something different.
  
   The 3 phases are more concrete.
  
   BACKUP --  CATCHUP---  STREAM
  
   When you connect you either do BACKUP or CATCHUP. Once in CATCHUP mode
   you never issue a BACKUP. Once we have caught up we move to STREAM. 
   That
   has nothing to do with idle/active.
 
  So how does a walsender that's waiting for a command from the client
  show up? Surely it's not in catchup mode yet?
 
  There is a trivial state between connect and first command. If you think
  that is worth publishing, feel free. STARTING?
 
  I think it's worth publishing.  STARTING would be OK, or maybe STARTUP
  to parallel the other two -UP states.

 Here's a patch for this. I chose IDLE, because that's what we call
 other backends that are waiting for commands...

 Does this seem correct?

 No

 You can be idle yet in STREAMING mode. What mode we are in has nothing
 to do with idle/active. Either STARTING/STARTUP/NULL but not IDLE.

 If you want that as well, then we need a second column, but personally
 it's too much information and its too hard to say what it actually
 means. For example, with sync rep, the WALSender might be idle, yet
 there might yet be backends waiting for a reply.

That's a good point.

Just to be clear, you're objecting to the *name* of the state, right,
not how/where it's set? In particular, how the catchup/streaming
things are set?

I agree that using a second column for it is unnecessary.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] system views for walsender activity

2011-01-11 Thread Simon Riggs
On Tue, 2011-01-11 at 12:41 +0100, Magnus Hagander wrote:

 Just to be clear, you're objecting to the *name* of the state, right,
 not how/where it's set? 

Yes

 In particular, how the catchup/streaming
 things are set?

You've set it in the right places.

I would personally constrain the state transitions, so that we can't
ever make illegal changes, such as CATCHUP - BACKUP.

I would also check the state locally before grabbing the spinlock, as is
typical in other xlog code. Continually resetting shared state to
exactly what it is already seems strange, to me. If we make the rule
that the state can only be changed by the WALSender itself, it won't
need to grab spinlock. If you don't like that, a local variable works.

Also, mixing upper camel case and uppercase for the STATe NamES looKS
weIRD. Uppercase makes them look more clearly like enum/states as used
elsewhere in similar code.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and 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] system views for walsender activity

2011-01-11 Thread Magnus Hagander
On Tue, Jan 11, 2011 at 12:58, Simon Riggs si...@2ndquadrant.com wrote:
 On Tue, 2011-01-11 at 12:41 +0100, Magnus Hagander wrote:

 Just to be clear, you're objecting to the *name* of the state, right,
 not how/where it's set?

 Yes

 In particular, how the catchup/streaming
 things are set?

 You've set it in the right places.

 I would personally constrain the state transitions, so that we can't
 ever make illegal changes, such as CATCHUP - BACKUP.

Well, once we enter the walsender loop we can never get out of it, so
it simply cannot happen...


 I would also check the state locally before grabbing the spinlock, as is
 typical in other xlog code. Continually resetting shared state to
 exactly what it is already seems strange, to me. If we make the rule
 that the state can only be changed by the WALSender itself, it won't
 need to grab spinlock. If you don't like that, a local variable works.

Hmm. I don't see why anybody other than the walsender should change
it, so yeah. You mean I can just drop the spinlock calls completely,
and then do an
if (walsnd-state != state)
   walsnd-state = state;


? Do I need to keep the volatile pointer, or should I drop that as well?



 Also, mixing upper camel case and uppercase for the STATe NamES looKS
 weIRD. Uppercase makes them look more clearly like enum/states as used
 elsewhere in similar code.

Yeah, I'll change that.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] system views for walsender activity

2011-01-11 Thread Simon Riggs
On Tue, 2011-01-11 at 13:04 +0100, Magnus Hagander wrote:
 On Tue, Jan 11, 2011 at 12:58, Simon Riggs si...@2ndquadrant.com wrote:
  On Tue, 2011-01-11 at 12:41 +0100, Magnus Hagander wrote:
 
  Just to be clear, you're objecting to the *name* of the state, right,
  not how/where it's set?
 
  Yes
 
  In particular, how the catchup/streaming
  things are set?
 
  You've set it in the right places.
 
  I would personally constrain the state transitions, so that we can't
  ever make illegal changes, such as CATCHUP - BACKUP.
 
 Well, once we enter the walsender loop we can never get out of it, so
 it simply cannot happen...

Accidentally it can, so I would like to protect against that.

Putting those checks in are like Asserts(), they help document what is
and is not possible by design.

  I would also check the state locally before grabbing the spinlock, as is
  typical in other xlog code. Continually resetting shared state to
  exactly what it is already seems strange, to me. If we make the rule
  that the state can only be changed by the WALSender itself, it won't
  need to grab spinlock. If you don't like that, a local variable works.
 
 Hmm. I don't see why anybody other than the walsender should change
 it, so yeah. You mean I can just drop the spinlock calls completely,
 and then do an
 if (walsnd-state != state)
walsnd-state = state;
 
 
 ? Do I need to keep the volatile pointer, or should I drop that as well?

No, do this at top

if (walsnd-state == state)
  return;

Keep spinlocks when actually setting it.

  Also, mixing upper camel case and uppercase for the STATe NamES looKS
  weIRD. Uppercase makes them look more clearly like enum/states as used
  elsewhere in similar code.
 
 Yeah, I'll change that.
 

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and 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] system views for walsender activity

2011-01-11 Thread Magnus Hagander
On Tue, Jan 11, 2011 at 13:18, Simon Riggs si...@2ndquadrant.com wrote:
 On Tue, 2011-01-11 at 13:04 +0100, Magnus Hagander wrote:
 On Tue, Jan 11, 2011 at 12:58, Simon Riggs si...@2ndquadrant.com wrote:
  On Tue, 2011-01-11 at 12:41 +0100, Magnus Hagander wrote:
 
  Just to be clear, you're objecting to the *name* of the state, right,
  not how/where it's set?
 
  Yes
 
  In particular, how the catchup/streaming
  things are set?
 
  You've set it in the right places.
 
  I would personally constrain the state transitions, so that we can't
  ever make illegal changes, such as CATCHUP - BACKUP.

 Well, once we enter the walsender loop we can never get out of it, so
 it simply cannot happen...

 Accidentally it can, so I would like to protect against that.

 Putting those checks in are like Asserts(), they help document what is
 and is not possible by design.

  I would also check the state locally before grabbing the spinlock, as is
  typical in other xlog code. Continually resetting shared state to
  exactly what it is already seems strange, to me. If we make the rule
  that the state can only be changed by the WALSender itself, it won't
  need to grab spinlock. If you don't like that, a local variable works.

 Hmm. I don't see why anybody other than the walsender should change
 it, so yeah. You mean I can just drop the spinlock calls completely,
 and then do an
 if (walsnd-state != state)
    walsnd-state = state;


 ? Do I need to keep the volatile pointer, or should I drop that as well?

 No, do this at top

 if (walsnd-state == state)
  return;

 Keep spinlocks when actually setting it.


Aha. Thanks for the pointers, pfa a new version.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/
*** a/doc/src/sgml/monitoring.sgml
--- b/doc/src/sgml/monitoring.sgml
***
*** 298,305  postgres: replaceableuser/ replaceabledatabase/ replaceablehost/ re
entrystructnamepg_stat_replication/indextermprimarypg_stat_replication/primary/indexterm/entry
entryOne row per WAL sender process, showing process acronymID/,
user OID, user name, application name, client's address and port number,
!   time at which the server process began execution, and transaction log
!   location.
   /entry
   /row
  
--- 298,305 
entrystructnamepg_stat_replication/indextermprimarypg_stat_replication/primary/indexterm/entry
entryOne row per WAL sender process, showing process acronymID/,
user OID, user name, application name, client's address and port number,
!   time at which the server process began execution, current WAL sender
!   state and transaction log location.
   /entry
   /row
  
*** a/src/backend/catalog/system_views.sql
--- b/src/backend/catalog/system_views.sql
***
*** 501,506  CREATE VIEW pg_stat_replication AS
--- 501,507 
  S.client_addr,
  S.client_port,
  S.backend_start,
+ W.state,
  W.sent_location
  FROM pg_stat_get_activity(NULL) AS S, pg_authid U,
  pg_stat_get_wal_senders() AS W
*** a/src/backend/replication/basebackup.c
--- b/src/backend/replication/basebackup.c
***
*** 24,29 
--- 24,30 
  #include libpq/pqformat.h
  #include nodes/pg_list.h
  #include replication/basebackup.h
+ #include replication/walsender.h
  #include storage/fd.h
  #include storage/ipc.h
  #include utils/builtins.h
***
*** 83,88  SendBaseBackup(const char *options)
--- 84,91 
  		   ALLOCSET_DEFAULT_MAXSIZE);
  	old_context = MemoryContextSwitchTo(backup_context);
  
+ 	WalSndSetState(WALSNDSTATE_BACKUP);
+ 
  	if (backup_label == NULL)
  		ereport(FATAL,
  (errcode(ERRCODE_PROTOCOL_VIOLATION),
*** a/src/backend/replication/walsender.c
--- b/src/backend/replication/walsender.c
***
*** 179,184  WalSndHandshake(void)
--- 179,185 
  	{
  		int			firstchar;
  
+ 		WalSndSetState(WALSNDSTATE_IDLE);
  		set_ps_display(idle, false);
  
  		/* Wait for a command to arrive */
***
*** 482,487  WalSndLoop(void)
--- 483,491 
  			if (!XLogSend(output_message, caughtup))
  break;
  		}
+ 
+ 		/* Update our state to indicate if we're behind or not */
+ 		WalSndSetState(caughtup ? WALSNDSTATE_STREAMING : WALSNDSTATE_CATCHUP);
  	}
  
  	/*
***
*** 533,538  InitWalSnd(void)
--- 537,543 
  			 */
  			walsnd-pid = MyProcPid;
  			MemSet(walsnd-sentPtr, 0, sizeof(XLogRecPtr));
+ 			walsnd-state = WALSNDSTATE_IDLE;
  			SpinLockRelease(walsnd-mutex);
  			/* don't need the lock anymore */
  			OwnLatch((Latch *) walsnd-latch);
***
*** 960,965  WalSndWakeup(void)
--- 965,1004 
  		SetLatch(WalSndCtl-walsnds[i].latch);
  }
  
+ /* Set state for current walsender (only called in walsender) */
+ void WalSndSetState(WalSndState state)
+ {
+ 	/* use volatile pointer to prevent code rearrangement */
+ 	

Re: [HACKERS] system views for walsender activity

2011-01-11 Thread Robert Haas
On Tue, Jan 11, 2011 at 7:24 AM, Magnus Hagander mag...@hagander.net wrote:
 No, do this at top

 if (walsnd-state == state)
  return;

 Keep spinlocks when actually setting it.

I think this is safe...

 Aha. Thanks for the pointers, pfa a new version.

...but I think you also need to take the spinlock when reading the value.

-- 
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] system views for walsender activity

2011-01-11 Thread Shigeru HANADA
On Tue, 11 Jan 2011 13:24:33 +0100
Magnus Hagander mag...@hagander.net wrote:
 Aha. Thanks for the pointers, pfa a new version.

Changing pg_stat replication view would require to fix regression test
rule. Please find attached patch.

Regards,
--
Shigeru Hanada


rule_test.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] system views for walsender activity

2011-01-11 Thread Andrew Dunstan



On 01/11/2011 10:24 PM, Shigeru HANADA wrote:

On Tue, 11 Jan 2011 13:24:33 +0100
Magnus Hagandermag...@hagander.net  wrote:

Aha. Thanks for the pointers, pfa a new version.


Changing pg_stat replication view would require to fix regression test
rule. Please find attached patch.




I have just committed a fix for this.

cheers

andrew

--
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] system views for walsender activity

2011-01-10 Thread Magnus Hagander
On Sun, Jan 9, 2011 at 15:53, Simon Riggs si...@2ndquadrant.com wrote:
 On Sun, 2011-01-09 at 12:52 +0100, Magnus Hagander wrote:

 One thing I noticed is that it gives an interesting property to my
 patch for streaming base backups - they now show up in
 pg_stat_replication, with a streaming location of 0/0.

 If the view is named pg_stat_replication, we probably want to filter
 that out somehow. But then, do we want a separate view listing the
 walsenders that are busy sending base backups?

 For that matter, do we want an indication that separates a walsender
 not sending data from one sending that happens to be at location 0/0?
 Most will leave 0/0 really quickly, but a walsender can be idle (not
 received a command yet), or it can be running IDENTIFY_SYSTEM for
 example.

 I think we need a status enum. ('BACKUP', 'CATCHUP', 'STREAM') for the 3
 phases of replication.

That seems reasonable. But if we keep BACKUP in there, should we
really have it called pg_stat_replication? (yeah, I know, I'm not
giving up :P)

(You'd need a 4th mode for WAITING or so, to indicate it's waiting for
a command)


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] system views for walsender activity

2011-01-10 Thread Simon Riggs
On Mon, 2011-01-10 at 15:20 +0100, Magnus Hagander wrote:
 On Sun, Jan 9, 2011 at 15:53, Simon Riggs si...@2ndquadrant.com wrote:
  On Sun, 2011-01-09 at 12:52 +0100, Magnus Hagander wrote:
 
  One thing I noticed is that it gives an interesting property to my
  patch for streaming base backups - they now show up in
  pg_stat_replication, with a streaming location of 0/0.
 
  If the view is named pg_stat_replication, we probably want to filter
  that out somehow. But then, do we want a separate view listing the
  walsenders that are busy sending base backups?
 
  For that matter, do we want an indication that separates a walsender
  not sending data from one sending that happens to be at location 0/0?
  Most will leave 0/0 really quickly, but a walsender can be idle (not
  received a command yet), or it can be running IDENTIFY_SYSTEM for
  example.
 
  I think we need a status enum. ('BACKUP', 'CATCHUP', 'STREAM') for the 3
  phases of replication.
 
 That seems reasonable. But if we keep BACKUP in there, should we
 really have it called pg_stat_replication? (yeah, I know, I'm not
 giving up :P)
 
 (You'd need a 4th mode for WAITING or so, to indicate it's waiting for
 a command)

That's something different.

The 3 phases are more concrete.

BACKUP -- CATCHUP --- STREAM

When you connect you either do BACKUP or CATCHUP. Once in CATCHUP mode
you never issue a BACKUP. Once we have caught up we move to STREAM. That
has nothing to do with idle/active.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and 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] system views for walsender activity

2011-01-10 Thread Heikki Linnakangas

On 10.01.2011 16:49, Simon Riggs wrote:

On Mon, 2011-01-10 at 15:20 +0100, Magnus Hagander wrote:

On Sun, Jan 9, 2011 at 15:53, Simon Riggssi...@2ndquadrant.com  wrote:

On Sun, 2011-01-09 at 12:52 +0100, Magnus Hagander wrote:


One thing I noticed is that it gives an interesting property to my
patch for streaming base backups - they now show up in
pg_stat_replication, with a streaming location of 0/0.

If the view is named pg_stat_replication, we probably want to filter
that out somehow. But then, do we want a separate view listing the
walsenders that are busy sending base backups?

For that matter, do we want an indication that separates a walsender
not sending data from one sending that happens to be at location 0/0?
Most will leave 0/0 really quickly, but a walsender can be idle (not
received a command yet), or it can be running IDENTIFY_SYSTEM for
example.


I think we need a status enum. ('BACKUP', 'CATCHUP', 'STREAM') for the 3
phases of replication.


That seems reasonable. But if we keep BACKUP in there, should we
really have it called pg_stat_replication? (yeah, I know, I'm not
giving up :P)

(You'd need a 4th mode for WAITING or so, to indicate it's waiting for
a command)


That's something different.

The 3 phases are more concrete.

BACKUP --  CATCHUP---  STREAM

When you connect you either do BACKUP or CATCHUP. Once in CATCHUP mode
you never issue a BACKUP. Once we have caught up we move to STREAM. That
has nothing to do with idle/active.


So how does a walsender that's waiting for a command from the client 
show up? Surely it's not in catchup mode yet?


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

--
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] system views for walsender activity

2011-01-10 Thread Simon Riggs
On Mon, 2011-01-10 at 17:05 +0200, Heikki Linnakangas wrote:
 On 10.01.2011 16:49, Simon Riggs wrote:
  On Mon, 2011-01-10 at 15:20 +0100, Magnus Hagander wrote:
  On Sun, Jan 9, 2011 at 15:53, Simon Riggssi...@2ndquadrant.com  wrote:
  On Sun, 2011-01-09 at 12:52 +0100, Magnus Hagander wrote:
 
  One thing I noticed is that it gives an interesting property to my
  patch for streaming base backups - they now show up in
  pg_stat_replication, with a streaming location of 0/0.
 
  If the view is named pg_stat_replication, we probably want to filter
  that out somehow. But then, do we want a separate view listing the
  walsenders that are busy sending base backups?
 
  For that matter, do we want an indication that separates a walsender
  not sending data from one sending that happens to be at location 0/0?
  Most will leave 0/0 really quickly, but a walsender can be idle (not
  received a command yet), or it can be running IDENTIFY_SYSTEM for
  example.
 
  I think we need a status enum. ('BACKUP', 'CATCHUP', 'STREAM') for the 3
  phases of replication.
 
  That seems reasonable. But if we keep BACKUP in there, should we
  really have it called pg_stat_replication? (yeah, I know, I'm not
  giving up :P)
 
  (You'd need a 4th mode for WAITING or so, to indicate it's waiting for
  a command)
 
  That's something different.
 
  The 3 phases are more concrete.
 
  BACKUP --  CATCHUP---  STREAM
 
  When you connect you either do BACKUP or CATCHUP. Once in CATCHUP mode
  you never issue a BACKUP. Once we have caught up we move to STREAM. That
  has nothing to do with idle/active.
 
 So how does a walsender that's waiting for a command from the client 
 show up? Surely it's not in catchup mode yet?

There is a trivial state between connect and first command. If you think
that is worth publishing, feel free. STARTING?

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and 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] system views for walsender activity

2011-01-10 Thread Euler Taveira de Oliveira

Em 10-01-2011 12:05, Heikki Linnakangas escreveu:

So how does a walsender that's waiting for a command from the client
show up? Surely it's not in catchup mode yet?

It is kind of initializing catchup. I think it is not worth representing 
those short lifespan states (in normal scenarios).



--
  Euler Taveira de Oliveira
  http://www.timbira.com/

--
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] system views for walsender activity

2011-01-10 Thread Magnus Hagander
On Mon, Jan 10, 2011 at 16:41, Simon Riggs si...@2ndquadrant.com wrote:
 On Mon, 2011-01-10 at 17:05 +0200, Heikki Linnakangas wrote:
 On 10.01.2011 16:49, Simon Riggs wrote:
  On Mon, 2011-01-10 at 15:20 +0100, Magnus Hagander wrote:
  On Sun, Jan 9, 2011 at 15:53, Simon Riggssi...@2ndquadrant.com  wrote:
  On Sun, 2011-01-09 at 12:52 +0100, Magnus Hagander wrote:
 
  One thing I noticed is that it gives an interesting property to my
  patch for streaming base backups - they now show up in
  pg_stat_replication, with a streaming location of 0/0.
 
  If the view is named pg_stat_replication, we probably want to filter
  that out somehow. But then, do we want a separate view listing the
  walsenders that are busy sending base backups?
 
  For that matter, do we want an indication that separates a walsender
  not sending data from one sending that happens to be at location 0/0?
  Most will leave 0/0 really quickly, but a walsender can be idle (not
  received a command yet), or it can be running IDENTIFY_SYSTEM for
  example.
 
  I think we need a status enum. ('BACKUP', 'CATCHUP', 'STREAM') for the 3
  phases of replication.
 
  That seems reasonable. But if we keep BACKUP in there, should we
  really have it called pg_stat_replication? (yeah, I know, I'm not
  giving up :P)
 
  (You'd need a 4th mode for WAITING or so, to indicate it's waiting for
  a command)
 
  That's something different.
 
  The 3 phases are more concrete.
 
  BACKUP --  CATCHUP---  STREAM
 
  When you connect you either do BACKUP or CATCHUP. Once in CATCHUP mode
  you never issue a BACKUP. Once we have caught up we move to STREAM. That
  has nothing to do with idle/active.

 So how does a walsender that's waiting for a command from the client
 show up? Surely it's not in catchup mode yet?

 There is a trivial state between connect and first command. If you think
 that is worth publishing, feel free. STARTING?

If we don't publish it, it'll implicitly be in one of the others..
Unless we say NULL, of course, but I definitely prefer STARTING then.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] system views for walsender activity

2011-01-10 Thread Magnus Hagander
On Mon, Jan 10, 2011 at 16:48, Euler Taveira de Oliveira
eu...@timbira.com wrote:
 Em 10-01-2011 12:05, Heikki Linnakangas escreveu:

 So how does a walsender that's waiting for a command from the client
 show up? Surely it's not in catchup mode yet?

 It is kind of initializing catchup. I think it is not worth representing
 those short lifespan states (in normal scenarios).

True, but it's quite important to detect and diagnose the abnormal ones...


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] system views for walsender activity

2011-01-10 Thread Robert Haas
On Mon, Jan 10, 2011 at 10:41 AM, Simon Riggs si...@2ndquadrant.com wrote:
  I think we need a status enum. ('BACKUP', 'CATCHUP', 'STREAM') for the 3
  phases of replication.
 
  That seems reasonable. But if we keep BACKUP in there, should we
  really have it called pg_stat_replication? (yeah, I know, I'm not
  giving up :P)
 
  (You'd need a 4th mode for WAITING or so, to indicate it's waiting for
  a command)
 
  That's something different.
 
  The 3 phases are more concrete.
 
  BACKUP --  CATCHUP---  STREAM
 
  When you connect you either do BACKUP or CATCHUP. Once in CATCHUP mode
  you never issue a BACKUP. Once we have caught up we move to STREAM. That
  has nothing to do with idle/active.

 So how does a walsender that's waiting for a command from the client
 show up? Surely it's not in catchup mode yet?

 There is a trivial state between connect and first command. If you think
 that is worth publishing, feel free. STARTING?

I think it's worth publishing.  STARTING would be OK, or maybe STARTUP
to parallel the other two -UP states.

-- 
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] system views for walsender activity

2011-01-09 Thread Magnus Hagander
On Fri, Jan 7, 2011 at 22:21, Robert Haas robertmh...@gmail.com wrote:
 On Fri, Jan 7, 2011 at 1:46 PM, Josh Berkus j...@agliodbs.com wrote:

 To my way of thinking, pg_stat_walsender and pg_stat_walreceiver would
 be more clear than pg_stat_replication_master and
 pg_stat_replication_slave.

 Let's commit it so that some of us can get a look at the data it
 contains, and then we can fix the name during beta.

 Well, the first half is committed, under the name pg_stat_replication.
  So go look at that, for starters...

One thing I noticed is that it gives an interesting property to my
patch for streaming base backups - they now show up in
pg_stat_replication, with a streaming location of 0/0.

If the view is named pg_stat_replication, we probably want to filter
that out somehow. But then, do we want a separate view listing the
walsenders that are busy sending base backups?

For that matter, do we want an indication that separates a walsender
not sending data from one sending that happens to be at location 0/0?
Most will leave 0/0 really quickly, but a walsender can be idle (not
received a command yet), or it can be running IDENTIFY_SYSTEM for
example.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] system views for walsender activity

2011-01-09 Thread Simon Riggs
On Sun, 2011-01-09 at 12:52 +0100, Magnus Hagander wrote:

 One thing I noticed is that it gives an interesting property to my
 patch for streaming base backups - they now show up in
 pg_stat_replication, with a streaming location of 0/0.
 
 If the view is named pg_stat_replication, we probably want to filter
 that out somehow. But then, do we want a separate view listing the
 walsenders that are busy sending base backups?
 
 For that matter, do we want an indication that separates a walsender
 not sending data from one sending that happens to be at location 0/0?
 Most will leave 0/0 really quickly, but a walsender can be idle (not
 received a command yet), or it can be running IDENTIFY_SYSTEM for
 example.

I think we need a status enum. ('BACKUP', 'CATCHUP', 'STREAM') for the 3
phases of replication.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and 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] system views for walsender activity

2011-01-07 Thread Simon Riggs
On Fri, 2011-01-07 at 12:13 +0900, Itagaki Takahiro wrote:

 pg_stat_replication seems to be the most understandable name.
 
  I would very much appreciate it if one of you could complete something
  here and commit in the next few days. That would then allow me to extend
  the view with sync rep specific info for monitoring and patch testing.

Please go with whatever you think best for now. I'm sure people will ask
for different names later anyway. Let's get this committed soon, to
reduce later patch conflicts. Thanks.

 What will we name xlog locations that have been received? We call
 xlog locations sent to standby as sentPtr. If we have sync rep,
 we will have two locations for each wal sender. For example,
 we can call them sent_location and sync_location.

Please add sent_location, I will add others.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and 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] system views for walsender activity

2011-01-07 Thread Itagaki Takahiro
On Fri, Jan 7, 2011 at 19:20, Simon Riggs si...@2ndquadrant.com wrote:
 pg_stat_replication seems to be the most understandable name.

 Please go with whatever you think best for now. I'm sure people will ask
 for different names later anyway. Let's get this committed soon, to
 reduce later patch conflicts. Thanks.

 Please add sent_location, I will add others.

OK, I added a view named s pg_stat_replication. The view is basically
based on Simon's patch, but I just skipped unused WalSnd entreis in
WalSndCtl rather than return NULLs. The applied patch attached.

I expect we will have two views for master and standby servers:

  * pg_stat_replication
   Activity of wal senders in master server.
  * pg_stat_standby (not yet)
   Activity of a wal receiver and a recovery process in standby servers.

I didn't use pg_stat_wal_sender/receiver as their names because standby
activity in slaves could contain not only a wal receiver but also a
recovery process.

-- 
Itagaki Takahiro


pg_stat_replication-20110107.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] system views for walsender activity

2011-01-07 Thread Magnus Hagander
On Fri, Jan 7, 2011 at 12:42, Itagaki Takahiro
itagaki.takah...@gmail.com wrote:
 On Fri, Jan 7, 2011 at 19:20, Simon Riggs si...@2ndquadrant.com wrote:
 pg_stat_replication seems to be the most understandable name.

 Please go with whatever you think best for now. I'm sure people will ask
 for different names later anyway. Let's get this committed soon, to
 reduce later patch conflicts. Thanks.

 Please add sent_location, I will add others.

 OK, I added a view named s pg_stat_replication. The view is basically
 based on Simon's patch, but I just skipped unused WalSnd entreis in
 WalSndCtl rather than return NULLs. The applied patch attached.

 I expect we will have two views for master and standby servers:

  * pg_stat_replication
       Activity of wal senders in master server.
  * pg_stat_standby (not yet)
       Activity of a wal receiver and a recovery process in standby servers.

Just to keep the bikeshedding up, should it in this case not be
pg_stat_replication_master and pg_stat_replication_standby or such?
Replication applies to both master and slave...


 I didn't use pg_stat_wal_sender/receiver as their names because standby
 activity in slaves could contain not only a wal receiver but also a
 recovery process.

Good point.


-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] system views for walsender activity

2011-01-07 Thread Itagaki Takahiro
On Fri, Jan 7, 2011 at 21:48, Magnus Hagander mag...@hagander.net wrote:
  * pg_stat_replication
  * pg_stat_standby (not yet)

 Just to keep the bikeshedding up, should it in this case not be
 pg_stat_replication_master and pg_stat_replication_standby or such?
 Replication applies to both master and slave...

The reason I didn't use term master is that pg_stat_replication is
information of *standby* servers on master server. Of course,
wal senders are processes in the master, but users probably think
they are the location standby servers receives.

I forgot to update SGML for the view. I'll do it soon.
Thanks for the heads-up.

-- 
Itagaki Takahiro

-- 
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] system views for walsender activity

2011-01-07 Thread Robert Haas
On Fri, Jan 7, 2011 at 8:09 AM, Itagaki Takahiro
itagaki.takah...@gmail.com wrote:
 On Fri, Jan 7, 2011 at 21:48, Magnus Hagander mag...@hagander.net wrote:
  * pg_stat_replication
  * pg_stat_standby (not yet)

 Just to keep the bikeshedding up, should it in this case not be
 pg_stat_replication_master and pg_stat_replication_standby or such?
 Replication applies to both master and slave...

 The reason I didn't use term master is that pg_stat_replication is
 information of *standby* servers on master server. Of course,
 wal senders are processes in the master, but users probably think
 they are the location standby servers receives.

To my way of thinking, pg_stat_walsender and pg_stat_walreceiver would
be more clear than pg_stat_replication_master and
pg_stat_replication_slave.

However, my way of thinking is of course not the only way of thinking.

-- 
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] system views for walsender activity

2011-01-07 Thread Josh Berkus

 To my way of thinking, pg_stat_walsender and pg_stat_walreceiver would
 be more clear than pg_stat_replication_master and
 pg_stat_replication_slave.

Let's commit it so that some of us can get a look at the data it
contains, and then we can fix the name during beta.

-- 
  -- Josh Berkus
 PostgreSQL Experts Inc.
 http://www.pgexperts.com

-- 
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] system views for walsender activity

2011-01-07 Thread Magnus Hagander
On Fri, Jan 7, 2011 at 19:46, Josh Berkus j...@agliodbs.com wrote:

 To my way of thinking, pg_stat_walsender and pg_stat_walreceiver would
 be more clear than pg_stat_replication_master and
 pg_stat_replication_slave.

 Let's commit it so that some of us can get a look at the data it
 contains, and then we can fix the name during beta.

We try to avoid inidb-requiring changes (like renaming a system
object...) during beta.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] system views for walsender activity

2011-01-07 Thread Simon Riggs
On Fri, 2011-01-07 at 19:48 +0100, Magnus Hagander wrote:
 On Fri, Jan 7, 2011 at 19:46, Josh Berkus j...@agliodbs.com wrote:
 
  To my way of thinking, pg_stat_walsender and pg_stat_walreceiver would
  be more clear than pg_stat_replication_master and
  pg_stat_replication_slave.
 
  Let's commit it so that some of us can get a look at the data it
  contains, and then we can fix the name during beta.
 
 We try to avoid inidb-requiring changes (like renaming a system
 object...) during beta.

Why?

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and 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] system views for walsender activity

2011-01-07 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes:
 On Fri, 2011-01-07 at 19:48 +0100, Magnus Hagander wrote:
 We try to avoid inidb-requiring changes (like renaming a system
 object...) during beta.

 Why?

So that beta testers won't be forced to do a dump and reload.

When and if pg_upgrade is actually 100% trustworthy, maybe the desire
to avoid initdb's after beta1 will vanish completely; but for now it's
still a good thing to avoid.

regards, tom lane

-- 
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] system views for walsender activity

2011-01-07 Thread Magnus Hagander
On Fri, Jan 7, 2011 at 20:51, Tom Lane t...@sss.pgh.pa.us wrote:
 Simon Riggs si...@2ndquadrant.com writes:
 On Fri, 2011-01-07 at 19:48 +0100, Magnus Hagander wrote:
 We try to avoid inidb-requiring changes (like renaming a system
 object...) during beta.

 Why?

 So that beta testers won't be forced to do a dump and reload.

 When and if pg_upgrade is actually 100% trustworthy, maybe the desire
 to avoid initdb's after beta1 will vanish completely; but for now it's
 still a good thing to avoid.


I think it's still a good thing to avoid. It's at that point no longer
as necessary, but it's still best if we can avoid to make those
changes after beta when possible.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] system views for walsender activity

2011-01-07 Thread Simon Riggs
On Fri, 2011-01-07 at 14:51 -0500, Tom Lane wrote:
 Simon Riggs si...@2ndquadrant.com writes:
  On Fri, 2011-01-07 at 19:48 +0100, Magnus Hagander wrote:
  We try to avoid inidb-requiring changes (like renaming a system
  object...) during beta.
 
  Why?
 
 So that beta testers won't be forced to do a dump and reload.
 
 When and if pg_upgrade is actually 100% trustworthy, maybe the desire
 to avoid initdb's after beta1 will vanish completely; but for now it's
 still a good thing to avoid.

Then before we go beta, we should confirm the names.

IIRC that is not until at least 15 Feb, perhaps 15 Mar.

ISTM there will be much discussion on a range of things, just as there
was last year and no doubt will be every year.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and 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] system views for walsender activity

2011-01-07 Thread Robert Haas
On Fri, Jan 7, 2011 at 1:46 PM, Josh Berkus j...@agliodbs.com wrote:

 To my way of thinking, pg_stat_walsender and pg_stat_walreceiver would
 be more clear than pg_stat_replication_master and
 pg_stat_replication_slave.

 Let's commit it so that some of us can get a look at the data it
 contains, and then we can fix the name during beta.

Well, the first half is committed, under the name pg_stat_replication.
 So go look at that, for starters...

-- 
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] system views for walsender activity

2011-01-06 Thread Itagaki Takahiro
On Wed, Jan 5, 2011 at 02:48, Simon Riggs si...@2ndquadrant.com wrote:
 The way I coded it was a new SRF that joins to the existing
 pg_stat_activity. So no initdb required, and this can also easily be
 included as an external module for 9.0.

 Please notice also that my coding of the new SRF does not have the O^2
 issue you mention, which I was keen to avoid.

Yeah, using SQL JOIN to avoid O(n^2) is a good idea. My only concern is
that pg_stat_get_activity(NULL) might return rows that are not actually
used. Is it an ignorable overhead?

 We should
 also include application name, since the user may set that in the
 standby for all the same reasons it is set elsewhere.

Ah, we can use application_name to name each wal senders.

 Small point: please lets not call this pg_stat_walsender?
 pg_stat_replication_sent and pg_stat_replication_received would be
 easier for normal humans to understand.

A list of proposed view names for replication master server:
 - pg_stat_replication
 - pg_stat_replication_sent
 - pg_stat_standby
 - pg_stat_walsender
 - pg_stat_walsender_activity

We have some functions for standby server activity
(pg_last_xlog_[receive|replay]_[location|timestamp])
but could have a view for them:
 - pg_stat_replication_received
 - pg_stat_walreceiver

pg_stat_replication and pg_stat_standby might not be good names
when we have a view for standby server because the names are not
clear for master server. But if we will have a view only on master,
pg_stat_replication seems to be the most understandable name.

 I would very much appreciate it if one of you could complete something
 here and commit in the next few days. That would then allow me to extend
 the view with sync rep specific info for monitoring and patch testing.

What will we name xlog locations that have been received? We call
xlog locations sent to standby as sentPtr. If we have sync rep,
we will have two locations for each wal sender. For example,
we can call them sent_location and sync_location.

-- 
Itagaki Takahiro

-- 
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] system views for walsender activity

2011-01-05 Thread Magnus Hagander
On Wed, Jan 5, 2011 at 02:32, Itagaki Takahiro
itagaki.takah...@gmail.com wrote:
 On Wed, Jan 5, 2011 at 04:56, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com wrote:
 I think pg_stat_replication is better than pg_stat_standby, but I'm
 still not convinced we shouldn't go with the obvious
 pg_stat_walsenders.

 How about pg_stat_replication_activity? If I understood correctly, the view
 is similar to pg_stat_activity, but displays information about connected
 standbys rather than regular backends. It's a bit long name, though.

 The view currently discussed is for *master* servers. We might have some
 views for replication activity in *standby* servers. So, I'd like to
 choose consistent and symmetric names for them -- for example,
 pg_stat_replication_master and pg_stat_replication_standby.
 I've expected they will be pg_stat_wal_[senders|receivers]
 when I was writing the patch, but any other better names welcome.

 However, we have max_wal_senders GUC parameter. So, users still
 need to know what wal_senders is.

An example to compare with could be pg_stat_bgwriter - that's also one
the really expects you to know some internals. Now, it so happens that
it's a very *bad* example, since it contains a bunch of information
that's *not* actually about the bgwriter these days :-)

But from that perspective, is it likely to ever contain anyting
*other* than walsender information? Given that it's keyed by the
process id of a walsender, I don't expect it would. Whereas a
pg_stat_replication or such could equally be expected to contain
information about other ways of replication - like the file based
modes or even slony.

+1 for pg_stat_walsender or pg_stat_walsender_activity

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] system views for walsender activity

2011-01-04 Thread Simon Riggs
On Tue, 2011-01-04 at 15:51 +0900, Itagaki Takahiro wrote:
 On Tue, Dec 28, 2010 at 22:17, Magnus Hagander mag...@hagander.net wrote:
  We definitely need the very basic level for 9.1, and we can always
  improve on it later :-)
 
  pg_stat_walsender. It would then only need the columns for procpid,
  usesysid, usename, client_addr, client_port, and the WALsender
  specific fields.
  Yeah, agreed. backend_start is probably the best one
 
 Here are patches for pg_stat_walsender.
 I split the feature into two pieces:
 
 * get_host_and_port.patch
 It separates host and port formatter as a subroutine from pg_stat_activity.
 In addition, make pg_stat_get_backend_client_addr/port() functions to
 use the subroutine to reduce duplicated codes.
 
 * pg_stat_walsender.patch
 It adds pg_stat_walsender system view. It has subset columns of
 pg_stat_activity and only one additional WAL sender specific information
 via WALSndStatus(). I named the column sending location because
 standby servers might not have received the WAL record; if we had
 synchronous replication, a new sent location wold be added.
 But the naming is still an open question. Comments welcome.
 
 There is O(max_wal_senders^2) complexity in the view, But I think
 it is not so serious problem because we can expect max_wal_senders
 is 10 or so at most.
 
 CREATE VIEW pg_stat_walsender AS
 SELECT
 S.procpid,
 S.usesysid,
 U.rolname AS usename,
 S.client_addr,
 S.client_port,
 S.backend_start,
 S.xlog_sending
 FROM pg_stat_get_walsender(NULL) AS S, pg_authid U
 WHERE S.usesysid = U.oid;

Just seen you started working on this again. Very good.

I enclose some snippets of code I was working on, which I am removing
from my patch in favour of your work as a separate commit.

The way I coded it was a new SRF that joins to the existing
pg_stat_activity. So no initdb required, and this can also easily be
included as an external module for 9.0.

Please notice also that my coding of the new SRF does not have the O^2
issue you mention, which I was keen to avoid.

The sent pointer is needed whether or not we have sync rep. We should
also include application name, since the user may set that in the
standby for all the same reasons it is set elsewhere.

Small point: please lets not call this pg_stat_walsender?
pg_stat_replication_sent and pg_stat_replication_received would be
easier for normal humans to understand.

I would very much appreciate it if one of you could complete something
here and commit in the next few days. That would then allow me to extend
the view with sync rep specific info for monitoring and patch testing.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 346eaaf..75419b7 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -488,6 +488,21 @@ CREATE VIEW pg_stat_activity AS
 WHERE S.datid = D.oid AND
 S.usesysid = U.oid;
 
+CREATE VIEW pg_stat_replication_activity AS
+SELECT
+S.procpid,
+S.usesysid,
+U.rolname AS usename,
+S.application_name,
+S.client_addr,
+S.client_port,
+S.backend_start,
+R.sent_location
+FROM pg_stat_get_activity(NULL) AS S, pg_authid U,
+pg_stat_get_replication_activity() AS R
+WHERE S.usesysid = U.oid AND
+S.procpid = R.procpid;
+
 CREATE VIEW pg_stat_database AS
 SELECT
 D.oid AS datid,
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index e9d8847..4422f5a 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -37,6 +37,7 @@
 #include signal.h
 #include unistd.h
 
+#include funcapi.h
 #include access/xlog_internal.h
 #include catalog/pg_type.h
 #include libpq/libpq.h
@@ -49,6 +50,7 @@
 #include storage/ipc.h
 #include storage/pmsignal.h
 #include tcop/tcopprot.h
+#include utils/builtins.h
 #include utils/guc.h
 #include utils/memutils.h
 #include utils/ps_status.h
@@ -1122,6 +1124,91 @@ WalSndWakeup(void)
 }
 
 /*
+ * Returns the Send position of walsenders with given pid.
+ * Or InvalidXLogRecPtr if none.
+ */
+Datum
+pg_stat_get_replication_activity(PG_FUNCTION_ARGS)
+{
+	FuncCallContext *funcctx;
+
+	if (SRF_IS_FIRSTCALL())
+	{
+		MemoryContext oldcontext;
+		TupleDesc	tupdesc;
+
+		funcctx = SRF_FIRSTCALL_INIT();
+
+		oldcontext = MemoryContextSwitchTo(funcctx-multi_call_memory_ctx);
+
+#define PG_STAT_GET_REP_ACTIVITY_COLS 	2
+		tupdesc = CreateTemplateTupleDesc(PG_STAT_GET_REP_ACTIVITY_COLS, false);
+		TupleDescInitEntry(tupdesc, (AttrNumber) 1, procpid, INT4OID, -1, 0);
+		TupleDescInitEntry(tupdesc, (AttrNumber) 2, sent_location, TEXTOID, -1, 0);
+
+		funcctx-tuple_desc = 

Re: [HACKERS] system views for walsender activity

2011-01-04 Thread Robert Haas
On Tue, Jan 4, 2011 at 12:48 PM, Simon Riggs si...@2ndquadrant.com wrote:
 The sent pointer is needed whether or not we have sync rep. We should
 also include application name, since the user may set that in the
 standby for all the same reasons it is set elsewhere.

 Small point: please lets not call this pg_stat_walsender?
 pg_stat_replication_sent and pg_stat_replication_received would be
 easier for normal humans to understand.

Eh... I may be showing my status as a non-normal human, but I know
exactly what pg_stat_walsender is (it's the view that shows you the
status of the WAL senders you've allowed by configuring
max_wal_senders0) but I have no idea what pg_stat_replication_sent
and pg_stat_replication_received are supposed to be.  Why two views?
*scratches head in confusion*

-- 
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] system views for walsender activity

2011-01-04 Thread Josh Berkus

 Eh... I may be showing my status as a non-normal human, but I know
 exactly what pg_stat_walsender is (it's the view that shows you the
 status of the WAL senders you've allowed by configuring
 max_wal_senders0) but I have no idea what pg_stat_replication_sent
 and pg_stat_replication_received are supposed to be.  Why two views?
 *scratches head in confusion*

How about one view, called pg_stat_replication?  This would be clear
even to newbies, which none of the other view names would ...

-- 
  -- Josh Berkus
 PostgreSQL Experts Inc.
 http://www.pgexperts.com

-- 
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] system views for walsender activity

2011-01-04 Thread Joshua D. Drake
On Tue, 2011-01-04 at 10:50 -0800, Josh Berkus wrote:
  Eh... I may be showing my status as a non-normal human, but I know
  exactly what pg_stat_walsender is (it's the view that shows you the
  status of the WAL senders you've allowed by configuring
  max_wal_senders0) but I have no idea what pg_stat_replication_sent
  and pg_stat_replication_received are supposed to be.  Why two views?
  *scratches head in confusion*
 
 How about one view, called pg_stat_replication?  This would be clear
 even to newbies, which none of the other view names would ...

hmmm I think pg_stat_standby might be more relevant but I definitely
agree something more newbie appropriate is in order.

Joshua D. Drake

 
 -- 
   -- Josh Berkus
  PostgreSQL Experts Inc.
  http://www.pgexperts.com
 

-- 
PostgreSQL.org Major Contributor
Command Prompt, Inc: http://www.commandprompt.com/ - 509.416.6579
Consulting, Training, Support, Custom Development, Engineering
http://twitter.com/cmdpromptinc | http://identi.ca/commandprompt


-- 
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] system views for walsender activity

2011-01-04 Thread Josh Berkus

 hmmm I think pg_stat_standby might be more relevant but I definitely
 agree something more newbie appropriate is in order.

I'd be fine with that name, too.

-- 
  -- Josh Berkus
 PostgreSQL Experts Inc.
 http://www.pgexperts.com

-- 
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] system views for walsender activity

2011-01-04 Thread Magnus Hagander
On Tue, Jan 4, 2011 at 20:28, Josh Berkus j...@agliodbs.com wrote:

 hmmm I think pg_stat_standby might be more relevant but I definitely
 agree something more newbie appropriate is in order.

 I'd be fine with that name, too.

That seems kind of backwards though - given that the view only
contains data on the master...

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] system views for walsender activity

2011-01-04 Thread Robert Haas
On Tue, Jan 4, 2011 at 2:31 PM, Magnus Hagander mag...@hagander.net wrote:
 On Tue, Jan 4, 2011 at 20:28, Josh Berkus j...@agliodbs.com wrote:

 hmmm I think pg_stat_standby might be more relevant but I definitely
 agree something more newbie appropriate is in order.

 I'd be fine with that name, too.

 That seems kind of backwards though - given that the view only
 contains data on the master...

I think pg_stat_replication is better than pg_stat_standby, but I'm
still not convinced we shouldn't go with the obvious
pg_stat_walsenders.

-- 
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] system views for walsender activity

2011-01-04 Thread Heikki Linnakangas

On 04.01.2011 21:43, Robert Haas wrote:

On Tue, Jan 4, 2011 at 2:31 PM, Magnus Hagandermag...@hagander.net  wrote:

On Tue, Jan 4, 2011 at 20:28, Josh Berkusj...@agliodbs.com  wrote:



hmmm I think pg_stat_standby might be more relevant but I definitely
agree something more newbie appropriate is in order.


I'd be fine with that name, too.


That seems kind of backwards though - given that the view only
contains data on the master...


I think pg_stat_replication is better than pg_stat_standby, but I'm
still not convinced we shouldn't go with the obvious
pg_stat_walsenders.


How about pg_stat_replication_activity? If I understood correctly, the 
view is similar to pg_stat_activity, but displays information about 
connected standbys rather than regular backends. It's a bit long name, 
though.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

--
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] system views for walsender activity

2011-01-04 Thread David Fetter
On Tue, Jan 04, 2011 at 10:50:12AM -0800, Josh Berkus wrote:
 
  Eh... I may be showing my status as a non-normal human, but I know
  exactly what pg_stat_walsender is (it's the view that shows you the
  status of the WAL senders you've allowed by configuring
  max_wal_senders0) but I have no idea what pg_stat_replication_sent
  and pg_stat_replication_received are supposed to be.  Why two views?
  *scratches head in confusion*
 
 How about one view, called pg_stat_replication?  This would be clear
 even to newbies, which none of the other view names would ...

Wait.  We can't do that.  We'd be breaking a decades-old tradition of
terrible names! ;)

Cheers,
David.
-- 
David Fetter da...@fetter.org http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com
iCal: webcal://www.tripit.com/feed/ical/people/david74/tripit.ics

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] system views for walsender activity

2011-01-04 Thread Itagaki Takahiro
On Wed, Jan 5, 2011 at 04:56, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 I think pg_stat_replication is better than pg_stat_standby, but I'm
 still not convinced we shouldn't go with the obvious
 pg_stat_walsenders.

 How about pg_stat_replication_activity? If I understood correctly, the view
 is similar to pg_stat_activity, but displays information about connected
 standbys rather than regular backends. It's a bit long name, though.

The view currently discussed is for *master* servers. We might have some
views for replication activity in *standby* servers. So, I'd like to
choose consistent and symmetric names for them -- for example,
pg_stat_replication_master and pg_stat_replication_standby.
I've expected they will be pg_stat_wal_[senders|receivers]
when I was writing the patch, but any other better names welcome.

However, we have max_wal_senders GUC parameter. So, users still
need to know what wal_senders is.

-- 
Itagaki Takahiro

-- 
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] system views for walsender activity

2011-01-03 Thread Itagaki Takahiro
On Tue, Dec 28, 2010 at 22:17, Magnus Hagander mag...@hagander.net wrote:
 We definitely need the very basic level for 9.1, and we can always
 improve on it later :-)

 pg_stat_walsender. It would then only need the columns for procpid,
 usesysid, usename, client_addr, client_port, and the WALsender
 specific fields.
 Yeah, agreed. backend_start is probably the best one

Here are patches for pg_stat_walsender.
I split the feature into two pieces:

* get_host_and_port.patch
It separates host and port formatter as a subroutine from pg_stat_activity.
In addition, make pg_stat_get_backend_client_addr/port() functions to
use the subroutine to reduce duplicated codes.

* pg_stat_walsender.patch
It adds pg_stat_walsender system view. It has subset columns of
pg_stat_activity and only one additional WAL sender specific information
via WALSndStatus(). I named the column sending location because
standby servers might not have received the WAL record; if we had
synchronous replication, a new sent location wold be added.
But the naming is still an open question. Comments welcome.

There is O(max_wal_senders^2) complexity in the view, But I think
it is not so serious problem because we can expect max_wal_senders
is 10 or so at most.

CREATE VIEW pg_stat_walsender AS
SELECT
S.procpid,
S.usesysid,
U.rolname AS usename,
S.client_addr,
S.client_port,
S.backend_start,
S.xlog_sending
FROM pg_stat_get_walsender(NULL) AS S, pg_authid U
WHERE S.usesysid = U.oid;

-- 
Itagaki Takahiro


get_host_and_port-20110104.patch
Description: Binary data


pg_stat_walsender-20110104.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] system views for walsender activity

2010-12-28 Thread Magnus Hagander
On Tue, Jun 22, 2010 at 06:18, Takahiro Itagaki
itagaki.takah...@oss.ntt.co.jp wrote:

 Magnus Hagander mag...@hagander.net wrote:

 The downside is that version 1 will require an initdb, and not version
 2, right?

 Unfortunately, 2 also requires initdb because pg_stat_activity will
 use LEFT JOIN instead of normal JOIN not to hide rows with databaseid = 0.
 All of them are items for 9.1.

Did this one end up on the floor?

We definitely need the very basic level for 9.1, and we can always
improve on it later :-) Do you want to keep working on it, or do you
want me to pick it up?

Any of the suggestions that includes the master showing data from the
slaves requires some kind of feedback going in from the slave, making
things a lot more complex (the slave is no longer passive) - let's
leave those for now. (you can use the 2ndquadrant replmgr to get some
of that :P)

I'm not sure it makes much sense to add walsenders to pg_stat_activity
- a lot of the fields would no longer make any sense (statement start?
query start?) - I think we're better off with a separate view for
pg_stat_walsender. It would then only need the columns for procpid,
usesysid, usename, client_addr, client_port, and the WALsender
specific fields.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] system views for walsender activity

2010-12-28 Thread Itagaki Takahiro
On Tue, Dec 28, 2010 at 21:46, Magnus Hagander mag...@hagander.net wrote:
 Unfortunately, 2 also requires initdb because pg_stat_activity will
 use LEFT JOIN instead of normal JOIN not to hide rows with databaseid = 0.
 All of them are items for 9.1.

 Did this one end up on the floor?

 We definitely need the very basic level for 9.1, and we can always
 improve on it later :-) Do you want to keep working on it, or do you
 want me to pick it up?

OK, I'll work for it.

 I'm not sure it makes much sense to add walsenders to pg_stat_activity
 - a lot of the fields would no longer make any sense (statement start?
 query start?) - I think we're better off with a separate view for
 pg_stat_walsender. It would then only need the columns for procpid,
 usesysid, usename, client_addr, client_port, and the WALsender
 specific fields.

+1 for the separate view. backend_start (or replication_start?)
might be also reasonable.

-- 
Itagaki Takahiro

-- 
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] system views for walsender activity

2010-12-28 Thread Magnus Hagander
On Tue, Dec 28, 2010 at 14:14, Itagaki Takahiro
itagaki.takah...@gmail.com wrote:
 On Tue, Dec 28, 2010 at 21:46, Magnus Hagander mag...@hagander.net wrote:
 Unfortunately, 2 also requires initdb because pg_stat_activity will
 use LEFT JOIN instead of normal JOIN not to hide rows with databaseid = 0.
 All of them are items for 9.1.

 Did this one end up on the floor?

 We definitely need the very basic level for 9.1, and we can always
 improve on it later :-) Do you want to keep working on it, or do you
 want me to pick it up?

 OK, I'll work for it.

Great.


 I'm not sure it makes much sense to add walsenders to pg_stat_activity
 - a lot of the fields would no longer make any sense (statement start?
 query start?) - I think we're better off with a separate view for
 pg_stat_walsender. It would then only need the columns for procpid,
 usesysid, usename, client_addr, client_port, and the WALsender
 specific fields.

 +1 for the separate view. backend_start (or replication_start?)
 might be also reasonable.

Yeah, agreed. backend_start is probably the best one -
replication_start may be considered conceptually different if the
connection was dropped and reconnected. backend_start is more
explicit.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] system views for walsender activity

2010-06-22 Thread Guillaume Lelarge
Le 22/06/2010 06:40, Takahiro Itagaki a écrit :
 [...]
 Tom Lane t...@sss.pgh.pa.us wrote:
 
 I'm of the opinion that this is a 9.1 problem.  It needs more thought
 than we can put into it now --- one obvious question is what about
 monitoring on the slave side?  Another is who should be able to see the
 data?
 
 Sure. We should research user's demands for monitoring and management
 of replication. I'll report some voices from users as of this moment:
 
 * Managers often ask DBAs How long standby servers are behind the master?
   We should provide such methods for DBAs. We have pg_xlog_location()
   functions, but they should be improved for:
 - The returned values are xxx/yyy texts, but more useful information
   is the difference of two values. Subtraction functions are required.
 - For easier management, the master server should provide not only
   sent/flush locations but also received/replayed locations for each
   standby servers. Users don't want to access both master and slaves.
 
 * Some developers want to pause and restart replication from the master
   server. They're going to use replication for application version
   managements. They'll pause all replications, and test their new features
   at the master, and restart replication to spread the changes to slaves.
 

I agree on these two.

Something I found lacking when I added support for Hot Standby /
Streaming Replication in pgAdmin (that was a really small patch, there
was not a lot to do) was that one cannot get the actual value of each
recovery.conf parameter. Try a SHOW primary_conninfo; and it will
juste reply that primary_conninfo is an unknown parameter. I already
talked about this to Heikki, but didn't get a chance to actually look at
the code.


-- 
Guillaume
 http://www.postgresql.fr
 http://dalibo.com

-- 
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] system views for walsender activity

2010-06-22 Thread Simon Riggs
On Tue, 2010-06-22 at 09:54 +0200, Guillaume Lelarge wrote:
 I added support for Hot Standby /
 Streaming Replication in pgAdmin (that was a really small patch, there
 was not a lot to do)

Well done.

Does this mean that pgAdmin has a read only mode now?

What are the details of that support? I couldn't easily see the commits
in the pgadmin list.

-- 
 Simon Riggs   www.2ndQuadrant.com


-- 
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] system views for walsender activity

2010-06-22 Thread Guillaume Lelarge
Le 22/06/2010 12:42, Simon Riggs a écrit :
 On Tue, 2010-06-22 at 12:19 +0200, Guillaume Lelarge wrote:
 Shamely simple : I only added some informations on the server's
 properties. See
 http://www.pgadmin.org/images/visualtour12/visualtour08.jpg. We only
 display the fact that the server is (or isn't) in recovery, and the
 result of the two admin functions (receive and replay location). 
 
 If you store the is-in-Recovery result you could set the .enabled
 property of many of the dialog boxes. I think its going to be painful
 for people to attempt to submit a DDL command and get an error.
 

That's what I first thought. But it would be weird that we disabled all
the OK button of the dialog properties only for hotstandby servers, but
not when a user doesn't have the permission. At least, that was the
reasonning I had at the time.

 Too bad the other admin functions aren't there, I could have used them
 (and hope to do so in 9.1). Too bad also we cannot know the primary
 server from a connection to the slave (that's why I would love to get
 the value of
 primary_conninfo, to found the alias/IP of the primary server).
 
 Agreed
 

:)


-- 
Guillaume
 http://www.postgresql.fr
 http://dalibo.com

-- 
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] system views for walsender activity

2010-06-22 Thread Guillaume Lelarge
Le 22/06/2010 11:41, Simon Riggs a écrit :
 On Tue, 2010-06-22 at 09:54 +0200, Guillaume Lelarge wrote:
 I added support for Hot Standby /
 Streaming Replication in pgAdmin (that was a really small patch, there
 was not a lot to do)
 
 Well done.
 
 Does this mean that pgAdmin has a read only mode now?
 

Nope, it does not really have one. Though I intend to work on having
pgAdmin more aware of the actual rights of the connected user (allowing
him to get to display the create table dialog when we should already
know he cannot is an issue, at least to me).

 What are the details of that support? I couldn't easily see the commits
 in the pgadmin list.
 

Shamely simple : I only added some informations on the server's
properties. See
http://www.pgadmin.org/images/visualtour12/visualtour08.jpg. We only
display the fact that the server is (or isn't) in recovery, and the
result of the two admin functions (receive and replay location). Too bad
the other admin functions aren't there, I could have used them (and hope
to do so in 9.1). Too bad also we cannot know the primary server from a
connection to the slave (that's why I would love to get the value of
primary_conninfo, to found the alias/IP of the primary server).


-- 
Guillaume
 http://www.postgresql.fr
 http://dalibo.com

-- 
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] system views for walsender activity

2010-06-22 Thread Simon Riggs
On Tue, 2010-06-22 at 12:19 +0200, Guillaume Lelarge wrote:
 Shamely simple : I only added some informations on the server's
 properties. See
 http://www.pgadmin.org/images/visualtour12/visualtour08.jpg. We only
 display the fact that the server is (or isn't) in recovery, and the
 result of the two admin functions (receive and replay location). 

If you store the is-in-Recovery result you could set the .enabled
property of many of the dialog boxes. I think its going to be painful
for people to attempt to submit a DDL command and get an error.

 Too bad the other admin functions aren't there, I could have used them
 (and hope to do so in 9.1). Too bad also we cannot know the primary
 server from a connection to the slave (that's why I would love to get
 the value of
 primary_conninfo, to found the alias/IP of the primary server).

Agreed

-- 
 Simon Riggs   www.2ndQuadrant.com
 PostgreSQL Development, 24x7 Support, Training and 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] system views for walsender activity

2010-06-21 Thread Takahiro Itagaki

Tom Lane t...@sss.pgh.pa.us wrote:

 I'm of the opinion that this is a 9.1 problem.  It needs more thought
 than we can put into it now --- one obvious question is what about
 monitoring on the slave side?  Another is who should be able to see the
 data?

Sure. We should research user's demands for monitoring and management
of replication. I'll report some voices from users as of this moment:

* Managers often ask DBAs How long standby servers are behind the master?
  We should provide such methods for DBAs. We have pg_xlog_location()
  functions, but they should be improved for:
- The returned values are xxx/yyy texts, but more useful information
  is the difference of two values. Subtraction functions are required.
- For easier management, the master server should provide not only
  sent/flush locations but also received/replayed locations for each
  standby servers. Users don't want to access both master and slaves.

* Some developers want to pause and restart replication from the master
  server. They're going to use replication for application version
  managements. They'll pause all replications, and test their new features
  at the master, and restart replication to spread the changes to slaves.

Regards,
---
Takahiro Itagaki
NTT Open Source Software Center



-- 
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] system views for walsender activity

2010-06-18 Thread Magnus Hagander
On Fri, Jun 18, 2010 at 04:33, Takahiro Itagaki
itagaki.takah...@oss.ntt.co.jp wrote:
 Hi,

 We don't have any statistic views for walsenders in SR's master server
 in 9.0, but such views would be useful to monitor and manage standby
 servers from the master server. I have two ideas for the solution -
 adding a new system view or recycling pg_stat_activity:

 1. Add another system view for walsenders, ex. pg_stat_replication.
   It would show pid, remote host, and sent location for each walsender.

 2. Make pg_stat_activity to show walsenders. We could use current_query
   to display walsender-specific information, like:
    =# SELECT * FROM my_stat_activity ;
    -[ RECORD 1 ]+-
    datid            | 16384
    snip
    current_query    | SELECT * FROM my_stat_activity ;
    -[ RECORD 2 ]+-
    datid            | 0
    datname          |
    procpid          | 4300
    usesysid         | 10
    usename          | itagaki
    application_name |
    client_addr      | ::1
    client_port      | 37710
    backend_start    | 2010-06-16 16:47:35.646486+09
    xact_start       |
    query_start      |
    waiting          | f
    current_query    | walsender: sent=0/701AAA8

 The attached patch is a WIP codes for the case 2, but it might be
 better to design management policy via SQL in 9.1 before such detailed
 implementation.  Comments welcome.

I like version 1 much better. It'll be a lot easier for a management
tool to get the data in proper columns and not have to parse it out of
an arbitrary string field.

The downside is that version 1 will require an initdb, and not version
2, right? In that light, #2 is probably the only one we can do for 9.0
(unless we stumble upon other initdb-forcing changes), so maybe we
should do that one as a temporary measure (and if so, note it both in
the documentation and code that it's a temporary thing).

Wouldn't we also need something similar for the receiving end?

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

-- 
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] system views for walsender activity

2010-06-18 Thread Simon Riggs
On Fri, 2010-06-18 at 11:33 +0900, Takahiro Itagaki wrote:

 1. Add another system view for walsenders, ex. pg_stat_replication.
It would show pid, remote host, and sent location for each walsender.

I prefer this option. I consider it an omission that we should correct.

Not sure I understand why you block up access to the WALSendPtr and then
propose re-accessing it in text form a few days later. Whatever else you
do you should revoke the commit that did that.

-- 
 Simon Riggs   www.2ndQuadrant.com


-- 
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] system views for walsender activity

2010-06-18 Thread Heikki Linnakangas

On 18/06/10 13:41, Simon Riggs wrote:

On Fri, 2010-06-18 at 11:33 +0900, Takahiro Itagaki wrote:


1. Add another system view for walsenders, ex. pg_stat_replication.
It would show pid, remote host, and sent location for each walsender.


I prefer this option. I consider it an omission that we should correct.

Not sure I understand why you block up access to the WALSendPtr and then
propose re-accessing it in text form a few days later. Whatever else you
do you should revoke the commit that did that.


Are you referring to the dead extern declaration of 
GetOldestWALSendPointer()? That was indeed dead, the function itself was 
already #ifdeffed out. If we go with the pg_state_replication view as 
suggested above, it would've been useless anyway because it returns only 
one value, not one for each walsender.


Let's discuss what the best possible user interface for the information 
would be first, and then decide if we need/want to force an initdb for 
that. We have pg_upgrade now, that makes initdb less painful, and if 
it's just a new view it might be possible to just add a note in the 
release notes that you'll have to run the CREATE VIEW manually if upgrading.


--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

--
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] system views for walsender activity

2010-06-18 Thread Tom Lane
Heikki Linnakangas heikki.linnakan...@enterprisedb.com writes:
 Let's discuss what the best possible user interface for the information 
 would be first, and then decide if we need/want to force an initdb for 
 that. We have pg_upgrade now, that makes initdb less painful, and if 
 it's just a new view it might be possible to just add a note in the 
 release notes that you'll have to run the CREATE VIEW manually if upgrading.

The view would presumably depend on a C-language SRF, which isn't there
either.

I'm of the opinion that this is a 9.1 problem.  It needs more thought
than we can put into it now --- one obvious question is what about
monitoring on the slave side?  Another is who should be able to see the
data?

regards, tom lane

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