Re: [HACKERS] pg_archivecleanup bug
On 03/19/2014 02:30 AM, Bruce Momjian wrote:
On Tue, Mar 18, 2014 at 09:13:28PM +0200, Heikki Linnakangas wrote:
On 03/18/2014 09:04 PM, Simon Riggs wrote:
On 18 March 2014 18:55, Alvaro Herrera alvhe...@2ndquadrant.com wrote:
That said, I don't find comma expression to be particularly not
simple.
Maybe, but we've not used it before anywhere in Postgres, so I don't
see a reason to start now. Especially since C is not the native
language of many people these days and people just won't understand
it.
Agreed. The psqlODBC code is littered with comma expressions, and
the first time I saw it, it took me a really long time to figure out
what if (foo = malloc(...), foo) { } meant. And I consider myself
quite experienced with C.
I can see how the comma syntax would be confusing, though it does the
job well. Attached is a patch that does the double-errno. However,
some of these loops are large, and there are 'continue' calls in there,
causing the addition of many new errno locations. I am not totally
comfortable that this new coding layout will stay unbroken.
Would people accept?
for (errno = 0; (dirent = readdir(dir)) != NULL; errno = 0)
That would keep the errno's together and avoid the 'continue' additions.
That's clever. A less clever way would be:
for (;;)
{
errno = 0;
if ((dirent = readdir(dir)) != NULL)
break;
...
}
I'm fine with either, but that's how I would naturally write it.
Yet another way would be to have a wrapper function for readdir that
resets errno, and just replace the current readdir() calls with that.
And now that I look at initdb.c, walkdir is using the comma expression
for this already. So there's some precedence, and it doesn't actually
look that bad. So I withdraw my objection for that approach; I'm fine
with any of the discussed alternatives, really.
- Heikki
--
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] Archive recovery won't be completed on some situation.
Hello, thank you for suggestions.
The *problematic* operation sequence I saw was performed by
pgsql-RA/Pacemaker. It stops a server already with immediate mode
and starts the Master as a Standby at first, then
promote. Focusing on this situation, there would be reasonable to
reset backup positions. 9.4 canceles backup mode even on
immediate shutdown so the operation causes no problem, but 9.3
and before are doesn't. Finally, needed amendments per versions
are
9.4: Nothing more is needed (but resetting backup mode by
resetxlog is acceptable)
9.3: Can be recovered without resetting backup positions in
controlfile. (but smarter with it)
9.2: Same to 9.3
9.1: Cannot be recoverd without directly resetting backup
position in controlfile. Resetting feature is needed.
At Mon, 17 Mar 2014 15:59:09 +0200, Heikki Linnakangas wrote
On 03/15/2014 05:59 PM, Fujii Masao wrote:
What about adding new option into pg_resetxlog so that we can
reset the pg_control's backup start location? Even after we've
accidentally entered into the situation that you described, we can
exit from that by resetting the backup start location in pg_control.
Also this option seems helpful to salvage the data as a last resort
from the corrupted backup.
Yeah, seems reasonable. After you run pg_resetxlog, there's no hope
that the backup end record would arrive any time later. And if it
does, it won't really do much good after you've reset the WAL.
We probably should just clear out the backup start/stop location
always when you run pg_resetxlog. Your database is potentially broken
if you reset the WAL before reaching consistency, but if forcibly do
that with pg_resetxlog -f, you've been warned.
Agreed. Attached patches do that and I could recover the
database state with following steps,
(1) Remove recovery.conf and do pg_resetxlog -bf
(the option name 'b' would be arguable)
(2) Start the server (with crash recovery)
(3) Stop the server (in any mode)
(4) Create recovery.conf and start the server with archive recovery.
Some annoyance in step 2 and 3 but I don't want to support the
pacemaker's in-a-sense broken sequence no further:(
This is alterable by the following steps suggested in Masao's
previous mail for 9.2 and alter, but 9.1 needs forcibly resetting
startBackupPoint.
At Sun, 16 Mar 2014 00:59:01 +0900, Fujii Masao wrote
Though this is formal way, you can exit from that situation by
(1) Remove recovery.conf and start the server with crash recovery
(2) Execute pg_start_backup() after crash recovery ends
(3) Copy backup_label to somewhere
(4) Execute pg_stop_backup() and shutdown the server
(5) Copy backup_label back to $PGDATA
(6) Create recovery.conf and start the server with archive recovery
This worked for 9.2, 9.3 and HEAD but failed for 9.1 at step 1.
| 2014-03-19 15:53:02.512 JST FATAL: WAL ends before end of online backup
| 2014-03-19 15:53:02.512 JST HINT: Online backup started with
pg_start_backup() must be ended with pg_stop_backup(), and all WAL up to that
point must be available at recovery.
This seems inevitable.
| if (InRecovery
| (XLByteLT(EndOfLog, minRecoveryPoint) ||
|!XLogRecPtrIsInvalid(ControlFile-backupStartPoint)))
| {
...
| /*
|* Ran off end of WAL before reaching end-of-backup WAL record, or
|* minRecoveryPoint.
|*/
| if (!XLogRecPtrIsInvalid(ControlFile-backupStartPoint))
| ereport(FATAL,
| (errmsg(WAL ends before end of online backup),
regards,
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/bin/pg_resetxlog/pg_resetxlog.c b/src/bin/pg_resetxlog/pg_resetxlog.c
index 28a4f19..7d9cf6d 100644
--- a/src/bin/pg_resetxlog/pg_resetxlog.c
+++ b/src/bin/pg_resetxlog/pg_resetxlog.c
@@ -85,6 +85,7 @@ main(int argc, char *argv[])
int c;
bool force = false;
bool noupdate = false;
+ bool resetbackuppos = false;
MultiXactId set_oldestmxid = 0;
char *endptr;
char *endptr2;
@@ -110,7 +111,7 @@ main(int argc, char *argv[])
}
- while ((c = getopt(argc, argv, fl:m:no:O:x:e:)) != -1)
+ while ((c = getopt(argc, argv, fl:m:no:O:x:e:b)) != -1)
{
switch (c)
{
@@ -122,6 +123,10 @@ main(int argc, char *argv[])
noupdate = true;
break;
+ case 'b':
+resetbackuppos = true;
+break;
+
case 'e':
set_xid_epoch = strtoul(optarg, endptr, 0);
if (endptr == optarg || *endptr != '\0')
@@ -350,6 +355,13 @@ main(int argc, char *argv[])
ControlFile.checkPointCopy.PrevTimeLineID = minXlogTli;
}
+ if (resetbackuppos)
+ {
+ ControlFile.backupStartPoint = InvalidXLogRecPtr;
+ ControlFile.backupEndPoint = InvalidXLogRecPtr;
+ ControlFile.backupEndRequired = false;
+ }
+
if (minXlogSegNo newXlogSegNo)
newXlogSegNo = minXlogSegNo;
@@ -1098,6 +1110,7 @@ usage(void)
printf(_( -O OFFSETset next multitransaction offset\n));
printf(_( -V, --versionoutput version information,
[HACKERS] Review: plpgsql.extra_warnings, plpgsql.extra_errors
Hello This patch introduce a possibility to implement some new checks without impact to current code. 1. there is a common agreement about this functionality, syntax, naming 2. patching is clean, compilation is without error and warning 3. all regress tests passed 4. feature is well documented 5. code is clean, documented and respect out codding standards Note: please, replace shadowed-variables by shadowed_variables This patch is ready for commit Regards Pavel Stehule
Re: [HACKERS] ALTER TABLE lock strength reduction patch is unsafe Reply-To:
On 18 March 2014 21:21, Noah Misch n...@leadboat.com wrote: On Tue, Mar 18, 2014 at 10:39:03AM +, Simon Riggs wrote: On 8 March 2014 11:14, Simon Riggs si...@2ndquadrant.com wrote: Implemented in attached patch, v22 I commend this patch to you for final review; I would like to commit this in a few days. I'm planning to commit this today at 1500UTC barring objections or negative reviews. Not an objection, but FYI, I'm currently in the midst of reviewing this. Thanks, I'll holdoff until I hear from you. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] Review: plpgsql.extra_warnings, plpgsql.extra_errors
On 19/03/14 09:45, Pavel Stehule wrote:
Hello
This patch introduce a possibility to implement some new checks without
impact to current code.
1. there is a common agreement about this functionality, syntax, naming
2. patching is clean, compilation is without error and warning
3. all regress tests passed
4. feature is well documented
5. code is clean, documented and respect out codding standards
Note: please, replace shadowed-variables by shadowed_variables
This patch is ready for commit
Thanks! Attached new version of the patch with the above change.
--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
index bddd458..0582c91 100644
--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -4711,6 +4711,51 @@ a_output := a_output || $$ if v_$$ || referrer_keys.kind || $$ like '$$
/variablelist
/sect2
+ sect2 id=plpgsql-extra-checks
+ titleAdditional compile-time checks/title
+
+ para
+To aid the user in finding instances of simple but common problems before
+they cause harm, applicationPL/PgSQL/ provides additional
+replaceablechecks/. When enabled, depending on the configuration, they
+can be used to emit either a literalWARNING/ or an literalERROR/
+during the compilation of a function.
+ /para
+
+ para
+ These additional checks are enabled through the configuration variables
+ varnameplpgsql.extra_warnings/ for warnings and
+ varnameplpgsql.extra_errors/ for errors. Both can be set either to
+ a comma-separated list of checks, literalnone/ or literalall/.
+ The default is literalnone/. Currently the list of available checks
+ includes only one:
+ variablelist
+ varlistentry
+termvarnameshadowed_variables/varname/term
+listitem
+ para
+ Checks if a declaration shadows a previously defined variable. For
+ example (with varnameplpgsql.extra_warnings/ set to
+ varnameshadowed_variables/varname):
+programlisting
+CREATE FUNCTION foo(f1 int) RETURNS int AS $$
+DECLARE
+f1 int;
+BEGIN
+RETURN f1;
+END
+$$ LANGUAGE plpgsql;
+WARNING: variable f1 shadows a previously defined variable
+LINE 3: f1 int;
+^
+CREATE FUNCTION
+/programlisting
+ /para
+/listitem
+ /varlistentry
+ /variablelist
+ /para
+ /sect2
/sect1
!-- Porting from Oracle PL/SQL --
diff --git a/src/pl/plpgsql/src/pl_comp.c b/src/pl/plpgsql/src/pl_comp.c
index 5afc2e5..8732efc 100644
--- a/src/pl/plpgsql/src/pl_comp.c
+++ b/src/pl/plpgsql/src/pl_comp.c
@@ -352,6 +352,9 @@ do_compile(FunctionCallInfo fcinfo,
function-out_param_varno = -1; /* set up for no OUT param */
function-resolve_option = plpgsql_variable_conflict;
function-print_strict_params = plpgsql_print_strict_params;
+ /* only promote extra warnings and errors at CREATE FUNCTION time */
+ function-extra_warnings = plpgsql_extra_warnings forValidator;
+ function-extra_errors = plpgsql_extra_errors forValidator;
if (is_dml_trigger)
function-fn_is_trigger = PLPGSQL_DML_TRIGGER;
@@ -849,6 +852,9 @@ plpgsql_compile_inline(char *proc_source)
function-out_param_varno = -1; /* set up for no OUT param */
function-resolve_option = plpgsql_variable_conflict;
function-print_strict_params = plpgsql_print_strict_params;
+ /* don't do extra validation for inline code as we don't want to add spam at runtime */
+ function-extra_warnings = false;
+ function-extra_errors = false;
plpgsql_ns_init();
plpgsql_ns_push(func_name);
diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y
index c0cb585..98f7ddd 100644
--- a/src/pl/plpgsql/src/pl_gram.y
+++ b/src/pl/plpgsql/src/pl_gram.y
@@ -727,6 +727,20 @@ decl_varname : T_WORD
$1.ident, NULL, NULL,
NULL) != NULL)
yyerror(duplicate declaration);
+
+ if (plpgsql_curr_compile-extra_warnings || plpgsql_curr_compile-extra_errors)
+ {
+ PLpgSQL_nsitem *nsi;
+ nsi = plpgsql_ns_lookup(plpgsql_ns_top(), false,
+ $1.ident, NULL, NULL, NULL);
+ if (nsi != NULL)
+ereport(plpgsql_curr_compile-extra_errors ? ERROR : WARNING,
+ (errcode(ERRCODE_DUPLICATE_ALIAS),
+ errmsg(variable \%s\ shadows a previously defined variable,
+$1.ident),
+ parser_errposition(@1)));
+ }
+
}
| unreserved_keyword
{
@@ -740,6 +754,20 @@ decl_varname : T_WORD
$1, NULL, NULL,
NULL) != NULL)
yyerror(duplicate declaration);
+
+ if (plpgsql_curr_compile-extra_warnings || plpgsql_curr_compile-extra_errors)
+ {
+ PLpgSQL_nsitem *nsi;
+ nsi = plpgsql_ns_lookup(plpgsql_ns_top(), false,
+ $1, NULL, NULL, NULL);
+ if (nsi != NULL)
+ereport(plpgsql_curr_compile-extra_errors ? ERROR : WARNING,
+ (errcode(ERRCODE_DUPLICATE_ALIAS),
+ errmsg(variable \%s\ shadows
Re: [HACKERS] Archive recovery won't be completed on some situation.
On Wed, Mar 19, 2014 at 5:28 PM, Kyotaro HORIGUCHI horiguchi.kyot...@lab.ntt.co.jp wrote: Hello, thank you for suggestions. The *problematic* operation sequence I saw was performed by pgsql-RA/Pacemaker. It stops a server already with immediate mode and starts the Master as a Standby at first, then promote. Focusing on this situation, there would be reasonable to reset backup positions. 9.4 canceles backup mode even on immediate shutdown so the operation causes no problem, but 9.3 and before are doesn't. Finally, needed amendments per versions are 9.4: Nothing more is needed (but resetting backup mode by resetxlog is acceptable) 9.3: Can be recovered without resetting backup positions in controlfile. (but smarter with it) 9.2: Same to 9.3 9.1: Cannot be recoverd without directly resetting backup position in controlfile. Resetting feature is needed. At Mon, 17 Mar 2014 15:59:09 +0200, Heikki Linnakangas wrote On 03/15/2014 05:59 PM, Fujii Masao wrote: What about adding new option into pg_resetxlog so that we can reset the pg_control's backup start location? Even after we've accidentally entered into the situation that you described, we can exit from that by resetting the backup start location in pg_control. Also this option seems helpful to salvage the data as a last resort from the corrupted backup. Yeah, seems reasonable. After you run pg_resetxlog, there's no hope that the backup end record would arrive any time later. And if it does, it won't really do much good after you've reset the WAL. We probably should just clear out the backup start/stop location always when you run pg_resetxlog. Your database is potentially broken if you reset the WAL before reaching consistency, but if forcibly do that with pg_resetxlog -f, you've been warned. Agreed. Attached patches do that and I could recover the database state with following steps, Adding new option looks like new feature rather than bug fix. I'm afraid that the backpatch of such a change to 9.3 or before is not acceptable. Regards, -- Fujii Masao -- 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] Archive recovery won't be completed on some situation.
On 03/19/2014 10:28 AM, Kyotaro HORIGUCHI wrote: The*problematic* operation sequence I saw was performed by pgsql-RA/Pacemaker. It stops a server already with immediate mode and starts the Master as a Standby at first, then promote. Focusing on this situation, there would be reasonable to reset backup positions. Well, that's scary. I would suggest doing a fast shutdown instead. But if you really want to do an immediate shutdown, you should delete the backup_label file after the shutdown When restarting after immediate shutdown and a backup_label file is present, the system doesn't know if the system crashed during a backup, and it needs to perform crash recovery, or if you're trying restore from a backup. It makes a compromise, and starts recovery from the checkpoint given in the backup_label, as if it was restoring from a backup, but if it doesn't see a backup-end WAL record, it just starts up anyway (which would be wrong if you are indeed restoring from a backup). But if you create a recovery.conf file, that indicates that you are definitely restoring from a backup, so it's more strict and insists that the backup-end record must be replayed. 9.4 canceles backup mode even on immediate shutdown so the operation causes no problem, but 9.3 and before are doesn't. Hmm, I don't think we've changed that behavior in 9.4. - Heikki -- 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] B-tree descend for insertion locking
On 18 March 2014 11:12, Heikki Linnakangas hlinnakan...@vmware.com wrote: When inserting into a B-tree index, all the pages are read-locked when descending the tree. When we reach the leaf page, the read-lock is exchanged for a write-lock. There's nothing wrong with that, but why don't we just directly grab a write-lock on the leaf page? When descending, we know the level we're on, and what level the child page is. The only downside I can see is that we would unnecessarily hold a write-lock when a read-lock would suffice, if the page was just split and we have to move right. But that seems like a really bad bet - hitting the page when it was just split is highly unlikely. Sounds good. Grabbing write lock directly will reduce contention on the buffer, not just reduce the code path. If we have a considerable number of duplicates we would normally step thru until we found a place to insert. Presumably that will happen with write lock enabled, rather than read lock. Would this slow down the insertion of highly duplicate keys under concurrent load? i.e. is this a benefit for nearly-unique but not for other cases? -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] Archive recovery won't be completed on some situation.
On Wed, Mar 19, 2014 at 7:57 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 03/19/2014 10:28 AM, Kyotaro HORIGUCHI wrote: The*problematic* operation sequence I saw was performed by pgsql-RA/Pacemaker. It stops a server already with immediate mode and starts the Master as a Standby at first, then promote. Focusing on this situation, there would be reasonable to reset backup positions. Well, that's scary. I would suggest doing a fast shutdown instead. But if you really want to do an immediate shutdown, you should delete the backup_label file after the shutdown When restarting after immediate shutdown and a backup_label file is present, the system doesn't know if the system crashed during a backup, and it needs to perform crash recovery, or if you're trying restore from a backup. It makes a compromise, and starts recovery from the checkpoint given in the backup_label, as if it was restoring from a backup, but if it doesn't see a backup-end WAL record, it just starts up anyway (which would be wrong if you are indeed restoring from a backup). But if you create a recovery.conf file, that indicates that you are definitely restoring from a backup, so it's more strict and insists that the backup-end record must be replayed. 9.4 canceles backup mode even on immediate shutdown so the operation causes no problem, but 9.3 and before are doesn't. Hmm, I don't think we've changed that behavior in 9.4. ISTM 82233ce7ea42d6ba519aaec63008aff49da6c7af changed immdiate shutdown that way. Regards, -- Fujii Masao -- 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] Review: plpgsql.extra_warnings, plpgsql.extra_errors
Hello all is pk Pavel 2014-03-19 11:28 GMT+01:00 Petr Jelinek p...@2ndquadrant.com: On 19/03/14 09:45, Pavel Stehule wrote: Hello This patch introduce a possibility to implement some new checks without impact to current code. 1. there is a common agreement about this functionality, syntax, naming 2. patching is clean, compilation is without error and warning 3. all regress tests passed 4. feature is well documented 5. code is clean, documented and respect out codding standards Note: please, replace shadowed-variables by shadowed_variables This patch is ready for commit Thanks! Attached new version of the patch with the above change. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
Re: [HACKERS] Archive recovery won't be completed on some situation.
Fujii Masao escribió: On Wed, Mar 19, 2014 at 7:57 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: 9.4 canceles backup mode even on immediate shutdown so the operation causes no problem, but 9.3 and before are doesn't. Hmm, I don't think we've changed that behavior in 9.4. ISTM 82233ce7ea42d6ba519aaec63008aff49da6c7af changed immdiate shutdown that way. Uh, interesting. I didn't see that secondary effect. I hope it's not for ill? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] Review: plpgsql.extra_warnings, plpgsql.extra_errors
Why start a new thread for this review? It seems to me it'd be more comfortable to keep the review as a reply on the original thread. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] Review: plpgsql.extra_warnings, plpgsql.extra_errors
2014-03-19 13:51 GMT+01:00 Alvaro Herrera alvhe...@2ndquadrant.com: Why start a new thread for this review? It seems to me it'd be more comfortable to keep the review as a reply on the original thread. I am sorry, I though so review should to start in separate thread Pavel -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services
Re: [HACKERS] logical decoding doc
On Tue, Mar 18, 2014 at 7:27 PM, Tatsuo Ishii is...@postgresql.org wrote: Maybe this is to demonstrating peek ahead does not consume changes, but IMO this is a little bit confusing for readers and I think there's a room to enhance the second sql session comment for example: I didn't write that text, but yeah, that was my interpretation of why it was like that. postgres=# -- Again you can also peek ahead in the change stream without consuming changes I don't especially like that particular text, but perhaps it could be made more clear in some way. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] four minor proposals for 9.5
Hello I wrote a few patches, that we use in our production. These patches are small, but I hope, so its can be interesting for upstream: 1. cancel time - we log a execution time cancelled statements 2. fatal verbose - this patch ensure a verbose log for fatal errors. It simplify a investigation about reasons of error. 3. relation limit - possibility to set session limit for maximum size of relations. Any relation cannot be extended over this limit in session, when this value is higher than zero. Motivation - we use lot of queries like CREATE TABLE AS SELECT .. , and some very big results decreased a disk free space too much. It was high risk in our multi user environment. Motivation is similar like temp_files_limit. 4. track statement lock - we are able to track a locking time for query and print this data in slow query log and auto_explain log. It help to us with lather slow query log analysis. Do you thinking so these patches can be generally useful? Regards Pavel
Re: [HACKERS] jsonb status
Hi, On 2014-03-13 17:00:33 -0400, Andrew Dunstan wrote: Peter Geoghegan has been doing a lot of great cleanup of the jsonb code, after moving in the bits we wanted from nested hstore. You can see the current state of the code at https://github.com/feodor/postgres/tree/jsonb_and_hstore I've pushed one commit with minor fixes, and one with several FIXMEs to http://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a=shortlog;h=refs/heads/jsonb_and_hstore The most imortant things are: * Jsonb vs JsonbValue is just bad, the latter needs to be renamed, and there needs to be a very clear explanation about why two forms exist and what each is used for. * The whole datastructure doesn't have any sensible highlevel documentation. * I don't really like the JsonbSuperHeader name/abstraction. What does the Super mean? The only interpetation I see is that it's the toplevel header (why not Top then?), but JEntry has the comment: /* May be accessed as superheader */... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] jsonb status
On 03/19/2014 09:28 AM, Andres Freund wrote: * The whole datastructure doesn't have any sensible highlevel documentation. Explain ... ? I'm planning on improving the docs through the beta period for this, so can you explain what kind of docs we're missing here? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] jsonb status
On 2014-03-19 09:55:03 -0700, Josh Berkus wrote: On 03/19/2014 09:28 AM, Andres Freund wrote: * The whole datastructure doesn't have any sensible highlevel documentation. Explain ... ? I'm planning on improving the docs through the beta period for this, so can you explain what kind of docs we're missing here? Ah, sorry, that was easy to misunderstand. I was talking about the on-disk datastructure on the code level, not the user/SQL exposed part. I've added more verbose FIXMEs to the relevant places where I think such documentation should be added. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] Wiki Page Draft for upcoming release
Tom Lane wrote: Josh Berkus j...@agliodbs.com writes: On 03/17/2014 05:49 PM, Josh Berkus wrote: https://wiki.postgresql.org/wiki/20140320UpdateIssues Anyone? We're going public with this in 36 hours, so I'd love for it to actually be correct. I did a bit more hacking on this page. Could use another look from Alvaro and/or Andres, I'm sure. Edited, mainly to remove mention of FOR NO KEY UPDATE as a possible cause of the problem. I don't know that this can cause an issue, since that lock level conflicts with updates. I wonder about suggesting other versions of ALTER TABLE that can do heap rewrites. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] pg_archivecleanup bug
On Wed, Mar 19, 2014 at 09:59:19AM +0200, Heikki Linnakangas wrote:
Would people accept?
for (errno = 0; (dirent = readdir(dir)) != NULL; errno = 0)
That would keep the errno's together and avoid the 'continue' additions.
That's clever. A less clever way would be:
for (;;)
{
errno = 0;
if ((dirent = readdir(dir)) != NULL)
break;
...
}
I'm fine with either, but that's how I would naturally write it.
Yet another way would be to have a wrapper function for readdir that
resets errno, and just replace the current readdir() calls with
that.
And now that I look at initdb.c, walkdir is using the comma
expression for this already. So there's some precedence, and it
doesn't actually look that bad. So I withdraw my objection for that
approach; I'm fine with any of the discussed alternatives, really.
OK, let's go with the comma. Ironically, the for() loop would be an odd
way to avoid commas as 'for' uses commas often:
for (i = 0, j = 1; i 10; i++, j++)
The attached patch is slightly updated. I will apply it to head and all
the back branches, including the stylistic change to pg_resetxlog (for
consistency) and remove the MinGW block in head.
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/contrib/pg_archivecleanup/pg_archivecleanup.c b/contrib/pg_archivecleanup/pg_archivecleanup.c
new file mode 100644
index 7b5484b..4f4d507
*** a/contrib/pg_archivecleanup/pg_archivecleanup.c
--- b/contrib/pg_archivecleanup/pg_archivecleanup.c
*** CleanupPriorWALFiles(void)
*** 106,112
if ((xldir = opendir(archiveLocation)) != NULL)
{
! while ((xlde = readdir(xldir)) != NULL)
{
strncpy(walfile, xlde-d_name, MAXPGPATH);
TrimExtension(walfile, additional_ext);
--- 106,112
if ((xldir = opendir(archiveLocation)) != NULL)
{
! while (errno = 0, (xlde = readdir(xldir)) != NULL)
{
strncpy(walfile, xlde-d_name, MAXPGPATH);
TrimExtension(walfile, additional_ext);
*** CleanupPriorWALFiles(void)
*** 164,170
}
}
}
! closedir(xldir);
}
else
fprintf(stderr, %s: could not open archive location \%s\: %s\n,
--- 164,182
}
}
}
!
! #ifdef WIN32
! /* Bug in old Mingw dirent.c; fixed in mingw-runtime-3.2, 2003-10-10 */
! if (GetLastError() == ERROR_NO_MORE_FILES)
! errno = 0;
! #endif
!
! if (errno)
! fprintf(stderr, %s: could not read archive location \%s\: %s\n,
! progname, archiveLocation, strerror(errno));
! if (!closedir(xldir))
! fprintf(stderr, %s: could not close archive location \%s\: %s\n,
! progname, archiveLocation, strerror(errno));
}
else
fprintf(stderr, %s: could not open archive location \%s\: %s\n,
diff --git a/contrib/pg_standby/pg_standby.c b/contrib/pg_standby/pg_standby.c
new file mode 100644
index 8ddd486..70fb387
*** a/contrib/pg_standby/pg_standby.c
--- b/contrib/pg_standby/pg_standby.c
*** CustomizableCleanupPriorWALFiles(void)
*** 245,251
*/
if ((xldir = opendir(archiveLocation)) != NULL)
{
! while ((xlde = readdir(xldir)) != NULL)
{
/*
* We ignore the timeline part of the XLOG segment identifiers
--- 245,251
*/
if ((xldir = opendir(archiveLocation)) != NULL)
{
! while (errno = 0, (xlde = readdir(xldir)) != NULL)
{
/*
* We ignore the timeline part of the XLOG segment identifiers
*** CustomizableCleanupPriorWALFiles(void)
*** 283,288
--- 283,298
}
}
}
+
+ #ifdef WIN32
+ /* Bug in old Mingw dirent.c; fixed in mingw-runtime-3.2, 2003-10-10 */
+ if (GetLastError() == ERROR_NO_MORE_FILES)
+ errno = 0;
+ #endif
+
+ if (errno)
+ fprintf(stderr, %s: could not read archive location \%s\: %s\n,
+ progname, archiveLocation, strerror(errno));
if (debug)
fprintf(stderr, \n);
}
*** CustomizableCleanupPriorWALFiles(void)
*** 290,296
fprintf(stderr, %s: could not open archive location \%s\: %s\n,
progname, archiveLocation, strerror(errno));
! closedir(xldir);
fflush(stderr);
}
}
--- 300,309
fprintf(stderr, %s: could not open archive location \%s\: %s\n,
progname, archiveLocation, strerror(errno));
! if (!closedir(xldir))
! fprintf(stderr, %s: could not close archive location \%s\: %s\n,
! progname, archiveLocation, strerror(errno));
!
fflush(stderr);
}
}
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
new file mode 100644
index 4dc809d..5158cfe
*** a/src/backend/storage/file/fd.c
--- b/src/backend/storage/file/fd.c
*** ReadDir(DIR *dir, const char *dirname)
*** 1957,1966
return dent;
#ifdef WIN32
! /*
! * This fix is in mingw cvs (runtime/mingwex/dirent.c rev 1.4), but not in
!
Re: [HACKERS] Patch: show relation and tuple infos of a lock to acquire
Alvaro Herrera escribió: Tom Lane escribió: I think the enum idea you floated is probably worth doing. It's certainly no more complex than passing a string, no? Okay, done that way, attached. I think this one solves all concerns there were. I hope the silence meant assent, because I have pushed this patch now. Thanks, -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] Wiki Page Draft for upcoming release
On 03/19/2014 10:37 AM, Alvaro Herrera wrote: Tom Lane wrote: Josh Berkus j...@agliodbs.com writes: On 03/17/2014 05:49 PM, Josh Berkus wrote: https://wiki.postgresql.org/wiki/20140320UpdateIssues Anyone? We're going public with this in 36 hours, so I'd love for it to actually be correct. I did a bit more hacking on this page. Could use another look from Alvaro and/or Andres, I'm sure. Edited, mainly to remove mention of FOR NO KEY UPDATE as a possible cause of the problem. I don't know that this can cause an issue, since that lock level conflicts with updates. I wonder about suggesting other versions of ALTER TABLE that can do heap rewrites. I don't want to recommend any version of ALTER TABLE until someone actually tests it on a corrupted database. What about simply CREATE TABLE AS SELECT? Presumably that kind of manual cleanup would work, no? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review: plpgsql.extra_warnings, plpgsql.extra_errors
Petr Jelinek escribió:
+ para
+ These additional checks are enabled through the configuration variables
+ varnameplpgsql.extra_warnings/ for warnings and
+ varnameplpgsql.extra_errors/ for errors. Both can be set either to
+ a comma-separated list of checks, literalnone/ or literalall/.
+ The default is literalnone/. Currently the list of available checks
+ includes only one:
+ variablelist
+ varlistentry
+termvarnameshadowed_variables/varname/term
+listitem
+ para
+ Checks if a declaration shadows a previously defined variable. For
+ example (with varnameplpgsql.extra_warnings/ set to
+ varnameshadowed_variables/varname):
+programlisting
+CREATE FUNCTION foo(f1 int) RETURNS int AS $$
+DECLARE
+f1 int;
+BEGIN
+RETURN f1;
+END
+$$ LANGUAGE plpgsql;
+WARNING: variable f1 shadows a previously defined variable
+LINE 3: f1 int;
+^
+CREATE FUNCTION
+/programlisting
+ /para
+/listitem
+ /varlistentry
+ /variablelist
As a matter of style, I think the example should go after the
variablelist is closed. Perhaps in the future, when we invent more
extra warnings/errors, we might want to show more than one in a single
example, for compactness.
+static bool
+plpgsql_extra_checks_check_hook(char **newvalue, void **extra, GucSource
source)
+{
+ if (strcmp(*newvalue, all) == 0 ||
+ strcmp(*newvalue, shadowed_variables) == 0 ||
+ strcmp(*newvalue, none) == 0)
+ return true;
+ return false;
+}
I'm not too clear on how this works when there is more than one possible
value.
+ DefineCustomStringVariable(plpgsql.extra_warnings,
+gettext_noop(List
of programming constructs which should produce a warning.),
+NULL,
+
plpgsql_extra_warnings_list,
+none,
+PGC_USERSET, 0,
+
plpgsql_extra_checks_check_hook,
+
plpgsql_extra_warnings_assign_hook,
+NULL);
I think this should have the GUC_LIST_INPUT flag, and ensure that when
multiple values are passed, we can process them all in a sane fashion.
Other than this, the patch looks sane to me in a quick skim.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training 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: show relation and tuple infos of a lock to acquire
Hi, On 19/03/14 15:12, Alvaro Herrera wrote: I hope the silence meant assent, because I have pushed this patch now. Great, thanks! Best regards, -- Christian Kruse http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services pgpYfuVjR_gg_.pgp Description: PGP signature
[HACKERS] Patch to send transaction commit/rollback stats to the stats collector unconditionally.
Please find attached the patch to send transaction commit/rollback stats to stats collector unconditionally. Without this patch, the transactions that do not read from/write to a database table do not get reported to the stats collector until the client disconnects. Hence the transaction counts in pg_stat_database do not increment gradually as one would expect them to. But when such a backend disconnects, the counts jump dramatically, giving the impression that the database processed a lot of transactions (potentially thousands) in an instant. Best regards, -- Gurjeet Singh http://gurjeet.singh.im/ EDB www.EnterpriseDB.com http://www.enterprisedb.com commit 500d11f96b21552872bad4e9655bd45db28efabd Author: Gurjeet Singh gurj...@singh.im Date: Wed Mar 19 13:41:55 2014 -0400 Send transaction stats to the collector even if no table stats to report. diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index 3dc280a..47008ed 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -825,11 +825,11 @@ pgstat_report_stat(bool force) } /* - * Send partial messages. If force is true, make sure that any pending - * xact commit/abort gets counted, even if no table stats to send. + * Send partial messages. Make sure that any pending xact commit/abort gets + * counted, even if no table stats to send. */ if (regular_msg.m_nentries 0 || - (force (pgStatXactCommit 0 || pgStatXactRollback 0))) + force || (pgStatXactCommit 0 || pgStatXactRollback 0)) pgstat_send_tabstat(regular_msg); if (shared_msg.m_nentries 0) pgstat_send_tabstat(shared_msg); -- 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] jsonb status
On 03/19/2014 03:58 PM, Peter Geoghegan wrote: On Wed, Mar 19, 2014 at 9:28 AM, Andres Freund and...@2ndquadrant.com wrote: * Jsonb vs JsonbValue is just bad, the latter needs to be renamed, and there needs to be a very clear explanation about why two forms exist and what each is used for. Agreed. We should probably emphasize the distinction. Perhaps Oleg and Teodor might like to chime in on this. * The whole datastructure doesn't have any sensible highlevel documentation. And this. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Review: plpgsql.extra_warnings, plpgsql.extra_errors
On 19/03/14 19:26, Alvaro Herrera wrote: Petr Jelinek escribió: + para + These additional checks are enabled through the configuration variables + varnameplpgsql.extra_warnings/ for warnings and + varnameplpgsql.extra_errors/ for errors. Both can be set either to + a comma-separated list of checks, literalnone/ or literalall/. + The default is literalnone/. Currently the list of available checks + includes only one: + variablelist + varlistentry +termvarnameshadowed_variables/varname/term +listitem + para + Checks if a declaration shadows a previously defined variable. For + example (with varnameplpgsql.extra_warnings/ set to + varnameshadowed_variables/varname): +programlisting +CREATE FUNCTION foo(f1 int) RETURNS int AS $$ +DECLARE +f1 int; +BEGIN +RETURN f1; +END +$$ LANGUAGE plpgsql; +WARNING: variable f1 shadows a previously defined variable +LINE 3: f1 int; +^ +CREATE FUNCTION +/programlisting + /para +/listitem + /varlistentry + /variablelist As a matter of style, I think the example should go after the variablelist is closed. Perhaps in the future, when we invent more extra warnings/errors, we might want to show more than one in a single example, for compactness. Ok I can change that. +static bool +plpgsql_extra_checks_check_hook(char **newvalue, void **extra, GucSource source) I'm not too clear on how this works when there is more than one possible value. + DefineCustomStringVariable(plpgsql.extra_warnings, + gettext_noop(List of programming constructs which should produce a warning.), + NULL, + plpgsql_extra_warnings_list, + none, + PGC_USERSET, 0, + plpgsql_extra_checks_check_hook, + plpgsql_extra_warnings_assign_hook, + NULL); I think this should have the GUC_LIST_INPUT flag, and ensure that when multiple values are passed, we can process them all in a sane fashion. Well, as we said with Marko in the original thread, the proper handling is left for whoever wants to add additional parameters, for the current implementation proper list handling is not really needed and it will only server to increase complexity of this simple patch quite late in the release cycle. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] jsonb status
On Wed, Mar 19, 2014 at 9:28 AM, Andres Freund and...@2ndquadrant.com wrote: I've pushed one commit with minor fixes, and one with several FIXMEs to http://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a=shortlog;h=refs/heads/jsonb_and_hstore Cool. * Jsonb vs JsonbValue is just bad, the latter needs to be renamed, and there needs to be a very clear explanation about why two forms exist and what each is used for. Agreed. We should probably emphasize the distinction. * The whole datastructure doesn't have any sensible highlevel documentation. I should probably go make some ascii art, per your comment. * I don't really like the JsonbSuperHeader name/abstraction. What does the Super mean? The only interpetation I see is that it's the toplevel header (why not Top then?), but JEntry has the comment: /* May be accessed as superheader */... What it means is that you can pass a pointer to just past the varlena Jsonb pointer in some contexts (a VARDATA() pointer), and a pointer into a jbvBinary buffer in others. The only reason that functions like findJsonbValueFromSuperHeader() take a super header rather than a Jsonb pointer is that if they did take a Jsonb*, you'd be making a representation that the varlena header wasn't garbage, which could not be ensured in all circumstances, as when the would-be varlena bytes are something else entirely, or perhaps are even at an address that is undefined to access. Sites with jbvBinary pointers (e.g. the iterator code) would have to worry about faking varlena. That comment is misleading, and the general idea does need to be explained better. What you propose for jsonb_typeof() looks incorrect to me. Both of those JsonbIteratorNext() calls are required, to get past the container pseudo array to get to the underlying single scalar element. -- Peter Geoghegan -- 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] Wiki Page Draft for upcoming release
Josh Berkus j...@agliodbs.com writes: On 03/19/2014 10:37 AM, Alvaro Herrera wrote: I wonder about suggesting other versions of ALTER TABLE that can do heap rewrites. I don't want to recommend any version of ALTER TABLE until someone actually tests it on a corrupted database. A test would be a good idea, yes. But in principle it ought to work. What about simply CREATE TABLE AS SELECT? Presumably that kind of manual cleanup would work, no? Well, it would leave you with a whole lot of error-prone manual cleanup to do, like repointing foreign key linkages, remaking indexes, etc. And what's possibly more relevant to this discussion, there's no very strong reason to believe that it'd result in data that's any cleaner than the ALTER TABLE way. Note that if you've already suffered some of the potential indirect consequences, like duplicate/missing keys, then there isn't going to be any automatic fix; you're gonna have to clean up the data by hand. But assuming that that hasn't happened, any seqscan-based data extraction ought to do the trick; and ALTER TABLE (as long as you avoid the no-op transformation pitfall) should be as good as other ways, with a lot less risk of human error than a manual recipe. 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 to send transaction commit/rollback stats to the stats collector unconditionally.
Gurjeet Singh gurj...@singh.im writes: Please find attached the patch to send transaction commit/rollback stats to stats collector unconditionally. That's intentional to reduce stats traffic. What kind of performance penalty does this patch impose? If the number of such transactions is large enough to create a noticeable jump in the counters, I would think that this would be a pretty darn expensive fix. 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] First draft of update announcement
On 03/18/2014 03:02 PM, Tom Lane wrote: Josh Berkus j...@agliodbs.com writes: Updated per feedback. CC'd to Advocacy now for additional corrections. A few thoughts: Changes incorporated. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] First-draft release notes for next week's releases
All, So, I'll ask again (because I didn't see a reply): is there any way users can *check* if they've been corrupted? Short of waiting for PK/FK violations? Given that all of the fixes we recommend involve extensive downtimes, nobody is going to want to do them just in case. How can they test for the issue? I can't see any reasonable way ... -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] First-draft release notes for next week's releases
Josh Berkus j...@agliodbs.com writes: So, I'll ask again (because I didn't see a reply): is there any way users can *check* if they've been corrupted? Short of waiting for PK/FK violations? I think it would work to do a REINDEX on each table (doesn't matter which index you select) and see if it complains about not being able to find any parent tuples. This would definitely catch the case as long as the orphaned HOT tuple is still live. If it's not, the immediate problem is gone ... but it's still possible you have follow-on corruption. 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] First-draft release notes for next week's releases
Josh Berkus wrote: All, So, I'll ask again (because I didn't see a reply): is there any way users can *check* if they've been corrupted? Short of waiting for PK/FK violations? Obviously there are queries you can run to check each FK -- the same queries that ri_triggers.c would run when you create an FK. It's cumbersome to write, but not impossible. In fact, it can be done mechanically. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] effective_cache_size cannot be changed by a reload
In 9.4dev, if the server is started with effective_cache_size = -1, then it cannot be changed away from that without a restart. If you change the config file and do a reload or pg_reload_conf(), it ignores the change without comment in the logs. If you start the server with a value other than -1, then you can change the value by editing the file and doing a reload. You can even change it to -1, and then change it back away from -1 again. It has been that way since at least 6b82f78ff95d7d4201d44359. Before that it was broken in other ways, so I don't know what the behavior would have been. I don't know if bugs reports (without patches) against pre-release versions are supposed to go to hackers or to bugs. If someone has a strong preference, they might want to clarify this on the bug report form. Cheers, Jeff
Re: [HACKERS] First-draft release notes for next week's releases
On 03/19/2014 02:01 PM, Alvaro Herrera wrote: Josh Berkus wrote: All, So, I'll ask again (because I didn't see a reply): is there any way users can *check* if they've been corrupted? Short of waiting for PK/FK violations? Obviously there are queries you can run to check each FK -- the same queries that ri_triggers.c would run when you create an FK. It's cumbersome to write, but not impossible. In fact, it can be done mechanically. Would users which this corruption necessarily have broken FKs which would show up as such on a simple query? That is, if I did: SELECT ref_id FROM referenced WHERE ref_id NOT IN ( SELECT ref_id FROM referencing ) ... or something similar, would that show the issue? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [pgsql-advocacy] [HACKERS] First draft of update announcement
On 2014-03-18, 2:42 PM, Josh Berkus wrote: Other PostgreSQL 9.3 only fixes in this update include: * Add read-only data_checksum parameter I recall being told last fall that this would not be added to 9.3.x (9.3.1 at the time I think) and only to 9.4.x because such a feature addition was something only allowed for major releases and not minor ones which were just supposed to be security and bug fixes. So what changed that it is added in 9.3.x after all? -- Darren Duncan -- 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 to send transaction commit/rollback stats to the stats collector unconditionally.
On Wed, Mar 19, 2014 at 4:22 PM, Tom Lane t...@sss.pgh.pa.us wrote: Gurjeet Singh gurj...@singh.im writes: Please find attached the patch to send transaction commit/rollback stats to stats collector unconditionally. That's intentional to reduce stats traffic. What kind of performance penalty does this patch impose? If the number of such transactions is large enough to create a noticeable jump in the counters, I would think that this would be a pretty darn expensive fix. I can't speak to the performance impact of this patch, except that it would depend on the fraction of transactions that behave this way. Perhaps the people who develop and/or aggressively use monitoring can pitch in. Presumably, on heavily used systems these transactions would form a small fraction. On relatively idle systems these transactions may be a larger fraction but that wouldn't affect the users since the database is not under stress anyway. Best regards, -- Gurjeet Singh http://gurjeet.singh.im/ EDB www.EnterpriseDB.com http://www.enterprisedb.com
Re: [HACKERS] First-draft release notes for next week's releases
Josh Berkus wrote: On 03/19/2014 02:01 PM, Alvaro Herrera wrote: Josh Berkus wrote: All, So, I'll ask again (because I didn't see a reply): is there any way users can *check* if they've been corrupted? Short of waiting for PK/FK violations? Some notes: 1. if there's been no crash with 9.3 installed in a single system, or in a master system, corruption cannot have occured. 2. replicas are very likely to have gotten corrupted if referenced tables are updated at all. Many workloads do not update referenced tables; those are not at risk. 3. Master that are failed-over at-risk replicas are thus very likely to have been corrupted. Obviously there are queries you can run to check each FK -- the same queries that ri_triggers.c would run when you create an FK. It's cumbersome to write, but not impossible. In fact, it can be done mechanically. Would users which this corruption necessarily have broken FKs which would show up as such on a simple query? That is, if I did: SELECT ref_id FROM referenced WHERE ref_id NOT IN ( SELECT ref_id FROM referencing ) ... or something similar, would that show the issue? Yes, AFAICT that would show the issue, as long as the query uses an index. I assume, without checking, that setting enable_seqscan to OFF would have that effect on most but the largest tables. I think it'd be better to write that as an EXISTS query, though. You also need to consider details such as the MATCH mode of the FK, for multicolumn ones. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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 to send transaction commit/rollback stats to the stats collector unconditionally.
Gurjeet Singh wrote: On Wed, Mar 19, 2014 at 4:22 PM, Tom Lane t...@sss.pgh.pa.us wrote: Gurjeet Singh gurj...@singh.im writes: Please find attached the patch to send transaction commit/rollback stats to stats collector unconditionally. That's intentional to reduce stats traffic. What kind of performance penalty does this patch impose? If the number of such transactions is large enough to create a noticeable jump in the counters, I would think that this would be a pretty darn expensive fix. Presumably, on heavily used systems these transactions would form a small fraction. On relatively idle systems these transactions may be a larger fraction but that wouldn't affect the users since the database is not under stress anyway. I'm not sure I understand the point of this whole thing. Realistically, how many transactions are there that do not access any database tables? If an application doesn't want to access stored data, why would it connect to the database in the first place? (I imagine you could use it to generate random numbers and such ...) -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training 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] four minor proposals for 9.5
On 03/19/2014 04:34 PM, Pavel Stehule wrote: Hello I wrote a few patches, that we use in our production. These patches are small, but I hope, so its can be interesting for upstream: 1. cancel time - we log a execution time cancelled statements +1 I even wrote a patch to do this, but it wasn't pretty so I never posted it. I would be happy to review yours or revive mine. 2. fatal verbose - this patch ensure a verbose log for fatal errors. It simplify a investigation about reasons of error. +0 3. relation limit - possibility to set session limit for maximum size of relations. Any relation cannot be extended over this limit in session, when this value is higher than zero. Motivation - we use lot of queries like CREATE TABLE AS SELECT .. , and some very big results decreased a disk free space too much. It was high risk in our multi user environment. Motivation is similar like temp_files_limit. -1 Also, I can't find any temp_files_limit setting anywhere. 4. track statement lock - we are able to track a locking time for query and print this data in slow query log and auto_explain log. It help to us with lather slow query log analysis. -0 Do you thinking so these patches can be generally useful? Yes and no, as scored above. -- Vik -- 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] jsonb status
On Wed, Mar 19, 2014 at 9:28 AM, Andres Freund and...@2ndquadrant.com wrote: * Jsonb vs JsonbValue is just bad, the latter needs to be renamed, and there needs to be a very clear explanation about why two forms exist and what each is used for. I've pushed some comments to Github that further clarify the distinction: https://github.com/feodor/postgres/commit/5835de0b55bdfc02ee59e2affb6ce25995587dc4 I did not change the name of JsonbValue, because I sincerely could not think of a better one. The distinction is subtle. -- Peter Geoghegan -- 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 to send transaction commit/rollback stats to the stats collector unconditionally.
Alvaro Herrera alvhe...@2ndquadrant.com writes: I'm not sure I understand the point of this whole thing. Realistically, how many transactions are there that do not access any database tables? I think that something like select * from pg_stat_activity might not bump any table-access counters, once the relevant syscache entries had gotten loaded. You could imagine that a monitoring app would do a long series of those and nothing else (whether any actually do or not is a different question). But still, it's a bit hard to credit that this patch is solving any real problem. Where's the user complaints about the existing behavior? That is, even granting that anybody has a workload that acts like this, why would they care ... and are they prepared to take a performance hit to avoid the counter jump after the monitoring app exits? 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] Review: plpgsql.extra_warnings, plpgsql.extra_errors
Petr Jelinek p...@2ndquadrant.com writes: On 19/03/14 19:26, Alvaro Herrera wrote: I think this should have the GUC_LIST_INPUT flag, and ensure that when multiple values are passed, we can process them all in a sane fashion. Well, as we said with Marko in the original thread, the proper handling is left for whoever wants to add additional parameters, for the current implementation proper list handling is not really needed and it will only server to increase complexity of this simple patch quite late in the release cycle. TBH, if I thought this specific warning was the only one that would ever be there, I'd probably be arguing to reject this patch altogether. Isn't the entire point to create a framework in which more tests will be added later? Also, adding GUC_LIST_INPUT later is not really cool since it changes the parsing behavior for the GUC. If it's going to be a list, it should be one from day zero. 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] jsonb status
On 03/19/2014 06:57 PM, Peter Geoghegan wrote: On Wed, Mar 19, 2014 at 9:28 AM, Andres Freund and...@2ndquadrant.com wrote: * Jsonb vs JsonbValue is just bad, the latter needs to be renamed, and there needs to be a very clear explanation about why two forms exist and what each is used for. I've pushed some comments to Github that further clarify the distinction: https://github.com/feodor/postgres/commit/5835de0b55bdfc02ee59e2affb6ce25995587dc4 I did not change the name of JsonbValue, because I sincerely could not think of a better one. The distinction is subtle. I didn't like the _state-state stuff either, but I think you changed the wrong name - it's the field name in the struct that needs changing. What you've done is inconsistent with the common idiom in jsonfuncs.c. 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] Review: plpgsql.extra_warnings, plpgsql.extra_errors
2014-03-20 0:32 GMT+01:00 Tom Lane t...@sss.pgh.pa.us: Petr Jelinek p...@2ndquadrant.com writes: On 19/03/14 19:26, Alvaro Herrera wrote: I think this should have the GUC_LIST_INPUT flag, and ensure that when multiple values are passed, we can process them all in a sane fashion. Well, as we said with Marko in the original thread, the proper handling is left for whoever wants to add additional parameters, for the current implementation proper list handling is not really needed and it will only server to increase complexity of this simple patch quite late in the release cycle. TBH, if I thought this specific warning was the only one that would ever be there, I'd probably be arguing to reject this patch altogether. Isn't the entire point to create a framework in which more tests will be added later? I plan to work on plpgsql redesign this summer, so plpgsql check with same functionality can be on next release, but should not be too. This functionality doesn't change anything - and when we will have a better tools, we can replace it without any cost, so I am for integration. It can helps - plpgsql_check is next level, but it is next level complexity and now it is not simply to integrate it. Proposed feature can server lot of users and it is good API when we integrate some more sophisticated tool. I like this interface - it is simple and good for almost all use cases that I can to see. Regards Pavel Also, adding GUC_LIST_INPUT later is not really cool since it changes the parsing behavior for the GUC. If it's going to be a list, it should be one from day zero. regards, tom lane
Re: [HACKERS] four minor proposals for 9.5
2014-03-19 23:31 GMT+01:00 Vik Fearing vik.fear...@dalibo.com: On 03/19/2014 04:34 PM, Pavel Stehule wrote: Hello I wrote a few patches, that we use in our production. These patches are small, but I hope, so its can be interesting for upstream: 1. cancel time - we log a execution time cancelled statements +1 I even wrote a patch to do this, but it wasn't pretty so I never posted it. I would be happy to review yours or revive mine. 2. fatal verbose - this patch ensure a verbose log for fatal errors. It simplify a investigation about reasons of error. +0 3. relation limit - possibility to set session limit for maximum size of relations. Any relation cannot be extended over this limit in session, when this value is higher than zero. Motivation - we use lot of queries like CREATE TABLE AS SELECT .. , and some very big results decreased a disk free space too much. It was high risk in our multi user environment. Motivation is similar like temp_files_limit. -1 Also, I can't find any temp_files_limit setting anywhere. any our server serves a about 100 users per seconds. We have system, that generate relative complex queries, where some Cartesian products are valid (although we are not happy). This limits removes a few (but real) critical issues, when we didn't enough free space on disc. After implementation of this limit we are able to ensure safe long time production with about 20% free space. In multiuser environment is more safe to kill some queries than lost your all production. 4. track statement lock - we are able to track a locking time for query and print this data in slow query log and auto_explain log. It help to us with lather slow query log analysis. -0 Do you thinking so these patches can be generally useful? Yes and no, as scored above. -- Vik
Re: [HACKERS] four minor proposals for 9.5
Pavel, I wrote a few patches, that we use in our production. These patches are small, but I hope, so its can be interesting for upstream: 1. cancel time - we log a execution time cancelled statements Manually cancelled? statement_timeout? Anyway, +1 to add the time to the existing log message, but not in an additional log line. BTW, what do folks think of the idea of adding a new log column called timing, which would record duration etc.? It would be really nice not to have to parse this out of the text message all the time ... 2. fatal verbose - this patch ensure a verbose log for fatal errors. It simplify a investigation about reasons of error. Configurable, or not? 3. relation limit - possibility to set session limit for maximum size of relations. Any relation cannot be extended over this limit in session, when this value is higher than zero. Motivation - we use lot of queries like CREATE TABLE AS SELECT .. , and some very big results decreased a disk free space too much. It was high risk in our multi user environment. Motivation is similar like temp_files_limit. I'd think the size of the relation you were creating would be difficult to measure. Also, would this apply to REINDEX/VACUUM FULL/ALTER? Or just CREATE TABLE AS/SELECT INTO? 4. track statement lock - we are able to track a locking time for query and print this data in slow query log and auto_explain log. It help to us with lather slow query log analysis. I'm very interested in this. What does it look like? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] jsonb status
On Wed, Mar 19, 2014 at 5:10 PM, Andrew Dunstan and...@dunslane.net wrote: I didn't like the _state-state stuff either, but I think you changed the wrong name - it's the field name in the struct that needs changing. What you've done is inconsistent with the common idiom in jsonfuncs.c. Okay. I've changed the variable name back, while changing the struct field as suggested. -- Peter Geoghegan -- 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 to send transaction commit/rollback stats to the stats collector unconditionally.
On Wed, Mar 19, 2014 at 7:27 PM, Tom Lane t...@sss.pgh.pa.us wrote: Alvaro Herrera alvhe...@2ndquadrant.com writes: I'm not sure I understand the point of this whole thing. Realistically, how many transactions are there that do not access any database tables? I think that something like select * from pg_stat_activity might not bump any table-access counters, once the relevant syscache entries had gotten loaded. You could imagine that a monitoring app would do a long series of those and nothing else (whether any actually do or not is a different question). But still, it's a bit hard to credit that this patch is solving any real problem. Where's the user complaints about the existing behavior? That is, even granting that anybody has a workload that acts like this, why would they care ... and are they prepared to take a performance hit to avoid the counter jump after the monitoring app exits? Well, EnterpriseDB has a monitoring product called Postgres Enterprise Manager (PEM) that sits around and does stuff like periodically reading statistics views. I think you can probably configure it to read from regular tables too, but it's hardly insane that someone would have a long-running monitoring connection that only reads statistics and monitoring views. (This is not a vote for or against the patch, which I have not read. It's just an observation.) -- 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] four minor proposals for 9.5
2014-03-20 1:28 GMT+01:00 Josh Berkus j...@agliodbs.com: Pavel, I wrote a few patches, that we use in our production. These patches are small, but I hope, so its can be interesting for upstream: 1. cancel time - we log a execution time cancelled statements Manually cancelled? statement_timeout? Manually cancelled - we have a more levels - user cancel 3..10 minutes, query executor timeout 20 minutes, and hard core statement limit 25 minutes. logging of execution time helps to us identify reason of cancel, and helps to us identify impatient user (and where is a limit). It is same like query time, when you log it. Anyway, +1 to add the time to the existing log message, but not in an additional log line. BTW, what do folks think of the idea of adding a new log column called timing, which would record duration etc.? It would be really nice not to have to parse this out of the text message all the time ... 2. fatal verbose - this patch ensure a verbose log for fatal errors. It simplify a investigation about reasons of error. Configurable, or not? we have not configuration, but it should be configurable naturally - A main motivation about this feature was a same message for more errors - and fatal level helps to us identify a source. But we cannot to enable verbose level as default due log size and log overhead. 3. relation limit - possibility to set session limit for maximum size of relations. Any relation cannot be extended over this limit in session, when this value is higher than zero. Motivation - we use lot of queries like CREATE TABLE AS SELECT .. , and some very big results decreased a disk free space too much. It was high risk in our multi user environment. Motivation is similar like temp_files_limit. I'd think the size of the relation you were creating would be difficult to measure. Also, would this apply to REINDEX/VACUUM FULL/ALTER? Or just CREATE TABLE AS/SELECT INTO? It was only relation limit without indexes or anything else. It just early stop for queries where statement timeout is too late (and allocated space on disc is too long). Our statement limit is 20 minutes and then a query can create table about 100GB - only ten unlimited users had to take our full free space on Amazon disc. 4. track statement lock - we are able to track a locking time for query and print this data in slow query log and auto_explain log. It help to us with lather slow query log analysis. I'm very interested in this. What does it look like? We divided locks to three kinds (levels): tables, tuples, and others. As results we print three numbers for any SQL statement - waiting to table's locks, waiting to tuple's locks and waiting to other's locks (extending page locks and similar). I don't remember so we used any info in this detail's level, but it is interesting for slow queries. You don't spend time over analyse of mystical fast/slow queries - you see clearly so problem was in locks. Regards Pavel -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com
Re: Custom Scan APIs (Re: [HACKERS] Custom Plan node)
Hello,
* Definition of add_join_path_hook
I didn't have idea to improve the definition and location of this
hook, so it is still on the tail of the add_paths_to_joinrel().
Its definition was a bit adjusted according to the feedback on the
pgsql-hackers. I omitted the mergeclause_list and semifactors
from the argument list. Indeed, these are specific to the built-in
MergeJoin logic and easy to reproduce.
After the submission, I'm still investigating better way to put a hook
to add custom join paths.
Regarding to the comment from Tom:
| * The API for add_join_path_hook seems overcomplex, as well as too full
| of implementation details that should remain private to joinpath.c.
| I particularly object to passing the mergeclause lists, which seem unlikely
| to be of interest for non-mergejoin plan types anyway.
| More generally, it seems likely that this hook is at the wrong level of
| abstraction; it forces the hook function to concern itself with a lot of
| stuff about join legality and parameterization (which I note your patch3
| code fails to do; but that's not an optional thing).
|
The earlier half was already done. My trouble is the later portion.
The overall jobs of add_join_path_hook are below:
1. construction of parameterized path information; being saved at
param_source_rel and extra_lateral_rels.
2. Try to add mergejoin paths with underlying Sort node
3. Try to add mergejoin/nestloop paths without underlying Sort node
4. Try to add hashjoin paths
It seems to me the check for join legality and parameterization are
built within individual routines for each join algorithm.
(what does the join legality check actually mean?)
Probably, it makes sense to provide a common utility function to be
called back from the extension if we can find out a common part for
all the join logics, however, I don't have clear idea to cut off the
common portion. What's jobs can be done independent from the join
algorithm??
I'd like to see ideas around this issue. Of course, I also think it
is still an option to handle it by extension on the initial version.
Thanks,
--
NEC OSS Promotion Center / PG-Strom Project
KaiGai Kohei kai...@ak.jp.nec.com
-Original Message-
From: pgsql-hackers-ow...@postgresql.org
[mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Kouhei Kaigai
Sent: Monday, March 17, 2014 9:29 AM
To: Kaigai Kouhei(海外 浩平); Tom Lane
Cc: Kohei KaiGai; Stephen Frost; Shigeru Hanada; Jim Mlodgenski; Robert
Haas; PgHacker; Peter Eisentraut
Subject: Re: Custom Scan APIs (Re: [HACKERS] Custom Plan node)
Hello,
I adjusted the custom-plan interface patch little bit for the cache-only
scan patch; that is a demonstration module for vacuum-page hook on top of
the custom-plan interface.
fix_scan_expr() looks to me useful for custom-plan providers that want to
implement its own relation scan logic, even though they can implement it
using fix_expr_common() being already exposed.
Also, I removed the hardcoded portion from the nodeCustom.c although, it
may make sense to provide a few template functions to be called by custom-
plan providers, that performs usual common jobs like construction of expr-
context, assignment of result-slot, open relations, and so on.
I though the idea during implementation of BeginCustomPlan handler.
(These template functions are not in the attached patch yet.) How about
your opinion?
The major portion of this patch is not changed from v10.
Thanks,
--
NEC OSS Promotion Center / PG-Strom Project KaiGai Kohei
kai...@ak.jp.nec.com
-Original Message-
From: pgsql-hackers-ow...@postgresql.org
[mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Kouhei Kaigai
Sent: Wednesday, March 12, 2014 1:55 PM
To: Tom Lane
Cc: Kohei KaiGai; Stephen Frost; Shigeru Hanada; Jim Mlodgenski;
Robert Haas; PgHacker; Peter Eisentraut
Subject: Re: Custom Scan APIs (Re: [HACKERS] Custom Plan node)
Hello,
The attached two patches are the revised custom-plan interface and
example usage that implements existing MergeJoin on top of this interface.
According to the discussion last week, I revised the portion where
custom-node is expected to perform a particular kind of task, like
scanning a relation, by putting polymorphism with a set of callbacks
set by custom-plan provider.
So, the core backend can handle this custom-plan node just an
abstracted plan-node with no anticipation.
Even though the subject of this message says custom-scan, I'd like
to name the interface custom-plan instead, because it became fully
arbitrary of extension whether it scan on a particular relation.
Definition of Custom data types were simplified:
typedef struct CustomPath
{
Pathpath;
const struct CustomPathMethods *methods;
} CustomPath;
typedef struct CustomPlan
{
Planplan;
const struct CustomPlanMethods *methods;
}
Re: [HACKERS] Patch to send transaction commit/rollback stats to the stats collector unconditionally.
On Wed, Mar 19, 2014 at 7:27 PM, Tom Lane t...@sss.pgh.pa.us wrote: Alvaro Herrera alvhe...@2ndquadrant.com writes: I'm not sure I understand the point of this whole thing. Realistically, how many transactions are there that do not access any database tables? I think that something like select * from pg_stat_activity might not bump any table-access counters, once the relevant syscache entries had gotten loaded. You could imagine that a monitoring app would do a long series of those and nothing else (whether any actually do or not is a different question). +1. This is exactly what I was doing; querying pg_stat_database from a psql session, to track transaction counters. But still, it's a bit hard to credit that this patch is solving any real problem. Where's the user complaints about the existing behavior? Consider my original email a user complaint, albeit with a patch attached. I was trying to ascertain TPS on a customer's instance when I noticed this behaviour; it violated POLA. Had I not decided to dig through the code to find the source of this behaviour, as a regular user I would've reported this anomaly as a bug, or maybe turned a blind eye to it labeling it as a quirky behaviour. That is, even granting that anybody has a workload that acts like this, why would they care ... To avoid surprises! This behaviour, at least in my case, causes Postgres to misreport the very thing I am trying to calculate. and are they prepared to take a performance hit to avoid the counter jump after the monitoring app exits? It doesn't look like we know how much of a performance hit this will cause. I don't see any reasons cited in the code, either. Could this be a case of premature optimization. The commit log shows that, during the overhaul (commit 77947c5), this check was put in place to emulate what the previous code did; code before that commit reported stats, including transaction stats, only if there were any regular or shared table stats to report. Besides, there's already a throttle built in using the PGSTAT_STAT_INTERVAL limit. Best regards, -- Gurjeet Singh http://gurjeet.singh.im/ EDB www.EnterpriseDB.com http://www.enterprisedb.com
Re: [HACKERS] Minimum supported version of Python?
I wrote: Peter Eisentraut pete...@gmx.net writes: It does pass the tests for me and others. If you are seeing something different, then we need to see some details, like what platform, what version, what Python version, how installed, what PostgreSQL version, how installed, actual diffs, etc. After further experimentation, I've found that 2.3 does pass the regression tests if one installs the separately-available cdecimal module. Well ... it passes in C locale, anyway. 9.1 appears to have a problem if using UTF8 encoding: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=prairiedogdt=2014-03-19%2017%3A00%3A48 I've had a hard time getting details, since Apple didn't supply debug symbols for their Python build, but the segfault is definitely happening down inside PyRun_String() called from plpython_validator(). If you just fire up a fresh session and execute the troublesome CREATE FUNCTION, it's fine; which says to me that some previous operation in the plpython_trigger test script tromped on data structures private to Python. Our other branches pass on the identical installation. Now, 9.0 didn't even have a validator, which might just mean that it's failing to trip over a memory clobber that happened anyway. And 9.2 is so completely refactored that it's hard to tell whether it incorporates a fix for a memory clobber compared to 9.1; or maybe it's just accidentally avoiding the crash symptom, too. But there's something rotten in the state of Denmark. 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] issue log message to suggest VACUUM FULL if a table is nearly empty
On Wed, Mar 19, 2014 at 6:25 AM, Wang, Jing ji...@fast.au.fujitsu.com wrote: On Friday, 14 March 2014 2:42 PM, Amit Kapila amit.kapil...@gmail.com wrote: I think it might be okay to even change this API to return the FreeSpace, as the other place it is used is for Index Vacuum, so even if we don't have any intention to print such a message for index in this patch, but similar information could be useful there as well to suggest a user that index has lot of free space. Enclosed please find the new patch which get the FreeSpace for one relation from the return of FreeSpaceMapVacuum() function. This function and the fsm_vacuum_page() function have been slightly modified to get the FreeSpace and no I/O burden increasing. The little side-effect is it will calculate FreeSpace for every table even the table is very small. I think that can also be avoided, because by the time you call FreeSpaceMapVacuum(), you already have the required information based on which you can decide not to ask for freespace if required. Can't we avoid the new calculation you have added in fsm_vacuum_page(), as this function already updates the size, so might be we can get it from current calculation done in function. + #define RELPAGES_VALUES_THRESHOLD 1000 + #define FREESPACE_PERCENTAGE_THRESHOLD 0.5 Is there any basis to define above hash defines, we already have one number similar to above for deciding Truncate of relation. In anycase, I think the patch's direction is better than previous and can be further discussed/reviewed during next CF, as it's already quite late for 9.4. With Regards, Amit Kapila. 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] four minor proposals for 9.5
On Wed, Mar 19, 2014 at 9:04 PM, Pavel Stehule pavel.steh...@gmail.com wrote: Hello I wrote a few patches, that we use in our production. These patches are small, but I hope, so its can be interesting for upstream: 1. cancel time - we log a execution time cancelled statements 2. fatal verbose - this patch ensure a verbose log for fatal errors. It simplify a investigation about reasons of error. 3. relation limit - possibility to set session limit for maximum size of relations. Any relation cannot be extended over this limit in session, when this value is higher than zero. Motivation - we use lot of queries like CREATE TABLE AS SELECT .. , and some very big results decreased a disk free space too much. It was high risk in our multi user environment. Motivation is similar like temp_files_limit. So if there is error on reaching max threshold size, won't it loose all data or is that expected? Also I think it might not be applicable for all table inserts, as normally checkpointer/bgrwriter flushes data, so you might not be able to estimate size immediately when your SQL statement is executing. Won't it better to have LIMIT Rows in Select statement or generically have table level option for Max Rows? With Regards, Amit Kapila. 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
