Re: [HACKERS] Fix Error Message for allocate_recordbuf() Failure
On 11 July 2016 at 12:04, Michael Paquier wrote: > On Sat, Jul 9, 2016 at 2:58 AM, Shoaib Lari wrote: > > Besides making the error message more informative, we had to modify > > allocate_recordbuf() to return the actual number of bytes that were being > > allocated. > > - report_invalid_record(state, "record length %u at %X/%X too long", > - total_len, > - (uint32) (RecPtr >> 32), (uint32) RecPtr); > + report_invalid_record(state, > + "cannot allocate %u bytes for record > length %u at %X/%X", > + newSizeOut, total_len, (uint32) (RecPtr >> > 32), > + (uint32) RecPtr); > > It does not look like a good idea to me to complicate the interface of > allocate_recordbuf just to make more verbose one error message, > meaning that it basically a complain related to the fact that > palloc_extended(MCXT_ALLOC_NO_OOM) does not mention to the user the > size that it has tried to allocate before returning NULL. Do you have > a use case that would prove to be useful if this extra piece of > information is provided? Because it seems to me that we don't really > care if we know how much memory it has failed to allocate, we only > want to know that it *has* failed and take actions only based on that. > I actually find details of how much memory we tried to allocate to be really useful in other places that do emit it. Sometimes it's been an important clue when trying to figure out what's going on on a remote system with no or limited access. Even if it just tells me "we were probably incrementally allocating lots of small values since this failure is small" vs "this allocation is huge, look for something unusually large" or "this allocation is impossibly huge / some suspicious value look for memory corruption". I tend to agree with your suggestion about a better approach but do think emitting details on allocation failures is useful. At least when people turn VM overcommit off ... -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: [HACKERS] pgbench - minor doc improvements
Hello Michaël, Minor pgbench documentation improvements so that the description is more precise: - a pgbench script may not contain SQL commands, it only needs not to be empty. Halfly true as far as I recall. This works and generates two queries: SELECT 1; \set two 3 Maybe it used to work, but not anymore: client 0 aborted in state 0: ERROR: syntax error at or near "\" LINE 1: SELECT 1 ; \set two 3 - point out explicitely variable setting meta commands. - the formula is short enough to fit on a line. -f(x) = PHI(2.0 * parameter * (x - mu) / (max - min + 1)) / - (2.0 * PHI(parameter) - 1) +f(x) = PHI(2.0 * parameter * (x - mu) / (max - min + 1)) / (2.0 * PHI(parameter) - 1) At full-length this is 85 characters. I agree on principle, however the issue is the "literal layout", the line is also broken in the resulting HTML and it looks pretty strange there: https://www.postgresql.org/docs/devel/static/pgbench.html But I agree that it is more readable to put that into a single line. Now we could as well trick the limit by using "param" instead of "parameter". ISTM that it would mean changing the text in a dozen places to be consistent, and it would not necessary be an improvement in some of those places... So I'm hesitant to follow on that suggestion. For me the "under 80 column" rule cannot really apply on a "literal layout" block. -- Fabien. -- 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] Disable WAL completely - Performance and Persistency research
On 10 July 2016 at 18:27, Netanel Katzburg wrote: > BUT, both options are not good, as they are stopping me from even running i > *nitdb.* > > > The easiest path for testing will be to use an unpatched PostgreSQL to `initdb` and create a new database. Then start up a patched one that simply skips WAL writing against an already-`initdb`'d data directory. You probably won't be able to safely restart PostgreSQL, but all you're doing is performance analsys so one-shot operation on a throw-away data directory is probably fine. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: [HACKERS] PSA: Systemd will kill PostgreSQL
On 11 July 2016 at 01:56, Joshua D. Drake wrote: > Hackers, > > This just came across my twitter feed: > > https://lists.freedesktop.org/archives/systemd-devel/2014-April/018373.html > > tl;dr; Systemd 212 defaults to remove all IPC (including SYSV memory) when > a user "fully" logs out. > > The underlying change sounds like a fix, not a problem. It ensures that when a user logs out, various dangling processes are cleaned up. Given the amount of work PostgreSQL has to do to try to make sure it's really gone, having systemd be able to just clobber everything is pretty nice. So long as there's control over it. However, it will break existing deployments that use "non-system" users to run PostgreSQL. I had a look and didn't find any useful definition of what systemd considers a "system user". Perhaps by uid threshold in login.defs? But then what happens for people who're managing users via a directory, who need to avoid conflicting with host-local UIDs, but also need some of those users to have systemd "system user" like behaviour? It's also not clear if there's any API apps can use to exempt themselves from this, or any wrapper command to spawn processes that aren't clobbered. With appropriate user privileges to permit it, at least. I've asked for clarification on the bug, so I'd better don my fire-proof suit. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: [COMMITTERS] [HACKERS] Logical decoding
On Mon, Jul 11, 2016 at 2:03 PM, Craig Ringer wrote: > On 9 July 2016 at 01:57, Joshua Bay wrote: >> and where are this code is in the codebase? > > src/backend/replication/logical/* > src/backend/replication/walsender.c > src/backend/access/transam/xlogreader.c > src/include/access/xlogreader.h > src/include/replication/output_plugin.h > contrib/test_decoding/ > doc/src/sgml/logicaldecoding.sgml Some other references: https://wiki.postgresql.org/images/7/77/Fosdem-2015-logical-decoding.pdf And some other examples of plugins: https://github.com/leptonix/decoding-json https://github.com/xstevens/decoderbufs https://github.com/confluentinc/bottledwater-pg (for kafka) https://github.com/michaelpq/pg_plugins/tree/master/decoder_raw (wrote this one) -- Michael -- 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] pgbench - minor fix for meta command only scripts
Hello Michaël, You may want to name your patches with .patch or .diff. Using .sql is disturbing style :) Indeed! :-) Indeed, not reporting the progress back to the client in the case of a script with only meta commands is non-intuitive. This looks good to me. I'd just rewrite the comment block with something like that, more simplified: Ok. Here is an updated version, with a better suffix and a simplified comment. Thanks, -- Fabien.diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index 87fb006..4e7449e 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -2172,8 +2172,12 @@ top: st->listen = true; } - /* after a meta command, immediately proceed with next command */ - goto top; + /* + * After a meta command immediately proceed with next command, + * but if it is the last command, just leave. + */ + if (commands[st->state + 1] != NULL) + goto top; } return 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] Logical decoding
On 9 July 2016 at 01:57, Joshua Bay wrote: > where are the entry points to logical decoding? > Well, it depends on what level you're looking at. Data is read using the xlogreader and fed into the decoding system by the walsender (when using a logical slot) and the SQL level pg_logical_slot_[get|peek]_changes functions. See XLogSendLogical() as called by WalSndLoop() and see pg_logical_slot_get_changes_guts() . WAL records are passed through to logical decoding and depending on the xlog entry type the reorder buffer and/or snapshot builder. Then when a transaction commit is detected the registered output plugin is invoked and its callbacks are used to process the buffered transaction. > Specifically, we want to know whether logical decoding happens immediately > after commit, or whether there is a polling thread that scans the Write > Ahead Log and then dumps to the special table? > There's no "polling thread", but that's probably the closer of the two. A walsender using a logical slot reads WAL as it becomes available. It sleeps on a latch if it runs out of WAL to read and is woken when there's more. It isn't dumped to some special table, it's managed by the reorder buffer infrastructure which uses its own special data structures. Reading may lag behind WAL generation since it's "pulled" by the client and happens lazily after commit. > and where are this code is in the codebase? > src/backend/replication/logical/* src/backend/replication/walsender.c src/backend/access/transam/xlogreader.c src/include/access/xlogreader.h src/include/replication/output_plugin.h contrib/test_decoding/ doc/src/sgml/logicaldecoding.sgml -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: [HACKERS] Issue with bgworker, SPI and pgstat_report_stat
On 11 July 2016 at 11:49, Michael Paquier wrote: > On Mon, Jul 11, 2016 at 3:36 AM, Julien Rouhaud > wrote: > > I'm not opposed, but in this case we should also provide a proper > > documentation of all the required actions to mimick normal backends. > > I'd rather just add a note like "Have a look at PostgresMain if you > want to imitate a backend able to run queries!" instead of listing all > the actions doable. If low-level things are added or removed in the > future in PostgresMain, it is very likely that the documentation will > not be updated at the same time, killing its purpose at the end. That sounds like a bug breeding ground. Especially with extensions whose bgworkers operate across Pg versions, extensions that get updated without re-checking PostgresMain and don't notice some new housekeeping task, etc. Rather than encouraging every extension author to copy and paste random chunks of PostgresMain, probably incorrectly, I really think factoring the required logic out into something reusable by bgworkers is going to be the way to go. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Re: [HACKERS] pgbench - minor doc improvements
On Sat, Jul 9, 2016 at 4:48 PM, Fabien COELHO wrote: > > Minor pgbench documentation improvements so that the description is more > precise: > > - a pgbench script may not contain SQL commands, it only needs not to be >empty. Halfly true as far as I recall. This works and generates two queries: SELECT 1; \set two 3 SELECT :two; But any query added after the meta-command cannot be parsed. > - point out explicitely variable setting meta commands. > - the formula is short enough to fit on a line. -f(x) = PHI(2.0 * parameter * (x - mu) / (max - min + 1)) / - (2.0 * PHI(parameter) - 1) +f(x) = PHI(2.0 * parameter * (x - mu) / (max - min + 1)) / (2.0 * PHI(parameter) - 1) At full-length this is 85 characters. But I agree that it is more readable to put that into a single line. Now we could as well trick the limit by using "param" instead of "parameter". -- Michael -- 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] pgbench - minor fix for meta command only scripts
On Sat, Jul 9, 2016 at 4:09 PM, Fabien COELHO wrote: > > While testing meta-command pgbench only scripts, I noticed that there is an > infinite loop in threadRun, which means that other tasks such as reporting > progress do not get a chance. > > The attached patch breaks this loop by always returning at the end of a > script. > > On "pgbench -T 3 -P 1 -f noop.sql", before this patch, the progress is not > shown, after it is. You may want to name your patches with .patch or .diff. Using .sql is disturbing style :) Indeed, not reporting the progress back to the client in the case of a script with only meta commands is non-intuitive. - /* after a meta command, immediately proceed with next command */ - goto top; + /* +* After a meta command, immediately proceed with next command... +* although not if last. This exception ensures that a meta command +* only script does not always loop in doCustom, so that other tasks +* in threadRun, eg progress reporting or switching client, get a chance. +*/ + if (commands[st->state + 1] != NULL) + goto top; This looks good to me. I'd just rewrite the comment block with something like that, more simplified: + /* +* After a meta command, immediately proceed with next command. +* But if this is the last command, just leave. +*/ -- Michael -- 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] Fix Error Message for allocate_recordbuf() Failure
On Sat, Jul 9, 2016 at 2:58 AM, Shoaib Lari wrote: > Besides making the error message more informative, we had to modify > allocate_recordbuf() to return the actual number of bytes that were being > allocated. - report_invalid_record(state, "record length %u at %X/%X too long", - total_len, - (uint32) (RecPtr >> 32), (uint32) RecPtr); + report_invalid_record(state, + "cannot allocate %u bytes for record length %u at %X/%X", + newSizeOut, total_len, (uint32) (RecPtr >> 32), + (uint32) RecPtr); It does not look like a good idea to me to complicate the interface of allocate_recordbuf just to make more verbose one error message, meaning that it basically a complain related to the fact that palloc_extended(MCXT_ALLOC_NO_OOM) does not mention to the user the size that it has tried to allocate before returning NULL. Do you have a use case that would prove to be useful if this extra piece of information is provided? Because it seems to me that we don't really care if we know how much memory it has failed to allocate, we only want to know that it *has* failed and take actions only based on that. And even if we make this thing more verbose, a better approach would be surely to generate a WARNING message for backends in mcxt.c and have something printed to stderr for frontends in fe_memutils.c without calling exit(). -- Michael -- 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 with bgworker, SPI and pgstat_report_stat
On Mon, Jul 11, 2016 at 3:36 AM, Julien Rouhaud wrote: > I'm not opposed, but in this case we should also provide a proper > documentation of all the required actions to mimick normal backends. I'd rather just add a note like "Have a look at PostgresMain if you want to imitate a backend able to run queries!" instead of listing all the actions doable. If low-level things are added or removed in the future in PostgresMain, it is very likely that the documentation will not be updated at the same time, killing its purpose at the end. -- Michael -- 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] Showing parallel status in \df+
On Mon, Jul 11, 2016 at 12:42 AM, Tom Lane wrote: > If we're keeping the "Source code" column, I'd be inclined to keep > "Language" adjacent to that. When thinking of a function as a black > box, both language and source code are implementation details; but > all the other properties listed here are of interest anyway. OK, no objections to that. And this gives the attached. > (Of course, if we were to get rid of "Source code", the point > would be moot ...) I still think that having source code is useful for debugging, so I left it out. Note for the committer who will perhaps pick up this patch: I left out "Source Code", but feel free to remove it if you think the contrary. It is easier to remove code than adding it back. -- Michael diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index aeffd63..e7bd2d7 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -1380,8 +1380,9 @@ testdb=> objects are shown; supply a pattern or the S modifier to include system objects. If the form \df+ is used, additional information -about each function is shown, including security classification, -volatility, owner, language, source code and description. +about each function is shown, including language, volatility, +parallel mode, owner, security classification, access privileges, +source code and description. diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index 2cdc5ac..8559b68 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -298,7 +298,7 @@ describeFunctions(const char *functypes, const char *pattern, bool verbose, bool PQExpBufferData buf; PGresult *res; printQueryOpt myopt = pset.popt; - static const bool translate_columns[] = {false, false, false, false, true, true, true, false, false, false, false}; + static const bool translate_columns[] = {false, false, false, false, true, true, false, false, true, false, false, true, false}; if (strlen(functypes) != strspn(functypes, "antwS+")) { @@ -410,28 +410,42 @@ describeFunctions(const char *functypes, const char *pattern, bool verbose, bool gettext_noop("Type")); if (verbose) + { appendPQExpBuffer(&buf, - ",\n CASE WHEN prosecdef THEN '%s' ELSE '%s' END AS \"%s\"" ",\n CASE\n" " WHEN p.provolatile = 'i' THEN '%s'\n" " WHEN p.provolatile = 's' THEN '%s'\n" " WHEN p.provolatile = 'v' THEN '%s'\n" " END as \"%s\"" - ",\n pg_catalog.pg_get_userbyid(p.proowner) as \"%s\",\n" - " l.lanname as \"%s\",\n" - " p.prosrc as \"%s\",\n" - " pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"", - gettext_noop("definer"), - gettext_noop("invoker"), - gettext_noop("Security"), + ",\n CASE\n" + " WHEN p.proparallel = 'r' THEN '%s'\n" + " WHEN p.proparallel = 's' THEN '%s'\n" + " WHEN p.proparallel = 'u' THEN '%s'\n" + " END as \"%s\"" + ",\n pg_catalog.pg_get_userbyid(p.proowner) as \"%s\"" + ",\n CASE WHEN prosecdef THEN '%s' ELSE '%s' END AS \"%s\"", gettext_noop("immutable"), gettext_noop("stable"), gettext_noop("volatile"), gettext_noop("Volatility"), + gettext_noop("restricted"), + gettext_noop("safe"), + gettext_noop("unsafe"), + gettext_noop("Parallel"), gettext_noop("Owner"), + gettext_noop("definer"), + gettext_noop("invoker"), + gettext_noop("Security")); + appendPQExpBufferStr(&buf, ",\n "); + printACLColumn(&buf, "p.proacl"); + appendPQExpBuffer(&buf, + ",\n l.lanname as \"%s\"" + ",\n p.prosrc as \"%s\"" + ",\n pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"", gettext_noop("Language"), gettext_noop("Source code"), gettext_noop("Description")); + } appendPQExpBufferStr(&buf, "\nFROM pg_catalog.pg_proc p" -- 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] [CF2016-9] Allow spaces in working path on tap-tests
On Sun, Jul 10, 2016 at 11:52 PM, Tom Lane wrote: > Michael Paquier writes: >> Thanks! What you have pushed looks fine to me. Also, the portion for >> src/tools/msvc needs to go further down, I should have precised that >> earlier. Do you want a patch for that? > > Yes, please --- I thought it'd all gotten done. OK, here are patches for 9.1, 9.2 and 9.3. -- Michael diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm index b779b31..bbb5c39 100644 --- a/src/tools/msvc/Install.pm +++ b/src/tools/msvc/Install.pm @@ -268,12 +268,19 @@ sub GenerateTimezoneFiles my $conf = shift; my $mf = read_file("src/timezone/Makefile"); $mf =~ s{\\\s*[\r\n]+}{}mg; -$mf =~ /^TZDATA\s*:?=\s*(.*)$/m || die "Could not find TZDATA row in timezone makefile\n"; +$mf =~ /^TZDATA\s*:?=\s*(.*)$/m || die "Could not find TZDATA line in timezone makefile\n"; my @tzfiles = split /\s+/,$1; -unshift @tzfiles,''; print "Generating timezone files..."; -system( -"$conf\\zic\\zic -d \"$target/share/timezone\" " . join(" src/timezone/data/", @tzfiles)); +my @args = ("$conf/zic/zic", +'-d', +"$target/share/timezone"); +foreach (@tzfiles) +{ +my $tzfile = $_; +push(@args, "src/timezone/data/$tzfile") +} + +system(@args); print "\n"; } @@ -490,8 +497,10 @@ sub CopyIncludeFiles next unless (-d "src/include/$d"); EnsureDirectories("$target/include/server/$d"); -system(qq{xcopy /s /i /q /r /y src\\include\\$d\\*.h "$ctarget\\include\\server\\$d\\"}) - && croak("Failed to copy include directory $d\n"); +my @args = ('xcopy', '/s', '/i', '/q', '/r', '/y', +"src\\include\\$d\\*.h", +"$ctarget\\include\\server\\$d\\"); +system(@args) && croak("Failed to copy include directory $d\n"); } closedir($D); @@ -545,9 +554,11 @@ sub GenerateNLSFiles $lang = $1; EnsureDirectories($target, "share/locale/$lang", "share/locale/$lang/LC_MESSAGES"); -system( -"\"$nlspath\\bin\\msgfmt\" -o \"$target\\share\\locale\\$lang\\LC_MESSAGES\\$prgm-$majorver.mo\" $_" -)&& croak("Could not run msgfmt on $dir\\$_"); +my @args = ("$nlspath\\bin\\msgfmt", +'-o', +"$target\\share\\locale\\$lang\\LC_MESSAGES\\$prgm-$majorver.mo", +$_); +system(@args) && croak("Could not run msgfmt on $dir\\$_"); print "."; } } diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm index 7205e23..671d860 100644 --- a/src/tools/msvc/Install.pm +++ b/src/tools/msvc/Install.pm @@ -309,12 +309,21 @@ sub GenerateTimezoneFiles my $mf = read_file("src/timezone/Makefile"); $mf =~ s{\\\s*[\r\n]+}{}mg; $mf =~ /^TZDATA\s*:?=\s*(.*)$/m - || die "Could not find TZDATA row in timezone makefile\n"; + || die "Could not find TZDATA line in timezone makefile\n"; my @tzfiles = split /\s+/, $1; - unshift @tzfiles, ''; + print "Generating timezone files..."; - system("$conf\\zic\\zic -d \"$target/share/timezone\" " - . join(" src/timezone/data/", @tzfiles)); + + my @args = ("$conf/zic/zic", +'-d', +"$target/share/timezone"); + foreach (@tzfiles) + { + my $tzfile = $_; + push(@args, "src/timezone/data/$tzfile") + } + + system(@args); print "\n"; } @@ -542,9 +551,10 @@ sub CopyIncludeFiles next unless (-d "src/include/$d"); EnsureDirectories("$target/include/server/$d"); - system( -qq{xcopy /s /i /q /r /y src\\include\\$d\\*.h "$ctarget\\include\\server\\$d\\"} - ) && croak("Failed to copy include directory $d\n"); + my @args = ('xcopy', '/s', '/i', '/q', '/r', '/y', + "src\\include\\$d\\*.h", + "$ctarget\\include\\server\\$d\\"); + system(@args) && croak("Failed to copy include directory $d\n"); } closedir($D); @@ -597,9 +607,11 @@ sub GenerateNLSFiles EnsureDirectories($target, "share/locale/$lang", "share/locale/$lang/LC_MESSAGES"); - system( -"\"$nlspath\\bin\\msgfmt\" -o \"$target\\share\\locale\\$lang\\LC_MESSAGES\\$prgm-$majorver.mo\" $_" - ) && croak("Could not run msgfmt on $dir\\$_"); + my @args = ("$nlspath\\bin\\msgfmt", + '-o', + "$target\\share\\locale\\$lang\\LC_MESSAGES\\$prgm-$majorver.mo", + $_); + system(@args) && croak("Could not run msgfmt on $dir\\$_"); print "."; } } diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl index adbfa9f..390de4e 100644 --- a/src/tools/msvc/vcregress.pl +++ b/src/tools/msvc/vcregress.pl @@ -270,7 +270,7 @@ sub upgradecheck Install($tmp_install, $config); # Install does a chdir, so change back after that chdir $cwd; - my ($bindir,$libdir,$oldsrc,$newsrc) = + my ($bindir,$libdir,$oldsrc,$newsrc) = ("$tmp_install/bin", "$tmp_install/lib", $topdir, $topdir); $ENV{PATH} = "$bindir;$ENV{PATH}"; my $data = "$tmp_root/da
Re: [HACKERS] doc: Incorrect return type of IsForeignScanParallelSafe in fdwhandler.sgml
On 2016/07/09 1:41, Tom Lane wrote: Etsuro Fujita writes: I noticed that the return type of IsForeignScanParallelSafe described in fdwhandler.sgml isn't correct; that should be bool, not Size. Please find attached a small patch for that. Pushed, thanks! Thank you. Best regards, Etsuro Fujita -- 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_hba_lookup function to get all matching pg_hba.conf entries
On Sat, Apr 9, 2016 at 6:36 AM, Tom Lane wrote: > > More generally, I'm not convinced about the use-case for this patch. > What problem is it supposed to help in dealing with, exactly? Not syntax > errors in the hba file, evidently, since it doesn't make any attempt to > instrument the file parser. And it's not very clear that it'll help > with "I can't connect", either, because if you can't connect you're > not going to be running this function. Apologies for replying an old thread. The main reason behind of this patch is for the administrators to control and verify the authentication mechanism that was deployed for several users in the database is correctly picking the assigned hba config or not? I feel this SQL function is useful for administrators and not for normal users. If anyone is not against to the above use case, i will update the patch based on the review comments and post it later. Regards, Hari Babu Fujitsu Australia -- 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] can we optimize STACK_DEPTH_SLOP
I wrote: > So, agreed, let's commit some temporary debug code and see what the > buildfarm can teach us. Will go work on that in a bit. After reviewing the buildfarm results, I'm feeling nervous about this whole idea again. For the most part, the unaccounted-for daylight between the maximum stack depth measured by check_stack_depth and the actually dirtied stack space reported by pmap is under 100K. But there are a pretty fair number of exceptions. The worst cases I found were on "dunlin", which approached 200K extra space in a couple of places: dunlin| 2016-07-09 22:05:09 | check.log | 72667000 268 208 208 rw--- [ stack ] dunlin| 2016-07-09 22:05:09 | check.log | max measured stack depth 14kB dunlin| 2016-07-09 22:05:09 | install-check-C.log | 7fffee65 268 200 200 rw--- [ stack ] dunlin| 2016-07-09 22:05:09 | install-check-C.log | max measured stack depth 14kB This appears to be happening in the tsdicts test script. Other machines also show a significant discrepancy between pmap and check_stack_depth results for that test, which suggests that maybe the tsearch code is being overly reliant on large local variables. But I haven't dug through it. Another area of concern is PLs. For instance, on capybara, a machine otherwise pretty unexceptional in stack-space appetite, quite a few of the PL tests ate ~100K of unaccounted-for space: capybara | 2016-07-09 21:15:56 | pl-install-check-C.log | 7ffc61bbe000 132 104 104 rw---[ stack ] capybara | 2016-07-09 21:15:56 | pl-install-check-C.log | 7ffc61bbe000 132 0 0 rw---[ stack ] capybara | 2016-07-09 21:15:56 | pl-install-check-C.log | max measured stack depth 8kB capybara | 2016-07-09 21:15:56 | pl-install-check-C.log | 7ffc61bbd000 136 136 136 rw---[ stack ] capybara | 2016-07-09 21:15:56 | pl-install-check-C.log | 7ffc61bbd000 136 0 0 rw---[ stack ] capybara | 2016-07-09 21:15:56 | pl-install-check-C.log | max measured stack depth 0kB capybara | 2016-07-09 21:15:56 | pl-install-check-C.log | 7ffc61bbe000 132 104 104 rw---[ stack ] capybara | 2016-07-09 21:15:56 | pl-install-check-C.log | 7ffc61bbe000 132 0 0 rw---[ stack ] capybara | 2016-07-09 21:15:56 | pl-install-check-C.log | max measured stack depth 5kB capybara | 2016-07-09 21:15:56 | pl-install-check-C.log | 7ffc61bbe000 132 116 116 rw---[ stack ] capybara | 2016-07-09 21:15:56 | pl-install-check-C.log | 7ffc61bbe000 132 0 0 rw---[ stack ] capybara | 2016-07-09 21:15:56 | pl-install-check-C.log | max measured stack depth 7kB Presumably that reflects some oddity of the local version of perl or python, but I have no idea what. So while we could possibly get away with reducing STACK_DEPTH_SLOP to 256K, there is good reason to think that that would be leaving little or no safety margin. At this point I'm inclined to think we should leave well enough alone. At the very least, if we were to try to reduce that number, I'd want to have some plan for tracking our stack space consumption better than we have done in the past. regards, tom lane PS: for amusement's sake, here are some numbers I extracted concerning the relative stack-hungriness of different buildfarm members. First, the number of recursion levels each machine could accomplish before hitting "stack too deep" in the errors.sql regression test (measured by counting the number of CONTEXT lines in the relevant error message): sysname| snapshot | count ---+-+--- protosciurus | 2016-07-10 12:03:06 | 731 chub | 2016-07-10 15:10:01 | 1033 quokka| 2016-07-10 02:17:31 | 1033 hornet| 2016-07-09 23:42:32 | 1156 clam | 2016-07-09 22:00:01 | 1265 anole | 2016-07-09 22:41:40 | 1413 spoonbill | 2016-07-09 23:00:05 | 1535 sungazer | 2016-07-09 23:51:33 | 1618 gaur | 2016-07-09 04:53:13 | 1634 kouprey | 2016-07-10 04:58:00 | 1653 nudibranch| 2016-07-10 09:18:10 | 1664 grouse| 2016-07-10 08:43:02 | 1708 sprat | 2016-07-10 08:43:55 | 1717 pademelon | 2016-07-09 06:12:10 | 1814 mandrill | 2016-07-10 00:10:02 | 2093 gharial | 2016-07-10 01:15:50 | 2248 francolin | 2016-07-10 13:00:01 | 2379 pi
Re: [HACKERS] Issue with bgworker, SPI and pgstat_report_stat
On 08/07/2016 01:53, Michael Paquier wrote: > On Fri, Jul 8, 2016 at 3:06 AM, Andres Freund wrote: >> On 2016-07-07 14:04:36 -0400, Robert Haas wrote: >>> On Thu, Jul 7, 2016 at 1:52 PM, Julien Rouhaud >>> wrote: Should a bgworker modifing data have to call pgstat_report_stat() to avoid this problem? I don't find any documentation suggesting it, and it seems that worker_spi (used as a template for this bgworker and I suppose a lot of other one) is also affected. >>> >>> That certainly seems like the simplest fix. Not sure if there's a better >>> one. >> >> I think a better fix would be to unify the startup & error handling >> code. We have way to many slightly diverging copies. But that's a bigger >> task, so I'm not protesting against making a more localized fix. > > It seems to me that the only fix is to have the bgworker call > pgstat_report_stat() and not mess up with the in-core backend code. > Personally, I always had the image of a bgworker to be an independent > process, so when it wants to do something, be it mimicking normal > backends, it should do it by itself. Take the example of SIGHUP > processing. If the bgworker does not ProcessConfigFile() no parameters > updates will happen in the context of the bgworker. > I'm not opposed, but in this case we should also provide a proper documentation of all the required actions to mimick normal backends. -- Julien Rouhaud http://dalibo.com - http://dalibo.org -- 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] PSA: Systemd will kill PostgreSQL
On 10/07/2016 19:56, Joshua D. Drake wrote: > Hackers, > > This just came across my twitter feed: > > https://lists.freedesktop.org/archives/systemd-devel/2014-April/018373.html > > tl;dr; Systemd 212 defaults to remove all IPC (including SYSV memory) > when a user "fully" logs out. > AFAIK it's only the case if the user is not a system user, and postgres user should be (at least with community packages). See https://github.com/systemd/systemd/issues/2039 -- Julien Rouhaud http://dalibo.com - http://dalibo.org -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] PSA: Systemd will kill PostgreSQL
Hackers, This just came across my twitter feed: https://lists.freedesktop.org/archives/systemd-devel/2014-April/018373.html tl;dr; Systemd 212 defaults to remove all IPC (including SYSV memory) when a user "fully" logs out. JD -- 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] Disable WAL completely - Performance and Persistency research
Hi Michael, Sorry for the delay, The answer is yes, I tried 2 things so far: 1. As I understand: *XLogRecPtr* *XLogInsert(RmgrId rmid, uint8 info)* is the primary insert function in xloginsert.c. I tried commenting the following line at this function, so I can return a phony pointer every time the function is called, just as in bootstrap mode. *if (IsBootstrapProcessingMode() && rmid != RM_XLOG_ID)* 2. At xlog.c, CopyXLogRecordToWAL(int write_len, bool isLogSwitch, XLogRecData *rdata, XLogRecPtr StartPos, XLogRecPtr EndPos), Commenting the memcpy syscall: ... memcpy(currpos, rdata_data, rdata_len); ... BUT, both options are not good, as they are stopping me from even running i *nitdb.* Maybe someone have a lead regarding changes to be done at xlog.c: *XLogInsertRecord(XLogRecData *rdata, XLogRecPtr fpw_lsn)* Any other lead regarding xloginsert.c is welcomed as well. Regards, Netanel On Thu, Jul 7, 2016 at 4:17 PM, Michael Paquier wrote: > On Thu, Jul 7, 2016 at 5:01 PM, Netanel Katzburg > wrote: > > 1. Disable the WAL by not writing anything to the xlog directory. I don't > > care about recovery/fault tolerance or PITR/ replication etc at the > moment. > > I'm aware that the WAL and checkpoint are bind in many ways and are > crucial > > for PG core features. > > Any guidance on how to do so would be appreciated :) > > WAL insertion routines are in xloginsert.c. Did you try to play with those? > -- > Michael >
Re: [HACKERS] Showing parallel status in \df+
Michael Paquier writes: >> - Reordering the columns, I'd suggest as follows): >> -- Schema >> -- Name >> -- Result data type >> -- Argument data types >> -- Type >> -- Language >> -- Volatility >> -- Parallel >> -- Owner >> -- Security >> -- ACL >> -- Source code >> -- Description If we're keeping the "Source code" column, I'd be inclined to keep "Language" adjacent to that. When thinking of a function as a black box, both language and source code are implementation details; but all the other properties listed here are of interest anyway. (Of course, if we were to get rid of "Source code", the point would be moot ...) 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] [CF2016-9] Allow spaces in working path on tap-tests
Michael Paquier writes: > Thanks! What you have pushed looks fine to me. Also, the portion for > src/tools/msvc needs to go further down, I should have precised that > earlier. Do you want a patch for that? Yes, please --- I thought it'd all gotten done. 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] Showing parallel status in \df+
On Sat, Jul 9, 2016 at 8:12 AM, Michael Paquier wrote: > So to sum up: > - Add "Parallel" column > - Add ACLs > - Reordering the columns, I'd suggest as follows): > -- Schema > -- Name > -- Result data type > -- Argument data types > -- Type > -- Language > -- Volatility > -- Parallel > -- Owner > -- Security > -- ACL > -- Source code > -- Description > Or by thema, 1) General info, 2) specificity (volatility, parallel, > type), 3) Ownership. > And regarding "source code", I think that's useful for debugging. Giving the attached, including doc clarifications and column reshuffling with translatable state set up as well. -- Michael diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index aeffd63..e7bd2d7 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -1380,8 +1380,9 @@ testdb=> objects are shown; supply a pattern or the S modifier to include system objects. If the form \df+ is used, additional information -about each function is shown, including security classification, -volatility, owner, language, source code and description. +about each function is shown, including language, volatility, +parallel mode, owner, security classification, access privileges, +source code and description. diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c index 2cdc5ac..e297891 100644 --- a/src/bin/psql/describe.c +++ b/src/bin/psql/describe.c @@ -298,7 +298,7 @@ describeFunctions(const char *functypes, const char *pattern, bool verbose, bool PQExpBufferData buf; PGresult *res; printQueryOpt myopt = pset.popt; - static const bool translate_columns[] = {false, false, false, false, true, true, true, false, false, false, false}; + static const bool translate_columns[] = {false, false, false, false, true, false, true, false, false, true, false, true, false}; if (strlen(functypes) != strspn(functypes, "antwS+")) { @@ -410,28 +410,42 @@ describeFunctions(const char *functypes, const char *pattern, bool verbose, bool gettext_noop("Type")); if (verbose) + { appendPQExpBuffer(&buf, - ",\n CASE WHEN prosecdef THEN '%s' ELSE '%s' END AS \"%s\"" + ",\n l.lanname as \"%s\"" ",\n CASE\n" " WHEN p.provolatile = 'i' THEN '%s'\n" " WHEN p.provolatile = 's' THEN '%s'\n" " WHEN p.provolatile = 'v' THEN '%s'\n" " END as \"%s\"" - ",\n pg_catalog.pg_get_userbyid(p.proowner) as \"%s\",\n" - " l.lanname as \"%s\",\n" - " p.prosrc as \"%s\",\n" - " pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"", - gettext_noop("definer"), - gettext_noop("invoker"), - gettext_noop("Security"), + ",\n CASE\n" + " WHEN p.proparallel = 'r' THEN '%s'\n" + " WHEN p.proparallel = 's' THEN '%s'\n" + " WHEN p.proparallel = 'u' THEN '%s'\n" + " END as \"%s\"" + ",\n pg_catalog.pg_get_userbyid(p.proowner) as \"%s\"" + ",\n CASE WHEN prosecdef THEN '%s' ELSE '%s' END AS \"%s\"", + gettext_noop("Language"), gettext_noop("immutable"), gettext_noop("stable"), gettext_noop("volatile"), gettext_noop("Volatility"), + gettext_noop("restricted"), + gettext_noop("safe"), + gettext_noop("unsafe"), + gettext_noop("Parallel"), gettext_noop("Owner"), - gettext_noop("Language"), + gettext_noop("definer"), + gettext_noop("invoker"), + gettext_noop("Security")); + appendPQExpBufferStr(&buf, ",\n "); + printACLColumn(&buf, "p.proacl"); + appendPQExpBuffer(&buf, + ",\n p.prosrc as \"%s\"" + ",\n pg_catalog.obj_description(p.oid, 'pg_proc') as \"%s\"", gettext_noop("Source code"), gettext_noop("Description")); + } appendPQExpBufferStr(&buf, "\nFROM pg_catalog.pg_proc p" -- 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] [CF2016-9] Allow spaces in working path on tap-tests
On Sun, Jul 10, 2016 at 5:50 AM, Tom Lane wrote: > This seemed like a bug fix to me ... Yes, it is. > ... so I went ahead and pushed it. I don't > have any ability to test the Windows parts, so it's possible I missed > something in the back-patching; please review. Thanks! What you have pushed looks fine to me. Also, the portion for src/tools/msvc needs to go further down, I should have precised that earlier. Do you want a patch for that? -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers