Tom Lane [Fri, 19 Mar 2021 02:43:03 +0000 (22:43 -0400)]
Blindly try to fix test script's tar invocation for MSYS.
Buildfarm member fairywren doesn't like the test case I added
in commit
081876d75. I'm guessing the reason is that I shouldn't
be using a perl2host-ified path in the tar command line.
Fujii Masao [Fri, 19 Mar 2021 02:28:54 +0000 (11:28 +0900)]
Fix comments in postmaster.c.
Commit
86c23a6eb2 changed the option to specify that postgres will
stop all other server processes by sending the signal SIGSTOP,
from -s to -T. But previously there were comments incorrectly
explaining that SIGSTOP behavior is set by -s option. This commit
fixes them.
Author: Kyotaro Horiguchi
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/
20210316.165141.
1400441966284654043.horikyota.ntt@gmail.com
Tom Lane [Fri, 19 Mar 2021 02:21:58 +0000 (22:21 -0400)]
Don't leak malloc'd error string in libpqrcv_check_conninfo().
We leaked the error report from PQconninfoParse, when there was
one. It seems unlikely that real usage patterns would repeat
the failure often enough to create serious bloat, but let's
back-patch anyway to keep the code similar in all branches.
Found via valgrind testing.
Back-patch to v10 where this code was added.
Discussion: https://postgr.es/m/
3816764.
1616104288@sss.pgh.pa.us
Tom Lane [Fri, 19 Mar 2021 02:09:41 +0000 (22:09 -0400)]
Don't leak malloc'd strings when a GUC setting is rejected.
Because guc.c prefers to keep all its string values in malloc'd
not palloc'd storage, it has to be more careful than usual to
avoid leaks. Error exits out of string GUC hook checks failed
to clear the proposed value string, and error exits out of
ProcessGUCArray() failed to clear the malloc'd results of
ParseLongOption().
Found via valgrind testing.
This problem is ancient, so back-patch to all supported branches.
Discussion: https://postgr.es/m/
3816764.
1616104288@sss.pgh.pa.us
Tom Lane [Fri, 19 Mar 2021 01:44:42 +0000 (21:44 -0400)]
Don't leak compiled regex(es) when an ispell cache entry is dropped.
The text search cache mechanisms assume that we can clean up
an invalidated dictionary cache entry simply by resetting the
associated long-lived memory context. However, that does not work
for ispell affixes that make use of regular expressions, because
the regex library deals in plain old malloc. Hence, we leaked
compiled regex(es) any time we dropped such a cache entry. That
could quickly add up, since even a fairly trivial regex can use up
tens of kB, and a large one can eat megabytes. Add a memory context
callback to ensure that a regex gets freed when its owning cache
entry is cleared.
Found via valgrind testing.
This problem is ancient, so back-patch to all supported branches.
Discussion: https://postgr.es/m/
3816764.
1616104288@sss.pgh.pa.us
Tom Lane [Fri, 19 Mar 2021 00:50:56 +0000 (20:50 -0400)]
Don't run RelationInitTableAccessMethod in a long-lived context.
Some code paths in this function perform syscache lookups, which
can lead to table accesses and possibly leakage of cruft into
the caller's context. If said context is CacheMemoryContext,
we eventually will have visible bloat. But fixing this is no
harder than moving one memory context switch step. (The other
callers don't have a problem.)
Andres Freund and I independently found this via valgrind testing.
Back-patch to v12 where this code was added.
Discussion: https://postgr.es/m/
20210317023101.anvejcfotwka6gaa@alap3.anarazel.de
Discussion: https://postgr.es/m/
3816764.
1616104288@sss.pgh.pa.us
Tom Lane [Fri, 19 Mar 2021 00:37:09 +0000 (20:37 -0400)]
Don't leak rd_statlist when a relcache entry is dropped.
Although these lists are usually NIL, and even when not empty
are unlikely to be large, constant relcache update traffic could
eventually result in visible bloat of CacheMemoryContext.
Found via valgrind testing.
Back-patch to v10 where this field was added.
Discussion: https://postgr.es/m/
3816764.
1616104288@sss.pgh.pa.us
Tomas Vondra [Fri, 19 Mar 2021 01:05:23 +0000 (02:05 +0100)]
Fix TAP test for remove_temp_files_after_crash
The test included in
cd91de0d17 had two simple flaws.
Firstly, the number of rows was low and on some platforms (e.g. 32-bit)
the sort did not require on-disk sort, so on those machines it was not
testing the automatic removal. The test was however failing, because
without any temporary files the base/pgsql_tmp directory was not even
created. Fixed by increasing the rowcount to 5000, which should be high
engough on any platform.
Secondly, the test used a simple sleep to wait for the temporary file to
be created. This is obviously problematic, because on slow machines (or
with valgrind, CLOBBER_CACHE_ALWAYS etc.) it may take a while to create
the temporary file. But we also want the tests run reasonably fast.
Fixed by instead relying on a UNIQUE constraint, blocking the query that
created the temporary file.
Author: Euler Taveira
Reviewed-by: Tomas Vondra
Discussion: https://postgr.es/m/CAH503wDKdYzyq7U-QJqGn%3DGm6XmoK%2B6_6xTJ-Yn5WSvoHLY1Ww%40mail.gmail.com
Michael Paquier [Fri, 19 Mar 2021 00:18:41 +0000 (09:18 +0900)]
Improve tab completion of IMPORT FOREIGN SCHEMA with \h in psql
Only "IMPORT" was showing as result of the completion, while IMPORT
FOREIGN SCHEMA is the only command using this keyword in first
position. This changes the completion to show the full command name
instead of just "IMPORT".
Reviewed-by: Georgios Kokolatos, Julien Rouhaud
Discussion: https://postgr.es/m/YFL6JneBiuMWYyoh@paquier.xyz
Tom Lane [Thu, 18 Mar 2021 23:24:22 +0000 (19:24 -0400)]
Fix misuse of foreach_delete_current().
Our coding convention requires this macro's result to be assigned
back to the original List variable. In this usage, since the
List could not become empty, there was no actual bug --- but
some compilers warned about it. Oversight in
be45be9c3.
Discussion: https://postgr.es/m/
35077b31-2d62-1e31-0e2e-
ddb52d590b73@enterprisedb.com
Tomas Vondra [Thu, 18 Mar 2021 16:45:38 +0000 (17:45 +0100)]
Implement GROUP BY DISTINCT
With grouping sets, it's possible that some of the grouping sets are
duplicate. This is especially common with CUBE and ROLLUP clauses. For
example GROUP BY CUBE (a,b), CUBE (b,c) is equivalent to
GROUP BY GROUPING SETS (
(a, b, c),
(a, b, c),
(a, b, c),
(a, b),
(a, b),
(a, b),
(a),
(a),
(a),
(c, a),
(c, a),
(c, a),
(c),
(b, c),
(b),
()
)
Some of the grouping sets are calculated multiple times, which is mostly
unnecessary. This commit implements a new GROUP BY DISTINCT feature, as
defined in the SQL standard, which eliminates the duplicate sets.
Author: Vik Fearing
Reviewed-by: Erik Rijkers, Georgios Kokolatos, Tomas Vondra
Discussion: https://postgr.es/m/
bf3805a8-d7d1-ae61-fece-
761b7ff41ecc@postgresfriends.org
Tomas Vondra [Thu, 18 Mar 2021 15:05:03 +0000 (16:05 +0100)]
Remove temporary files after backend crash
After a crash of a backend using temporary files, the files used to be
left behind, on the basis that it might be useful for debugging. But we
don't have any reports of anyone actually doing that, and it means the
disk usage may grow over time due to repeated backend failures (possibly
even hitting ENOSPC). So this behavior is a bit unfortunate, and fixing
it required either manual cleanup (deleting files, which is error-prone)
or restart of the instance (i.e. service disruption).
This implements automatic cleanup of temporary files, controled by a new
GUC remove_temp_files_after_crash. By default the files are removed, but
it can be disabled to restore the old behavior if needed.
Author: Euler Taveira
Reviewed-by: Tomas Vondra, Michael Paquier, Anastasia Lubennikova, Thomas Munro
Discussion: https://postgr.es/m/CAH503wDKdYzyq7U-QJqGn%3DGm6XmoK%2B6_6xTJ-Yn5WSvoHLY1Ww%40mail.gmail.com
Magnus Hagander [Thu, 18 Mar 2021 10:17:42 +0000 (11:17 +0100)]
Fix function name in error hint
pg_read_file() is the function that's in core, pg_file_read() is in
adminpack. But when using pg_file_read() in adminpack it calls the *C*
level function pg_read_file() in core, which probably threw the original
author off. But the error hint should be about the SQL function.
Reported-By: Sergei Kornilov
Backpatch-through: 11
Discussion: https://postgr.es/m/
373021616060475@mail.yandex.ru
Amit Kapila [Thu, 18 Mar 2021 10:04:55 +0000 (15:34 +0530)]
Doc: Update description for parallel insert reloption.
Commit
c8f78b6161 added a new reloption to enable inserts in parallel-mode
but forgot to update at one of the places about the same in docs. In
passing, fix a typo in the same commit.
Reported-by: Justin Pryzby
Author: Justin Pryzby
Reviewed-by: "Hou, Zhijie", Amit Kapila
Discussion: https://postgr.es/m/
20210318025228.GE11765@telsasoft.com
Amit Kapila [Thu, 18 Mar 2021 01:55:27 +0000 (07:25 +0530)]
Add a new GUC and a reloption to enable inserts in parallel-mode.
Commit
05c8482f7f added the implementation of parallel SELECT for
"INSERT INTO ... SELECT ..." which may incur non-negligible overhead in
the additional parallel-safety checks that it performs, even when, in the
end, those checks determine that parallelism can't be used. This is
normally only ever a problem in the case of when the target table has a
large number of partitions.
A new GUC option "enable_parallel_insert" is added, to allow insert in
parallel-mode. The default is on.
In addition to the GUC option, the user may want a mechanism to allow
inserts in parallel-mode with finer granularity at table level. The new
table option "parallel_insert_enabled" allows this. The default is true.
Author: "Hou, Zhijie"
Reviewed-by: Greg Nancarrow, Amit Langote, Takayuki Tsunakawa, Amit Kapila
Discussion: https://postgr.es/m/CAA4eK1K-cW7svLC2D7DHoGHxdAdg3P37BLgebqBOC2ZLc9a6QQ%40mail.gmail.com
Discussion: https://postgr.es/m/CAJcOf-cXnB5cnMKqWEp2E2z7Mvcd04iLVmV=qpFJrR3AcrTS3g@mail.gmail.com
Andres Freund [Wed, 17 Mar 2021 23:18:37 +0000 (16:18 -0700)]
Fix memory lifetime issues of replication slot stats.
When accessing replication slot stats, introduced in
98681675002d,
pgstat_read_statsfiles() reads the data into newly allocated
memory. Unfortunately the current memory context at that point is the
callers, leading to leaks and use-after-free dangers.
The fix is trivial, explicitly use pgStatLocalContext. There's some
potential for further improvements, but that's outside of the scope of
this bugfix.
No backpatch necessary, feature is only in HEAD.
Author: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/
20210317230447.c7uc4g3vbs4wi32i@alap3.anarazel.de
Tom Lane [Wed, 17 Mar 2021 20:39:58 +0000 (16:39 -0400)]
Doc: remove duplicated step in RLS example.
Seems to have been a copy-and-paste mistake in
093129c9d.
Per report from max1@inbox.ru.
Discussion: https://postgr.es/m/
161591740692.24273.
4202054598867879464@wrigleys.postgresql.org
Tom Lane [Wed, 17 Mar 2021 20:18:46 +0000 (16:18 -0400)]
Code review for server's handling of "tablespace map" files.
While looking at Robert Foggia's report, I noticed a passel of
other issues in the same area:
* The scheme for backslash-quoting newlines in pathnames is just
wrong; it will misbehave if the last ordinary character in a pathname
is a backslash. I'm not sure why we're bothering to allow newlines
in tablespace paths, but if we're going to do it we should do it
without introducing other problems. Hence, backslashes themselves
have to be backslashed too.
* The author hadn't read the sscanf man page very carefully, because
this code would drop any leading whitespace from the path. (I doubt
that a tablespace path with leading whitespace could happen in
practice; but if we're bothering to allow newlines in the path, it
sure seems like leading whitespace is little less implausible.) Using
sscanf for the task of finding the first space is overkill anyway.
* While I'm not 100% sure what the rationale for escaping both \r and
\n is, if the idea is to allow Windows newlines in the file then this
code failed, because it'd throw an error if it saw \r followed by \n.
* There's no cross-check for an incomplete final line in the map file,
which would be a likely apparent symptom of the improper-escaping
bug.
On the generation end, aside from the escaping issue we have:
* If needtblspcmapfile is true then do_pg_start_backup will pass back
escaped strings in tablespaceinfo->path values, which no caller wants
or is prepared to deal with. I'm not sure if there's a live bug from
that, but it looks like there might be (given the dubious assumption
that anyone actually has newlines in their tablespace paths).
* It's not being very paranoid about the possibility of random stuff
in the pg_tblspc directory. IMO we should ignore anything without an
OID-like name.
The escaping rule change doesn't seem back-patchable: it'll require
doubling of backslashes in the tablespace_map file, which is basically
a basebackup format change. The odds of that causing trouble are
considerably more than the odds of the existing bug causing trouble.
The rest of this seems somewhat unlikely to cause problems too,
so no back-patch.
Tom Lane [Wed, 17 Mar 2021 20:10:37 +0000 (16:10 -0400)]
Prevent buffer overrun in read_tablespace_map().
Robert Foggia of Trustwave reported that read_tablespace_map()
fails to prevent an overrun of its on-stack input buffer.
Since the tablespace map file is presumed trustworthy, this does
not seem like an interesting security vulnerability, but still
we should fix it just in the name of robustness.
While here, document that pg_basebackup's --tablespace-mapping option
doesn't work with tar-format output, because it doesn't. To make it
work, we'd have to modify the tablespace_map file within the tarball
sent by the server, which might be possible but I'm not volunteering.
(Less-painful solutions would require changing the basebackup protocol
so that the source server could adjust the map. That's not very
appetizing either.)
Tom Lane [Wed, 17 Mar 2021 18:52:55 +0000 (14:52 -0400)]
Add end-to-end testing of pg_basebackup's tar-format output.
The existing test script does run pg_basebackup with the -Ft option,
but it makes no real attempt to verify the sanity of the results.
We wouldn't know if the output is incompatible with standard "tar"
programs, nor if the server fails to start from the restored output.
Notably, this means that xlog.c's read_tablespace_map() is not being
meaningfully tested, since that code is used only in the tar-format
case. (We do have reasonable coverage of restoring from plain-format
output, though it's over in src/test/recovery not here.)
Hence, attempt to untar the output and start a server from it,
rather just hoping it's OK.
This test assumes that the local "tar" has the "-C directory"
switch. Although that's not promised by POSIX, my research
suggests that all non-extinct tar implementations have it.
Should the buildfarm's opinion differ, we can complicate the
test a bit to avoid requiring that.
Possibly this should be back-patched, but I'm unsure about
whether it could work on Windows before
d66b23b03.
Tom Lane [Wed, 17 Mar 2021 17:09:13 +0000 (13:09 -0400)]
Doc: improve discussion of variable substitution in PL/pgSQL.
This was a bit disjointed, partly because of a not-well-considered
decision to document SQL commands that don't return result rows as
though they had nothing in common with commands that do. Rearrange
so that we have one discussion of variable substitution that clearly
applies to all types of SQL commands, and then handle the question
of processing command output separately. Clarify that EXPLAIN,
CREATE TABLE AS SELECT, and similar commands that incorporate an
optimizable statement will act like optimizable statements for the
purposes of variable substitution. Do a bunch of minor wordsmithing
in the same area.
David Johnston and Tom Lane, reviewed by Pavel Stehule and David
Steele
Discussion: https://postgr.es/m/CAKFQuwYvMKucM5fnZvHSo-ah4S=_n9gmKeu6EAo=_fTrohunqQ@mail.gmail.com
Thomas Munro [Wed, 17 Mar 2021 11:35:04 +0000 (00:35 +1300)]
Revert "Fix race in Parallel Hash Join batch cleanup."
This reverts commit
378802e3713c6c0fce31d2390c134cd5d7c30157.
This reverts commit
3b8981b6e1a2aea0f18384c803e21e9391de669a.
Discussion: https://postgr.es/m/CA%2BhUKGJmcqAE3MZeDCLLXa62cWM0AJbKmp2JrJYaJ86bz36LFA%40mail.gmail.com
Michael Paquier [Wed, 17 Mar 2021 09:07:00 +0000 (18:07 +0900)]
Fix comment in indexing.c
578b229, that removed support for WITH OIDS, has changed
CatalogTupleInsert() to not return an Oid, but one comment was still
mentioning that.
Author: Vik Fearing
Discussion: https://postgr.es/m/
fef01975-ed10-3601-7b9e-
80ecef72d00b@postgresfriends.org
Peter Eisentraut [Wed, 17 Mar 2021 07:17:33 +0000 (08:17 +0100)]
Small error message improvement
Thomas Munro [Wed, 17 Mar 2021 05:24:45 +0000 (18:24 +1300)]
Update the names of Parallel Hash Join phases.
Commit
3048898e dropped -ING from some wait event names that correspond
to barrier phases. Update the phases' names to match.
While we're here making cosmetic changes, also rename "DONE" to "FREE".
That pairs better with "ALLOCATE", and describes the activity that
actually happens in that phase (as we do for the other phases) rather
than describing a state. The distinction is clearer after bugfix commit
3b8981b6 split the phase into two. As for the growth barriers, rename
their "ALLOCATE" phase to "REALLOCATE", which is probably a better
description of what happens then. Also improve the comments about
the phases a bit.
Discussion: https://postgr.es/m/CA%2BhUKG%2BMDpwF2Eo2LAvzd%3DpOh81wUTsrwU1uAwR-v6OGBB6%2B7g%40mail.gmail.com
Thomas Munro [Wed, 17 Mar 2021 04:46:39 +0000 (17:46 +1300)]
Fix race in Parallel Hash Join batch cleanup.
With very unlucky timing and parallel_leader_participation off, PHJ
could attempt to access per-batch state just as it was being freed.
There was code intended to prevent that by checking for a cleared
pointer, but it was buggy.
Fix, by introducing an extra barrier phase. The new phase
PHJ_BUILD_RUNNING means that it's safe to access the per-batch state to
find a batch to help with, and PHJ_BUILD_DONE means that it is too late.
The last to detach will free the array of per-batch state as before, but
now it will also atomically advance the phase at the same time, so that
late attachers can avoid the hazard, without the data race. This
mirrors the way per-batch hash tables are freed (see phases
PHJ_BATCH_PROBING and PHJ_BATCH_DONE).
Revealed by a one-off build farm failure, where BarrierAttach() failed a
sanity check assertion, because the memory had been clobbered by
dsa_free().
Back-patch to 11, where the code arrived.
Reported-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/
20200929061142.GA29096%40paquier.xyz
Thomas Munro [Wed, 17 Mar 2021 04:13:43 +0000 (17:13 +1300)]
Fix transaction.sql tests in higher isolation levels.
It seems like a useful sanity check to be able to run "installcheck"
against a cluster running with default_transaction_level set to
serializable or repeatable read. Only one thing currently fails in
those configurations, so let's fix that.
No back-patch for now, because it fails in many other places in some of
the stable branches. We'd have to go back and fix those too if we
included this configuration in automated testing.
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CA%2BhUKGJUaHeK%3DHLATxF1JOKDjKJVrBKA-zmbPAebOM0Se2FQRg%40mail.gmail.com
Amit Kapila [Wed, 17 Mar 2021 02:45:12 +0000 (08:15 +0530)]
Fix race condition in drop subscription's handling of tablesync slots.
Commit
ce0fdbfe97 made tablesync slots permanent and allow Drop
Subscription to drop such slots. However, it is possible that before
tablesync worker could get the acknowledgment of slot creation, drop
subscription stops it and that can lead to a dangling slot on the
publisher. Prevent cancel/die interrupts while creating a slot in the
tablesync worker.
Reported-by: Thomas Munro as per buildfarm
Author: Amit Kapila
Reviewed-by: Vignesh C, Takamichi Osumi
Discussion: https://postgr.es/m/CA+hUKGJG9dWpw1cOQ2nzWU8PHjm=PTraB+KgE5648K9nTfwvxg@mail.gmail.com
Amit Kapila [Wed, 17 Mar 2021 02:10:23 +0000 (07:40 +0530)]
Doc: Add a description of substream in pg_subscription.
Commit
464824323e added a new column substream in pg_subscription but
forgot to update the docs.
Reported-by: Peter Smith
Author: Amit Kapila
Reviewed-by: Peter Smith
Discussion: https://postgr.es/m/CAHut+PuPGGASnh2Dy37VYODKULVQo-5oE=Shc6gwtRizDt==cA@mail.gmail.com
Thomas Munro [Wed, 17 Mar 2021 00:43:08 +0000 (13:43 +1300)]
Enable parallelism in REFRESH MATERIALIZED VIEW.
Pass CURSOR_OPT_PARALLEL_OK to pg_plan_query() so that parallel plans
are considered when running the underlying SELECT query. This wasn't
done in commit
e9baa5e9, which did this for CREATE MATERIALIZED VIEW,
because it wasn't yet known to be safe.
Since REFRESH always inserts into a freshly created table before later
merging or swapping the data into place with separate operations, we can
enable such plans here too.
Author: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Reviewed-by: Hou, Zhijie <houzj.fnst@cn.fujitsu.com>
Reviewed-by: Luc Vlaming <luc@swarm64.com>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://postgr.es/m/CALj2ACXg-4hNKJC6nFnepRHYT4t5jJVstYvri%2BtKQHy7ydcr8A%40mail.gmail.com
Peter Geoghegan [Tue, 16 Mar 2021 20:38:52 +0000 (13:38 -0700)]
Fix comment about promising tuples.
Oversight in commit
d168b666823, which added bottom-up index deletion.
Peter Geoghegan [Tue, 16 Mar 2021 20:11:17 +0000 (13:11 -0700)]
amcheck: Reduce debug message verbosity.
Empty sibling pages can occasionally be much more common than any other
event that we report on at elevel DEBUG1. Increase the elevel for
relevant cases to DEBUG2 to avoid overwhelming the user with relatively
insignificant details.
Tom Lane [Tue, 16 Mar 2021 20:02:49 +0000 (16:02 -0400)]
Avoid corner-case memory leak in SSL parameter processing.
After reading the root cert list from the ssl_ca_file, immediately
install it as client CA list of the new SSL context. That gives the
SSL context ownership of the list, so that SSL_CTX_free will free it.
This avoids a permanent memory leak if we fail further down in
be_tls_init(), which could happen if bogus CRL data is offered.
The leak could only amount to something if the CRL parameters get
broken after server start (else we'd just quit) and then the server
is SIGHUP'd many times without fixing the CRL data. That's rather
unlikely perhaps, but it seems worth fixing, if only because the
code is clearer this way.
While we're here, add some comments about the memory management
aspects of this logic.
Noted by Jelte Fennema and independently by Andres Freund.
Back-patch to v10; before commit
de41869b6 it doesn't matter,
since we'd not re-execute this code during SIGHUP.
Discussion: https://postgr.es/m/16160-
18367e56e9a28264@postgresql.org
Robert Haas [Tue, 16 Mar 2021 19:42:20 +0000 (15:42 -0400)]
Fix a confusing amcheck corruption message.
Don't complain about the last TOAST chunk number being different
from what we expected if there are no TOAST chunks at all.
In such a case, saying that the final chunk number is 0 is not
really accurate, and the fact the value is missing from the
TOAST table is reported separately anyway.
Mark Dilger
Discussion: http://postgr.es/m/
AA5506CE-7D2A-42E4-A51D-
358635E3722D@enterprisedb.com
Stephen Frost [Tue, 16 Mar 2021 18:46:48 +0000 (14:46 -0400)]
Use pre-fetching for ANALYZE
When we have posix_fadvise() available, we can improve the performance
of an ANALYZE by quite a bit by using it to inform the kernel of the
blocks that we're going to be asking for. Similar to bitmap index
scans, the number of buffers pre-fetched is based off of the
maintenance_io_concurrency setting (for the particular tablespace or,
if not set, globally, via get_tablespace_maintenance_io_concurrency()).
Reviewed-By: Heikki Linnakangas, Tomas Vondra
Discussion: https://www.postgresql.org/message-id/VI1PR0701MB69603A433348EDCF783C6ECBF6EF0%40VI1PR0701MB6960.eurprd07.prod.outlook.com
Stephen Frost [Tue, 16 Mar 2021 18:46:48 +0000 (14:46 -0400)]
Improve logging of auto-vacuum and auto-analyze
When logging auto-vacuum and auto-analyze activity, include the I/O
timing if track_io_timing is enabled. Also, for auto-analyze, add the
read rate and the dirty rate, similar to how that information has
historically been logged for auto-vacuum.
Stephen Frost and Jakub Wartak
Reviewed-By: Heikki Linnakangas, Tomas Vondra
Discussion: https://www.postgresql.org/message-id/VI1PR0701MB69603A433348EDCF783C6ECBF6EF0%40VI1PR0701MB6960.eurprd07.prod.outlook.com
Tom Lane [Tue, 16 Mar 2021 15:16:41 +0000 (11:16 -0400)]
Improve logging of bad parameter values in BIND messages.
Since commit
ba79cb5dc, values of bind parameters have been logged
during errors in extended query mode. However, we only did that after
we'd collected and converted all the parameter values, thus failing to
offer any useful localization of invalid-parameter problems. Add a
separate callback that's used during parameter collection, and have it
print the parameter number, along with the input string if text input
format is used.
Justin Pryzby and Tom Lane
Discussion: https://postgr.es/m/
20210104170939.GH9712@telsasoft.com
Discussion: https://postgr.es/m/CANfkH5k-6nNt-4cSv1vPB80nq2BZCzhFVR5O4VznYbsX0wZmow@mail.gmail.com
Alvaro Herrera [Tue, 16 Mar 2021 13:36:28 +0000 (10:36 -0300)]
(Blind) fix Perl splitting of strings at newlines
I forgot that Windows represents newlines as \r\n, so splitting a string
at /\s/ creates additional empty strings. Let's rewrite that as /\s+/
to see if that avoids those. (There's precedent for using that pattern
on Windows in other scripts.)
Previously:
91bdf499b37b,
8ed428dc977f,
650b96707672.
Per buildfarm, via Tom Lane.
Discussion: https://postgr.es/m/
3144460.
1615860259@sss.pgh.pa.us
Michael Paquier [Tue, 16 Mar 2021 00:55:43 +0000 (09:55 +0900)]
Add some basic tests for progress reporting of COPY
This tests some basic features for progress reporting of COPY, relying
on an INSERT trigger that gets fired when doing COPY FROM with a file or
stdin, checking for sizes, number of tuples processed, and number of
tuples excluded by a WHERE clause.
Author: Josef Šimánek, Matthias van de Meent
Reviewed-by: Michael Paquier, Justin Pryzby, Bharath Rupireddy, Tomas
Vondra
Discussion: https://postgr.es/m/CAEze2WiOcgdH4aQA8NtZq-4dgvnJzp8PohdeKchPkhMY-jWZXA@mail.gmail.com
Alvaro Herrera [Mon, 15 Mar 2021 21:33:03 +0000 (18:33 -0300)]
Add libpq pipeline mode support to pgbench
New metacommands \startpipeline and \endpipeline allow the user to run
queries in libpq pipeline mode.
Author: Daniel Vérité <daniel@manitou-mail.org>
Reviewed-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/
b4e34135-2bd9-4b8a-94ca-
27d760da26d7@manitou-mail.org
Alvaro Herrera [Mon, 15 Mar 2021 21:13:42 +0000 (18:13 -0300)]
Implement pipeline mode in libpq
Pipeline mode in libpq lets an application avoid the Sync messages in
the FE/BE protocol that are implicit in the old libpq API after each
query. The application can then insert Sync at its leisure with a new
libpq function PQpipelineSync. This can lead to substantial reductions
in query latency.
Co-authored-by: Craig Ringer <craig.ringer@enterprisedb.com>
Co-authored-by: Matthieu Garrigues <matthieu.garrigues@gmail.com>
Co-authored-by: Álvaro Herrera <alvherre@alvh.no-ip.org>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Aya Iwata <iwata.aya@jp.fujitsu.com>
Reviewed-by: Daniel Vérité <daniel@manitou-mail.org>
Reviewed-by: David G. Johnston <david.g.johnston@gmail.com>
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Reviewed-by: Kirk Jamison <k.jamison@fujitsu.com>
Reviewed-by: Michael Paquier <michael.paquier@gmail.com>
Reviewed-by: Nikhil Sontakke <nikhils@2ndquadrant.com>
Reviewed-by: Vaishnavi Prabakaran <VaishnaviP@fast.au.fujitsu.com>
Reviewed-by: Zhihong Yu <zyu@yugabyte.com>
Discussion: https://postgr.es/m/CAMsr+YFUjJytRyV4J-16bEoiZyH=4nj+sQ7JP9ajwz=B4dMMZw@mail.gmail.com
Discussion: https://postgr.es/m/CAJkzx4T5E-2cQe3dtv2R78dYFvz+in8PY7A8MArvLhs_pg75gg@mail.gmail.com
Tom Lane [Mon, 15 Mar 2021 16:34:17 +0000 (12:34 -0400)]
Work around issues in MinGW-64's setjmp/longjmp support.
It's hard to avoid the conclusion that there is something wrong with
setjmp/longjmp on MinGW-64, as we have seen failures come and go after
entirely-unrelated-looking changes in our own code. Other projects
such as Ruby have given up and started using gcc's setjmp/longjmp
builtins on that platform; this patch just follows that lead.
Note that this is a pretty fundamental ABI break for functions
containining either setjmp or longjmp, so we can't really consider
a back-patch.
Per reports from Regina Obe and Heath Lord, as well as recent failures
on buildfarm member walleye, and less-recent failures on fairywren.
Juan José Santamaría Flecha
Discussion: https://postgr.es/m/
000401d716a0$
1ed0fc70$
5c72f550$@pcorp.us
Discussion: https://postgr.es/m/CA+BEBhvHhM-Bn628pf-LsjqRh3Ang7qCSBG0Ga+7KwhGqrNUPw@mail.gmail.com
Discussion: https://postgr.es/m/
f1caef93-9640-022e-9211-
bbe8755a56b0@2ndQuadrant.com
Thomas Munro [Mon, 15 Mar 2021 10:27:08 +0000 (23:27 +1300)]
Drop SERIALIZABLE workaround from parallel query tests.
SERIALIZABLE no longer inhibits parallelism, so we can drop some
outdated workarounds and comments from regression tests. The change
came in release 12, commit
bb16aba5, but it's not really worth
back-patching.
Also fix a typo.
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://postgr.es/m/CA%2BhUKGJUaHeK%3DHLATxF1JOKDjKJVrBKA-zmbPAebOM0Se2FQRg%40mail.gmail.com
Fujii Masao [Mon, 15 Mar 2021 04:13:14 +0000 (13:13 +0900)]
Make archiver process an auxiliary process.
This commit changes WAL archiver process so that it's treated as
an auxiliary process and can use shared memory. This is an infrastructure
patch required for upcoming shared-memory based stats collector patch
series. These patch series basically need any processes including archiver
that can report the statistics to access to shared memory. Since this patch
itself is useful to simplify the code and when users monitor the status of
archiver, it's committed separately in advance.
This commit simplifies the code for WAL archiving. For example, previously
backends need to signal to archiver via postmaster when they notify
archiver that there are some WAL files to archive. On the other hand,
this commit removes that signal to postmaster and enables backends to
notify archier directly using shared latch.
Also, as the side of this change, the information about archiver process
becomes viewable at pg_stat_activity view.
Author: Kyotaro Horiguchi
Reviewed-by: Andres Freund, Álvaro Herrera, Julien Rouhaud, Tomas Vondra, Arthur Zakirov, Fujii Masao
Discussion: https://postgr.es/m/
20180629.173418.
190173462.horiguchi.kyotaro@lab.ntt.co.jp
Peter Geoghegan [Mon, 15 Mar 2021 01:05:57 +0000 (18:05 -0700)]
Notice that heap page has dead items during VACUUM.
Consistently set a flag variable that tracks whether the current heap
page has a dead item during lazy vacuum's heap scan. We missed the
common case where there is an preexisting (or even a new) LP_DEAD heap
line pointer.
Also make it clear that the variable might be affected by an existing
line pointer, say from an earlier opportunistic pruning operation. This
distinction is important because it's the main reason why we can't just
use the nearby tups_vacuumed variable instead.
No backpatch. In theory failing to set the page level flag variable had
no consequences. Currently it is only used to defensively check if a
page marked all visible has dead items, which should never happen anyway
(if it does then the table must be corrupt).
Author: Masahiko Sawada <sawada.mshk@gmail.com>
Diagnosed-By: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAD21AoAtZb4+HJT_8RoOXvu4HM-Zd4HKS3YSMCH6+-W=bDyh-w@mail.gmail.com
Tom Lane [Sat, 13 Mar 2021 15:51:27 +0000 (10:51 -0500)]
Doc: add note about how to run the pg_amcheck regression tests.
It's not immediately obvious what you have to do to get "make
installcheck" to work here, so document that along the same lines
as we've used elsewhere.
Robert Haas [Sat, 13 Mar 2021 15:55:33 +0000 (10:55 -0500)]
In pg_amcheck tests, don't depend on perl's Q/q pack code.
It does not work on all versions of perl across all platforms.
To avoid endian-ness issues, pick a new value for column a
that has the same upper 4 bytes as lower 4 bytes. Try to
make it something that isn't likely to occur anywhere nearby
in the page.
Discussion: http://postgr.es/m/
29DA079B-0658-4E66-BDAA-
0EFD7B64D9C6@enterprisedb.com
Tom Lane [Sat, 13 Mar 2021 05:06:56 +0000 (00:06 -0500)]
pg_amcheck: Keep trying to fix the tests.
Fix another example of non-portable option ordering in the tests.
Oversight in
24189277f.
Mark Dilger
Discussion: https://postgr.es/m/
C37D28BA-3BA3-4776-B812-
17F05F3472D8@enterprisedb.com
Thomas Munro [Sat, 13 Mar 2021 04:21:01 +0000 (17:21 +1300)]
Fix new pthread code to respect --disable-thread-safety.
Don't try to compile src/port/pthread_barrier_wait.c if we opted out of
threads at configure time. Revealed by build farm member gaur, which
can't compile this code because of problems with its pthread
implementation. It shouldn't be trying to, because it's using
--disable-thread-safety.
Defect in commit
44bf3d50.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/
2568537.
1615603606%40sss.pgh.pa.us
Amit Kapila [Sat, 13 Mar 2021 03:43:21 +0000 (09:13 +0530)]
Improve FK trigger parallel-safety check added by
05c8482f7f.
Commit
05c8482f7f added special logic related to parallel-safety of FK
triggers. This is a bit of a hack and should have instead been done by
simply setting appropriate proparallel values on those trigger functions
themselves.
Suggested-by: Tom Lane
Author: Greg Nancarrow
Reviewed-by: Amit Kapila
Discussion: https://postgr.es/m/
2309260.
1615485644@sss.pgh.pa.us
Robert Haas [Sat, 13 Mar 2021 02:59:56 +0000 (21:59 -0500)]
pg_amcheck: Keep trying to fix the tests.
Commit
24189277f6ff3169b15c7bc82926a372ca7f2dbf managed to remove
one of the two places where we were checking for a "no such user"
error while leaving the other one right next to it. So remove that
too. In fact, remove the entire test, because the whole point of
this test was to see which message we got on a failure.
Robert Haas [Sat, 13 Mar 2021 01:11:47 +0000 (20:11 -0500)]
pg_amcheck: Try to fix still more test failures.
Avoid use of non-portable option ordering in command_checks_all().
The use of bare command line arguments before switches doesn't work
everywhere. Per buildfarm members drongo and hoverfly.
Avoid testing for the message "role \"%s\" does not exist", because
some buildfarm machines report a different error. fairywren complains
about "SSPI authentication failed for user \"%s\"", for example.
Mark Dilger
Discussion: http://postgr.es/m/
9E76E46A-48B2-4869-BD0C-
422204C1F767@enterprisedb.com
Discussion: http://postgr.es/m/
F0A1FD70-A2F4-4528-8A03-
8650CAEC0554%40enterprisedb.com
Robert Haas [Sat, 13 Mar 2021 00:00:41 +0000 (19:00 -0500)]
Try to avoid apparent platform-dependency in IPC::Run
It's hard to believe, but buildfarm results from the new pg_amcheck
suggest that command_checks_all() perform shell expansion on some
machines but not others, apparently due to an underlying behavior
difference in IPC::Run. Let's see if we can work around that - and
confirm that it is the real problem - by passing '-S*' as a single
argument rather than '-S' and '*' as two separate ones.
Failures were observed on jacana and hoverfly.
Mark Dilger
Discussion: http://postgr.es/m/
9E76E46A-48B2-4869-BD0C-
422204C1F767@enterprisedb.com
Robert Haas [Fri, 12 Mar 2021 22:30:17 +0000 (17:30 -0500)]
Fix portability issues in pg_amcheck's 004_verify_heapam.pl.
Test #12 overwrote a 1-byte varlena header to make it look like the
initial byte of a 4-byte varlena header, but the results were
endian-dependent. Also, the byte "abc" that followed the overwritten
byte would be interpreted differently depending on endian-ness.
Overwrite 4 bytes instead, in an endian-aware manner.
Test #13 accidentally managed to depend on TOAST_MAX_CHUNK_SIZE,
which varies slightly depending on MAXIMUM_ALIGNOF. That's not
the point anyway, so make the regexp insensitive to the expected
number of chunks.
Mark Dilger
Discussion: http://postgr.es/m/
A80D68F6-E38F-482D-9522-
E2FB6AAFE8A1@enterprisedb.com
Peter Geoghegan [Fri, 12 Mar 2021 21:11:47 +0000 (13:11 -0800)]
Consolidate nbtree VACUUM metapage routines.
Simplify _bt_vacuum_needs_cleanup() functions's signature (it only needs
a single 'rel' argument now), and move it next to its sibling function
in nbtpage.c.
I believe that _bt_vacuum_needs_cleanup() was originally located in
nbtree.c due to an include dependency issue. That's no longer an issue.
Follow-up to commit
9f3665fb.
Robert Haas [Fri, 12 Mar 2021 20:04:10 +0000 (15:04 -0500)]
Move PG_USED_FOR_ASSERTS_ONLY before initializer.
Erik Rijkers reported a compile failure, and I think this is probably
the reason.
Robert Haas [Fri, 12 Mar 2021 19:55:40 +0000 (14:55 -0500)]
Adjust perl style.
Per buildfarm member crake.
Robert Haas [Fri, 12 Mar 2021 19:35:10 +0000 (14:35 -0500)]
Try to fix compiler warnings.
Per report from Peter Geoghegan.
Discussion: http://postgr.es/m/CAH2-WznpwULZ3uJ1_6WXvNMXYbOy8k8tYs3r=qSdGmZeRd6tDw@mail.gmail.com
Robert Haas [Fri, 12 Mar 2021 18:00:01 +0000 (13:00 -0500)]
Add pg_amcheck, a CLI for contrib/amcheck.
This makes it a lot easier to run the corruption checks that are
implemented by contrib/amcheck against lots of relations and get
the result in an easily understandable format. It has a wide variety
of options for choosing which relations to check and which checks
to perform, and it can run checks in parallel if you want.
Mark Dilger, reviewed by Peter Geoghegan and by me.
Discussion: http://postgr.es/m/
12ED3DA8-25F0-4B68-937D-
D907CFBF08E7@enterprisedb.com
Discussion: http://postgr.es/m/
BA592F2D-F928-46FF-9516-
2B827F067F57@enterprisedb.com
Tom Lane [Fri, 12 Mar 2021 17:20:15 +0000 (12:20 -0500)]
Fix race condition in psql \e's detection of file modification.
psql's editing commands decide whether the user has edited the file
by checking for change of modification timestamp. This is probably
fine for a pre-existing file, but with a temporary file that is
created within the command, it's possible for a fast typist to
save-and-exit in less than the one-second granularity of stat(2)
timestamps. On Windows FAT filesystems the granularity is even
worse, 2 seconds, making the race a bit easier to hit.
To fix, try to set the temp file's mod time to be two seconds ago.
It's unlikely this would fail, but then again the race condition
itself is unlikely, so just ignore any error.
Also, we might as well check the file size as well as its mod time.
While this is a difficult bug to hit, it still seems worth
back-patching, to ensure that users' edits aren't lost.
Laurenz Albe, per gripe from Jacob Champion; based on fix suggestions
from Jacob and myself
Discussion: https://postgr.es/m/
0ba3f2a658bac6546d9934ab6ba63a805d46a49b.camel@cybertec.at
Tom Lane [Fri, 12 Mar 2021 16:08:42 +0000 (11:08 -0500)]
Forbid marking an identity column as nullable.
GENERATED ALWAYS AS IDENTITY implies NOT NULL, but the code failed
to complain if you overrode that with "GENERATED ALWAYS AS IDENTITY
NULL". One might think the old behavior was a feature, but it was
inconsistent because the outcome varied depending on the order of
the clauses, so it seems to have been just an oversight.
Per bug #16913 from Pavel Boev. Back-patch to v10 where identity
columns were introduced.
Vik Fearing (minor tweaks by me)
Discussion: https://postgr.es/m/16913-
3b5198410f67d8c6@postgresql.org
Thomas Munro [Fri, 12 Mar 2021 10:56:02 +0000 (23:56 +1300)]
Specialize checkpointer sort functions.
When sorting a potentially large number of dirty buffers, the
checkpointer can benefit from a faster sort routine. One reported
improvement on a large buffer pool system was 1.4s -> 0.6s.
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/CA%2BhUKGJ2-eaDqAum5bxhpMNhvuJmRDZxB_Tow0n-gse%2BHG0Yig%40mail.gmail.com
Amit Kapila [Fri, 12 Mar 2021 10:12:08 +0000 (15:42 +0530)]
Fix size overflow in calculation introduced by commits
d6ad34f3 and
bea449c6.
Reported-by: Thomas Munro
Author: Takayuki Tsunakawa
Reviewed-by: Kyotaro Horiguchi
Discussion: https://postgr.es/m/CA+hUKG+oPoFizjABt=GXZWTEHx3oev5rAe2scjW2r6F1rguo5w@mail.gmail.com
Amit Kapila [Fri, 12 Mar 2021 09:44:41 +0000 (15:14 +0530)]
Fix use of relcache TriggerDesc field introduced by commit
05c8482f7f.
The commit added code which used a relcache TriggerDesc field across
another cache access, which it shouldn't because the relcache doesn't
guarantee it won't get moved.
Diagnosed-by: Tom Lane
Author: Greg Nancarrow
Reviewed-by: Hou Zhijie, Amit Kapila
Discussion: https://postgr.es/m/
2309260.
1615485644@sss.pgh.pa.us
Thomas Munro [Fri, 12 Mar 2021 06:08:52 +0000 (19:08 +1300)]
Poll postmaster less frequently in recovery.
Since commits
9f095299 and
f98b8476 we don't poll the postmaster
pipe at all during crash recovery on Linux and FreeBSD, but on other
operating systems we were still doing it for every WAL record. Do it
less frequently on operating systems where system calls are required, at
the cost of delaying exit a bit after postmaster death. This avoids
expensive system calls reported to slow down CPU-bound recovery by as
much as 10-30%.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Fujii Masao <masao.fujii@oss.nttdata.com>
Reviewed-by: Michael Paquier <michael@paquier.xyz>
Discussion: https://postgr.es/m/CA%2BhUKGK1607VmtrDUHQXrsooU%3Dap4g4R2yaoByWOOA3m8xevUQ%40mail.gmail.com
Discussion: https://postgr.es/m/
7261eb39-0369-f2f4-1bb5-
62f3b6083b5e@iki.fi
Thomas Munro [Fri, 12 Mar 2021 06:07:27 +0000 (19:07 +1300)]
Add condition variable for walreceiver shutdown.
Use this new CV to wait for walreceiver shutdown without a sleep/poll
loop, while also benefiting from standard postmaster death handling.
Discussion: https://postgr.es/m/CA%2BhUKGK1607VmtrDUHQXrsooU%3Dap4g4R2yaoByWOOA3m8xevUQ%40mail.gmail.com
Thomas Munro [Fri, 12 Mar 2021 06:03:52 +0000 (19:03 +1300)]
Add condition variable for recovery resume.
Replace a sleep loop with a CV, to get a fast reaction time when
recovery is resumed or the postmaster exits via standard infrastructure.
Unfortunately we still need to wake up every second to perform extra
polling during the recovery pause loop.
Discussion: https://postgr.es/m/CA%2BhUKGK1607VmtrDUHQXrsooU%3Dap4g4R2yaoByWOOA3m8xevUQ%40mail.gmail.com
Fujii Masao [Fri, 12 Mar 2021 05:23:00 +0000 (14:23 +0900)]
Send statistics collected during shutdown checkpoint to the stats collector.
When shutdown is requested, checkpointer performs checkpoint or
restartpoint, and updates the statistics, before it exits. But previously
checkpointer didn't send those statistics to the stats collector.
Shutdown checkpoint and restartpoint are treated as requested ones
instead of scheduled ones, so the number of them are counted in
pg_stat_bgwriter.checkpoints_req column.
Author: Masahiro Ikeda
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/
0509ad67b585a5b86a83d445dfa75392@oss.nttdata.com
Fujii Masao [Fri, 12 Mar 2021 04:29:59 +0000 (13:29 +0900)]
Force to send remaining WAL stats to the stats collector at walwriter exit.
In walwriter's main loop, WAL stats message is only sent if enough time
has passed since last one was sent to reach PGSTAT_STAT_INTERVAL msecs.
This is necessary to avoid overloading to the stats collector. But this
can cause recent WAL stats to be unsent when walwriter exits.
To ensure that all the WAL stats are sent, this commit makes walwriter
force to send remaining WAL stats to the collector when it exits because
of shutdown request. Note that those remaining WAL stats can still be
unsent when walwriter exits with non-zero exit code (e.g., FATAL error).
This is OK because that walwriter exit leads to server crash and
subsequent recovery discards all the stats. So there is no need to send
remaining stats in that case.
Author: Masahiro Ikeda
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/
0509ad67b585a5b86a83d445dfa75392@oss.nttdata.com
Thomas Munro [Fri, 12 Mar 2021 02:24:28 +0000 (15:24 +1300)]
Minor modernization for README.barrier.
Itanium is very uncommon and being discontinued. ARM is everywhere.
Prefer ARM as an example of an architecture with weak memory ordering.
Peter Geoghegan [Thu, 11 Mar 2021 22:18:23 +0000 (14:18 -0800)]
Save a few cycles during nbtree VACUUM.
Avoid calling RelationGetNumberOfBlocks() unnecessarily in the common
case where there are no deleted but not yet recycled pages to recycle
during a cleanup-only nbtree VACUUM operation.
Follow-up to commit
e5d8a999, which (among other things) taught the
"skip full scan" nbtree VACUUM mechanism to only trigger a full index
scan when the absolute number of deleted pages in the index is
considered excessive.
Peter Geoghegan [Thu, 11 Mar 2021 20:42:46 +0000 (12:42 -0800)]
Add back vacuum_cleanup_index_scale_factor parameter.
Commit
9f3665fb removed the vacuum_cleanup_index_scale_factor storage
parameter. However, that creates dump/reload hazards when moving across
major versions.
Add back the vacuum_cleanup_index_scale_factor parameter (though not the
GUC of the same name) purely to avoid problems when using tools like
pg_upgrade. The parameter remains disabled and undocumented.
No backpatch to Postgres 13, since vacuum_cleanup_index_scale_factor was
only disabled by REL_13_STABLE's version of master branch commit
9f3665fb in the first place -- the parameter already looks like this on
REL_13_STABLE.
Discussion: https://postgr.es/m/YEm/a3Ko3nKnBuVq@paquier.xyz
Robert Haas [Thu, 11 Mar 2021 19:52:32 +0000 (14:52 -0500)]
Be clear about whether a recovery pause has taken effect.
Previously, the code and documentation seem to have essentially
assumed than a call to pg_wal_replay_pause() would take place
immediately, but that's not the case, because we only check for a
pause in certain places. This means that a tool that uses this
function and then wants to do something else afterward that is
dependent on the pause having taken effect doesn't know how long it
needs to wait to be sure that no more WAL is going to be replayed.
To avoid that, add a new function pg_get_wal_replay_pause_state()
which returns either 'not paused', 'paused requested', or 'paused'.
After calling pg_wal_replay_pause() the status will immediate change
from 'not paused' to 'pause requested'; when the startup process
has noticed this, the status will change to 'pause'. For backward
compatibility, pg_is_wal_replay_paused() still exists and returns
the same thing as before: true if a pause has been requested,
whether or not it has taken effect yet; and false if not.
The documentation is updated to clarify.
To improve the changes that a pause request is quickly confirmed
effective, adjust things so that WaitForWALToBecomeAvailable will
swiftly reach a call to recoveryPausesHere() when a pause request
is made.
Dilip Kumar, reviewed by Simon Riggs, Kyotaro Horiguchi, Yugo Nagata,
Masahiko Sawada, and Bharath Rupireddy.
Discussion: http://postgr.es/m/CAFiTN-vcLLWEm8Zr%3DYK83rgYrT9pbC8VJCfa1kY9vL3AUPfu6g%40mail.gmail.com
Tom Lane [Thu, 11 Mar 2021 19:43:45 +0000 (14:43 -0500)]
Re-simplify management of inStart in pqParseInput3's subroutines.
Commit
92785dac2 copied some logic related to advancement of inStart
from pqParseInput3 into getRowDescriptions and getAnotherTuple,
because it wanted to allow user-defined row processor callbacks to
potentially longjmp out of the library, and inStart would have to be
updated before that happened to avoid an infinite loop. We later
decided that that API was impossibly fragile and reverted it, but
we didn't undo all of the related code changes, and this bit of
messiness survived. Undo it now so that there's just one place in
pqParseInput3's processing where inStart is advanced; this will
simplify addition of better tracing support.
getParamDescriptions had grown similar processing somewhere along
the way (not in
92785dac2; I didn't track down just when), but it's
actually buggy because its handling of corrupt-message cases seems to
have been copied from the v2 logic where we lacked a known message
length. The cases where we "goto not_enough_data" should not simply
return EOF, because then we won't consume the message, potentially
creating an infinite loop. That situation now represents a
definitively corrupt message, and we should report it as such.
Although no field reports of getParamDescriptions getting stuck in
a loop have been seen, it seems appropriate to back-patch that fix.
I chose to back-patch all of this to keep the logic looking more alike
in supported branches.
Discussion: https://postgr.es/m/
2217283.
1615411989@sss.pgh.pa.us
Robert Haas [Thu, 11 Mar 2021 18:17:46 +0000 (13:17 -0500)]
Refactor and generalize the ParallelSlot machinery.
Create a wrapper object, ParallelSlotArray, to encapsulate the
number of slots and the slot array itself, plus some other relevant
bits of information. This reduces the number of parameters we have
to pass around all over the place.
Allow for a ParallelSlotArray to contain slots connected to
different databases within a single cluster. The current clients
of this mechanism don't need this, but it is expected to be used
by future patches.
Defer connecting to databases until we actually need the connection
for something. This is a slight behavior change for vacuumdb and
reindexdb. If you specify a number of jobs that is larger than the
number of objects, the extra connections will now not be used.
But, on the other hand, if you specify a number of jobs that is
so large that it's going to fail, the failure would previously have
happened before any operations were actually started, and now it
won't.
Mark Dilger, reviewed by me.
Discussion: http://postgr.es/m/
12ED3DA8-25F0-4B68-937D-
D907CFBF08E7@enterprisedb.com
Discussion: http://postgr.es/m/
BA592F2D-F928-46FF-9516-
2B827F067F57@enterprisedb.com
Michael Paquier [Thu, 11 Mar 2021 08:14:25 +0000 (17:14 +0900)]
Set libcrypto callbacks for all connection threads in libpq
Based on an analysis of the OpenSSL code with Jacob, moving to EVP for
the cryptohash computations makes necessary the setup of the libcrypto
callbacks that were getting set only for SSL connections, but not for
connections without SSL. Not setting the callbacks makes the use of
threads potentially unsafe for connections calling cryptohashes during
authentication, like MD5 or SCRAM, if a failure happens during a
cryptohash computation. The logic setting the libssl and libcrypto
states is then split into two parts, both using the same locking, with
libcrypto being set up for SSL and non-SSL connections, while SSL
connections set any libssl state afterwards as needed.
Prior to this commit, only SSL connections would have set libcrypto
callbacks that are necessary to ensure a proper thread locking when
using multiple concurrent threads in libpq (ENABLE_THREAD_SAFETY). Note
that this is only required for OpenSSL 1.0.2 and 1.0.1 (oldest version
supported on HEAD), as 1.1.0 has its own internal locking and it has
dropped support for CRYPTO_set_locking_callback().
Tests with up to 300 threads with OpenSSL 1.0.1 and 1.0.2, mixing SSL
and non-SSL connection threads did not show any performance impact after
some micro-benchmarking. pgbench can be used here with -C and a
mostly-empty script (with one \set meta-command for example) to stress
authentication requests, and we have mixed that with some custom
programs for testing.
Reported-by: Jacob Champion
Author: Michael Paquier
Reviewed-by: Jacob Champion
Discussion: https://postgr.es/m/
fd3ba610085f1ff54623478cf2f7adf5af193cbb.camel@vmware.com
Peter Geoghegan [Thu, 11 Mar 2021 06:10:36 +0000 (22:10 -0800)]
Doc: B-Tree only has one additional parameter.
Oversight in commit
9f3665fb.
Backpatch: 13-, just like commit
9f3665fb.
Thomas Munro [Thu, 11 Mar 2021 02:58:05 +0000 (15:58 +1300)]
Improve comment for struct BufferDesc.
Add a note that per-buffer I/O condition variables currently live
outside the BufferDesc struct. Follow-up for commit
d8725104.
Reported-by: Julien Rouhaud <rjuju123@gmail.com>
Discussion: https://postgr.es/m/
20210311031118.hucytmrgwlktjxgq%40nol
Bruce Momjian [Thu, 11 Mar 2021 01:25:19 +0000 (20:25 -0500)]
tutorial: land height is "elevation", not "altitude"
This is a follow-on patch to
92c12e46d5. In that patch, we renamed
"altitude" to "elevation" in the docs, based on these details:
https://mapscaping.com/blogs/geo-candy/what-is-the-difference-between-elevation-relief-and-altitude
This renames the tutorial SQL files to match the documentation.
Reported-by: max1@inbox.ru
Discussion: https://postgr.es/m/
161512392887.1046.
3137472627109459518@wrigleys.postgresql.org
Backpatch-through: 9.6
Peter Geoghegan [Thu, 11 Mar 2021 01:07:57 +0000 (17:07 -0800)]
VACUUM ANALYZE: Always update pg_class.reltuples.
vacuumlazy.c sometimes fails to update pg_class entries for each index
(to ensure that pg_class.reltuples is current), even though analyze.c
assumed that that must have happened during VACUUM ANALYZE. There are
at least a couple of reasons for this. For example, vacuumlazy.c could
fail to update pg_class when the index AM indicated that its statistics
are merely an estimate, per the contract for amvacuumcleanup() routines
established by commit
e57345975cf back in 2006.
Stop assuming that pg_class must have been updated with accurate
statistics within VACUUM ANALYZE -- update pg_class for indexes at the
same time as the table relation in all cases. That way VACUUM ANALYZE
will never fail to keep pg_class.reltuples reasonably accurate.
The only downside of this approach (compared to the old approach) is
that it might inaccurately set pg_class.reltuples for indexes whose heap
relation ends up with the same inaccurate value anyway. This doesn't
seem too bad. We already consistently called vac_update_relstats() (to
update pg_class) for the heap/table relation twice during any VACUUM
ANALYZE -- once in vacuumlazy.c, and once in analyze.c. We now make
sure that we call vac_update_relstats() at least once (though often
twice) for each index.
This is follow up work to commit
9f3665fb, which dealt with issues in
btvacuumcleanup(). Technically this fixes an unrelated issue, though.
btvacuumcleanup() no longer provides an accurate num_index_tuples value
following commit
9f3665fb (when there was no btbulkdelete() call during
the VACUUM operation in question), but hashvacuumcleanup() has worked in
the same way for many years now.
Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAH2-WzknxdComjhqo4SUxVFk_Q1171GJO2ZgHZ1Y6pion6u8rA@mail.gmail.com
Backpatch: 13-, just like commit
9f3665fb.
Peter Geoghegan [Thu, 11 Mar 2021 00:27:01 +0000 (16:27 -0800)]
Don't consider newly inserted tuples in nbtree VACUUM.
Remove the entire idea of "stale stats" within nbtree VACUUM (stop
caring about stats involving the number of inserted tuples). Also
remove the vacuum_cleanup_index_scale_factor GUC/param on the master
branch (though just disable them on postgres 13).
The vacuum_cleanup_index_scale_factor/stats interface made the nbtree AM
partially responsible for deciding when pg_class.reltuples stats needed
to be updated. This seems contrary to the spirit of the index AM API,
though -- it is not actually necessary for an index AM's bulk delete and
cleanup callbacks to provide accurate stats when it happens to be
inconvenient. The core code owns that. (Index AMs have the authority
to perform or not perform certain kinds of deferred cleanup based on
their own considerations, such as page deletion and recycling, but that
has little to do with pg_class.reltuples/num_index_tuples.)
This issue was fairly harmless until the introduction of the
autovacuum_vacuum_insert_threshold feature by commit
b07642db, which had
an undesirable interaction with the vacuum_cleanup_index_scale_factor
mechanism: it made insert-driven autovacuums perform full index scans,
even though there is no real benefit to doing so. This has been tied to
a regression with an append-only insert benchmark [1].
Also have remaining cases that perform a full scan of an index during a
cleanup-only nbtree VACUUM indicate that the final tuple count is only
an estimate. This prevents vacuumlazy.c from setting the index's
pg_class.reltuples in those cases (it will now only update pg_class when
vacuumlazy.c had TIDs for nbtree to bulk delete). This arguably fixes
an oversight in deduplication-related bugfix commit
48e12913.
[1] https://smalldatum.blogspot.com/2021/01/insert-benchmark-postgres-is-still.html
Author: Peter Geoghegan <pg@bowt.ie>
Reviewed-By: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/CAD21AoA4WHthN5uU6+WScZ7+J_RcEjmcuH94qcoUPuB42ShXzg@mail.gmail.com
Backpatch: 13-, where autovacuum_vacuum_insert_threshold was added.
Bruce Momjian [Wed, 10 Mar 2021 22:03:10 +0000 (17:03 -0500)]
C comments: improve description of GiST NSN and GistBuildLSN
GiST indexes are complex, so adding more details in the code might help
someone.
Discussion: https://postgr.es/m/
20210302164021.GA364@momjian.us
Thomas Munro [Wed, 10 Mar 2021 21:05:58 +0000 (10:05 +1300)]
Replace buffer I/O locks with condition variables.
1. Backends waiting for buffer I/O are now interruptible.
2. If something goes wrong in a backend that is currently performing
I/O, waiting backends no longer wake up until that backend reaches
AbortBufferIO() and broadcasts on the CV. Previously, any waiters would
wake up (because the I/O lock was automatically released) and then
busy-loop until AbortBufferIO() cleared BM_IO_IN_PROGRESS.
3. LWLockMinimallyPadded is removed, as it would now be unused.
Author: Robert Haas <robertmhaas@gmail.com>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Reviewed-by: Julien Rouhaud <rjuju123@gmail.com>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us> (earlier version, 2016)
Discussion: https://postgr.es/m/CA%2BhUKGJ8nBFrjLuCTuqKN0pd2PQOwj9b_jnsiGFFMDvUxahj_A%40mail.gmail.com
Discussion: https://postgr.es/m/CA+Tgmoaj2aPti0yho7FeEf2qt-JgQPRWb0gci_o1Hfr=C56Xng@mail.gmail.com
Tom Lane [Wed, 10 Mar 2021 19:22:31 +0000 (14:22 -0500)]
Avoid creating duplicate cached plans for inherited FK constraints.
When a foreign key constraint is applied to a partitioned table, each
leaf partition inherits a similar FK constraint. We were processing all
of those constraints independently, meaning that in large partitioning
trees we'd build up large collections of cached FK-checking query plans.
However, in all cases but one, the generated queries are actually
identical for all members of the inheritance tree (because, in most
cases, the query only mentions the topmost table of the other side of
the FK relationship). So we can share a single cached plan among all
the partitions, saving memory, not to mention time to build and maintain
the cached plans.
Keisuke Kuroda and Amit Langote
Discussion: https://postgr.es/m/
cab4b85d-9292-967d-adf2-
be0d803c3e23@nttcom.co.jp_1
Tom Lane [Wed, 10 Mar 2021 17:38:43 +0000 (12:38 -0500)]
Doc: get rid of <foreignphrase> tags.
We italicized some, but not all, instances of "per se", "pro forma", and
"ad hoc". These phrases are widespread in formal registers of English,
so it"s debatable whether they even qualify as foreign. We could instead
try to be more consistent in the use of <foreignphrase>, but that"s
difficult to enforce, so let"s just remove the tags for those words.
The one case that seems to deserve the tag is "voilà". Instead of keeping
just one instance of the tag, change that to a more standard phrase.
John Naylor
Discussion: https://postgr.es/m/CAFBsxsHtWs_NsccAVgQ=tTUKkXHpHdkjZXtp_Cd9dGWyBDxfbQ@mail.gmail.com
Tom Lane [Wed, 10 Mar 2021 16:33:50 +0000 (11:33 -0500)]
Doc: improve introductory information about procedures.
Clarify the discussion in "User-Defined Procedures", by laying out
the key differences between functions and procedures in a bulleted
list. Notably, this avoids burying the lede about procedures being
able to do transaction control. Make the back-link in the CREATE
FUNCTION reference page more prominent, and add one in CREATE
PROCEDURE.
Per gripe from Guyren Howe. Thanks to David Johnston for discussion.
Discussion: https://postgr.es/m/BYAPR03MB4903C53A8BB7EFF5EA289674A6949@BYAPR03MB4903.namprd03.prod.outlook.com
Tom Lane [Wed, 10 Mar 2021 15:59:48 +0000 (10:59 -0500)]
Doc: fix missing mention of procedure OUT parameters.
Small oversight in commit
2453ea142.
Peter Eisentraut [Wed, 10 Mar 2021 14:19:37 +0000 (15:19 +0100)]
Add bound check before bsearch() for performance
In the current lazy vacuum implementation, some index AMs such as
btree indexes call lazy_tid_reaped() for each index tuple during
ambulkdelete to check if the index tuple points to the (collected)
garbage tuple. In that function, we simply call bsearch(), but we
should be able to know the result without bsearch() if the index tuple
points to the heap tuple that is out of range of the collected garbage
tuples. Therefore, add a simple bound check before resorting to
bsearch(). Testing has shown that this can give significant
performance benefits.
Author: Masahiko Sawada <masahiko.sawada@2ndquadrant.com>
Discussion: https://www.postgresql.org/message-id/flat/CA+fd4k76j8jKzJzcx8UqEugvayaMSnQz0iLUt_XgBp-_-bd22A@mail.gmail.com
Thomas Munro [Wed, 10 Mar 2021 09:22:12 +0000 (22:22 +1300)]
Fix another portability bug in recent pgbench commit.
Commit
547f04e7 produced errors on AIX/xlc while building plpython. The
new code appears to be incompatible with the hack installed by commit
a11cf433. Without access to an AIX system to check, my guess is that
_POSIX_C_SOURCE may be required for <time.h> to declare the things the
header needs to see, but plpython.h undefines it.
For now, to unbreak build farm animal hoverfly, just move the new
pg_time_usec_t support into pgbench.c. Perhaps later we could figure
out what to rearrange to put it back into a header for wider use.
Discussion: https://postgr.es/m/CA%2BhUKG%2BP%2BjcD%3Dx9%2BagyTdWtjpOT64MYiGic%2Bcbu_TD8CV%3D6A3w%40mail.gmail.com
Thomas Munro [Wed, 10 Mar 2021 07:18:15 +0000 (20:18 +1300)]
Try to fix portability bugs in recent pgbench commits.
1. pg_time_usec_t needs to be printed with INT64_FORMAT, not %ld, or 32
bit systems complain, per lapwing.
2. Some Windows compilers didn't like a thread function not marked with
__stdcall, per whelk; let's see if this fixes the problem.
Peter Eisentraut [Wed, 10 Mar 2021 07:16:38 +0000 (08:16 +0100)]
Small debug message tweak
This makes the wording of the delete case match the update case.
Michael Paquier [Wed, 10 Mar 2021 05:50:00 +0000 (14:50 +0900)]
Move tablespace path re-creation from the makefiles to pg_regress
Moving this logic into pg_regress fixes a potential failure with
parallel tests when pg_upgrade and the main regression test suite both
trigger the makefile rule that cleaned up testtablespace/ under
src/test/regress. Even if pg_upgrade was triggering this rule, it has
no need to do so as it uses a different tablespace path. So if
pg_upgrade triggered the makefile rule for the tablespace setup while
the main regression test suite ran the tablespace cases, it would fail.
61be85a was a similar attempt at achieving that, but that broke cases
where the regression tests require to run under an Administrator
account, like with Appveyor.
Reported-by: Andres Freund, Kyotaro Horiguchi
Reviewed-by: Peter Eisentraut
Discussion: https://postgr.es/m/
20201209012911.uk4d6nxcnkp7ehrx@alap3.anarazel.de
Thomas Munro [Wed, 10 Mar 2021 03:17:34 +0000 (16:17 +1300)]
pgbench: Synchronize client threads.
Wait until all pgbench threads are connected before benchmarking begins.
This fixes a problem where some connections could take a very long time
to be established because of lock contention from earlier connections,
making results unstable and bogus with high connection counts.
Author: Andres Freund <andres@anarazel.de>
Author: Fabien COELHO <coelho@cri.ensmp.fr>
Reviewed-by: Marina Polyakova <m.polyakova@postgrespro.ru>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/
20200227180100.zyvjwzcpiokfsqm2%40alap3.anarazel.de
Thomas Munro [Wed, 10 Mar 2021 02:40:17 +0000 (15:40 +1300)]
Add missing pthread_barrier_t.
Supply a simple implementation of the missing pthread_barrier_t type and
functions, for macOS.
Discussion: https://postgr.es/m/
20200227180100.zyvjwzcpiokfsqm2%40alap3.anarazel.de
Thomas Munro [Wed, 10 Mar 2021 03:09:50 +0000 (16:09 +1300)]
pgbench: Improve time logic.
Instead of instr_time (struct timespec) and the INSTR_XXX macros,
introduce pg_time_usec_t and use integer arithmetic. Don't include the
connection time in TPS unless using -C mode, but report it separately.
Author: Fabien COELHO <coelho@cri.ensmp.fr>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: Hayato Kuroda <kuroda.hayato@fujitsu.com>
Discussion: https://postgr.es/m/
20200227180100.zyvjwzcpiokfsqm2%40alap3.anarazel.de
Thomas Munro [Wed, 10 Mar 2021 02:39:08 +0000 (15:39 +1300)]
pgbench: Refactor thread portability support.
Instead of maintaining an incomplete emulation of POSIX threads for
Windows, let's use an extremely minimalist macro-based abstraction for
now. A later patch will extend this, without the need to supply more
complicated pthread emulation code. (There may be a need for a more
serious portable thread abstraction in later projects, but this is not
it.)
Minor incidental problems fixed: it wasn't OK to use (pthread_t) 0 as a
special value, it wasn't OK to compare thread_t values with ==, and we
incorrectly assumed that pthread functions set errno.
Discussion: https://postgr.es/m/
20200227180100.zyvjwzcpiokfsqm2%40alap3.anarazel.de
Amit Kapila [Wed, 10 Mar 2021 04:34:20 +0000 (10:04 +0530)]
Fix valgrind issue in commit
05c8482f7f.
Initialize other newly added variables in max_parallel_hazard_context via
is_parallel_safe() because we don't check the parallel-safety of target
relations in that function.
Reported-by: Tom Lane as per buildfarm
Author: Amit Kapila
Discussion: https://postgr.es/m/
2060179.
1615347455@sss.pgh.pa.us
Amit Kapila [Wed, 10 Mar 2021 02:08:58 +0000 (07:38 +0530)]
Enable parallel SELECT for "INSERT INTO ... SELECT ...".
Parallel SELECT can't be utilized for INSERT in the following cases:
- INSERT statement uses the ON CONFLICT DO UPDATE clause
- Target table has a parallel-unsafe: trigger, index expression or
predicate, column default expression or check constraint
- Target table has a parallel-unsafe domain constraint on any column
- Target table is a partitioned table with a parallel-unsafe partition key
expression or support function
The planner is updated to perform additional parallel-safety checks for
the cases listed above, for determining whether it is safe to run INSERT
in parallel-mode with an underlying parallel SELECT. The planner will
consider using parallel SELECT for "INSERT INTO ... SELECT ...", provided
nothing unsafe is found from the additional parallel-safety checks, or
from the existing parallel-safety checks for SELECT.
While checking parallel-safety, we need to check it for all the partitions
on the table which can be costly especially when we decide not to use a
parallel plan. So, in a separate patch, we will introduce a GUC and or a
reloption to enable/disable parallelism for Insert statements.
Prior to entering parallel-mode for the execution of INSERT with parallel
SELECT, a TransactionId is acquired and assigned to the current
transaction state. This is necessary to prevent the INSERT from attempting
to assign the TransactionId whilst in parallel-mode, which is not allowed.
This approach has a disadvantage in that if the underlying SELECT does not
return any rows, then the TransactionId is not used, however that
shouldn't happen in practice in many cases.
Author: Greg Nancarrow, Amit Langote, Amit Kapila
Reviewed-by: Amit Langote, Hou Zhijie, Takayuki Tsunakawa, Antonin Houska, Bharath Rupireddy, Dilip Kumar, Vignesh C, Zhihong Yu, Amit Kapila
Tested-by: Tang, Haiying
Discussion: https://postgr.es/m/CAJcOf-cXnB5cnMKqWEp2E2z7Mvcd04iLVmV=qpFJrR3AcrTS3g@mail.gmail.com
Discussion: https://postgr.es/m/CAJcOf-fAdj=nDKMsRhQzndm-O13NY4dL6xGcEvdX5Xvbbi0V7g@mail.gmail.com
Michael Paquier [Wed, 10 Mar 2021 00:35:42 +0000 (09:35 +0900)]
Revert changes for SSL compression in libpq
This partially reverts
096bbf7 and
9d2d457, undoing the libpq changes as
it could cause breakages in distributions that share one single libpq
version across multiple major versions of Postgres for extensions and
applications linking to that.
Note that the backend is unchanged here, and it still disables SSL
compression while simplifying the underlying catalogs that tracked if
compression was enabled or not for a SSL connection.
Per discussion with Tom Lane and Daniel Gustafsson.
Discussion: https://postgr.es/m/YEbq15JKJwIX+S6m@paquier.xyz
Alexander Korotkov [Tue, 9 Mar 2021 15:16:03 +0000 (18:16 +0300)]
Fix vague comment in jsonb documentation
The sample query fails because of an attempt to update the key of a numeric.
But the comment says it's just because of the missing object key. That's not
correct because jsonb subscription automatically adds missing keys.
Reported-by: Nikita Konev