Re: [HACKERS] WIP: RangeTypes
Updated patch. Summary of changes: * More generic functions * pg_dump support * remove typmod support until it can be done correctly * added some tests There is still quite a bit left, including (numbers match up with previous TODO list): 1. Generic functions -- still more work to do here. Handling the combination of continuous range semantics with NULLs requires quite a lot of special cases, because it's hard to share code among functions. Even something as simple as equals is not as trivial as it sounds. Perhaps I'm missing some cleaner abstractions, or perhaps I'm over-thinking the null semantics. 3. perhaps fix typmod 4. documentation 5. more tests 7. better parser Regards, Jeff Davis rangetypes-20110114.patch.gz Description: GNU Zip compressed 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] pg_depend explained
2011/1/12 Alvaro Herrera alvhe...@commandprompt.com: I think this code should live in the Wiki somewhere: http://wiki.postgresql.org/wiki/Snippets This file contains only the relevant remapping of pg_depend, folding the internal linkages properly: https://github.com/gluefinance/pov/blob/master/sql/schema/pov/views/pg_depend_remapped.sql It can be tested stand-alone and does not require any other components from the pov project. Can I create a wiki snippet myself or do I need some kind of admin access? -- Best regards, Joel Jacobson Glue Finance -- 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] pg_depend explained
On Fri, Jan 14, 2011 at 09:39, Joel Jacobson j...@gluefinance.com wrote: 2011/1/12 Alvaro Herrera alvhe...@commandprompt.com: I think this code should live in the Wiki somewhere: http://wiki.postgresql.org/wiki/Snippets This file contains only the relevant remapping of pg_depend, folding the internal linkages properly: https://github.com/gluefinance/pov/blob/master/sql/schema/pov/views/pg_depend_remapped.sql It can be tested stand-alone and does not require any other components from the pov project. Can I create a wiki snippet myself or do I need some kind of admin access? Absolutely, no admin access required. As long as you have (or create) a community account, you can edit or create pages. -- 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] Streaming base backups
On 14.01.2011 08:45, Fujii Masao wrote: On Fri, Jan 14, 2011 at 4:13 AM, Magnus Hagandermag...@hagander.net wrote: At the end of the backup by walsender, it forces a switch to a new WAL file and waits until the last WAL file has been archived. So we should change postmaster so that it doesn't cause the archiver to end before walsender ends when shutdown is requested? Um. I have to admit I'm not entirely following what you mean enough to confirm it, but it *sounds* correct :-) What scenario exactly is the problematic one? 1. Smart shutdown is requested while walsender is sending a backup. 2. Shutdown causes archiver to end. (Though shutdown sends SIGUSR2 to walsender to exit, walsender running backup doesn't respond for now) 3. At the end of backup, walsender calls do_pg_stop_backup, which forces a switch to a new WAL file and waits until the last WAL file has been archived. *BUT*, since archiver has already been dead, walsender waits for that forever. Not only does it wait forever, but it writes the end-of-backup WAL record after bgwriter has already exited and written the shutdown checkpoint record. I think postmaster should treat a walsender as a regular backend, until it has started streaming. We can achieve that by starting up the child as PM_CHILD_ACTIVE, and changing the state to PM_CHILD_WALSENDER later, when streaming is started. Looking at the postmaster.c, that should be safe, postmaster will treat a backend as a regular backend anyway until it has connected to shared memory. It is *not* safe to switch a walsender back to a regular process, but we have no need to do that. -- 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] SSI patch version 8
On 01/14/2011 02:21 AM, Kevin Grittner wrote: I hope you have no objection to having the code you wrote included in the test suite which is part of the patch. Well, if you do, I'll pull it back out and invent something similar... ;-) No objection. - Anssi -- 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] [RRR] reviewers needed!
Robert Haas robertmh...@gmail.com writes: So far I have 6 people who have volunteered to be round-robin reviewers, and 7 people who are listed as reviewers on the CF site already. That leaves 45 patches without a reviewer, plus whatever comes in in the next day or so. This is not going to work unless a lot more people pitch in. I will enroll later this week-end or early next week, have some other duties meanwhile. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Recovery control functions
Functions to control recovery, to aid PITR and Hot Standby. pg_is_xlog_replay_paused() pg_xlog_replay_pause() pg_xlog_replay_resume() recovery.conf parameter: pause_at_recovery_target (bool) -- Simon Riggs http://www.2ndQuadrant.com/books/ PostgreSQL Development, 24x7 Support, Training and Services diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 17a1d34..5b90e9e 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -42,7 +42,8 @@ ALTER TABLE replaceable class=PARAMETERname/replaceable ALTER [ COLUMN ] replaceable class=PARAMETERcolumn/replaceable SET ( replaceable class=PARAMETERattribute_option/replaceable = replaceable class=PARAMETERvalue/replaceable [, ... ] ) ALTER [ COLUMN ] replaceable class=PARAMETERcolumn/replaceable RESET ( replaceable class=PARAMETERattribute_option/replaceable [, ... ] ) ALTER [ COLUMN ] replaceable class=PARAMETERcolumn/replaceable SET STORAGE { PLAIN | EXTERNAL | EXTENDED | MAIN } -ADD replaceable class=PARAMETERtable_constraint/replaceable +ADD replaceable class=PARAMETERtable_constraint/replaceable [ NOT VALID ] +VALIDATE CONSTRAINT replaceable class=PARAMETERconstraint_name/replaceable DROP CONSTRAINT [ IF EXISTS ] replaceable class=PARAMETERconstraint_name/replaceable [ RESTRICT | CASCADE ] DISABLE TRIGGER [ replaceable class=PARAMETERtrigger_name/replaceable | ALL | USER ] ENABLE TRIGGER [ replaceable class=PARAMETERtrigger_name/replaceable | ALL | USER ] @@ -220,11 +221,27 @@ ALTER TABLE replaceable class=PARAMETERname/replaceable /varlistentry varlistentry -termliteralADD replaceable class=PARAMETERtable_constraint/replaceable/literal/term +termliteralADD replaceable class=PARAMETERtable_constraint/replaceable + [ NOT VALID ]/literal/term listitem para This form adds a new constraint to a table using the same syntax as - xref linkend=SQL-CREATETABLE. + xref linkend=SQL-CREATETABLE. Newly added foreign key constraints can + also be defined as literalNOT VALID/literal to avoid the + potentially lengthy initial check that must otherwise be performed. + Constraint checks are skipped at create table time, so + xref linkend=SQL-CREATETABLE does not contain this option. + /para +/listitem + /varlistentry + + varlistentry +termliteralVALIDATE CONSTRAINT/literal/term +listitem + para + This form validates a foreign key constraint that was previously created + as literalNOT VALID/literal. Constraints already marked valid do not + cause an error response. /para /listitem /varlistentry diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 4c55db7..7cc521d 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -1835,6 +1835,7 @@ StoreRelCheck(Relation rel, char *ccname, Node *expr, CONSTRAINT_CHECK, /* Constraint Type */ false, /* Is Deferrable */ false, /* Is Deferred */ + true, /* Is Validated */ RelationGetRelid(rel), /* relation */ attNos, /* attrs in the constraint */ keycount, /* # attrs in the constraint */ diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 4dd89e1..a19b139 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -777,6 +777,7 @@ index_create(Oid heapRelationId, constraintType, deferrable, initdeferred, + true, heapRelationId, indexInfo-ii_KeyAttrNumbers, indexInfo-ii_NumIndexAttrs, diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index 3a38518..6619eed 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -46,6 +46,7 @@ CreateConstraintEntry(const char *constraintName, char constraintType, bool isDeferrable, bool isDeferred, + bool isValidated, Oid relId, const int16 *constraintKey, int constraintNKeys, @@ -158,6 +159,7 @@ CreateConstraintEntry(const char *constraintName, values[Anum_pg_constraint_contype - 1] = CharGetDatum(constraintType); values[Anum_pg_constraint_condeferrable - 1] = BoolGetDatum(isDeferrable); values[Anum_pg_constraint_condeferred - 1] = BoolGetDatum(isDeferred); + values[Anum_pg_constraint_convalidated - 1] = BoolGetDatum(isValidated); values[Anum_pg_constraint_conrelid - 1] = ObjectIdGetDatum(relId); values[Anum_pg_constraint_contypid - 1] = ObjectIdGetDatum(domainId); values[Anum_pg_constraint_conindid - 1] = ObjectIdGetDatum(indexRelId); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index f3bd565..8041a9b 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -254,6 +254,7 @@ static void
Re: [HACKERS] Recovery control functions
On Fri, 2011-01-14 at 11:09 +, Simon Riggs wrote: Functions to control recovery, to aid PITR and Hot Standby. pg_is_xlog_replay_paused() pg_xlog_replay_pause() pg_xlog_replay_resume() recovery.conf parameter: pause_at_recovery_target (bool) And now with the correct patch. -- Simon Riggs http://www.2ndQuadrant.com/books/ PostgreSQL Development, 24x7 Support, Training and Services diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index d15..55fa70d 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -14173,6 +14173,63 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup()); /table para +The functions shown in xref +linkend=functions-recovery-control-table control the progress of recovery. +These functions may be executed only during recovery. + /para + + table id=functions-recovery-control-table +titleRecovery Control Functions/title +tgroup cols=3 + thead + rowentryName/entry entryReturn Type/entry entryDescription/entry + /row + /thead + + tbody + row + entry +literalfunctionpg_is_xlog_replay_paused()/function/literal +/entry + entrytypebool/type/entry + entryTrue if recovery is paused. + /entry + /row + row + entry +literalfunctionpg_xlog_replay_pause()/function/literal +/entry + entrytypevoid/type/entry + entryPauses recovery immediately. + /entry + /row + row + entry +literalfunctionpg_xlog_replay_resume()/function/literal +/entry + entrytypevoid/type/entry + entryRestarts recovery if it was paused. + /entry + /row + /tbody +/tgroup + /table + + para +While recovery is paused no further database changes are applied. +If in hot standby, all queries will see the same consistent snapshot +of the database, and no query conflicts will be generated. + /para + + para +If streaming replication is disabled, the paused state may continue +indefinitely without problem. While streaming replication is in +progress WAL records will continue to be received, which will +eventually fill available disk space, depending upon the duration of +the pause, the rate of WAL generation and available disk space. + /para + + para The functions shown in xref linkend=functions-admin-dbsize calculate the disk space usage of database objects. /para diff --git a/doc/src/sgml/recovery-config.sgml b/doc/src/sgml/recovery-config.sgml index bd9dfd1..b5f9cfa 100644 --- a/doc/src/sgml/recovery-config.sgml +++ b/doc/src/sgml/recovery-config.sgml @@ -228,6 +228,31 @@ restore_command = 'copy C:\\server\\archivedir\\%f %p' # Windows /listitem /varlistentry + varlistentry id=pause-at-recovery-target + xreflabel=pause_at_recovery_target + termvarnamepause_at_recovery_target/varname +(typeboolean/type) + /term + indexterm +primaryvarnamepause_at_recovery_target/ recovery parameter/primary + /indexterm + listitem + para +Specifies whether recovery should pause when the recovery target +is reached. The default is true, if a recovery target is set. +This is intended to allow queries to be executed against the +database to check if this recovery target is the most desirable +point for recovery. The paused state can be resumed by using +functionpg_xlog_replay_resume()/ (See +xref linkend=functions-recovery-control-table), which then +causes recovery to end. If this recovery target is not the +desired stopping point, then shutdown the server, change the +recovery target settings to a later target and restart to +continue recovery. + /para + /listitem + /varlistentry + /variablelist /sect1 diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 818a59f..6a8c500 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -184,6 +184,7 @@ static char *recoveryEndCommand = NULL; static char *archiveCleanupCommand = NULL; static RecoveryTargetType recoveryTarget = RECOVERY_TARGET_UNSET; static bool recoveryTargetInclusive = true; +static bool recoveryPauseAtTarget = false; static TransactionId recoveryTargetXid; static TimestampTz recoveryTargetTime; @@ -416,6 +417,8 @@ typedef struct XLogCtlData XLogRecPtr recoveryLastRecPtr; /* timestamp of last COMMIT/ABORT record replayed (or being replayed) */ TimestampTz recoveryLastXTime; + /* Are we requested to pause recovery? */ + bool recoveryPause; slock_t info_lck; /* locks shared variables shown above */ } XLogCtlData; @@ -563,6 +566,9 @@ static void readRecoveryCommandFile(void); static void exitArchiveRecovery(TimeLineID endTLI, uint32 endLogId, uint32 endLogSeg); static bool
Re: [HACKERS] ALTER TABLE ... ADD FOREIGN KEY ... NOT ENFORCED
On Mon, 2010-12-13 at 17:15 +, Peter Geoghegan wrote: On 13 December 2010 16:08, Robert Haas robertmh...@gmail.com wrote: On Mon, Dec 13, 2010 at 10:49 AM, Simon Riggs si...@2ndquadrant.com wrote: 2. pg_validate_foreign_key('constraint name'); Returns immediately if FK is valid Returns SETOF rows that violate the constraint, or if no rows are returned it updates constraint to show it is now valid. Lock held: AccessShareLock I'm less sure about this part. I think there should be a DDL statement to validate the foreign key. The return the problem rows behavior could be done some other way, or just left to the user to write their own query. +1. I think that a DDL statement is more appropriate, because it makes the process sort of symmetrical. Patch to implement the proposed feature attached, for CFJan2011. 2 sub-command changes: ALTER TABLE foo ADD FOREIGN KEY fkoo ... NOT VALID; ALTER TABLE foo VALIDATE CONSTRAINT fkoo; -- Simon Riggs http://www.2ndQuadrant.com/books/ PostgreSQL Development, 24x7 Support, Training and Services diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml index 17a1d34..5b90e9e 100644 --- a/doc/src/sgml/ref/alter_table.sgml +++ b/doc/src/sgml/ref/alter_table.sgml @@ -42,7 +42,8 @@ ALTER TABLE replaceable class=PARAMETERname/replaceable ALTER [ COLUMN ] replaceable class=PARAMETERcolumn/replaceable SET ( replaceable class=PARAMETERattribute_option/replaceable = replaceable class=PARAMETERvalue/replaceable [, ... ] ) ALTER [ COLUMN ] replaceable class=PARAMETERcolumn/replaceable RESET ( replaceable class=PARAMETERattribute_option/replaceable [, ... ] ) ALTER [ COLUMN ] replaceable class=PARAMETERcolumn/replaceable SET STORAGE { PLAIN | EXTERNAL | EXTENDED | MAIN } -ADD replaceable class=PARAMETERtable_constraint/replaceable +ADD replaceable class=PARAMETERtable_constraint/replaceable [ NOT VALID ] +VALIDATE CONSTRAINT replaceable class=PARAMETERconstraint_name/replaceable DROP CONSTRAINT [ IF EXISTS ] replaceable class=PARAMETERconstraint_name/replaceable [ RESTRICT | CASCADE ] DISABLE TRIGGER [ replaceable class=PARAMETERtrigger_name/replaceable | ALL | USER ] ENABLE TRIGGER [ replaceable class=PARAMETERtrigger_name/replaceable | ALL | USER ] @@ -220,11 +221,27 @@ ALTER TABLE replaceable class=PARAMETERname/replaceable /varlistentry varlistentry -termliteralADD replaceable class=PARAMETERtable_constraint/replaceable/literal/term +termliteralADD replaceable class=PARAMETERtable_constraint/replaceable + [ NOT VALID ]/literal/term listitem para This form adds a new constraint to a table using the same syntax as - xref linkend=SQL-CREATETABLE. + xref linkend=SQL-CREATETABLE. Newly added foreign key constraints can + also be defined as literalNOT VALID/literal to avoid the + potentially lengthy initial check that must otherwise be performed. + Constraint checks are skipped at create table time, so + xref linkend=SQL-CREATETABLE does not contain this option. + /para +/listitem + /varlistentry + + varlistentry +termliteralVALIDATE CONSTRAINT/literal/term +listitem + para + This form validates a foreign key constraint that was previously created + as literalNOT VALID/literal. Constraints already marked valid do not + cause an error response. /para /listitem /varlistentry diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 4c55db7..7cc521d 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -1835,6 +1835,7 @@ StoreRelCheck(Relation rel, char *ccname, Node *expr, CONSTRAINT_CHECK, /* Constraint Type */ false, /* Is Deferrable */ false, /* Is Deferred */ + true, /* Is Validated */ RelationGetRelid(rel), /* relation */ attNos, /* attrs in the constraint */ keycount, /* # attrs in the constraint */ diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 4dd89e1..a19b139 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -777,6 +777,7 @@ index_create(Oid heapRelationId, constraintType, deferrable, initdeferred, + true, heapRelationId, indexInfo-ii_KeyAttrNumbers, indexInfo-ii_NumIndexAttrs, diff --git a/src/backend/catalog/pg_constraint.c b/src/backend/catalog/pg_constraint.c index 3a38518..6619eed 100644 --- a/src/backend/catalog/pg_constraint.c +++ b/src/backend/catalog/pg_constraint.c @@ -46,6 +46,7 @@ CreateConstraintEntry(const char *constraintName, char constraintType, bool isDeferrable, bool isDeferred, + bool isValidated, Oid relId, const int16 *constraintKey, int constraintNKeys, @@ -158,6 +159,7 @@ CreateConstraintEntry(const char
Re: [HACKERS] Streaming base backups
On Fri, Jan 14, 2011 at 11:19, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 14.01.2011 08:45, Fujii Masao wrote: On Fri, Jan 14, 2011 at 4:13 AM, Magnus Hagandermag...@hagander.net wrote: At the end of the backup by walsender, it forces a switch to a new WAL file and waits until the last WAL file has been archived. So we should change postmaster so that it doesn't cause the archiver to end before walsender ends when shutdown is requested? Um. I have to admit I'm not entirely following what you mean enough to confirm it, but it *sounds* correct :-) What scenario exactly is the problematic one? 1. Smart shutdown is requested while walsender is sending a backup. 2. Shutdown causes archiver to end. (Though shutdown sends SIGUSR2 to walsender to exit, walsender running backup doesn't respond for now) 3. At the end of backup, walsender calls do_pg_stop_backup, which forces a switch to a new WAL file and waits until the last WAL file has been archived. *BUT*, since archiver has already been dead, walsender waits for that forever. Not only does it wait forever, but it writes the end-of-backup WAL record after bgwriter has already exited and written the shutdown checkpoint record. I think postmaster should treat a walsender as a regular backend, until it has started streaming. We can achieve that by starting up the child as PM_CHILD_ACTIVE, and changing the state to PM_CHILD_WALSENDER later, when streaming is started. Looking at the postmaster.c, that should be safe, postmaster will treat a backend as a regular backend anyway until it has connected to shared memory. It is *not* safe to switch a walsender back to a regular process, but we have no need to do that. Seems reasonable to me. I've applied a patch that exits base backups when the postmaster is shutting down - I'm happily waiting for Heikki to submit one that changes the shutdown logic in the postmaster :-) -- 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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.
2011/1/13 KaiGai Kohei kai...@ak.jp.nec.com: I tried to pick up this patch to review. It seems to me fine, enough simple and works as explained in the implementation level, apart from reasonability of this feature. (Tom was not 100% agree with this feature 1.5month ago.) Did you check whether this updated the code for 100% of the object types where this could apply? I'm not certain whether the current regression test should be updated, or not. The pg_regress launches psql with -q option, so completionTag is always ignored. Well, I don't see any easy way of regression testing it, then. Am I missing something? Also, I don't really like the way this spreads knowledge of the completionTag out all over the backend. I think it would be better to follow the existing model used by the COPY and COMMIT commands, whereby the return value indicates what happened and standard_ProcessUtility() uses that to set the command tag. -- 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] Recovery control functions
On Fri, Jan 14, 2011 at 12:15, Simon Riggs si...@2ndquadrant.com wrote: On Fri, 2011-01-14 at 11:09 +, Simon Riggs wrote: Functions to control recovery, to aid PITR and Hot Standby. pg_is_xlog_replay_paused() pg_xlog_replay_pause() pg_xlog_replay_resume() recovery.conf parameter: pause_at_recovery_target (bool) Awesome, I've been waiting for these! :-) How hard would it be to have a pg_xlog_replay_until(xlog location or timestamp), to have it resume recovery up to that point and then pause again? -- 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] Recovery control functions
On 14.01.2011 13:15, Simon Riggs wrote: /* + * Recheck shared recoveryPause by polling. + * + * XXX It might seem we should do this via a shared Latch, but + * currently we only support one shared latch per process and + * that is already taken for Startup process. Polling is used + * in other places in xlog.c already, so not a concern. + */ There is no such limitation with latches. -- 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] Recovery control functions
On Fri, 2011-01-14 at 12:41 +0100, Magnus Hagander wrote: On Fri, Jan 14, 2011 at 12:15, Simon Riggs si...@2ndquadrant.com wrote: On Fri, 2011-01-14 at 11:09 +, Simon Riggs wrote: Functions to control recovery, to aid PITR and Hot Standby. pg_is_xlog_replay_paused() pg_xlog_replay_pause() pg_xlog_replay_resume() recovery.conf parameter: pause_at_recovery_target (bool) Awesome, I've been waiting for these! :-) How hard would it be to have a pg_xlog_replay_until(xlog location or timestamp), to have it resume recovery up to that point and then pause again? You can already do that for timestamps. What you can't do is dynamically set recovery targets via functions, since currently that is set via recovery.conf parameters. Which requires restart. Jaime has a separate patch about recovery targets as well, which does some more of what you want. Some things are straightforward, some things require overhaul of the recovery.conf mechanisms, which is not the right time to do that, nor have we even discussed let alone agreed what we would change it to. -- 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] Recovery control functions
On Fri, 2011-01-14 at 13:47 +0200, Heikki Linnakangas wrote: On 14.01.2011 13:15, Simon Riggs wrote: /* + * Recheck shared recoveryPause by polling. + * + * XXX It might seem we should do this via a shared Latch, but + * currently we only support one shared latch per process and + * that is already taken for Startup process. Polling is used + * in other places in xlog.c already, so not a concern. + */ There is no such limitation with latches. SIGUSR1 handler can only handle one shared latch -- 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] Recovery control functions
On 14.01.2011 14:01, Simon Riggs wrote: On Fri, 2011-01-14 at 13:47 +0200, Heikki Linnakangas wrote: On 14.01.2011 13:15, Simon Riggs wrote: /* + * Recheck shared recoveryPause by polling. + * + * XXX It might seem we should do this via a shared Latch, but + * currently we only support one shared latch per process and + * that is already taken for Startup process. Polling is used + * in other places in xlog.c already, so not a concern. + */ There is no such limitation with latches. SIGUSR1 handler can only handle one shared latch You can only *wait* for one latch at a time, but you can own more than that. AFAICS you would never need to wait for the recovery-pause-latch at the same time as the other latch. (That you can't wait for more than one latch at a time isn't a limitation of the SIGUSR1 handler either. The signal handler and the self-pipe mechanism wouldn't need any changes to support multi-latch waits. We're just missing a WaitMultipleLatches() function that would check the is_set flag on multiple latches.) -- 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] reviewers needed!
On Thu, Jan 13, 2011 at 4:54 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Jan 11, 2011 at 9:17 PM, Robert Haas robertmh...@gmail.com wrote: [ abject plea for reviewers ] [ second abject please for reviewers ] OK, I believe I've sent an off-list email to everyone who volunteered to review and asked me to assign them a patch, which totaled 11 people. If you got an email of this type, and the patch you were assigned is OK, then please go to commitfest.postgresql.org, edit your patch, and put your name next to it so other people know you've grabbed it. Then review it and post your review on -hackers. Then add a comment to the patch with your review and mark it as either Waiting on Author or Ready for Committer. If you did not get an email of this type and want one, email me and I'll assign you a patch. If you are a committer or an experience reviewer looking for a challenge, please pick up a hard patch and review it, as many people are not up to picking these up. Some good choices: Change pg_last_xlog_receive_location not to move backwards (is this safe?), SSPI client auth on non-Windows systems (needs Windows expertise), FDW API (complicated), Range Types (also complicated), ALTER EXTENSION UPGRADE (needs design comments as well as code review). At this point, our goal should be to get all patches that are marked as Needs Review to either Waiting on Author or Ready for Committer as quickly as possible. All help is appreciated! -- 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] [COMMITTERS] pgsql: Exit from base backups when shutdown is requested
On 14.01.2011 14:10, Simon Riggs wrote: Please make that the last commit in this area for next week. I want to get a base from which to solidify the sync rep code, and associated patches. I'm not saying there will be a code clash, but there may be a clash of detail or intention that will make life more difficult for testing. I'm sorry, but it doesn't work that way. There's at least one serious bug in there still (http://archives.postgresql.org/message-id/4d302326.1000...@enterprisedb.com), and the multiple-concurrent-backups patch is pending review/commit. And there's one more feature me Magnus would like to get in before the release: including all the WAL files required to restore from the backup in the backup tar itself - we'll see if that makes the release or not. None of that should conflict seriously with the synch rep code, although it's hard to tell without seeing the patch. And even if it does, we'll just resolve the conflict, merge conflicts are not such a big deal. -- 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] Recovery control functions
On Fri, 2011-01-14 at 14:08 +0200, Heikki Linnakangas wrote: On 14.01.2011 14:01, Simon Riggs wrote: On Fri, 2011-01-14 at 13:47 +0200, Heikki Linnakangas wrote: On 14.01.2011 13:15, Simon Riggs wrote: /* + * Recheck shared recoveryPause by polling. + * + * XXX It might seem we should do this via a shared Latch, but + * currently we only support one shared latch per process and + * that is already taken for Startup process. Polling is used + * in other places in xlog.c already, so not a concern. + */ There is no such limitation with latches. SIGUSR1 handler can only handle one shared latch You can only *wait* for one latch at a time, but you can own more than that. AFAICS you would never need to wait for the recovery-pause-latch at the same time as the other latch. Yes, I understand. Trouble is, if you wait on Latch X and other processes send wakeup assuming you were waiting on Latch Y, then this will erroneously wake you up. So a process can have more than one shared latch, BUT other processes don't know and can't tell which latch you're waiting on. Yes, you can get around that, but not via the direct support of the latch module, as currently written. Polling is fine in that case, and we already do elsewhere, so it wasn't critical for me to extend latch support to implement this. (That you can't wait for more than one latch at a time isn't a limitation of the SIGUSR1 handler either. The signal handler and the self-pipe mechanism wouldn't need any changes to support multi-latch waits. We're just missing a WaitMultipleLatches() function that would check the is_set flag on multiple latches.) Something for the future. -- 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] [COMMITTERS] pgsql: Exit from base backups when shutdown is requested
On Fri, Jan 14, 2011 at 13:16, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 14.01.2011 14:10, Simon Riggs wrote: Please make that the last commit in this area for next week. I want to get a base from which to solidify the sync rep code, and associated patches. I'm not saying there will be a code clash, but there may be a clash of detail or intention that will make life more difficult for testing. I'm sorry, but it doesn't work that way. There's at least one serious bug in there still (http://archives.postgresql.org/message-id/4d302326.1000...@enterprisedb.com), and the multiple-concurrent-backups patch is pending review/commit. And there's one more feature me Magnus would like to get in before the release: including all the WAL files required to restore from the backup in the backup tar itself - we'll see if that makes the release or not. There's one more before that, which is the walsender parser patch. I intend to get that in RSN, but the include all wal files will be a while, though, I don't think either of us has started that one? -- 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] SQL/MED - FDW API
Attached are rebased version of patches for FDW API. To make review easier, I split core functionality into 3 patches. Please apply these patches in the following order. 1) fdw_handler - this patch adds HANDLER option to both syntax and catalog of FOREIGN DATA WRAPPER. 2) foreign_scan - this patch adds following: ForeignScan executor-node, hooks in planner and executor, and FdwRoutine (FDW API). 3) fdw_catalog_lookup - this patch adds GetForeignTable() whicch returns ForeignTable object, similar to GetForeignDataWrapper(), GetForeignServer(), and GetUserMapping(). This function is assumed to be used by FDWs. You would be able to test these patches with 20110114 version of file_fdw wrapper patches which will be posted in another thread. Regards, -- Shigeru Hanada 20110114-fdw_handler.patch.gz Description: Binary data 20110114-foreign_scan.patch.gz Description: Binary data 20110114-fdw_catalog_lookup.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
[HACKERS] Re: [COMMITTERS] pgsql: Exit from base backups when shutdown is requested
On Fri, 2011-01-14 at 14:16 +0200, Heikki Linnakangas wrote: On 14.01.2011 14:10, Simon Riggs wrote: Please make that the last commit in this area for next week. I want to get a base from which to solidify the sync rep code, and associated patches. I'm not saying there will be a code clash, but there may be a clash of detail or intention that will make life more difficult for testing. I'm sorry, but it doesn't work that way. There's at least one serious bug in there still I'm not clear why you haven't submitted this code to the CommitFest, especially if it clearly hasn't been very well tested. None of that should conflict seriously with the synch rep code, although it's hard to tell without seeing the patch. And even if it does, we'll just resolve the conflict, merge conflicts are not such a big deal. It's even easier if you don't cause them in the first place. My request is not unreasonable. Please be helpful, as you would expect me to be if the roles were reversed. -- 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] Recovery control functions
On 14.01.2011 14:18, Simon Riggs wrote: On Fri, 2011-01-14 at 14:08 +0200, Heikki Linnakangas wrote: On 14.01.2011 14:01, Simon Riggs wrote: On Fri, 2011-01-14 at 13:47 +0200, Heikki Linnakangas wrote: On 14.01.2011 13:15, Simon Riggs wrote: /* + * Recheck shared recoveryPause by polling. + * + * XXX It might seem we should do this via a shared Latch, but + * currently we only support one shared latch per process and + * that is already taken for Startup process. Polling is used + * in other places in xlog.c already, so not a concern. + */ There is no such limitation with latches. SIGUSR1 handler can only handle one shared latch You can only *wait* for one latch at a time, but you can own more than that. AFAICS you would never need to wait for the recovery-pause-latch at the same time as the other latch. Yes, I understand. Trouble is, if you wait on Latch X and other processes send wakeup assuming you were waiting on Latch Y, then this will erroneously wake you up. The signal will wake up the process, but WaitLatch will quickly go back to sleep if it wasn't for the latch we're waiting for. I don't think that causes any meaningful performance issues. So that's all handled within the latch code. -- 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] Re: [COMMITTERS] pgsql: Exit from base backups when shutdown is requested
On Fri, Jan 14, 2011 at 7:25 AM, Simon Riggs si...@2ndquadrant.com wrote: My request is not unreasonable. Please be helpful, as you would expect me to be if the roles were reversed. I can't remember anyone other than you ever requesting that. -- 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] [COMMITTERS] pgsql: Exit from base backups when shutdown is requested
On 14.01.2011 14:25, Simon Riggs wrote: On Fri, 2011-01-14 at 14:16 +0200, Heikki Linnakangas wrote: None of that should conflict seriously with the synch rep code, although it's hard to tell without seeing the patch. And even if it does, we'll just resolve the conflict, merge conflicts are not such a big deal. It's even easier if you don't cause them in the first place. My request is not unreasonable. Please be helpful, as you would expect me to be if the roles were reversed. Feel free to submit the patch against the snapshot of today. I will fix any merge conflict that arises from changes to the base-backup stuff. -- 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] Recovery control functions
On Fri, 2011-01-14 at 14:27 +0200, Heikki Linnakangas wrote: Trouble is, if you wait on Latch X and other processes send wakeup assuming you were waiting on Latch Y, then this will erroneously wake you up. The signal will wake up the process, but WaitLatch will quickly go back to sleep if it wasn't for the latch we're waiting for. I don't think that causes any meaningful performance issues. Neither does polling. So that's all handled within the latch code. I'll adjust the comment. -- 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] Allowing multiple concurrent base backups
On Thu, Jan 13, 2011 at 22:32, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: On 13.01.2011 22:57, Josh Berkus wrote: On 1/13/11 12:11 PM, Robert Haas wrote: That's going to depend on the situation. If the database fits in memory, then it's just going to work. If it fits on disk, it's less obvious whether it'll be good or bad, but an arbitrary limitation here doesn't serve us well. FWIW, if we had this feature right now in 9.0 we (PGX) would be using it. We run into the case of DB in memory, multiple slaves fairly often these days. Anyway, here's an updated patch with all the known issues fixed. It's kind of crude that do_pg_stop_backup() has to parse the backuplabel with sscanf() when it's coming from a non-exclusive backup - we could pass the location as a parameter instead, to make it cleaner. There might be a point to it being the same in both codepaths, but I'm not sure it's that important... Doesn't this code put the backup label in *every* tarfile, and not just the main one? From what I can tell the code in SendBackupDirectory() relies on labelfile being NULL in tar files for other tablespaces, but the caller never sets this. Finally, I'd move the addition of the labelfile to it's own function, but that's just me ;) -- 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] Error code for terminating connection due to conflict with recovery
On Thu, Jan 13, 2011 at 7:31 PM, Tatsuo Ishii is...@postgresql.org wrote: Tatsuo Ishii is...@postgresql.org writes: Please add this to the currently open CommitFest: https://commitfest.postgresql.org/action/commitfest_view/open Done. Comments are welcome. Unless there's objection, I will commit it this weekend. If you're expecting anyone to actually *review* it during the CF, that's a bit premature. No problem to wait for longer. I will wait by the end of January for the present. Review: The only possible point of concern I see here is the naming of the C identifier. Everything else in class 40 uses ERRCODE_T_R_whatever, with T_R standing for transaction rollback. It's not obvious to me that that convention has any real value, but perhaps we ought to follow it here for the sake of consistency? -- 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] Database file copy
Alvaro Herrera wrote: Excerpts from Bruce Momjian's message of jue ene 13 00:05:53 -0300 2011: Srini Raghavan wrote: Thank you very much for reviewing, appreciate the feedback.? As pointed out by you, it is always best to test it out with the latest version, so, I tested the same approach with postgres 9.0.2 on windows just now, and it works! I forgot to mention earlier that in addition to setting vacuum_freeze_table_age to 0, vacuum_freeze_min_age must also be set to 0 to reset xmin with the FrozenXid. I wonder if you should be using VACUUM FREEZE instead of having to set variables. The documentation says you shouldn't: FREEZE Selects aggressive freezing of tuples. Specifying FREEZE is equivalent to performing VACUUM with the vacuum_freeze_min_age parameter set to zero. The FREEZE option is deprecated and will be removed in a future release; set the parameter instead. http://www.postgresql.org/docs/9.0/static/sql-vacuum.html I didn't know that. I added the -z(freeze) option to vacuumdb in 8.4 for use by pg_upgrade. I think the original idea was that people should never need to freeze anything, but it turns out pg_upgrade and this user need it so maybe depricating is not a good idea. I guess pg_upgrade could call vacuumdb with a PGOPTIONS flag to force a vacuum_freeze_min_age value. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- 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] LOCK for non-tables
On Tue, Jan 11, 2011 at 8:31 PM, Robert Haas robertmh...@gmail.com wrote: In that case, can I have some comments on approaches mentioned in my OP? Tom - I am willing to implement this if you think it's valuable, but I'd like your input on the syntax. http://archives.postgresql.org/pgsql-hackers/2011-01/msg00472.php -- 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] Database file copy
On Fri, Jan 14, 2011 at 9:13 AM, Bruce Momjian br...@momjian.us wrote: Alvaro Herrera wrote: Excerpts from Bruce Momjian's message of jue ene 13 00:05:53 -0300 2011: Srini Raghavan wrote: Thank you very much for reviewing, appreciate the feedback.? As pointed out by you, it is always best to test it out with the latest version, so, I tested the same approach with postgres 9.0.2 on windows just now, and it works! I forgot to mention earlier that in addition to setting vacuum_freeze_table_age to 0, vacuum_freeze_min_age must also be set to 0 to reset xmin with the FrozenXid. I wonder if you should be using VACUUM FREEZE instead of having to set variables. The documentation says you shouldn't: FREEZE Selects aggressive freezing of tuples. Specifying FREEZE is equivalent to performing VACUUM with the vacuum_freeze_min_age parameter set to zero. The FREEZE option is deprecated and will be removed in a future release; set the parameter instead. http://www.postgresql.org/docs/9.0/static/sql-vacuum.html I didn't know that. I added the -z(freeze) option to vacuumdb in 8.4 for use by pg_upgrade. I think the original idea was that people should never need to freeze anything, but it turns out pg_upgrade and this user need it so maybe depricating is not a good idea. I guess pg_upgrade could call vacuumdb with a PGOPTIONS flag to force a vacuum_freeze_min_age value. I'd rather remove the deprecating warning. -- 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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.
Excerpts from Robert Haas's message of vie ene 14 08:40:07 -0300 2011: Also, I don't really like the way this spreads knowledge of the completionTag out all over the backend. I think it would be better to follow the existing model used by the COPY and COMMIT commands, whereby the return value indicates what happened and standard_ProcessUtility() uses that to set the command tag. Yeah, that looks ugly. However it's already ugly elsewhere: for example see PerformPortalFetch. I am not sure if it should be this patch's responsability to clean that stuff up. (Maybe we should decree that at least this patch shouldn't make the situation worse.) -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] Database file copy
Excerpts from Robert Haas's message of vie ene 14 11:18:16 -0300 2011: I'd rather remove the deprecating warning. +1 -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.
On Fri, Jan 14, 2011 at 9:24 AM, Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Robert Haas's message of vie ene 14 08:40:07 -0300 2011: Also, I don't really like the way this spreads knowledge of the completionTag out all over the backend. I think it would be better to follow the existing model used by the COPY and COMMIT commands, whereby the return value indicates what happened and standard_ProcessUtility() uses that to set the command tag. Yeah, that looks ugly. However it's already ugly elsewhere: for example see PerformPortalFetch. I am not sure if it should be this patch's responsability to clean that stuff up. (Maybe we should decree that at least this patch shouldn't make the situation worse.) Agreed: it's not the patch's job to clean it up, but it shouldn't make the situation worse. -- 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] Database file copy
Alvaro Herrera alvhe...@commandprompt.com wrote: Excerpts from Robert Haas's message: I'd rather remove the deprecating warning. +1 +1 -Kevin -- 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
Tom Lane t...@sss.pgh.pa.us writes: Another little problem with the quick and dirty solution is that stuff that's important enough to warrant a log_line_prefix escape is generally thought to be important enough to warrant inclusion in CSV logs. That would imply adding a column and taking the resultant compatibility hit. Well if we're down to adding columns to the CSV format, what about adding an explicit column where to output the query duration as an interval literal, rather than putting it in the query string (IIRC)? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] kill -KILL: What happens?
Excerpts from Robert Haas's message of vie ene 14 00:03:53 -0300 2011: On Thu, Jan 13, 2011 at 8:28 PM, Tom Lane t...@sss.pgh.pa.us wrote: True. It strikes me also that the postmaster does provide some services other than accepting new connections: * ensuring that everybody gets killed if a backend crashes While you could probably live without these in the scenario of let my honking big query finish before restarting, you would not want to do without them in unattended operation. Yep. I'm pretty doubtful that you're going to want them even in that case, but you're surely not going to want them in unattended operation. I'm sure you don't want that. The reason postmaster causes a restart of all backends in case one of them crashes is that it could have left some corrupted state behind. If postmaster dies, and then another backend crashes, then your backend running your honking big query could run across corrupted state and then you'd be in serious trouble. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] Error code for terminating connection due to conflict with recovery
Review: The only possible point of concern I see here is the naming of the C identifier. Everything else in class 40 uses ERRCODE_T_R_whatever, with T_R standing for transaction rollback. It's not obvious to me that that convention has any real value, but perhaps we ought to follow it here for the sake of consistency? Yeah. Actually at first I used T_R convention. After a few seconds thought, I realized that T_R is not appropreate by the same reason you feel. Possible other argument might be Terminating connection always involves transaction rollback. So using T_R is ok. I'm not sure this argument is reasonable enough though. -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese: http://www.sraoss.co.jp -- 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] kill -KILL: What happens?
Alvaro Herrera alvhe...@commandprompt.com wrote: If postmaster dies, and then another backend crashes, then your backend running your honking big query could run across corrupted state and then you'd be in serious trouble. Worst of all, it could give bogus results without error. I really don't see a production use case for letting backends continue after postmaster failure -- unless you only kinda, sorta care whether committed data is actually retrievable or reported data is actually accurate. -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Maintenance downtime for commitfest.postgresql.org and media.postgresql.org
Hi all! In preperation of some further improvments to our infrastructure, the sysadmin team needs to move coridian.postgresql.org (aka commitfest.postgresql.org) and mintaka.postgresql.org (media.postgresql.org) to a different host within the same datacenter. We are planning to do that move starting Sunday January 16th 12:00 to 13:00 (GMT). The expected downtime is ~30min per VM and we will send a all is good again after the work is done - it would be good to not make any chances to the commitfest interface until you see that notice. In preparation for the move we also have reduced the TTLs of the affected records down to 5 minutes to make the DNS-migration go as fast as possible. Stefan -- 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] kill -KILL: What happens?
On Jan14, 2011, at 17:22 , Kevin Grittner wrote: Alvaro Herrera alvhe...@commandprompt.com wrote: If postmaster dies, and then another backend crashes, then your backend running your honking big query could run across corrupted state and then you'd be in serious trouble. Worst of all, it could give bogus results without error. I really don't see a production use case for letting backends continue after postmaster failure -- unless you only kinda, sorta care whether committed data is actually retrievable or reported data is actually accurate. I gather that the behaviour we want is for normal backends to exit once the postmaster is gone, and for utility processes (bgwriter, ...) to exit once all the backends are gone. The test program I posted in this thread proves that FIFOs and select() can be used to implement this, if we're ready to check for EOF on the socket in CHECK_FOR_INTERRUPTS() every few seconds. Is this a viable route to take? best regards, Florian Pflug -- 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
On 01/12/2011 08:59 PM, Tom Lane wrote: I'm not actually concerned about adding a few extra cycles to SET ROLE for this. What bothered me more was the cost of adding another output column to CSV log mode. That's not something you're going to be able to finesse such that only people who care pay the cost. I think it's time to revisit the design of CSV logs again, now we have two or three releases worth of experience with it. It needs some flexibility and refinement. 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] Add support for logging the current role
* Tom Lane (t...@sss.pgh.pa.us) wrote: I seem to recall that the assign hook for role stores the string form of the role name anyway. Indeed it does, and it's already exposed through show_role() since it's needed in guc.c. Based on my review and understanding of the comments and calls, it also doesn't do anything particularly complicated or any syscache searches or anything. It wouldn't track the effects of RENAME ROLE against an actively-used role, but then again neither does %u. Right, I didn't specifically point that out in the documentation changes, but I can if people feel it's neceessary. What bothered me more was the cost of adding another output column to CSV log mode. That's not something you're going to be able to finesse such that only people who care pay the cost. I definitely feel this is something that we should be logging in the CSV also, and you're right, there doesn't appear to be a way to do that without just outright changing the format and causing people to have to update anything/everything that uses it. I have a hard time with the idea that we'll commit to never changing that format though, so do we want to provide a way for users to specify the format (ala log_line_prefix), or just ask users to expect and deal with format changes when they happen..? I noticed Dimitri would like another change to the CSV log format (which looked reasonable to me, asking to have something split out from the query string itself), it'd certainly be better to change both in the same release than split them across two (of course, we might come up with something else in the future...). I have to admit to being a bit suprised the CSV logging wasn't implemented with a 'format' type option. I'm not sure if I have the cycles or even if we would want to try and add that now, but it strikes me as something we should probably do. Updated patch attached, included new comments for elog.c too. Thanks, Stephen commit 4e27ab79ef9b0d0c3c9824d672e06160dd227cc2 Author: Stephen Frost sfr...@snowman.net Date: Wed Jan 12 12:22:16 2011 -0500 Improve comments at the top of elog.c Add in some comments about how certain usually available backend systems may be unavailable or which won't function properly in elog.c due to the current transaction being in a failed state. commit d3ca4063ba8e16930278947c32c336b5b80cdaba Author: Stephen Frost sfr...@snowman.net Date: Fri Jan 14 11:19:45 2011 -0500 Add %U option to log_line_prefix for current role This adds a new option to log_line_prefix (%U) to allow the current role to be logged, which is valuable information when an application or user is using SET ROLE and roles which are set 'noinherit'. This also changes the current definition of %u to be 'Session user', to avoid confusion when a superuser uses 'SET SESSION AUTHORIZATION'. Otherwise, a log might read 'login_user none' but actually be running as a different user due to SET SESSION AUTHORIZATION. The 'username' field for CSV logging was also updated to be 'Session user'. Note: SET SESSION AUTHORIZATION is only allowed for superusers, and the logged username will only change if SET SESSION AUTHORIZATION is called, so this is unlikely to have signifigant user impact. Last, but certainly not least, role_name was added as a new column to the CSV log output and corresponding example CSV table definition. This is a user-visible change which should be called out in the release notes. *** a/doc/src/sgml/config.sgml --- b/doc/src/sgml/config.sgml *** *** 3504,3510 local0.*/var/log/postgresql /row row entryliteral%u/literal/entry ! entryUser name/entry entryyes/entry /row row --- 3504,3523 /row row entryliteral%u/literal/entry ! entrySession user name, typically the user name which was used ! to authenticate to productnamePostgreSQL/productname with, ! but can be changed by a superuser, see commandSET SESSION ! AUTHORIZATION//entry ! entryyes/entry ! /row ! row ! entryliteral%U/literal/entry ! entryCurrent role name, when set with commandSET ROLE/; ! the current role identifier is relevant for permission checking; ! Returns 'none' if the current role matches the session user. ! Note: Log messages from inside literalSECURITY DEFINER/ ! functions will show the calling role, not the effective role ! inside the literalSECURITY DEFINER/ function/entry entryyes/entry /row row *** *** 3731,3737 FROM pg_stat_activity; (acronymCSV/) format, with these columns:
Re: [HACKERS] Add support for logging the current role
Andrew Dunstan and...@dunslane.net writes: On 01/12/2011 08:59 PM, Tom Lane wrote: I'm not actually concerned about adding a few extra cycles to SET ROLE for this. What bothered me more was the cost of adding another output column to CSV log mode. That's not something you're going to be able to finesse such that only people who care pay the cost. I think it's time to revisit the design of CSV logs again, now we have two or three releases worth of experience with it. It needs some flexibility and refinement. It would definitely be nice to support optional columns a little better. I'm not even sure whether the runtime overhead is worth worrying about (maybe it is, maybe it isn't, I have no data). But I do know that adding a column to the CSV output format spec causes a flag day for users. How can we avoid that? 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] kill -KILL: What happens?
On Fri, Jan 14, 2011 at 11:28 AM, Florian Pflug f...@phlo.org wrote: I gather that the behaviour we want is for normal backends to exit once the postmaster is gone, and for utility processes (bgwriter, ...) to exit once all the backends are gone. The test program I posted in this thread proves that FIFOs and select() can be used to implement this, if we're ready to check for EOF on the socket in CHECK_FOR_INTERRUPTS() every few seconds. Is this a viable route to take? I don't think there's much point in getting excited about the order in which things exit. If we're agreed (and we seem to be, modulo Tom) that the backends should exit quickly if the postmaster dies, then worrying about whether the utility processes exit slightly before or slightly after that doesn't excite me very much. -- 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] Error code for terminating connection due to conflict with recovery
On Fri, Jan 14, 2011 at 10:53 AM, Tatsuo Ishii is...@postgresql.org wrote: Review: The only possible point of concern I see here is the naming of the C identifier. Everything else in class 40 uses ERRCODE_T_R_whatever, with T_R standing for transaction rollback. It's not obvious to me that that convention has any real value, but perhaps we ought to follow it here for the sake of consistency? Yeah. Actually at first I used T_R convention. After a few seconds thought, I realized that T_R is not appropreate by the same reason you feel. Possible other argument might be Terminating connection always involves transaction rollback. So using T_R is ok. I'm not sure this argument is reasonable enough though. Looking at this a bit more carefully, I notice that there are two cases when a recovery conflict occurs: - we cancel the currently running statement, or - we kill the whole connection Should those use the same error code, or different ones? This patch doesn't appear to update all the places where recovery conflicts can occur, which is probably not ideal. -- 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
* Tom Lane (t...@sss.pgh.pa.us) wrote: Andrew Dunstan and...@dunslane.net writes: I think it's time to revisit the design of CSV logs again, now we have two or three releases worth of experience with it. It needs some flexibility and refinement. It would definitely be nice to support optional columns a little better. I'm not even sure whether the runtime overhead is worth worrying about (maybe it is, maybe it isn't, I have no data). But I do know that adding a column to the CSV output format spec causes a flag day for users. How can we avoid that? My first thought would be to have a 'log_csv_format' GUC that's very similar to 'log_line_prefix' (and uses the same variables if possible..). We could then ship a default in postgresql.conf that matches what the current format is while adding the other options if people want to use them. If we could have all the processing to generate that line go through the same function for log_line_prefix and log_csv_format, that'd be even better. Makes me tempted to throw out the current notion of 'log_line_*prefix*' and replace it with 'log_line_*format*' to match exactly the 'log_csv_format' that I'm proposing. That'd undoubtably cause more user headaches tho... :( Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Error code for terminating connection due to conflict with recovery
Tatsuo Ishii is...@postgresql.org writes: Review: The only possible point of concern I see here is the naming of the C identifier. Everything else in class 40 uses ERRCODE_T_R_whatever, with T_R standing for transaction rollback. It's not obvious to me that that convention has any real value, but perhaps we ought to follow it here for the sake of consistency? Yeah. Actually at first I used T_R convention. After a few seconds thought, I realized that T_R is not appropreate by the same reason you feel. Possible other argument might be Terminating connection always involves transaction rollback. So using T_R is ok. I'm not sure this argument is reasonable enough though. This is not only a matter of some macro name or other. According to the SQL standard, class 40 itself is defined as transaction rollback. If the error condition can't reasonably be regarded as a subcase of that, you're making a bad choice of SQLSTATE code. 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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.
Alvaro Herrera alvhe...@commandprompt.com writes: Excerpts from Robert Haas's message of vie ene 14 08:40:07 -0300 2011: Also, I don't really like the way this spreads knowledge of the completionTag out all over the backend. I think it would be better to follow the existing model used by the COPY and COMMIT commands, whereby the return value indicates what happened and standard_ProcessUtility() uses that to set the command tag. Yeah, that looks ugly. However it's already ugly elsewhere: for example see PerformPortalFetch. I am not sure if it should be this patch's responsability to clean that stuff up. (Maybe we should decree that at least this patch shouldn't make the situation worse.) I thought we were going to reject the patch outright anyway. The compatibility consequences of changing command tags are not worth the benefit, independently of how ugly the backend-side code may or may not be. 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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.
On Fri, 2011-01-14 at 12:07 -0500, Tom Lane wrote: I thought we were going to reject the patch outright anyway. The compatibility consequences of changing command tags are not worth the benefit, independently of how ugly the backend-side code may or may not be. +1 -- 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] Database file copy
Kevin Grittner kevin.gritt...@wicourts.gov writes: Alvaro Herrera alvhe...@commandprompt.com wrote: I'd rather remove the deprecating warning. +1 +1 The reason for wanting to deprecate and ultimately remove that syntax is so we can get rid of FREEZE as a reserved word. We could probably still allow the new-style syntax VACUUM (FREEZE) ... but VACUUM FREEZE really needs to be killed. pg_upgrade is NOT a good reason to have a nonstandard reserved word in the grammar. 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] Re: [COMMITTERS] pgsql: Exit from base backups when shutdown is requested
Robert Haas robertmh...@gmail.com writes: On Fri, Jan 14, 2011 at 7:25 AM, Simon Riggs si...@2ndquadrant.com wrote: My request is not unreasonable. Please be helpful, as you would expect me to be if the roles were reversed. I can't remember anyone other than you ever requesting that. Yes. As Heikki said, it doesn't work that way. 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
[HACKERS] Add ENCODING option to COPY
Here's the patch to add ENCODING option to COPY command. The fundamental issue was explained months ago: http://archives.postgresql.org/message-id/AANLkTikCt6bHXZjO_oX+JS7+G=jaq7gvzpu0owjcs...@mail.gmail.com In short, client_encoding is not appropriate for copy operation so we should need the specialized option for it. Robert Haas agreed with its need later in the thread. Thanks. The patch overrides client_encoding by the added ENCODING option, and restores it as soon as copy is done. I see some complaints ask to use pg_do_encoding_conversion() instead of pg_client_to_server/server_to_client(), but the former will surely add slight overhead per reading line and I believe copy is performance-sensitive command. I'll add this to the CF app. Regards, -- Hitoshi Harada copy_encoding.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] Error code for terminating connection due to conflict with recovery
On Fri, 2011-01-14 at 12:04 -0500, Tom Lane wrote: Tatsuo Ishii is...@postgresql.org writes: Review: The only possible point of concern I see here is the naming of the C identifier. Everything else in class 40 uses ERRCODE_T_R_whatever, with T_R standing for transaction rollback. It's not obvious to me that that convention has any real value, but perhaps we ought to follow it here for the sake of consistency? Yeah. Actually at first I used T_R convention. After a few seconds thought, I realized that T_R is not appropreate by the same reason you feel. Possible other argument might be Terminating connection always involves transaction rollback. So using T_R is ok. I'm not sure this argument is reasonable enough though. This is not only a matter of some macro name or other. According to the SQL standard, class 40 itself is defined as transaction rollback. If the error condition can't reasonably be regarded as a subcase of that, you're making a bad choice of SQLSTATE code. This whole thing is confused. No change is appropriate here at all. We issue ERRCODE_T_R_SERIALIZATION_FAILURE almost all of the time for recovery conflicts. We issue ERRCODE_ADMIN_SHUTDOWN only if the conflict is non-retryable, which occurs if someone drops the database that the user was connected to when they get kicked off. That code exists specifically to inform the user that they *cannot* reconnect. So pgpool should not be trying to trap that error and reconnect. So the whole basis for the existence of this patch is moot. -- 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] Database file copy
Tom Lane t...@sss.pgh.pa.us wrote: The reason for wanting to deprecate and ultimately remove that syntax is so we can get rid of FREEZE as a reserved word. We could probably still allow the new-style syntax VACUUM (FREEZE) Oh, OK. I can go along with that. If we're going that route, though, shouldn't we be getting support for the new syntax added, so there can be a release or two supporting both? -Kevin -- 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] limiting hint bit I/O
Robert Haas robertmh...@gmail.com writes: On Thu, Jan 13, 2011 at 10:43 PM, Tom Lane t...@sss.pgh.pa.us wrote: This appears to remove the BM_JUST_DIRTIED logic. Please explain why that's not completely broken. Even if it isn't completely broken, it would seem better to do something like that as a separate patch. Well, the only point of BM_JUST_DIRTIED is to detect whether BM_DIRTY has been set while a buffer write is in progress. With this patch, only BM_HINT_BITS can be set while the buffer write is in progress; BM_DIRTY cannot. Perhaps one could make the argument that this would be a good cleanup anyway: in the unpatched code, BM_DIRTY can only be set while a buffer I/O is in progress if it is set due to a hint-bit update, and then we don't really care if the update gets lost. Although that seems a bit confusing... [ thinks some more... ] If memory serves, the BM_JUST_DIRTIED mechanism dates from a time when checkpoints would write dirty buffers without taking any lock on them; if somebody changed the page meanwhile, the buffer was just considered to remain dirty. We later decided that was a bad idea and set up the current arrangement whereby only hint-bit changes are allowed while a write is in progress. So you're right that it would be dead code if we don't consider that a hint-bit change is really dirtying the page. I'm not for removing it altogether though, because it seems like something we could possibly want again in the future (for instance, we might decide to go back to write-without-lock to reduce lock contention). It's not like we are short of buffer flag bits. Moreover this whole business of not treating hint-bit setting as a page-dirtying operation is completely experimental/unproven IMO, so it would be better to keep the patch footprint as small as possible. I'd suggest leaving BM_JUST_DIRTIED as-is and just adding BM_HINT_BITS_DIRTY as a new flag. 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] limiting hint bit I/O
On Fri, Jan 14, 2011 at 12:47 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Jan 13, 2011 at 10:43 PM, Tom Lane t...@sss.pgh.pa.us wrote: This appears to remove the BM_JUST_DIRTIED logic. Please explain why that's not completely broken. Even if it isn't completely broken, it would seem better to do something like that as a separate patch. Well, the only point of BM_JUST_DIRTIED is to detect whether BM_DIRTY has been set while a buffer write is in progress. With this patch, only BM_HINT_BITS can be set while the buffer write is in progress; BM_DIRTY cannot. Perhaps one could make the argument that this would be a good cleanup anyway: in the unpatched code, BM_DIRTY can only be set while a buffer I/O is in progress if it is set due to a hint-bit update, and then we don't really care if the update gets lost. Although that seems a bit confusing... [ thinks some more... ] If memory serves, the BM_JUST_DIRTIED mechanism dates from a time when checkpoints would write dirty buffers without taking any lock on them; if somebody changed the page meanwhile, the buffer was just considered to remain dirty. We later decided that was a bad idea and set up the current arrangement whereby only hint-bit changes are allowed while a write is in progress. So you're right that it would be dead code if we don't consider that a hint-bit change is really dirtying the page. I'm not for removing it altogether though, because it seems like something we could possibly want again in the future (for instance, we might decide to go back to write-without-lock to reduce lock contention). It's not like we are short of buffer flag bits. Moreover this whole business of not treating hint-bit setting as a page-dirtying operation is completely experimental/unproven IMO, so it would be better to keep the patch footprint as small as possible. I'd suggest leaving BM_JUST_DIRTIED as-is and just adding BM_HINT_BITS_DIRTY as a new flag. I have some concerns about that proposal, but it might be the right way to go. Before we get too far off into the weeds, though, let's back up and talk about something more fundamental: this seems to be speeding up the first run by 6x at the expense of slowing down many subsequent runs by 10-15%. Does that make this whole idea dead on arrival? -- 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] Allowing multiple concurrent base backups
On Thu, 2011-01-13 at 23:32 +0200, Heikki Linnakangas wrote: On 13.01.2011 22:57, Josh Berkus wrote: On 1/13/11 12:11 PM, Robert Haas wrote: That's going to depend on the situation. If the database fits in memory, then it's just going to work. If it fits on disk, it's less obvious whether it'll be good or bad, but an arbitrary limitation here doesn't serve us well. FWIW, if we had this feature right now in 9.0 we (PGX) would be using it. We run into the case of DB in memory, multiple slaves fairly often these days. Anyway, here's an updated patch with all the known issues fixed. It's good we have this as an option and I like the way you've done this. -- 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] FOR KEY LOCK foreign keys
On Jan 13, 2011, at 1:58 PM, Alvaro Herrera wrote: Something else that might be of interest: the patch as presented here does NOT solve the deadlock problem originally presented by Joel Jacobson. It does solve the second, simpler example I presented in my blog article referenced above, however. I need to have a closer look at that problem to figure out if we could fix the deadlock too. Sounds like a big win already. Should this be considered a WIP patch, though, if you still plan to look at Joel's deadlock example? Best, David -- 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] limiting hint bit I/O
Robert Haas robertmh...@gmail.com wrote: this seems to be speeding up the first run by 6x at the expense of slowing down many subsequent runs by 10-15%. If the overall throughput when measured far enough out to have hit a steady state again is anywhere in the neighborhood of the unpatched throughput, the leveling of the response times has enough value to merit the change. At least in my world. -Kevin -- 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] Database file copy
Kevin Grittner kevin.gritt...@wicourts.gov writes: Tom Lane t...@sss.pgh.pa.us wrote: The reason for wanting to deprecate and ultimately remove that syntax is so we can get rid of FREEZE as a reserved word. Oh, OK. I can go along with that. If we're going that route, though, shouldn't we be getting support for the new syntax added, so there can be a release or two supporting both? You mean like 9.0? 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] limiting hint bit I/O
Robert Haas robertmh...@gmail.com writes: On Fri, Jan 14, 2011 at 12:47 PM, Tom Lane t...@sss.pgh.pa.us wrote: Moreover this whole business of not treating hint-bit setting as a page-dirtying operation is completely experimental/unproven IMO, so it would be better to keep the patch footprint as small as possible. I have some concerns about that proposal, but it might be the right way to go. Before we get too far off into the weeds, though, let's back up and talk about something more fundamental: this seems to be speeding up the first run by 6x at the expense of slowing down many subsequent runs by 10-15%. Does that make this whole idea dead on arrival? Well, it reinforces my opinion that it's experimental ;-). But first run of what, exactly? And are you sure you're taking a wholistic view of the costs/benefits? 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] limiting hint bit I/O
On Fri, Jan 14, 2011 at 1:02 PM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: Robert Haas robertmh...@gmail.com wrote: this seems to be speeding up the first run by 6x at the expense of slowing down many subsequent runs by 10-15%. If the overall throughput when measured far enough out to have hit a steady state again is anywhere in the neighborhood of the unpatched throughput, the leveling of the response times has enough value to merit the change. At least in my world. I think it would eventually settle down to the same speed, but it might take a really long time. I got impatient before I got that far. I'm hoping some will pick it up and play with it some more (hint, hint). -- 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] limiting hint bit I/O
On Fri, Jan 14, 2011 at 1:06 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Jan 14, 2011 at 12:47 PM, Tom Lane t...@sss.pgh.pa.us wrote: Moreover this whole business of not treating hint-bit setting as a page-dirtying operation is completely experimental/unproven IMO, so it would be better to keep the patch footprint as small as possible. I have some concerns about that proposal, but it might be the right way to go. Before we get too far off into the weeds, though, let's back up and talk about something more fundamental: this seems to be speeding up the first run by 6x at the expense of slowing down many subsequent runs by 10-15%. Does that make this whole idea dead on arrival? Well, it reinforces my opinion that it's experimental ;-). But first run of what, exactly? See the test case in my OP. The runs in question are select sum(1) from s. And are you sure you're taking a wholistic view of the costs/benefits? No. -- 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] FOR KEY LOCK foreign keys
On Fri, Jan 14, 2011 at 1:00 PM, David E. Wheeler da...@kineticode.com wrote: On Jan 13, 2011, at 1:58 PM, Alvaro Herrera wrote: Something else that might be of interest: the patch as presented here does NOT solve the deadlock problem originally presented by Joel Jacobson. It does solve the second, simpler example I presented in my blog article referenced above, however. I need to have a closer look at that problem to figure out if we could fix the deadlock too. Sounds like a big win already. Should this be considered a WIP patch, though, if you still plan to look at Joel's deadlock example? Alvaro, are you planning to add this to the CF? -- 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] Re: [COMMITTERS] pgsql: Exit from base backups when shutdown is requested
On Fri, 2011-01-14 at 07:30 -0500, Robert Haas wrote: On Fri, Jan 14, 2011 at 7:25 AM, Simon Riggs si...@2ndquadrant.com wrote: My request is not unreasonable. Please be helpful, as you would expect me to be if the roles were reversed. I can't remember anyone other than you ever requesting that. Meaning what? You're playing compliment to my originality? Thanks ;-) -- 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] Database file copy
Tom Lane t...@sss.pgh.pa.us wrote: Kevin Grittner kevin.gritt...@wicourts.gov writes: shouldn't we be getting support for the new syntax added, so there can be a release or two supporting both? You mean like 9.0? Yeah, just like that. If we're going to be supporting that long term, we should probably change the note about FREEZE being deprecated, though. So, still +1 on removing the wording about FREEZE being deprecated, but instead we should mention what actually *is* deprecated (the omission of the parentheses). -Kevin -- 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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.
On Fri, Jan 14, 2011 at 12:07 PM, Tom Lane t...@sss.pgh.pa.us wrote: Alvaro Herrera alvhe...@commandprompt.com writes: Excerpts from Robert Haas's message of vie ene 14 08:40:07 -0300 2011: Also, I don't really like the way this spreads knowledge of the completionTag out all over the backend. I think it would be better to follow the existing model used by the COPY and COMMIT commands, whereby the return value indicates what happened and standard_ProcessUtility() uses that to set the command tag. Yeah, that looks ugly. However it's already ugly elsewhere: for example see PerformPortalFetch. I am not sure if it should be this patch's responsability to clean that stuff up. (Maybe we should decree that at least this patch shouldn't make the situation worse.) I thought we were going to reject the patch outright anyway. The compatibility consequences of changing command tags are not worth the benefit, independently of how ugly the backend-side code may or may not be. My previous response to this criticism was here: http://archives.postgresql.org/pgsql-hackers/2010-11/msg01899.php Your response, which seemed at least partially in agreement, is here: http://archives.postgresql.org/pgsql-hackers/2010-11/msg01901.php If we're going to reject this patch on backwards-compatibility grounds, we need to make an argument that the backward-compatibility hazards are a real concern. So, again, has anyone complained about the changes we made in this area in 9.0? And under what circumstances do we foresee someone relying on the command tag of a command that always returns the same tag? I'm as quick as anyone to bow before a compelling argument, but I don't think anyone's made such an argument. -- 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] FOR KEY LOCK foreign keys
Excerpts from David E. Wheeler's message of vie ene 14 15:00:48 -0300 2011: On Jan 13, 2011, at 1:58 PM, Alvaro Herrera wrote: Something else that might be of interest: the patch as presented here does NOT solve the deadlock problem originally presented by Joel Jacobson. It does solve the second, simpler example I presented in my blog article referenced above, however. I need to have a closer look at that problem to figure out if we could fix the deadlock too. Sounds like a big win already. Should this be considered a WIP patch, though, if you still plan to look at Joel's deadlock example? Not necessarily -- we can implement that as a later refinement/improvement. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] FOR KEY LOCK foreign keys
Excerpts from Robert Haas's message of vie ene 14 15:08:27 -0300 2011: On Fri, Jan 14, 2011 at 1:00 PM, David E. Wheeler da...@kineticode.com wrote: On Jan 13, 2011, at 1:58 PM, Alvaro Herrera wrote: Something else that might be of interest: the patch as presented here does NOT solve the deadlock problem originally presented by Joel Jacobson. It does solve the second, simpler example I presented in my blog article referenced above, however. I need to have a closer look at that problem to figure out if we could fix the deadlock too. Sounds like a big win already. Should this be considered a WIP patch, though, if you still plan to look at Joel's deadlock example? Alvaro, are you planning to add this to the CF? Eh, yes. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- 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] limiting hint bit I/O
Robert Haas robertmh...@gmail.com writes: On Fri, Jan 14, 2011 at 1:06 PM, Tom Lane t...@sss.pgh.pa.us wrote: Well, it reinforces my opinion that it's experimental ;-). But first run of what, exactly? See the test case in my OP. The runs in question are select sum(1) from s. And are you sure you're taking a wholistic view of the costs/benefits? No. Well, IMO it would be a catastrophic mistake to evaluate a patch like this on the basis of any single test case, let alone one as simplistic as that. I would observe in particular that your test case creates a table containing only one distinct value of xmin, which means that the single-transaction cache in transam.c is 100% effective, which doesn't seem to me to be a very realistic test condition. I think this is vastly understating the cost of missing hint bits. So what it needs now is a lot more testing. pg_bench might be worth trying if you want something with minimal development effort, though I'm not sure if its clog access pattern is particularly realistic either. 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] limiting hint bit I/O
Robert Haas robertmh...@gmail.com wrote: I'm hoping some will pick it up and play with it some more (hint, hint). That was a bit of a pun, eh? Anyway, there are so many ideas in this area, it's hard to keep them all straight. Personally, if I was going to start with something, it would probably be to better establish what the impact is on various workloads of *eliminating* hint bits. If the impact is negative to a significant degree, my next step might be to try background *freezing* of tuples (in a manner somewhat similar to what you've done in this test) with the hint bits gone. I know some people find them useful for forensics to a degree that they would prefer not to see this, but I think it makes sense to establish what cost people are paying every day to maintain forensic information in this format. In previous discussions there has been some talk about being able to get better forensics from WAL files if certain barriers could be overcome -- having hard numbers on the performance benefits which might also accrue might put that work in a different perspective. -Kevin -- 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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.
Robert Haas robertmh...@gmail.com writes: If we're going to reject this patch on backwards-compatibility grounds, we need to make an argument that the backward-compatibility hazards are a real concern. So, again, has anyone complained about the changes we made in this area in 9.0? That 9.0 change was far less invasive than this: it only added a count field to SELECT and CTAS result tags. Quite aside from the fact that the tag name stayed the same, in the SELECT case it's unlikely anyone would have checked the tag at all rather than just testing for PQresultStatus() == PGRES_TUPLES_OK. So it was basically only changing the result for *one* command type. I don't think it's a good basis for arguing that this patch won't cause problems. 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] limiting hint bit I/O
On Fri, Jan 14, 2011 at 1:34 PM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: Robert Haas robertmh...@gmail.com wrote: I'm hoping some will pick it up and play with it some more (hint, hint). That was a bit of a pun, eh? Unintentional... Anyway, there are so many ideas in this area, it's hard to keep them all straight. Personally, if I was going to start with something, it would probably be to better establish what the impact is on various workloads of *eliminating* hint bits. If the impact is negative to a significant degree, my next step might be to try background *freezing* of tuples (in a manner somewhat similar to what you've done in this test) with the hint bits gone. Background freezing plays havoc with Hot Standby, and this test is sufficient to show that eliminating hint bits altogether would a significant regression on some workloads. I don't think either of those ideas can get off the ground. -- 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] limiting hint bit I/O
Kevin Grittner kevin.gritt...@wicourts.gov writes: Anyway, there are so many ideas in this area, it's hard to keep them all straight. Personally, if I was going to start with something, it would probably be to better establish what the impact is on various workloads of *eliminating* hint bits. I know some people find them useful for forensics to a degree that they would prefer not to see this, Um, yeah, I think you're having a problem keeping all the ideas straight ;-). The argument about forensics has to do with how soon we're willing to freeze tuples, ie replace the XID with a constant. Not about hint bits. 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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.
On Fri, Jan 14, 2011 at 1:35 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: If we're going to reject this patch on backwards-compatibility grounds, we need to make an argument that the backward-compatibility hazards are a real concern. So, again, has anyone complained about the changes we made in this area in 9.0? That 9.0 change was far less invasive than this: it only added a count field to SELECT and CTAS result tags. Quite aside from the fact that the tag name stayed the same, in the SELECT case it's unlikely anyone would have checked the tag at all rather than just testing for PQresultStatus() == PGRES_TUPLES_OK. So it was basically only changing the result for *one* command type. I don't think it's a good basis for arguing that this patch won't cause problems. Yeah, but that one command tag was SELECT. That's a pretty commonly used command. Most production environments probably use all of the commands affected by this patch together an order of magnitude less often than they use SELECT. Again, on what basis are we arguing that people are going to be looking at the command tag of a command that always returns the same tag? That seems pretty darn unlikely to me. -- 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] Error code for terminating connection due to conflict with recovery
On Fri, Jan 14, 2011 at 12:29 PM, Simon Riggs si...@2ndquadrant.com wrote: This whole thing is confused. No change is appropriate here at all. We issue ERRCODE_T_R_SERIALIZATION_FAILURE almost all of the time for recovery conflicts. We issue ERRCODE_ADMIN_SHUTDOWN only if the conflict is non-retryable, which occurs if someone drops the database that the user was connected to when they get kicked off. That code exists specifically to inform the user that they *cannot* reconnect. So pgpool should not be trying to trap that error and reconnect. CheckRecoveryConflictDeadlock() uses ERRCODE_QUERY_CANCELLED. ProcessInterrupts() sometimes uses ERRCODE_T_R_SERIALIZATION_FAILURE and sometimes uses ERRCODE_ADMIN_SHUTDOWN. It seems to me that it wouldn't be a bad thing to be a bit more consistent, and perhaps to have dedicated error codes for recovery conflicts. This bit strikes me as particularly strange: else if (RecoveryConflictPending RecoveryConflictRetryable) { pgstat_report_recovery_conflict(RecoveryConflictReason); ereport(FATAL, (errcode(ERRCODE_T_R_SERIALIZATION_FAILURE), errmsg(terminating connection due to conflict with recovery), errdetail_recovery_conflict())); } else if (RecoveryConflictPending) { pgstat_report_recovery_conflict(RecoveryConflictReason); ereport(FATAL, (errcode(ERRCODE_ADMIN_SHUTDOWN), errmsg(terminating connection due to conflict with recovery), errdetail_recovery_conflict())); } That's the same error message at the same severity level with two different SQLSTATEs depending on RecoveryConflictRetryable. Seems a bit cryptic. -- 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] limiting hint bit I/O
On Fri, Jan 14, 2011 at 1:42 PM, Tom Lane t...@sss.pgh.pa.us wrote: Kevin Grittner kevin.gritt...@wicourts.gov writes: Anyway, there are so many ideas in this area, it's hard to keep them all straight. Personally, if I was going to start with something, it would probably be to better establish what the impact is on various workloads of *eliminating* hint bits. I know some people find them useful for forensics to a degree that they would prefer not to see this, Um, yeah, I think you're having a problem keeping all the ideas straight ;-). The argument about forensics has to do with how soon we're willing to freeze tuples, ie replace the XID with a constant. Not about hint bits. Those things are related, though. Freezing sooner could be viewed as an alternative to hint bits. Trouble is, it breaks Hot Standby, badly. -- 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] limiting hint bit I/O
Robert Haas robertmh...@gmail.com wrote: Background freezing plays havoc with Hot Standby I must have missed or forgotten the issue of background vacuums and hot standby. Can you summarize why that's worse than hitting thresholds where autovacuum is freezing things? this test is sufficient to show that eliminating hint bits altogether would a significant regression on some workloads. That wasn't clear to me from what you posted -- I thought that the reduced performance might be partly (largely? mostly?) due to competition with the background writer's work pushing the hinted pages out. Maybe I'm missing something or you didn't post everything you observed in this regard -Kevin -- 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] LOCK for non-tables
Robert Haas robertmh...@gmail.com writes: Tom - I am willing to implement this if you think it's valuable, but I'd like your input on the syntax. http://archives.postgresql.org/pgsql-hackers/2011-01/msg00472.php It looks to me like the reason why there's a shift/reduce conflict is not so much that TABLE is optional as that we allow the syntax LOCK tablename NOWAIT If that weren't possible, then a table name would have to be followed by EOL or IN (which is full-reserved), while an optional object type name could not be followed by either, so there would be no shift/reduce conflict. So we broke it when we added NOWAIT, not when TABLE was made optional. So it looks to me like there are at least two fixes other than the ones you enumerated: 1. Make NOWAIT a reserved word. Not good, but perhaps better than reserving all the different object type names. 2. Disallow the above abbreviated syntax; allow NOWAIT only after an explicit IN ... MODE phrase. This would probably break a couple of applications, but I bet a lot fewer than changing the longer-established parts of the command syntax would break. I think #2 might be the best choice here. 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] limiting hint bit I/O
Robert Haas robertmh...@gmail.com wrote: Freezing sooner could be viewed as an alternative to hint bits. Exactly. And as your test showed, things run faster frozen than unfrozen with hint bits set. Trouble is, it breaks Hot Standby, badly. You're really starting to worry me here. Both for performance and to reduce the WAN bandwidth demands of our backup strategy we are very aggressive with our freezing. Do off-hours VACUUM (FREEZE) runs break hot standby? Autovacuum freezing? What are the symptoms? -Kevin -- 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] LOCK for non-tables
On Fri, 2011-01-14 at 13:58 -0500, Tom Lane wrote: Robert Haas robertmh...@gmail.com writes: Tom - I am willing to implement this if you think it's valuable, but I'd like your input on the syntax. http://archives.postgresql.org/pgsql-hackers/2011-01/msg00472.php It looks to me like the reason why there's a shift/reduce conflict is not so much that TABLE is optional as that we allow the syntax LOCK tablename NOWAIT If that weren't possible, then a table name would have to be followed by EOL or IN (which is full-reserved), while an optional object type name could not be followed by either, so there would be no shift/reduce conflict. So we broke it when we added NOWAIT, not when TABLE was made optional. So it looks to me like there are at least two fixes other than the ones you enumerated: 1. Make NOWAIT a reserved word. Not good, but perhaps better than reserving all the different object type names. 2. Disallow the above abbreviated syntax; allow NOWAIT only after an explicit IN ... MODE phrase. This would probably break a couple of applications, but I bet a lot fewer than changing the longer-established parts of the command syntax would break. I think #2 might be the best choice here. There's a similar issue on my new syntax for skipping the validation check on FKs. I'd appreciate it if you could propose a solution there also. I'm not sure whether I solved it, or am adding issues for the future. -- 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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.
Robert Haas robertmh...@gmail.com writes: On Fri, Jan 14, 2011 at 1:35 PM, Tom Lane t...@sss.pgh.pa.us wrote: That 9.0 change was far less invasive than this: it only added a count field to SELECT and CTAS result tags. Quite aside from the fact that the tag name stayed the same, in the SELECT case it's unlikely anyone would have checked the tag at all rather than just testing for PQresultStatus() == PGRES_TUPLES_OK. Yeah, but that one command tag was SELECT. That's a pretty commonly used command. You're ignoring the point that people would probably use PQresultStatus in preference to checking the tag at all, when dealing with SELECT. psql itself is an example --- it never looks at the tag, nor shows it to the user, in the SELECT case. That patch only really changed the exposed behavior for CREATE TABLE AS SELECT / SELECT INTO, neither of which can be claimed to be hugely popular things for programs to issue. The other side of the argument that needs to be considered is what the benefit is. There was a fairly clear functional gain from reporting the rowcount for CTAS. I'm less convinced that sending back REPLACE is a big benefit worth taking big compatibility risks for. 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] LOCK for non-tables
On Fri, Jan 14, 2011 at 1:58 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Tom - I am willing to implement this if you think it's valuable, but I'd like your input on the syntax. http://archives.postgresql.org/pgsql-hackers/2011-01/msg00472.php It looks to me like the reason why there's a shift/reduce conflict is not so much that TABLE is optional as that we allow the syntax LOCK tablename NOWAIT If that weren't possible, then a table name would have to be followed by EOL or IN (which is full-reserved), while an optional object type name could not be followed by either, so there would be no shift/reduce conflict. So we broke it when we added NOWAIT, not when TABLE was made optional. Hmm, OK. So it looks to me like there are at least two fixes other than the ones you enumerated: 1. Make NOWAIT a reserved word. Not good, but perhaps better than reserving all the different object type names. 2. Disallow the above abbreviated syntax; allow NOWAIT only after an explicit IN ... MODE phrase. This would probably break a couple of applications, but I bet a lot fewer than changing the longer-established parts of the command syntax would break. I think #2 might be the best choice here. That strikes me as pretty unintuitive. I'd rather take the hit of forcing people to write LOCK TABLE foo instead of just LOCK foo than try to explain why they have to include IN ACCESS EXCLUSIVE MODE if they want to stick NOWAIT on the end. However, I guess it's a matter of opinion so... anyone else have an opinion? -- 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] limiting hint bit I/O
Robert Haas robertmh...@gmail.com writes: On Fri, Jan 14, 2011 at 1:42 PM, Tom Lane t...@sss.pgh.pa.us wrote: Um, yeah, I think you're having a problem keeping all the ideas straight ;-). The argument about forensics has to do with how soon we're willing to freeze tuples, ie replace the XID with a constant. Not about hint bits. Those things are related, though. Freezing sooner could be viewed as an alternative to hint bits. Freezing sooner isn't likely to reduce I/O compared to hint bits. What that does is create I/O that you *have* to execute ... both in the pages themselves, and in WAL. 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] limiting hint bit I/O
On Fri, Jan 14, 2011 at 2:01 PM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: Trouble is, it breaks Hot Standby, badly. You're really starting to worry me here. Both for performance and to reduce the WAN bandwidth demands of our backup strategy we are very aggressive with our freezing. Do off-hours VACUUM (FREEZE) runs break hot standby? Autovacuum freezing? What are the symptoms? Freezing removes XIDs, so latestRemovedXid advances. VACUUM (FREEZE) is fine if you do it when there are no queries running on your Hot Standby server, but if there ARE queries running on the Hot Standby server, they'll be cancelled once max_standby_delay expires. -- 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] Determining client_encoding from client locale
On lör, 2009-07-25 at 01:41 -0500, Jaime Casanova wrote: On Fri, Jul 24, 2009 at 2:23 AM, Magnus Hagandermag...@hagander.net wrote: 1) it introduces a dependency for -lpgport when compiling a client that uses libpq http://archives.postgresql.org/pgsql-hackers/2009-07/msg01511.php For other parts of libpgport that are needed, we pull in the individual source files. We specifically *don't* link libpq with libpgport, for a reason. There's a comment in the Makefile that explains why. ok, attached a version that modifies src/interfaces/libpq/Makefile to push chklocale.o and eliminate the dependency on libpgport, this change also fixes the compile problem on windows I have adjusted your old patch for the current tree, and it seems to work. I think it was just forgotten last time because the move to PQconnectdbParams had to happen first. But I'll throw it back into the ring now. diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index fe661b8..cb3f6e3 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -245,6 +245,21 @@ PGconn *PQconnectdbParams(const char **keywords, const char **values, int expand /listitem /varlistentry +varlistentry id=libpq-connect-client-encoding xreflabel=client_encoding + termliteralclient_encoding/literal/term + listitem + para + This sets the varnameclient_encoding/varname + configuration parameter for this connection. In addition to + the values accepted by the corresponding server option, you + can use literalauto/literal to determine the right + encoding from the current locale in the client + (envarLC_CTYPE/envar environment variable on Unix + systems). + /para + /listitem +/varlistentry + varlistentry id=libpq-connect-options xreflabel=options termliteraloptions/literal/term listitem @@ -6341,6 +6356,16 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough) linkend=libpq-connect-connect-timeout connection parameter. /para /listitem + +listitem + para + indexterm + primaryenvarPGCLIENTENCODING/envar/primary + /indexterm + envarPGCLIENTENCODING/envar behaves the same as xref + linkend=libpq-connect-client-encoding connection parameter. + /para +/listitem /itemizedlist /para @@ -6377,17 +6402,6 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough) listitem para indexterm - primaryenvarPGCLIENTENCODING/envar/primary - /indexterm - envarPGCLIENTENCODING/envar sets the default client character - set encoding. (Equivalent to literalSET client_encoding TO - .../literal.) - /para -/listitem - -listitem - para - indexterm primaryenvarPGGEQO/envar/primary /indexterm envarPGGEQO/envar sets the default mode for the genetic query diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 5f61561..1716b3b 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -593,6 +593,17 @@ $ userinputpsql service=myservice sslmode=require/userinput privileges, server is not running on the targeted host, etc.), applicationpsql/application will return an error and terminate. /para + +para + If at least one of standard input or standard output are a + terminal, then applicationpsql/application sets the client + encoding to quoteauto/quote, which will detect the + appropriate client encoding from the locale settings + (envarLC_CTYPE/envar environment variable on Unix systems). + If this doesn't work out as expected, the client encoding can be + overridden using the environment + variable envarPGCLIENTENCODING/envar. +/para /refsect2 refsect2 id=R2-APP-PSQL-4 diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 962c13c..37f696a 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -1475,7 +1475,7 @@ do_connect(char *dbname, char *user, char *host, char *port) while (true) { -#define PARAMS_ARRAY_SIZE 7 +#define PARAMS_ARRAY_SIZE 8 const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords)); const char **values = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*values)); @@ -1491,8 +1491,10 @@ do_connect(char *dbname, char *user, char *host, char *port) values[4] = dbname; keywords[5] = fallback_application_name; values[5] = pset.progname; - keywords[6] = NULL; - values[6] = NULL; + keywords[6] = client_encoding; + values[6] = (pset.notty || getenv(PGCLIENTENCODING)) ? NULL : auto; + keywords[7] = NULL; + values[7] = NULL; n_conn = PQconnectdbParams(keywords, values, true); diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c index 4f3815a..539f8be 100644 --- a/src/bin/psql/startup.c +++ b/src/bin/psql/startup.c @@ -171,7
Re: [HACKERS] limiting hint bit I/O
On Fri, Jan 14, 2011 at 1:52 PM, Kevin Grittner kevin.gritt...@wicourts.gov wrote: Robert Haas robertmh...@gmail.com wrote: Background freezing plays havoc with Hot Standby I must have missed or forgotten the issue of background vacuums and hot standby. Can you summarize why that's worse than hitting thresholds where autovacuum is freezing things? The critical issue is whether the tuples get frozen while they're still invisible to some transactions on the standby server. That's when you get query cancellations. this test is sufficient to show that eliminating hint bits altogether would a significant regression on some workloads. That wasn't clear to me from what you posted -- I thought that the reduced performance might be partly (largely? mostly?) due to competition with the background writer's work pushing the hinted pages out. Maybe I'm missing something or you didn't post everything you observed in this regard Well, let me put together a quick patch that obliterates hint bits entirely, and we can measure that. The background writer has always pushed out hint bit pages; I think the reduced performance was probably due to needing to reset hint bits on pages that we threw away without pushing them out. -- 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] limiting hint bit I/O
On Fri, Jan 14, 2011 at 2:09 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Jan 14, 2011 at 1:42 PM, Tom Lane t...@sss.pgh.pa.us wrote: Um, yeah, I think you're having a problem keeping all the ideas straight ;-). The argument about forensics has to do with how soon we're willing to freeze tuples, ie replace the XID with a constant. Not about hint bits. Those things are related, though. Freezing sooner could be viewed as an alternative to hint bits. Freezing sooner isn't likely to reduce I/O compared to hint bits. What that does is create I/O that you *have* to execute ... both in the pages themselves, and in WAL. It depends on which way you tilt your head - right now, we rewrite each table 3x - once to populate, once to hint, and once to freeze. If the table is doomed to survive long enough to go through all three of those, then freezing is better than hinting. Of course, that's not always the case, but people keep complaining about the way this shakes out. -- 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] Database file copy
Kevin Grittner kevin.gritt...@wicourts.gov writes: If we're going to be supporting that long term, we should probably change the note about FREEZE being deprecated, though. So, still +1 on removing the wording about FREEZE being deprecated, but instead we should mention what actually *is* deprecated (the omission of the parentheses). If we're going to do that, we should deprecate the unparenthesized syntax altogether, with an eye to de-reserving VERBOSE and ANALYZE as well. 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] limiting hint bit I/O
Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: Those things are related, though. Freezing sooner could be viewed as an alternative to hint bits. Freezing sooner isn't likely to reduce I/O compared to hint bits. What that does is create I/O that you *have* to execute ... both in the pages themselves, and in WAL. In an environment where the vast majority of tuples live long enough to need to be frozen anyway, freezing sooner doesn't really do that to you. Granted, explicit freezing off-hours prevents autovacuum from doing that to you in large bursts at unexpected times, but if you're comparing background writer freezing to autovacuum freezing, I'm not clear on where the extra pain comes from. I am assuming that the background writer would be sane about how it did this, of course. We could all set up straw man implementations which would clobber performance. I suspect that you can envision a hueristic which would be no more bothersome than autovacuum. -Kevin -- 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] limiting hint bit I/O
Robert Haas robertmh...@gmail.com wrote: The critical issue is whether the tuples get frozen while they're still invisible to some transactions on the standby server. That's when you get query cancellations. Oh, OK; I get that. That seems easy enough to at least mitigate to a large degree by some threshold GUC. But of course, the longer you wait to freeze so that you don't cancel queries on the standby, the more you pay to recalculate visibility, so it'd be a fussy thing to tune. Perhaps such freeze information could be queued until a safe time on the standby. (Now that I've learned the joys of SLRU, I can see all sorts of possible uses for it) Well, let me put together a quick patch that obliterates hint bits entirely, and we can measure that. The background writer has always pushed out hint bit pages; I think the reduced performance was probably due to needing to reset hint bits on pages that we threw away without pushing them out. It would be good to confirm and quantify. -Kevin -- 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] [PATCH] Return command tag 'REPLACE X' for CREATE OR REPLACE statements.
On Fri, Jan 14, 2011 at 2:07 PM, Tom Lane t...@sss.pgh.pa.us wrote: Robert Haas robertmh...@gmail.com writes: On Fri, Jan 14, 2011 at 1:35 PM, Tom Lane t...@sss.pgh.pa.us wrote: That 9.0 change was far less invasive than this: it only added a count field to SELECT and CTAS result tags. Quite aside from the fact that the tag name stayed the same, in the SELECT case it's unlikely anyone would have checked the tag at all rather than just testing for PQresultStatus() == PGRES_TUPLES_OK. Yeah, but that one command tag was SELECT. That's a pretty commonly used command. You're ignoring the point that people would probably use PQresultStatus in preference to checking the tag at all, when dealing with SELECT. I would assume they would also use PQresultStatus() when checking whether CREATE OR REPLACE FUNCTION worked. Even if they were using PQcmdStatus() for some reason, which seems like an odd thing to do, there'd be no reason to check for anything beyond is it empty?. The idea that there are massive amounts of code out there that are expecting the command tag to be *exactly* CREATE FUNCTION and will break if it differs by a byte seems quite improbable. Can you produce an example of any such code? The other side of the argument that needs to be considered is what the benefit is. There was a fairly clear functional gain from reporting the rowcount for CTAS. I'm less convinced that sending back REPLACE is a big benefit worth taking big compatibility risks for. Asserting that there are big compatibility risks doesn't make it so - you've offered no evidence of that. Even if a handful of people had complained about that one, I would still felt it was a good change, but it doesn't seem that there are any at all. I classify this as one of a dozen or two minor usability enhancements that we make in every release, and most people don't care, and those who do go oh, that's handy. I think before we reject a patch for breaking things, we ought to be able to identify either some actual application that is broken by it, or at least some reasonably plausible coding pattern that would blow up. -- 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] LOCK for non-tables
Robert Haas robertmh...@gmail.com writes: On Fri, Jan 14, 2011 at 1:58 PM, Tom Lane t...@sss.pgh.pa.us wrote: 2. Disallow the above abbreviated syntax; allow NOWAIT only after an explicit IN ... MODE phrase. This would probably break a couple of applications, but I bet a lot fewer than changing the longer-established parts of the command syntax would break. That strikes me as pretty unintuitive. I'd rather take the hit of forcing people to write LOCK TABLE foo instead of just LOCK foo than try to explain why they have to include IN ACCESS EXCLUSIVE MODE if they want to stick NOWAIT on the end. However, I guess it's a matter of opinion so... anyone else have an opinion? It doesn't seem amazingly unintuitive to me; the syntax diagram just changes from LOCK ... [ IN lockmode MODE ] [ NOWAIT ] to LOCK ... [ IN lockmode MODE [ NOWAIT ] ] If it had been that way to start with, nobody would have questioned it. In any case I'd rather break apps using LOCK foo NOWAIT than break every application using any form of LOCK at all, which is what I think your proposal will amount to in practice. People aren't that eager to write useless noise words, which is what TABLE has been up to now in this command. 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] Database file copy
Tom Lane t...@sss.pgh.pa.us wrote: Kevin Grittner kevin.gritt...@wicourts.gov writes: So, still +1 on removing the wording about FREEZE being deprecated, but instead we should mention what actually *is* deprecated (the omission of the parentheses). If we're going to do that, we should deprecate the unparenthesized syntax altogether, with an eye to de-reserving VERBOSE and ANALYZE as well. +1 -Kevin -- 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] limiting hint bit I/O
Robert Haas robertmh...@gmail.com writes: On Fri, Jan 14, 2011 at 2:09 PM, Tom Lane t...@sss.pgh.pa.us wrote: Freezing sooner isn't likely to reduce I/O compared to hint bits. What that does is create I/O that you *have* to execute ... both in the pages themselves, and in WAL. It depends on which way you tilt your head - right now, we rewrite each table 3x - once to populate, once to hint, and once to freeze. If the table is doomed to survive long enough to go through all three of those, then freezing is better than hinting. Of course, that's not always the case, but people keep complaining about the way this shakes out. The people whose tables are mostly insert-only complain about it, but that's not the majority of our userbase IMO. We just happen to have a couple of particularly vocal ones, like Berkus. 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] LOCK for non-tables
Le 14 janv. 2011 à 20:08, Robert Haas robertmh...@gmail.com a écrit : On Fri, Jan 14, 2011 at 1:58 PM, Tom Lane t...@sss.pgh.pa.us wrote: So it looks to me like there are at least two fixes other than the ones you enumerated: 1. Make NOWAIT a reserved word. Not good, but perhaps better than reserving all the different object type names. 2. Disallow the above abbreviated syntax; allow NOWAIT only after an explicit IN ... MODE phrase. This would probably break a couple of applications, but I bet a lot fewer than changing the longer-established parts of the command syntax would break. I think #2 might be the best choice here. +1 That strikes me as pretty unintuitive. I'd rather take the hit of forcing people to write LOCK TABLE foo instead of just LOCK foo than try to explain why they have to include IN ACCESS EXCLUSIVE MODE if they want to stick NOWAIT on the end. However, I guess it's a matter of opinion so... anyone else have an opinion? Since you ask, see above. :) -- dim -- 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] LOCK for non-tables
On Fri, 2011-01-14 at 14:48 -0500, Tom Lane wrote: In any case I'd rather break apps using LOCK foo NOWAIT than break every application using any form of LOCK at all, which is what I think your proposal will amount to in practice. Can I suggest that we don't break anything at all? pg_lock_object(objectname, objecttype, mode); or pg_lock_sequence(name, mode); is all we need... -- 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] SQL/MED - FDW API
On 01/14/2011 07:23 AM, Shigeru HANADA wrote: You would be able to test these patches with 20110114 version of file_fdw wrapper patches which will be posted in another thread. Have you actually posted this version of file_fdw? I haven't seen it. 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] LOCK for non-tables
Simon Riggs si...@2ndquadrant.com writes: On Fri, 2011-01-14 at 14:48 -0500, Tom Lane wrote: In any case I'd rather break apps using LOCK foo NOWAIT than break every application using any form of LOCK at all, which is what I think your proposal will amount to in practice. Can I suggest that we don't break anything at all? pg_lock_object(objectname, objecttype, mode); or pg_lock_sequence(name, mode); is all we need... No, that will not work at all. LOCK has to be a utility command. A function called by SELECT isn't a substitute, because SELECT will acquire a transaction snapshot before executing the function, and that breaks many use cases for locks. 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