mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Move I/O before the index_update_stats() buffer lock region.

Browse Source 
Commit a07e03fd8fa7daf4d1356f7cb501ffe784ea6257 enlarged the work done
here under the pg_class heap buffer lock.  Two preexisting actions are
best done before holding that lock.  Both RelationGetNumberOfBlocks()
and visibilitymap_count() do I/O, and the latter might exclusive-lock a
visibility map buffer.  Moving these reduces contention and risk of
undetected LWLock deadlock.  Back-patch to v12, like that commit.

Discussion: https://postgr.es/m/20241031200139.b4@rfd.leadboat.com
This commit is contained in:
Noah Misch 2024-11-02 09:04:55 -07:00
parent 4c70887295
commit bb30542976
1 changed files with 36 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

54
src/backend/catalog/index.c
View File

@ -2792,6 +2792,9 @@ index_update_stats(Relation rel,
bool hasindex, bool hasindex,
double reltuples) double reltuples)
{ {
bool update_stats;
BlockNumber relpages;
BlockNumber relallvisible;
Oid relid = RelationGetRelid(rel); Oid relid = RelationGetRelid(rel);
Relation pg_class; Relation pg_class;
ScanKeyData key[1]; ScanKeyData key[1];
@ -2800,6 +2803,38 @@ index_update_stats(Relation rel,
Form_pg_class rd_rel; Form_pg_class rd_rel;
bool dirty; bool dirty;
/*
* As a special hack, if we are dealing with an empty table and the
* existing reltuples is -1, we leave that alone. This ensures that
* creating an index as part of CREATE TABLE doesn't cause the table to
* prematurely look like it's been vacuumed. The rd_rel we modify may
* differ from rel->rd_rel due to e.g. commit of concurrent GRANT, but the
* commands that change reltuples take locks conflicting with ours. (Even
* if a command changed reltuples under a weaker lock, this affects only
* statistics for an empty table.)
*/
if (reltuples == 0 && rel->rd_rel->reltuples < 0)
reltuples = -1;
update_stats = reltuples >= 0;
/*
* Finish I/O and visibility map buffer locks before
* systable_inplace_update_begin() locks the pg_class buffer. The rd_rel
* we modify may differ from rel->rd_rel due to e.g. commit of concurrent
* GRANT, but no command changes a relkind from non-index to index. (Even
* if one did, relallvisible doesn't break functionality.)
*/
if (update_stats)
{
relpages = RelationGetNumberOfBlocks(rel);
if (rel->rd_rel->relkind != RELKIND_INDEX)
visibilitymap_count(rel, &relallvisible, NULL);
else /* don't bother for indexes */
relallvisible = 0;
}
/* /*
* We always update the pg_class row using a non-transactional, * We always update the pg_class row using a non-transactional,
* overwrite-in-place update. There are several reasons for this: * overwrite-in-place update. There are several reasons for this:
@ -2844,15 +2879,6 @@ index_update_stats(Relation rel,
/* Should this be a more comprehensive test? */ /* Should this be a more comprehensive test? */
Assert(rd_rel->relkind != RELKIND_PARTITIONED_INDEX); Assert(rd_rel->relkind != RELKIND_PARTITIONED_INDEX);
/*
* As a special hack, if we are dealing with an empty table and the
* existing reltuples is -1, we leave that alone. This ensures that
* creating an index as part of CREATE TABLE doesn't cause the table to
* prematurely look like it's been vacuumed.
*/
if (reltuples == 0 && rd_rel->reltuples < 0)
reltuples = -1;
/* Apply required updates, if any, to copied tuple */ /* Apply required updates, if any, to copied tuple */
dirty = false; dirty = false;
@ -2862,16 +2888,8 @@ index_update_stats(Relation rel,
dirty = true; dirty = true;
} }
if (reltuples >= 0) if (update_stats)
{ {
BlockNumber relpages = RelationGetNumberOfBlocks(rel);
BlockNumber relallvisible;
if (rd_rel->relkind != RELKIND_INDEX)
visibilitymap_count(rel, &relallvisible, NULL);
else /* don't bother for indexes */
relallvisible = 0;
if (rd_rel->relpages != (int32) relpages) if (rd_rel->relpages != (int32) relpages)
{ {
rd_rel->relpages = (int32) relpages; rd_rel->relpages = (int32) relpages;