pg.ddx.io pgsql-committers@postgresql.org mailing list archive
help / color / mirror / Atom feedpgsql: Move I/O before the index_update_stats() buffer lock region.
11+ messages / 2 participants
[nested] [flat]
* pgsql: Move I/O before the index_update_stats() buffer lock region.
@ 2024-11-02 16:08 Noah Misch <noah@leadboat.com>
0 siblings, 0 replies; 11+ messages in thread
From: Noah Misch @ 2024-11-02 16:08 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Move I/O before the index_update_stats() buffer lock region.
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
Branch
------
REL_15_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/6d5b4031b927328e548088ebcc24781223b3b803
Modified Files
--------------
src/backend/catalog/index.c | 54 ++++++++++++++++++++++++++++++---------------
1 file changed, 36 insertions(+), 18 deletions(-)
^ permalink raw reply [nested|flat] 11+ messages in thread
* pgsql: Move I/O before the index_update_stats() buffer lock region.
@ 2024-11-02 16:08 Noah Misch <noah@leadboat.com>
0 siblings, 0 replies; 11+ messages in thread
From: Noah Misch @ 2024-11-02 16:08 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Move I/O before the index_update_stats() buffer lock region.
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
Branch
------
REL_12_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/d729f1ea5a46963f3d4b0dd1268f3583b1cdbe9e
Modified Files
--------------
src/backend/catalog/index.c | 45 ++++++++++++++++++++++++++++++++++++---------
1 file changed, 36 insertions(+), 9 deletions(-)
^ permalink raw reply [nested|flat] 11+ messages in thread
* pgsql: Move I/O before the index_update_stats() buffer lock region.
@ 2024-11-02 16:08 Noah Misch <noah@leadboat.com>
0 siblings, 0 replies; 11+ messages in thread
From: Noah Misch @ 2024-11-02 16:08 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Move I/O before the index_update_stats() buffer lock region.
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
Branch
------
REL_17_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/0bcb9d07940c790912b0aff781ef605ec22e1a2d
Modified Files
--------------
src/backend/catalog/index.c | 62 +++++++++++++++++++++++++++++----------------
1 file changed, 40 insertions(+), 22 deletions(-)
^ permalink raw reply [nested|flat] 11+ messages in thread
* pgsql: Move I/O before the index_update_stats() buffer lock region.
@ 2024-11-02 16:08 Noah Misch <noah@leadboat.com>
0 siblings, 0 replies; 11+ messages in thread
From: Noah Misch @ 2024-11-02 16:08 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Move I/O before the index_update_stats() buffer lock region.
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
Branch
------
REL_14_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/bb30542976618e6d0cd9eb2358824b741566ffbf
Modified Files
--------------
src/backend/catalog/index.c | 54 ++++++++++++++++++++++++++++++---------------
1 file changed, 36 insertions(+), 18 deletions(-)
^ permalink raw reply [nested|flat] 11+ messages in thread
* pgsql: Move I/O before the index_update_stats() buffer lock region.
@ 2024-11-02 16:08 Noah Misch <noah@leadboat.com>
0 siblings, 0 replies; 11+ messages in thread
From: Noah Misch @ 2024-11-02 16:08 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Move I/O before the index_update_stats() buffer lock region.
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
Branch
------
REL_16_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/6c837c237bf821ee78522259c847f417d1e11fd7
Modified Files
--------------
src/backend/catalog/index.c | 54 ++++++++++++++++++++++++++++++---------------
1 file changed, 36 insertions(+), 18 deletions(-)
^ permalink raw reply [nested|flat] 11+ messages in thread
* pgsql: Move I/O before the index_update_stats() buffer lock region.
@ 2024-11-02 16:08 Noah Misch <noah@leadboat.com>
0 siblings, 1 reply; 11+ messages in thread
From: Noah Misch @ 2024-11-02 16:08 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Move I/O before the index_update_stats() buffer lock region.
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
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/b412f402d1e020c5dac94f3bf4a005db69519b99
Modified Files
--------------
src/backend/catalog/index.c | 62 +++++++++++++++++++++++++++++----------------
1 file changed, 40 insertions(+), 22 deletions(-)
^ permalink raw reply [nested|flat] 11+ messages in thread
* pgsql: Move I/O before the index_update_stats() buffer lock region.
@ 2024-11-02 16:08 Noah Misch <noah@leadboat.com>
0 siblings, 0 replies; 11+ messages in thread
From: Noah Misch @ 2024-11-02 16:08 UTC (permalink / raw)
To: pgsql-committers@lists.postgresql.org
Move I/O before the index_update_stats() buffer lock region.
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
Branch
------
REL_13_STABLE
Details
-------
https://git.postgresql.org/pg/commitdiff/6b01cac0be6dd044fa3c2d08df7e893b96dfafe6
Modified Files
--------------
src/backend/catalog/index.c | 45 ++++++++++++++++++++++++++++++++++++---------
1 file changed, 36 insertions(+), 9 deletions(-)
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: pgsql: Move I/O before the index_update_stats() buffer lock region.
@ 2024-11-03 02:32 Tom Lane <tgl@sss.pgh.pa.us>
parent: Noah Misch <noah@leadboat.com>
0 siblings, 1 reply; 11+ messages in thread
From: Tom Lane @ 2024-11-03 02:32 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: pgsql-committers@lists.postgresql.org
Noah Misch <noah@leadboat.com> writes:
> Move I/O before the index_update_stats() buffer lock region.
In branches before 17, this commit is causing some compilers
to complain:
index.c: In function 'index_update_stats':
index.c:2913:24: warning: 'relpages' may be used uninitialized in this function [-Wmaybe-uninitialized]
if (rd_rel->relpages != (int32) relpages)
~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
I'm seeing that with RHEL8's gcc 8.5.0, and trawling the buildfarm
shows at least 40 animals similarly unhappy. (I noticed this because
mamba, which is using -Werror, is actually failing.)
regards, tom lane
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: pgsql: Move I/O before the index_update_stats() buffer lock region.
@ 2024-11-03 03:10 Noah Misch <noah@leadboat.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 1 reply; 11+ messages in thread
From: Noah Misch @ 2024-11-03 03:10 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org
On Sat, Nov 02, 2024 at 10:32:56PM -0400, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > Move I/O before the index_update_stats() buffer lock region.
>
> In branches before 17, this commit is causing some compilers
> to complain:
>
> index.c: In function 'index_update_stats':
> index.c:2913:24: warning: 'relpages' may be used uninitialized in this function [-Wmaybe-uninitialized]
> if (rd_rel->relpages != (int32) relpages)
> ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
>
> I'm seeing that with RHEL8's gcc 8.5.0, and trawling the buildfarm
> shows at least 40 animals similarly unhappy. (I noticed this because
> mamba, which is using -Werror, is actually failing.)
Hopefully now the code won't fool any compiler.
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: pgsql: Move I/O before the index_update_stats() buffer lock region.
@ 2024-11-03 03:13 Tom Lane <tgl@sss.pgh.pa.us>
parent: Noah Misch <noah@leadboat.com>
0 siblings, 1 reply; 11+ messages in thread
From: Tom Lane @ 2024-11-03 03:13 UTC (permalink / raw)
To: Noah Misch <noah@leadboat.com>; +Cc: pgsql-committers@lists.postgresql.org
Noah Misch <noah@leadboat.com> writes:
> On Sat, Nov 02, 2024 at 10:32:56PM -0400, Tom Lane wrote:
>> In branches before 17, this commit is causing some compilers
>> to complain:
> Hopefully now the code won't fool any compiler.
I saw your commit, and I'm content with it, but did you figure
out why the warning only appears pre-17? (At least, that's
what I got locally.)
regards, tom lane
^ permalink raw reply [nested|flat] 11+ messages in thread
* Re: pgsql: Move I/O before the index_update_stats() buffer lock region.
@ 2024-11-03 03:21 Noah Misch <noah@leadboat.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 11+ messages in thread
From: Noah Misch @ 2024-11-03 03:21 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: pgsql-committers@lists.postgresql.org
On Sat, Nov 02, 2024 at 11:13:57PM -0400, Tom Lane wrote:
> Noah Misch <noah@leadboat.com> writes:
> > On Sat, Nov 02, 2024 at 10:32:56PM -0400, Tom Lane wrote:
> >> In branches before 17, this commit is causing some compilers
> >> to complain:
>
> > Hopefully now the code won't fool any compiler.
>
> I saw your commit, and I'm content with it, but did you figure
> out why the warning only appears pre-17? (At least, that's
> what I got locally.)
Nope. The principles behind the variable's lifecycle didn't change. Here's
the function's diff between v16 and v17. (Same diff all the way from v12 to
v17.) Perhaps the reference to non-local variable IsBinaryUpgrade changed the
compiler's deductive capability, somehow.
@@ -2836,7 +2806,11 @@ index_update_stats(Relation rel,
if (reltuples == 0 && rel->rd_rel->reltuples < 0)
reltuples = -1;
- update_stats = reltuples >= 0;
+ /*
+ * Don't update statistics during binary upgrade, because the indexes are
+ * created before the data is moved into place.
+ */
+ update_stats = reltuples >= 0 && !IsBinaryUpgrade;
/*
* Finish I/O and visibility map buffer locks before
^ permalink raw reply [nested|flat] 11+ messages in thread
end of thread, other threads:[~2024-11-03 03:21 UTC | newest]
Thread overview: 11+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-11-02 16:08 pgsql: Move I/O before the index_update_stats() buffer lock region. Noah Misch <noah@leadboat.com>
2024-11-03 02:32 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-03 03:10 ` Noah Misch <noah@leadboat.com>
2024-11-03 03:13 ` Tom Lane <tgl@sss.pgh.pa.us>
2024-11-03 03:21 ` Noah Misch <noah@leadboat.com>
2024-11-02 16:08 pgsql: Move I/O before the index_update_stats() buffer lock region. Noah Misch <noah@leadboat.com>
2024-11-02 16:08 pgsql: Move I/O before the index_update_stats() buffer lock region. Noah Misch <noah@leadboat.com>
2024-11-02 16:08 pgsql: Move I/O before the index_update_stats() buffer lock region. Noah Misch <noah@leadboat.com>
2024-11-02 16:08 pgsql: Move I/O before the index_update_stats() buffer lock region. Noah Misch <noah@leadboat.com>
2024-11-02 16:08 pgsql: Move I/O before the index_update_stats() buffer lock region. Noah Misch <noah@leadboat.com>
2024-11-02 16:08 pgsql: Move I/O before the index_update_stats() buffer lock region. Noah Misch <noah@leadboat.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox