The primary bottlenecks for relation extension are:
1) The extension lock is held while acquiring a victim buffer for the new
page. Acquiring a victim buffer can require writing out the old page
contents including possibly needing to flush WAL.
2) When extending via ReadBuffer() et al, we write a zero page during the
extension, and then later write out the actual page contents. This can
nearly double the write rate.
3) The existing bulk relation extension infrastructure in hio.c just amortized
the cost of acquiring the relation extension lock, but none of the other
costs.
Unfortunately 1) cannot currently be addressed in a central manner as the
callers to ReadBuffer() need to acquire the extension lock. To address that,
this this commit moves the responsibility for acquiring the extension lock
into bufmgr.c functions. That allows to acquire the relation extension lock
for just the required time. This will also allow us to improve relation
extension further, without changing callers.
The reason we write all-zeroes pages during relation extension is that we hope
to get ENOSPC errors earlier that way (largely works, except for CoW
filesystems). It is easier to handle out-of-space errors gracefully if the
page doesn't yet contain actual tuples. This commit addresses 2), by using the
recently introduced smgrzeroextend(), which extends the relation, without
dirtying the kernel page cache for all the extended pages.
To address 3), this commit introduces a function to extend a relation by
multiple blocks at a time.
There are three new exposed functions: ExtendBufferedRel() for extending the
relation by a single block, ExtendBufferedRelBy() to extend a relation by
multiple blocks at once, and ExtendBufferedRelTo() for extending a relation up
to a certain size.
To avoid duplicating code between ReadBuffer(P_NEW) and the new functions,
ReadBuffer(P_NEW) now implements relation extension with
ExtendBufferedRel(), using a flag to tell ExtendBufferedRel() that the
relation lock is already held.
Note that this commit does not yet lead to a meaningful performance or
scalability improvement - for that uses of ReadBuffer(P_NEW) will need to be
converted to ExtendBuffered*(), which will be done in subsequent commits.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
Discussion: https://postgr.es/m/20221029025420.eplyow6k7tgu6he3@awork3.anarazel.de
This adds a new option to libpq's sslrootcert, "system", which will load
the system trusted CA roots for certificate verification. This is a more
convenient way to achieve this than pointing to the system CA roots
manually since the location can differ by installation and be locally
adjusted by env vars in OpenSSL.
When sslrootcert is set to system, sslmode is forced to be verify-full
as weaker modes aren't providing much security for public CAs.
Changing the location of the system roots by setting environment vars is
not supported by LibreSSL so the tests will use a heuristic to determine
if the system being tested is LibreSSL or OpenSSL.
The workaround in .cirrus.yml is required to handle a strange interaction
between homebrew and the openssl@3 formula; hopefully this can be removed
in the near future.
The original patch was written by Thomas Habets, which was later revived
by Jacob Champion.
Author: Jacob Champion <jchampion@timescale.com>
Author: Thomas Habets <thomas@habets.se>
Reviewed-by: Jelte Fennema <postgres@jeltef.nl>
Reviewed-by: Andrew Dunstan <andrew@dunslane.net>
Reviewed-by: Magnus Hagander <magnus@hagander.net>
Discussion: https://www.postgresql.org/message-id/flat/CA%2BkHd%2BcJwCUxVb-Gj_0ptr3_KZPwi3%2B67vK6HnLFBK9MzuYrLA%40mail.gmail.com
A future patch will add support for extending relations by multiple blocks at
once. To be concurrency safe, the buffers for those blocks need to be marked
as BM_IO_IN_PROGRESS. Until now we only had infrastructure for recovering from
an IO error for a single buffer. This commit extends that infrastructure to
multiple buffers by using the resource owner infrastructure.
This commit increases the size of the ResourceOwnerData struct, which appears
to have a just about measurable overhead in very extreme workloads. Medium
term we are planning to substantially shrink the size of
ResourceOwnerData. Short term the increase is small enough to not worry about
it for now.
Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
Discussion: https://postgr.es/m/20221029025420.eplyow6k7tgu6he3@awork3.anarazel.de
Discussion: https://postgr.es/m/20221029200025.w7bvlgvamjfo6z44@awork3.anarazel.de
Merge and hash joins can support antijoin with the non-nullable input
on the right, using very simple combinations of their existing logic
for right join and anti join. This gives the planner more freedom
about how to order the join. It's particularly useful for hash join,
since we may now have the option to hash the smaller table instead
of the larger.
Richard Guo, reviewed by Ronan Dunklau and myself
Discussion: https://postgr.es/m/CAMbWs48xh9hMzXzSy3VaPzGAz+fkxXXTUbCLohX1_L8THFRm2Q@mail.gmail.com
Previously we held buffer locks for two buffer mapping partitions at the same
time to change the identity of buffers. Particularly for extending relations
needing to hold the extension lock while acquiring a victim buffer is
painful.But it also creates a bottleneck for workloads that just involve
reads.
Now we instead first acquire a victim buffer and write it out, if
necessary. Then we remove that buffer from the old partition with just the old
partition's partition lock held and insert it into the new partition with just
that partition's lock held.
By separating out the victim buffer acquisition, future commits will be able
to change relation extensions to scale better.
On my workstation, a micro-benchmark exercising buffered reads strenuously and
under a lot of concurrency, sees a >2x improvement.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
Discussion: https://postgr.es/m/20221029025420.eplyow6k7tgu6he3@awork3.anarazel.de
Commit 47bb9db75 taught AcquireExecutorLocks to re-acquire locks
on views using data from their RTE_SUBQUERY replacements, but
it now seems like we should make AcquirePlannerLocks do the same.
In this way, if a view has been redefined, we will notice that
a bit earlier while checking validity of a cached plan and thereby
avoid some wasted work.
Report and patch by Amit Langote.
Discussion: https://postgr.es/m/CA+HiwqH0xZOQ+GQAdKeckY1R4NOeHdzhtfxkAMJLSchpapNk5w@mail.gmail.com
Allow pg_dump to use the zstd compression, in addition to gzip/lz4. Bulk
of the new compression method is implemented in compress_zstd.{c,h},
covering the pg_dump compression APIs. The rest of the patch adds test
and makes various places aware of the new compression method.
The zstd library (which this patch relies on) supports multithreaded
compression since version 1.5. We however disallow that feature for now,
as it might interfere with parallel backups on platforms that rely on
threads (e.g. Windows). This can be improved / relaxed in the future.
This also fixes a minor issue in InitDiscoverCompressFileHandle(), which
was not updated to check if the file already has the .lz4 extension.
Adding zstd compression was originally proposed in 2020 (see the second
thread), but then was reworked to use the new compression API introduced
in e9960732a9. I've considered both threads when compiling the list of
reviewers.
Author: Justin Pryzby
Reviewed-by: Tomas Vondra, Jacob Champion, Andreas Karlsson
Discussion: https://postgr.es/m/20230224191840.GD1653@telsasoft.com
Discussion: https://postgr.es/m/20201221194924.GI30237@telsasoft.com
This adds a few more assertions against a buffer being local in places we
don't expect, and extracts the check for a buffer being pinned exactly once
from LockBufferForCleanup() into its own function. Later commits will use this
function.
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
Discussion: http://postgr.es/m/419312fd-9255-078c-c3e3-f0525f911d7f@iki.fi
smgrzeroextend() uses FileFallocate() to efficiently extend files by multiple
blocks. When extending by a small number of blocks, use FileZero() instead, as
using posix_fallocate() for small numbers of blocks is inefficient for some
file systems / operating systems. FileZero() is also used as the fallback for
FileFallocate() on platforms / filesystems that don't support fallocate.
A big advantage of using posix_fallocate() is that it typically won't cause
dirty buffers in the kernel pagecache. So far the most common pattern in our
code is that we smgrextend() a page full of zeroes and put the corresponding
page into shared buffers, from where we later write out the actual contents of
the page. If the kernel, e.g. due to memory pressure or elapsed time, already
wrote back the all-zeroes page, this can lead to doubling the amount of writes
reaching storage.
There are no users of smgrzeroextend() as of this commit. That will follow in
future commits.
Reviewed-by: Melanie Plageman <melanieplageman@gmail.com>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: John Naylor <john.naylor@enterprisedb.com>
Discussion: https://postgr.es/m/20221029025420.eplyow6k7tgu6he3@awork3.anarazel.de
In v13 and v14, the ENABLE/DISABLE TRIGGER USER variant malfunctioned
on cloned triggers, failing to find the clones because it thought they
were system triggers. Other variants of ENABLE/DISABLE TRIGGER would
improperly apply a superuserness check. Fix by adjusting the is-it-
a-system-trigger check to match reality in those branches. (As far
as I can find, this is the only place that got it wrong.)
There's no such bug in v15/HEAD, because we revised the catalog
representation of system triggers to be what this code was expecting.
However, add the test case to these branches anyway, because this area
is visibly pretty fragile. Also remove an obsoleted comment.
The recent v15/HEAD commit 6949b921d fixed a nearby bug. I now see
that my commit message for that was inaccurate: the behavior of
recursing to clone triggers is older than v15, but it didn't apply
to the case in v13/v14 because in those branches parent partitioned
tables have no pg_trigger entries for foreign-key triggers. But add
the test case from that commit to v13/v14, just to show what is
happening there.
Per bug #17886 from DzmitryH.
Discussion: https://postgr.es/m/17886-5406d5d828aa4aa3@postgresql.org
The read path needs to be able to initialize pages anyway, as relation
extensions are not durable. By avoiding initializing pages, we can, in a
future patch, extend the relation by multiple blocks at once.
Using smgrextend() for {vm,fsm}_extend() is not a good idea in general, as at
least one page of the VM/FSM will be read immediately after, always causing a
cache miss, requiring us to read content we just wrote.
Discussion: https://postgr.es/m/20230301223515.pucbj7nb54n4i4nv@awork3.anarazel.de
This is mainly a light renumbering to match the sections in the
standard.
The comments for SQL_IMPLEMENTATION_INFO and SQL_SIZING are no longer
applicable because the required information has been moved into part
9.
Since 8b9e9644d, the messages for failed permissions checks report
"table" where appropriate, rather than "relation".
Backpatch to all supported branches
The meson docs generation hardcoded using the website style so far. Make it
configurable via a meson option.
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Reported-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/3fc3bb9b-f7f8-d442-35c1-ec82280c564a@enterprisedb.com
Until now the meson built docs did not have a working reference to the css
stylesheet, it was copied in the make target. Instead of duplicating that for
meson, use the docbook-xsl parameter custom.css.source to reference it. An
additional benefit of that approach is that the stylesheet is now included in
the single-file HTML documentation.
Reported-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/3fc3bb9b-f7f8-d442-35c1-ec82280c564a@enterprisedb.com
Until now the meson built HTML docs had non-working references to images. They
were copied in the make target. Instead of duplicating that for meson, copy
them as part of the xslt stylesheet.
Reported-by: Peter Eisentraut <peter.eisentraut@enterprisedb.com>
Discussion: https://postgr.es/m/3fc3bb9b-f7f8-d442-35c1-ec82280c564a@enterprisedb.com
Detect and report if the tools necessary to build documentation are available
during configure. This is represented as two new options 'docs' and
'docs_pdf', both defaulting to 'auto'.
This should also fix a meson error about the installdocs target, when none of
the doc tools are found.
Reviewed-by: Justin Pryzby <pryzby@telsasoft.com>
Discussion: https://postgr.es/m/20230325201414.sh7c6xlut2fpunnv@awork3.anarazel.de
Discussion: https://postgr.es/m/ZB8331v5IhUA/pNu@telsasoft.com
We had partial support for generating documentation suitable for .chm
files. However, we only had wired up generating the input files using
docbook-xsl, not generating an actual .chm file. Nor did we document how to do
so. Additionally, it was very slow to generate htmlhelp, as we never applied
the docbook-xsl stylesheet performance improvements to htmlhelp.
It doesn't look like there's any interest in the htmlhelp output, so remove
it, instead of spending cycles to finish the support.
Discussion: https://postgr.es/m/20230324165822.wcrj3akllbqquy7u@awork3.anarazel.de
fill_seq_fork_with_data(), used to initialize a new sequence relation, only
locked the buffer after calling PageInit(), even though PageInit() modifies
page contents.
This is unlikely to cause real-world issues, as the relation is exclusively
locked at that point, and the buffer not yet marked dirty, so other processes
should not access the buffer.
This issue looks to have been present since the introduction of sequences in
e8647c45d6.
Given the low risk, it does not seem worth backpatching the fix.
Discussion: https://postgr.es/m/20230404185501.wdkmo3k7bedlx6qk@awork3.anarazel.de
The explanation describing the dependency to system read() calls for
these two functions has been removed in ddfc2d9. And after more
discussion about d69c404, we have concluded that adding more details
makes them easier to understand.
While on it, use the term "block read requests" (maybe found in cache)
rather than "buffers fetched" and "buffer hits".
Per discussion with Melanie Plageman, Kyotaro Horiguchi, Bertrand
Drouvot and myself.
Discussion: https://postgr.es/m/CAAKRu_ZmdiScT4q83OAbfmR5AH-L5zWya3SXjaxiJvhCob-e2A@mail.gmail.com
Backpatch-through: 11
Convert to BCP47 language tags before storing in the catalog, except
during binary upgrade or when the locale comes from an existing
collation or template database.
The resulting language tags can vary slightly between ICU
versions. For instance, "@colBackwards=yes" is converted to
"und-u-kb-true" in older versions of ICU, and to the simpler (but
equivalent) "und-u-kb" in newer versions.
The process of canonicalizing to a language tag also understands more
input locale string formats than ucol_open(). For instance,
"fr_CA.UTF-8" is misinterpreted by ucol_open() and the region is
ignored; effectively treating it the same as the locale "fr" and
opening the wrong collator. Canonicalization properly interprets the
language and region, resulting in the language tag "fr-CA", which can
then be understood by ucol_open().
This commit fixes a problem in prior versions due to ucol_open()
misinterpreting locale strings as described above. For instance,
creating an ICU collation with locale "fr_CA.UTF-8" would store that
string directly in the catalog, which would later be passed to (and
misinterpreted by) ucol_open(). After this commit, the locale string
will be canonicalized to language tag "fr-CA" in the catalog, which
will be properly understood by ucol_open(). Because this fix affects
the resulting collator, we cannot change the locale string stored in
the catalog for existing databases or collations; otherwise we'd risk
corrupting indexes. Therefore, only canonicalize locales for
newly-created (not upgraded) collations/databases. For similar
reasons, do not backport.
Discussion: https://postgr.es/m/8c7af6820aed94dc7bc259d2aa7f9663518e6137.camel@j-davis.com
Reviewed-by: Peter Eisentraut
Invent "GET DIAGNOSTICS oid_variable = PG_ROUTINE_OID".
This is useful for avoiding the maintenance nuisances that come
with embedding a function's name in its body, as one might do
for logging purposes for example. Typically users would cast the
result to regproc or regprocedure to get something human-readable,
but we won't pre-judge whether that's appropriate.
Pavel Stehule, reviewed by Kirk Wolak and myself
Discussion: https://postgr.es/m/CAFj8pRA4zMd5pY-B89Gm64bDLRt-L+akOd34aD1j4PEstHHSVQ@mail.gmail.com
This option is normally false, but can be set to true to obtain
the legacy behavior where the subscription runs with the permissions
of the subscription owner rather than the permissions of the
table owner. The advantages of this mode are (1) it doesn't require
that the subscription owner have permission to SET ROLE to each
table owner and (2) since no role switching occurs, the
SECURITY_RESTRICTED_OPERATION restrictions do not apply.
On the downside, it allows any table owner to easily usurp
the privileges of the subscription owner - basically, to take
over their account. Because that's generally quite undesirable,
we don't make this mode the default, but we do make it available,
just in case the new behavior causes too many problems for someone.
Discussion: http://postgr.es/m/CA+TgmoZ-WEeG6Z14AfH7KhmpX2eFh+tZ0z+vf0=eMDdbda269g@mail.gmail.com
Up until now, logical replication actions have been performed as the
subscription owner, who will generally be a superuser. Commit
cec57b1a0f documented hazards
associated with that situation, namely, that any user who owns a
table on the subscriber side could assume the privileges of the
subscription owner by attaching a trigger, expression index, or
some other kind of executable code to it. As a remedy, it suggested
not creating configurations where users who are not fully trusted
own tables on the subscriber.
Although that will work, it basically precludes using logical
replication in the way that people typically want to use it,
namely, to replicate a database from one node to another
without necessarily having any restrictions on which database
users can own tables. So, instead, change logical replication to
execute INSERT, UPDATE, DELETE, and TRUNCATE operations as the
table owner when they are replicated.
Since this involves switching the active user frequently within
a session that is authenticated as the subscription user, also
impose SECURITY_RESTRICTED_OPERATION restrictions on logical
replication code. As an exception, if the table owner can SET
ROLE to the subscription owner, these restrictions have no
security value, so don't impose them in that case.
Subscription owners are now required to have the ability to
SET ROLE to every role that owns a table that the subscription
is replicating. If they don't, replication will fail. Superusers,
who normally own subscriptions, satisfy this property by default.
Non-superusers users who own subscriptions will need to be
granted the roles that own relevant tables.
Patch by me, reviewed (but not necessarily in its entirety) by
Jelte Fennema, Jeff Davis, and Noah Misch.
Discussion: http://postgr.es/m/CA+TgmoaSCkg9ww9oppPqqs+9RVqCexYCE6Aq=UsYPfnOoDeFkw@mail.gmail.com
- At the last minute and for no particularly good reason, I changed the
WITHOUT token to be marked especially for lookahead, from the one in
WITHOUT TIME to the one in WITHOUT UNIQUE. Study of upcoming patches
(where a new WITHOUT ARRAY WRAPPER clause is added) showed me that the
former was better, so put it back the way the original patch had it.
- update exprTypmod() for JsonConstructorExpr to return the typmod of
the RETURNING clause, as a comment there suggested. Perhaps it's
possible for this to make a difference with datetime types, but I
didn't try to build a test case.
- The nodeFuncs.c support code for new nodes was calling walker()
directly instead of the WALK() macro as introduced by commit 1c27d16e6e.
Modernize that. Also add exprLocation() support for a couple of nodes
that missed it. Lastly, reorder the code more sensibly.
The WITHOUT_LA -> WITHOUT change means that stored rules containing
either WITHOUT TIME ZONE or WITHOUT UNIQUE KEYS would change
representation. Therefore, bump catversion.
Discussion: https://postgr.es/m/20230329181708.e64g2tpy7jyufqkr@alvherre.pgsql
The trace point was using the relfileno / fork / block for the to-be-read-in
buffer. Some upcoming work would make that more expensive to provide. We still
have buffer-flush-start/done, which can serve most tracing needs that
buffer-write-dirty could serve.
Discussion: https://postgr.es/m/f5164e7a-eef6-8972-75a3-8ac622ed0c6e@iki.fi
Commit 61b313e4 made VACUUM pass down a heaprel to index AM bulkdelete
and vacuumcleanup routines. Although this was primarily intended as
preparation for logical decoding on standbys, it also made it easy to
correct an old deficiency in how we determine how to cleanup SP-GiST
redirect and placeholder tuples.
Pass the heaprel to GlobalVisTestFor() during cleanup of redirect and
placeholder tuples, rather than pessimistically passing NULL.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/02392033-f030-a3c8-c7d0-5c27eb529fec@gmail.com
Commit 61b313e4 made nbtree consistently pass down a heaprel to low
level routines like _bt_getbuf(). Although this was primarily intended
as preparation for logical decoding on standbys, it also made it easy to
correct an old deficiency in how nbtree VACUUM determines whether or not
it's now safe to recycle deleted pages.
Pass the heaprel to GlobalVisTestFor() in nbtree routines that deal with
recycle safety. nbtree now makes less pessimistic assumptions about
recycle safety within non-catalog relations. This enhancement
complements the recycling enhancement added by commit 9dd963ae25.
nbtree remains just as pessimistic as ever when it comes to recycle
safety within indexes on catalog relations. There is no fundamental
reason why we need to treat catalog relations differently, though. The
behavioral inconsistency is a consequence of the way that nbtree uses
nextXID values to implement what Lanin and Shasha call "the drain
technique". Note in particular that it has nothing to do with whether
or not index tuples might still be required for an older MVCC snapshot.
Author: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Discussion: https://postgr.es/m/CAH2-WzkaiDxCje0yPuH=3Uh2p1V_2pFGY==xfbZoZu7Ax_NB8g@mail.gmail.com
In USE_VALGRIND builds, add code to print the text of the current query
to the valgrind log whenever the valgrind error count has increased
during the query. Valgrind will already have printed its report,
if the error is distinct from ones already seen, so that this works
out fairly well as a way of annotating the log.
Onur Tirtir and Tom Lane
Discussion: https://postgr.es/m/AM9PR83MB0498531E804DC8DF8CFF0E8FE9D09@AM9PR83MB0498.EURPRD83.prod.outlook.com
The new name better reflects what the field is - the size of the buffers[]
array. ring_size sounded more like it is in units of bytes.
An upcoming commit allows a BufferAccessStrategy of custom sizes, so it
seems relevant to improve this beforehand.
Author: Melanie Plageman
Reviewed-by: David Rowley
Discussion: https://postgr.es/m/CAAKRu_YefVbhg4rAxU2frYFdTap61UftH-kUNQZBvAs%2BYK81Zg%40mail.gmail.com
Traditionally, vacuum always makes use of a buffer access strategy 32
buffers in size. This means that running vacuums tend not to cause too
many shared buffers to become dirty, however, this can cause vacuums to
run much more slowly than they otherwise could as WAL flushes will occur
more frequently due to having to flush WAL out to the LSN of the dirty
page before that page can be written to disk.
When we are performing failsafe VACUUMs (as added in 1e55e7d17), we really
want to make the vacuum work go as quickly as possible, so here we disable
the buffer access strategy when entering failsafe mode while vacuuming a
relation.
Per idea and analyis from Andres Freund.
In passing, also include some changes I had intended for 32fbe0239.
Author: Melanie Plageman
Reviewed-by: Justin Pryzby, David Rowley
Discussion: https://postgr.es/m/20230111182720.ejifsclfwymw2reb%40awork3.anarazel.de
VACUUM FULL and VACUUM ONLY_DATABASE_STATS will not use the vacuum
strategy ring created in vacuum(), so don't waste effort making it in
those cases.
There are other conceivable cases where the buffer strategy also won't be
used, but those are probably less common and not worth troubling over too
much. For example VACUUM (PROCESS_MAIN false, PROCESS_TOAST false).
There are other cases too, but many of these are only discovered once
inside vacuum_rel().
Author: Melanie Plageman
Reviewed-by: David Rowley
Discussion: https://postgr.es/m/CAAKRu_ZLRuzkM3zKogiZAz2hUony37yLY4aaLb8fPf8fgqs5Og@mail.gmail.com
Using global variables because we don't want to pass these values around
as parameters does not really seem like a great idea, so let's remove
these two global variables and adjust a few functions to accept these
values as parameters instead.
This is part of a wider patch which intends to allow the size of the
buffer access strategy that vacuum uses to be adjusted.
Author: Melanie Plageman
Reviewed-by: Bharath Rupireddy
Discussion: https://postgr.es/m/CAAKRu_b1q_07uquUtAvLqTM%3DW9nzee7QbtzHwA4XdUo7KX_Cnw%40mail.gmail.com
Andres Freund