date:20110218

[HACKERS] Re: [COMMITTERS] pgsql: Hot Standby feedback for avoidance of cleanup conflicts on stand

2011-02-18 Thread Simon Riggs

On Fri, 2011-02-18 at 11:44 +0900, Fujii Masao wrote:
  This begs the questions what is the xmin of all the normal
 backends?
  and Whats is the xmin of prepared transactions? as well. I wasn't
 sure  that we should expose that information for walsenders when we
 don't do  it for everybody else. If we do it would require major
 sections in the  docs explaining it all, etc..
 
 We can *presume* which backend (or prepared transaction) unexpectedly
 prevents VACUUM by seeing pg_stat_activity (or pg_prepared_xacts) and
 checking whether there is long-running transaction. But there is no
 way to presume which standby does that, I'm concerned.

Currently there is no information available to users about xmin.

It isn't any *harder* to work out standby xmins than it is to work out
prepared transaction xmins or other backend xmins. For example, we don't
show which transactions use serializable mode on pg_stat_activity and so
we might make the mistake of reading the last statement start time
rather than the last transaction start time.

I see it as a major piece of work to add xmin in *all* the right places
and to fully document how to use that information as a user. The debug
information at DEBUG2 is sufficient to show the code works and to debug
issues if needed.

I agree with you that it would be nice to have a which thing is
bloating my tables device, but I'm not assuming the responsibility to
create such a thing and fully document it, and definitely not Right Now.

-- 
 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] Transaction-scope advisory locks

2011-02-18 Thread Marko Tiikkaja


On 2011-02-18 7:16 AM +0200, Itagaki Takahiro wrote:

Committed with a few typo fixes. Thanks, Marko!


Thanks a lot!


Regards,
Marko Tiikkaja

--
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] Sync Rep for 2011CF1

2011-02-18 Thread Simon Riggs

On Fri, 2011-02-18 at 00:48 +, Simon Riggs wrote:

 Complete but rough hack, for comments, but nothing surprising.

This is an implicit requirement from our earlier agreed API, so its
blocking further work on Sync Rep.

I'm looking to commit this in about 3-4 hours unless I get comments.

-- 
 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

[HACKERS] pg_basebackup and wal streaming

2011-02-18 Thread Magnus Hagander

Better late than never (or?), here's the final cleanup of
pg_streamrecv for moving into the main distribution, per discussion
back in late dec or early jan. It also includes the stream logs in
parallel to backup part that was not completed on pg_basebackup.

Other than that, the only changes to pg_basebackup are the moving of a
couple of functions into streamutil.c to make them usable from both,
and the progress format output fix Fujii-san mentioned recently.

Should be complete except for Win32 support (needs thread/fork thing
for the  background streaming feature. Shouldn't be too hard, and I
guess that falls on me anyway..) and the reference documentation.

And with no feedback to my question here
(http://archives.postgresql.org/pgsql-hackers/2011-02/msg00805.php), I
went with the duplicate the macros that are needed to avoid loading
postgres.h path.

Yes, I realize that this is far too late in the CF process really, but
I wanted to post it anyway... If it's too late to be acceptable it
should be possible to maintain this outside the main repository until
9.2, since it only changes frontend binaries. So I'm not actually
going to put it on the CF page unless someone else says that's a good
idea, to at least share the blame from Robert ;)

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/
diff --git a/src/bin/pg_basebackup/Makefile b/src/bin/pg_basebackup/Makefile
index ccb1502..5bbe52d 100644
--- a/src/bin/pg_basebackup/Makefile
+++ b/src/bin/pg_basebackup/Makefile
@@ -18,21 +18,26 @@ include $(top_builddir)/src/Makefile.global
 
 override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS)
 
-OBJS=	pg_basebackup.o $(WIN32RES)
+OBJS=receivelog.o streamutil.o $(WIN32RES)
 
-all: pg_basebackup
+all: pg_basebackup pg_receivexlog
 
-pg_basebackup: $(OBJS) | submake-libpq submake-libpgport
-	$(CC) $(CFLAGS) $(OBJS) $(libpq_pgport) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
+pg_basebackup: pg_basebackup.o $(OBJS) | submake-libpq submake-libpgport
+	$(CC) $(CFLAGS) pg_basebackup.o $(OBJS) $(libpq_pgport) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
+
+pg_receivexlog: pg_receivexlog.o $(OBJS) | submake-libpq submake-libpgport
+	$(CC) $(CFLAGS) pg_receivexlog.o $(OBJS) $(libpq_pgport) $(LDFLAGS) $(LDFLAGS_EX) $(LIBS) -o $@$(X)
 
 install: all installdirs
 	$(INSTALL_PROGRAM) pg_basebackup$(X) '$(DESTDIR)$(bindir)/pg_basebackup$(X)'
+	$(INSTALL_PROGRAM) pg_receivexlog$(X) '$(DESTDIR)$(bindir)/pg_receivexlog(X)'
 
 installdirs:
 	$(MKDIR_P) '$(DESTDIR)$(bindir)'
 
 uninstall:
 	rm -f '$(DESTDIR)$(bindir)/pg_basebackup$(X)'
+	rm -f '$(DESTDIR)$(bindir)/pg_receivexlog(X)'
 
 clean distclean maintainer-clean:
-	rm -f pg_basebackup$(X) $(OBJS)
+	rm -f pg_basebackup$(X) pg_receivexlog$(X) $(OBJS) pg_basebackup.o pg_receivexlog.o
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 61aa1d3..7442bbe 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -17,6 +17,8 @@
 #include unistd.h
 #include dirent.h
 #include sys/stat.h
+#include sys/types.h
+#include sys/wait.h
 
 #ifdef HAVE_LIBZ
 #include zlib.h
@@ -24,9 +26,11 @@
 
 #include getopt_long.h
 
+#include receivelog.h
+#include streamutil.h
+
 
 /* Global options */
-static const char *progname;
 char	   *basedir = NULL;
 char		format = 'p';		/* p(lain)/t(ar) */
 char	   *label = pg_basebackup base backup;
@@ -34,38 +38,35 @@ bool		showprogress = false;
 int			verbose = 0;
 int			compresslevel = 0;
 bool		includewal = false;
+bool		streamwal = false;
 bool		fastcheckpoint = false;
-char	   *dbhost = NULL;
-char	   *dbuser = NULL;
-char	   *dbport = NULL;
-int			dbgetpassword = 0;	/* 0=auto, -1=never, 1=always */
 
 /* Progress counters */
 static uint64 totalsize;
 static uint64 totaldone;
 static int	tablespacecount;
 
-/* Connection kept global so we can disconnect easily */
-static PGconn *conn = NULL;
+/* Pipe to communicate with background wal receiver process */
+static int	bgpipe[2] = {-1, -1};
 
-#define disconnect_and_exit(code)\
-	{			\
-	if (conn != NULL) PQfinish(conn);			\
-	exit(code);	\
-	}
+/* Handle to child process */
+static pid_t bgchild = -1;
+
+/* End position for xlog streaming, empty string if unknown yet */
+static XLogRecPtr xlogendptr;
+static bool has_xlogendptr = false;
 
 /* Function headers */
-static char *xstrdup(const char *s);
-static void *xmalloc0(int size);
 static void usage(void);
 static void verify_dir_is_empty_or_create(char *dirname);
 static void progress_report(int tablespacenum, char *fn);
-static PGconn *GetConnection(void);
 
 static void ReceiveTarFile(PGconn *conn, PGresult *res, int rownum);
 static void ReceiveAndUnpackTarFile(PGconn *conn, PGresult *res, int rownum);
 static void BaseBackup();
 
+static bool segment_callback(XLogRecPtr segendpos, uint32 timeline);
+
 #ifdef HAVE_LIBZ
 static const char *
 get_gz_error(gzFile *gzf)
@@ -81,39 +82,6 @@ get_gz_error(gzFile *gzf)
 }

Re: [HACKERS] ECPG dynamic cursor fix for UPDATE/DELETE ... WHERE CURRENT OF :curname

2011-02-18 Thread Michael Meskes

 Did this ever get applied?  If so, I can't find it.

No, my bad, I simply forgot about it, sorry.

Will work on this now.

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at googlemail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

-- 
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] Re: ECPG dynamic cursor fix for UPDATE/DELETE ... WHERE CURRENT OF :curname

2011-02-18 Thread Michael Meskes

 sorry for the late answer. Here is a minimal patch against the

My answer is way later, so I have to apologize. 

 current GIT tree, so the WHERE CURRENT OF accepts
 dynamic cursornames,  plus the test case that shows the problem.

It doesn't on my test system. I just committed the minimal patch, so everyone
can test it, but your test case works just nicely on my system.

No idea why your results are different.

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org
Jabber: michael.meskes at googlemail dot com
VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL

-- 
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] Debian readline/libedit breakage

2011-02-18 Thread Magnus Hagander

On Fri, Feb 18, 2011 at 02:01,  charles.mcdev...@emc.com wrote:
 On 02/17/2011 12:34 PM, Bruce Momjian wrote:
  Andrew Dunstan wrote:
 
  On 02/17/2011 12:13 PM, Bruce Momjian wrote:
  FWIW, the only interactively usable version of psql for windows I know
  of is the one that runs under Cygwin. It can be build with readline and
  works as expected.
  Uh, don't we have a psql built via MSVC?  Doesn't it work interactively?
 
  Not if you want readline. And in my book that's a requirement of a psql
  that's usable interactively. It's pretty horrible to use without it.
  Well, as horrible as other Windows apps.  I will leave others to decide
  if that is usable.  ;-)  I am unclear if we would ship readline support
  on Windows even if we didn't have a license issue (no OS readline
  version on Windows).
 

 Windows developers almost universally work from GUIs and not using
 console apps (and that's true of many Unix developers also, particularly
 those who can't recall a time when X-Windows wasn't almost universally
 available). Microsoft has de-emphasized console apps for 15 years. So
 the only people who are likely to be interested in using an enhanced
 psql on Windows are old Unix-heads like you and me. It's not worth a lot
 of effort, IMNSHO.

 cheers

 andrew


 I think this is wrong...  There are plenty of people who use the command line 
 in Windows, and Microsoft has been adding better support for this, including 
 PowerShell and interfaces to every administrative operation from command line 
 scripts.

While I haven't used the latest version of powershell, my experience
has always been that the focus has really been on scripts and not
necessarily the interactive experience.


 Psql on Windows is ugly... Readline does (supposedly) run on Windows, but no 
 one has done the work to get it to happen.

Readline does not run on Windows, if you want to use any interesting
keys. Such as the alt or alt-gr keys. Basically, readline for windows
doesn't work outside of english locales... For example, you can't
access any of the backslash commands in many locales, which makes psql
a lot less useful...

 IIRC I even brought this up with the readline folks, and they simply
don't give a  about woe32 as they call it, so there should be no
faith in that ever getting fixed.

Granted, I haven't checked if they actually fixed it in the past
couple of years, but there seemed to be zero interest before that a
least.


-- 
 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] SSI bug?

2011-02-18 Thread Magnus Hagander

On Thu, Feb 17, 2011 at 23:11, Kevin Grittner
kevin.gritt...@wicourts.gov wrote:
 Dan Ports d...@csail.mit.edu wrote:

 Oops. Those are both definitely bugs (and my fault). Your patch
 looks correct. Thanks for catching that!

 Could a committer please apply the slightly modified version here?:

 http://archives.postgresql.org/message-id/4d5c46bb02250003a...@gw.wicourts.gov

 It is a pretty straightforward bug fix to initialize some currently
 uninitialized data which is causing occasional but severe problems,
 especially during vacuum.

Done, thanks.


-- 
 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] COPY ENCODING revisited

2011-02-18 Thread Itagaki Takahiro

On Fri, Feb 18, 2011 at 03:57, Robert Haas robertmh...@gmail.com wrote:
 On Wed, Feb 16, 2011 at 10:45 PM, Itagaki Takahiro
 I am not qualified to fully review this patch because I'm not all that
 familiar with the encoding stuff, but it looks reasonably sensible on
 a quick read-through.  I am supportive of making a change in this area
 even at this late date, because it seems to me that if we're not going
 to change this then we're pretty much giving up on having a usable
 file_fdw in 9.1.  And since postgresql_fdw isn't in very good shape
 either, that would mean we may as well give up on SQL/MED.  We might
 have to do that anyway, but I don't think we should do it just because
 of this issue, if there's a reasonable fix.

One design issue is the new function names:
  extern char *pg_client_to_server(const char *s, int len);
  extern char *pg_server_to_client(const char *s, int len);
+ extern char *pg_any_to_server(const char *s, int len, int encoding);
+ extern char *pg_server_to_any(const char *s, int len, int encoding);

They don't contain any words related to encoding or conversion.

Ishii-san, do you have comments? I guess you designed the area.
Please let me know if there are better alternatives.

-- 
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] ALTER TYPE COLLATABLE?

2011-02-18 Thread Peter Eisentraut

On tor, 2011-02-17 at 17:50 -0500, Tom Lane wrote:
 What are we going to do to allow the citext update script to fix this?
 I see no sign that ALTER TYPE can fix it (and am unsure that we'd want
 to add such a feature, particularly not right now).

How would this normally be handled if a type changes properties or wants
to make use of a new property?  I guess the answer is that there is no
normally.

 Is it time for a direct UPDATE on the pg_type row?  If so, to what?  I see
 pg_type.typcollation is supposed to be an OID, so how the heck does
 one map a bool CREATE TYPE parameter into the catalog entry?

It's 100, which is the OID of default in pg_collation.  The value may
be different for domains.  (Earlier versions of the feature had a
boolean column and a separate collation column for domains, but somehow
it turned out to be quite redundant.)




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

[HACKERS] Assertion failure on UNLOGGED VIEW and SEQUENCE

2011-02-18 Thread Itagaki Takahiro

UNLOGGED is defigned in OptTemp in gram.y, so the parser accepts
CREATE UNLOGGED VIEW and CREATE UNLOGGED SEQUENCE, but they are
actually not supported.

=# CREATE UNLOGGED VIEW uv AS SELECT 1;
TRAP: FailedAssertion(!(relkind == 'r' || relkind == 't'), File:
heap.c, Line: 1246)

=# CREATE UNLOGGED SEQUENCE us;
TRAP: FailedAssertion(!(relkind == 'r' || relkind == 't'), File:
heap.c, Line: 1246)

The most easiest fix would be preventing them in parser level.

-- 
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

[HACKERS] Server Name

2011-02-18 Thread Simon Riggs


We agreed to add a parameter called sync_standbys
http://archives.postgresql.org/message-id/4d1dcf5a.7070...@enterprisedb.com.
that required the concept of a server name.

I've written this patch to add a server name parameter and to send an
info message which returns the server name, attached.

For now, Sync Rep will be written to match sync_standbys against the
application_name instead. It may be possible we agree it is the right
way to go, so I've not rushed to apply the patch given here after all. 

This patch now has a lower priority than the main sync rep patch.

-- 
 Simon Riggs   http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services
 
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index c7f5bd5..46096e6 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -125,6 +125,7 @@ static void XLogWalRcvWrite(char *buf, Size nbytes, XLogRecPtr recptr);
 static void XLogWalRcvFlush(bool dying);
 static void XLogWalRcvSendReply(void);
 static void XLogWalRcvSendHSFeedback(void);
+static void XLogWalRcvSendInfo(void);
 
 /* Signal handlers */
 static void WalRcvSigHupHandler(SIGNAL_ARGS);
@@ -276,6 +277,9 @@ WalReceiverMain(void)
 	walrcv_connect(conninfo, startpoint);
 	DisableWalRcvImmediateExit();
 
+	/* Give the primary our identification and configuration information */
+	XLogWalRcvSendInfo();
+
 	/* Loop until end-of-streaming or error */
 	for (;;)
 	{
@@ -305,6 +309,7 @@ WalReceiverMain(void)
 		{
 			got_SIGHUP = false;
 			ProcessConfigFile(PGC_SIGHUP);
+			XLogWalRcvSendInfo();
 		}
 
 		/* Wait a while for data to arrive */
@@ -702,3 +707,26 @@ XLogWalRcvSendHSFeedback(void)
 	memcpy(buf[1], feedback_message, sizeof(StandbyHSFeedbackMessage));
 	walrcv_send(buf, sizeof(StandbyHSFeedbackMessage) + 1);
 }
+
+/*
+ * Send info message to primary.
+ */
+static void
+XLogWalRcvSendInfo(void)
+{
+	char		buf[sizeof(StandbyInfoMessage) + 1];
+	StandbyInfoMessage	info_message;
+
+	/* Get current timestamp. */
+	info_message.sendTime = GetCurrentTimestamp();
+	strncpy(info_message.servername, ServerName, strlen(ServerName));
+	info_message.servername[strlen(ServerName)] = '\0';
+
+	elog(DEBUG2, sending standby info servername \%s\,
+ info_message.servername);
+
+	/* Prepend with the message type and send it. */
+	buf[0] = 'i';
+	memcpy(buf[1], info_message, sizeof(StandbyInfoMessage));
+	walrcv_send(buf, sizeof(StandbyInfoMessage) + 1);
+}
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index e04d59e..43e47d9 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -119,6 +119,7 @@ static void StartReplication(StartReplicationCmd * cmd);
 static void ProcessStandbyMessage(void);
 static void ProcessStandbyReplyMessage(void);
 static void ProcessStandbyHSFeedbackMessage(void);
+static void ProcessStandbyInfoMessage(void);
 static void ProcessRepliesIfAny(void);
 
 
@@ -537,6 +538,10 @@ ProcessStandbyMessage(void)
 			ProcessStandbyHSFeedbackMessage();
 			break;
 
+		case 'i':
+			ProcessStandbyInfoMessage();
+			break;
+
 		default:
 			ereport(COMMERROR,
 	(errcode(ERRCODE_PROTOCOL_VIOLATION),
@@ -655,6 +660,16 @@ ProcessStandbyHSFeedbackMessage(void)
 	}
 }
 
+static void
+ProcessStandbyInfoMessage(void)
+{
+	StandbyInfoMessage	info;
+
+	pq_copymsgbytes(reply_message, (char *) info, sizeof(StandbyInfoMessage));
+
+	elog(DEBUG2, server name %s, info.servername);
+}
+
 /* Main loop of walsender process */
 static int
 WalSndLoop(void)
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 55cbf75..fd733e0 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -186,6 +186,7 @@ static const char *assign_pgstat_temp_directory(const char *newval, bool doit, G
 static const char *assign_application_name(const char *newval, bool doit, GucSource source);
 static const char *show_unix_socket_permissions(void);
 static const char *show_log_file_mode(void);
+static const char *assign_server_name(const char *newval, bool doit, GucSource source);
 
 static char *config_enum_get_options(struct config_enum * record,
 		const char *prefix, const char *suffix,
@@ -405,6 +406,9 @@ int			tcp_keepalives_idle;
 int			tcp_keepalives_interval;
 int			tcp_keepalives_count;
 
+char		*ServerName = NULL;
+
+
 /*
  * These variables are all dummies that don't do anything, except in some
  * cases provide the value for SHOW to display.  The real state is elsewhere
@@ -2365,6 +2369,15 @@ static struct config_string ConfigureNamesString[] =
 	},
 
 	{
+		{server_name, PGC_POSTMASTER, CLIENT_CONN_STATEMENT,
+			gettext_noop(Allows setting of a unique name for this server.),
+			NULL
+		},
+		ServerName,
+		, assign_server_name, NULL
+	},
+
+	{
 		{temp_tablespaces, PGC_USERSET, CLIENT_CONN_STATEMENT,
 			gettext_noop(Sets the tablespace(s) to use for temporary tables

[HACKERS] Re: [COMMITTERS] pgsql: Separate messages for standby replies and hot standby feedback.

2011-02-18 Thread Robert Haas

On Fri, Feb 18, 2011 at 6:34 AM, Simon Riggs si...@2ndquadrant.com wrote:
 Separate messages for standby replies and hot standby feedback.
 Allow messages to be sent at different times, and greatly reduce
 the frequency of hot standby feedback. Refactor to allow additional
 message types.

You could refactor this some more to avoid calling
GetCurrentTimestamp() and TimestampDifferenceExceeds() twice each, but
I'm not sure whether it's worth it.

-- 
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] About the performance of startup after dropping many tables

2011-02-18 Thread Robert Haas

On Thu, Feb 17, 2011 at 10:37 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Gan Jiadong ga...@huawei.com writes:
 we have PG 8.3.13 in our system. When running performance cases, we find the
 startup recovery cost about 3 minutes. It is too long in our system.

 Maybe you should rethink the assumption that dropping 4 tables is a
 cheap operation.  Why do you have that many in the first place, let
 alone that many that you drop and recreate frequently?  Almost
 certainly, you need a better-conceived schema.

Possibly, but it's not necessarily a bad idea to improve performance
for people with crazy schemas.

What concerns me a little bit about the proposed scheme, though, is
that it's only going to work if all over those tables are dropped by a
single transaction.  You still need one pass through all of
shared_buffers for every transaction that drops one or more relations.
 Now, I'm not sure, maybe there's no help for that, but ever since
commit c2281ac87cf4828b6b828dc8585a10aeb3a176e0 it's been on my mind
that loops that iterate through the entire buffer cache are bad for
scalability.

Conventional wisdom seems to be that performance tops out at, or just
before, 8GB, but it's already the case that that's a quite a small
fraction of the memory on a large machine, and that's only going to
keep getting worse.  Admittedly, the existing places where we loop
through the whole buffer cache are probably not the primary reason for
that limitation, but...

-- 
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] SQL/MED - file_fdw

2011-02-18 Thread Shigeru HANADA

On Wed, 16 Feb 2011 16:48:33 +0900
Itagaki Takahiro itagaki.takah...@gmail.com wrote:

On Tue, Feb 8, 2011 at 00:30, Shigeru HANADA han...@metrosystems.co.jp
wrote:
Fixed version is attached.

I reviewed your latest git version, that is a bit newer than the attached
patch.
http://git.postgresql.org/gitweb?p=users/hanada/postgres.git;a=commit;h=0e1a1e1b0e168cb3d8ff4d637747d0ba8f7b8d55

The code still works with small adjustment, but needs to be rebased on the
latest master, especially for extension support and copy API changes.

Here are a list of comments and suggestions:

Thanks for the comments. Revised version of patch has been attached.

* You might forget some combination or unspecified options in
file_fdw_validator().
For example, format == NULL or !csv header cases. I've not tested all
cases, but please recheck validations used in BeginCopy().

Right, I've revised validation based on BeginCopy(), and added
regression tests about validation.

* estimate_costs() needs own row estimation rather than using baserel.
What value does baserel-tuples have?
Foreign tables are never analyzed for now. Is the number correct?
No, baserel-tuples is always 0, and baserel-pages is 0 too.
In addition, width and rows are set to 100 and 1, as default values.

It means baserel is not reliable at all, right?

Right, tables which has not been ANALYZEd have default stats in
baserel. But getting # of records needs another parsing for the file...

If so, we need alternative
solution in estimate_costs(). We adjust statistics with runtime relation
size in estimate_rel_size(). Also, we use get_rel_data_width() for not
analyzed tables. We could use similar technique in file_fdw, too.

Ah, using get_relation_data_width(), exported version of
get_rel_data_width(), seems to help estimation. I'll research around
it little more. By the way, adding ANALYZE support for foreign tables
is reasonable idea for this issue?

* Should use int64 for file byte size (or, uint32 in blocks).
unsigned long might be 32bit. ulong is not portable.

* Oid List (list_make1_oid) might be more handy than Const to hold relid
in FdwPlan.fdw_private.

* I prefer FileFdwExecutionState to FileFdwPrivate, because there are
two different 'fdw_private' variables in FdwPlan and FdwExecutionState.

* The comment in fileIterate seems wrong. It should be
/* Store the next tuple as a virtual tuple. */ , right?

* #include sys/stat.h is needed.

Fixed all of above.

Regards,
--
Shigeru Hanada

20110218-file_fdw.patch.gz
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] contrib loose ends: 9.0 to 9.1 incompatibilities

2011-02-18 Thread Robert Haas

On Thu, Feb 17, 2011 at 9:13 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 I wrote:
 Robert Haas robertmh...@gmail.com writes:
 I think we should try to make the state match as closely as possible,
 no matter how you got there.  Otherwise, I think we're storing up a
 host of future pain for ourselves.

 Well, if you're willing to hold your nose for the UPDATE pg_proc hack,
 we can make it so.

 I believe I've now fixed all the discrepancies between fresh installs
 and 9.0 updates of contrib modules, except for these:

 1. citext COLLATABLE option (see adjacent thread)

 2. intarray and tsearch2 use some core support functions in their
 GIN opclasses, and those support functions changed signatures in 9.1.
 The current solution to this involves having stub functions in core
 with the old signatures; when you do an upgrade from the 9.0 version
 of one of these contrib modules, its opclass will be pointing at the
 stub version instead of the preferred version.  I guess we could fix
 that with a direct UPDATE on pg_amproc but I'm not sure that's a
 good idea.  Note these functions aren't actually *members* of the
 extensions, just things it references, so the odds of future trouble
 seem pretty small.  On the other hand, if we don't do this, it's
 unclear when we'll ever be able to get rid of the stubs.

 Comments?

ISTM that the pg_amproc entries are part of the operator class, which
is owned by the extension.  So it's the upgrade script's job to leave
the operator class in the right state.

-- 
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] Add support for logging the current role

2011-02-18 Thread Robert Haas

On Thu, Feb 17, 2011 at 10:40 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 Robert Haas robertmh...@gmail.com writes:
 On Thu, Feb 17, 2011 at 4:53 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 In short, add a bit of overhead at SetUserId time in order to make this
 cheap (and accurate) in elog.c.

 As Stephen says, I think this is utterly impractical; those routines
 can't ever throw any kind of error.

 Why would they need to throw an error?  It'd be on the caller's head to
 supply the role name along with OID.  We can keep the name in a static
 buffer of size NAMEDATALEN, so don't tell me about palloc failures
 either.

OK, but there are not a small number of callers of that function, and
they don't necessarily have the correct info at hand.  For example,
you'd need to add prevUserAsText to TransactionStateData, which
doesn't seem appealing.

 The logging design as it stands seems to me to be a Rube Goldberg device
 that is probably going to have corner-case bugs quite aside from its
 possible performance issues.

I think you're overreacting.

-- 
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] WIP - Add ability to constrain backend temporary file space

2011-02-18 Thread Robert Haas

On Thu, Feb 17, 2011 at 10:17 PM, Mark Kirkwood
mark.kirkw...@catalyst.net.nz wrote:
 This is WIP, it does seem to work ok, but some areas/choices I'm not
 entirely clear about are mentioned in the patch itself. Mainly:

 - name of the guc... better suggestions welcome
 - datatype for the guc - real would be good, but at the moment the nice
 parse KB/MB/GB business only works for int

Please add this to the next CommitFest:

https://commitfest.postgresql.org/action/commitfest_view/open

With respect to the datatype of the GUC, int seems clearly correct.
Why would you want to use a float?

-- 
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] Assertion failure on UNLOGGED VIEW and SEQUENCE

2011-02-18 Thread Robert Haas

On Fri, Feb 18, 2011 at 6:42 AM, Itagaki Takahiro
itagaki.takah...@gmail.com wrote:
 UNLOGGED is defigned in OptTemp in gram.y, so the parser accepts
 CREATE UNLOGGED VIEW and CREATE UNLOGGED SEQUENCE, but they are
 actually not supported.

 =# CREATE UNLOGGED VIEW uv AS SELECT 1;
 TRAP: FailedAssertion(!(relkind == 'r' || relkind == 't'), File:
 heap.c, Line: 1246)

 =# CREATE UNLOGGED SEQUENCE us;
 TRAP: FailedAssertion(!(relkind == 'r' || relkind == 't'), File:
 heap.c, Line: 1246)

 The most easiest fix would be preventing them in parser level.

Well, that sucks.  I had intended for that to be disallowed at the
parser level, but obviously I fail.  It seems that disallowing this in
the parser will require duplicating the OptTemp production.  Or
alternatively we can just add an error check to the CREATE VIEW and
CREATE SEQUENCE productions, i.e.

if ($4 == RELPERSISTENCE_UNLOGGED)
ereport(ERROR, ...);

I am somewhat leaning toward the latter option, to avoid unnecessarily
bloating the size of the parser tables, but I can do it the other way
if people prefer.

In scrutinizing this code again, I notice that I accidentally added
the ability to create an unlogged table using SELECT INTO, as in
SELECT 1 INTO UNLOGGED foo, just as you can also do SELECT 1 INTO
TEMP foo.  I don't see any particular reason to disallow that, but I
should probably update the documentation.

-- 
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

[HACKERS] Re: [COMMITTERS] pgsql: Separate messages for standby replies and hot standby feedback.

2011-02-18 Thread Simon Riggs

On Fri, 2011-02-18 at 07:11 -0500, Robert Haas wrote:
 On Fri, Feb 18, 2011 at 6:34 AM, Simon Riggs si...@2ndquadrant.com wrote:
  Separate messages for standby replies and hot standby feedback.
  Allow messages to be sent at different times, and greatly reduce
  the frequency of hot standby feedback. Refactor to allow additional
  message types.
 
 You could refactor this some more to avoid calling
 GetCurrentTimestamp() and TimestampDifferenceExceeds() twice each, but
 I'm not sure whether it's worth it.

Thanks for the observation.

-- 
 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

1 2 >

1 - 100 of 108 matches

Mail list logo