pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM
6+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2024-09-27 09620ea091 1532599a85 afbd3dc7d2 a613edc5c2 6530b869c3 911eda9f3c 2f33e68a5c Fix incorrect memory access in VACUUM FULL with invalid toast indexes
* BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM
@ 2024-09-25 03:00 PG Bug reporting form <noreply@postgresql.org>
2024-09-25 09:28 ` Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM Tender Wang <tndrwang@gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: PG Bug reporting form @ 2024-09-25 03:00 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: exclusion@gmail.com
The following bug has been logged on the website:
Bug reference: 18630
Logged by: Alexander Lakhin
Email address: exclusion@gmail.com
PostgreSQL version: 17rc1
Operating system: Ubuntu 22.04
Description:
The following script:
psql -c "SELECT pg_sleep(5)" &
echo "
SET lock_timeout = '3s';
CREATE TABLE t(i int, t text);
REINDEX TABLE CONCURRENTLY t;
SELECT pg_sleep(3);
" | psql
psql -c "VACUUM (PROCESS_MAIN FALSE, FULL TRUE) t;"
produces:
WARNING: cannot reindex invalid index "pg_toast.pg_toast_16384_index_ccnew"
on TOAST table, skipping
and then a Valgrind-detected error:
==00:00:00:10.727 3193327== Invalid read of size 4
==00:00:00:10.727 3193327== at 0x5A6D80: list_member_oid (list.c:726)
==00:00:00:10.727 3193327== by 0x33FE2F: ReindexIsProcessingIndex
(index.c:4083)
==00:00:00:10.727 3193327== by 0x27B43F: systable_beginscan
(genam.c:396)
==00:00:00:10.727 3193327== by 0x4CE8F9: vac_update_datfrozenxid
(vacuum.c:1723)
==00:00:00:10.727 3193327== by 0x4CCFAB: vacuum (vacuum.c:691)
==00:00:00:10.727 3193327== by 0x4CC910: ExecVacuum (vacuum.c:449)
==00:00:00:10.727 3193327== by 0x7CE082: standard_ProcessUtility
(utility.c:859)
==00:00:00:10.727 3193327== by 0x7CD61D: ProcessUtility (utility.c:523)
==00:00:00:10.727 3193327== by 0x7CBE98: PortalRunUtility
(pquery.c:1158)
==00:00:00:10.727 3193327== by 0x7CC10F: PortalRunMulti (pquery.c:1316)
==00:00:00:10.727 3193327== by 0x7CB559: PortalRun (pquery.c:791)
==00:00:00:10.727 3193327== by 0x7C3C7A: exec_simple_query
(postgres.c:1284)
==00:00:00:10.727 3193327== Address 0x72f4878 is 7,496 bytes inside a
recently re-allocated block of size 8,192 alloc'd
==00:00:00:10.727 3193327== at 0x4848899: malloc
(vg_replace_malloc.c:381)
==00:00:00:10.727 3193327== by 0x9FDA95: AllocSetContextCreateInternal
(aset.c:444)
==00:00:00:10.727 3193327== by 0x2E0BBC: AtStart_Memory (xact.c:1206)
==00:00:00:10.727 3193327== by 0x2E1C56: StartTransaction (xact.c:2143)
==00:00:00:10.727 3193327== by 0x2E2CA8: StartTransactionCommand
(xact.c:3050)
==00:00:00:10.727 3193327== by 0x9DF444: InitPostgres (postinit.c:830)
==00:00:00:10.727 3193327== by 0x7C8B3A: PostgresMain (postgres.c:4349)
==00:00:00:10.727 3193327== by 0x7BF5AE: BackendMain
(backend_startup.c:107)
==00:00:00:10.727 3193327== by 0x6D1E75: postmaster_child_launch
(launch_backend.c:274)
==00:00:00:10.727 3193327== by 0x6D7CE8: BackendStartup
(postmaster.c:3420)
==00:00:00:10.727 3193327== by 0x6D539A: ServerLoop (postmaster.c:1653)
==00:00:00:10.727 3193327== by 0x6D4C92: PostmasterMain
(postmaster.c:1351)
==00:00:00:10.727 3193327==
...
2024-09-25 02:44:16.496 UTC|||66f378f6.30b9b3|LOG: server process (PID
3193327) exited with exit code 1
2024-09-25 02:44:16.496 UTC|||66f378f6.30b9b3|DETAIL: Failed process was
running: VACUUM (PROCESS_MAIN FALSE, FULL TRUE) t;
or an assertion failure (when executed without Valgrind):
TRAP: failed Assert("IsOidList(list)"), File: "list.c", Line: 726, PID:
3213057
Reproduced on REL_16_STABLE (starting from 4211fbd84) .. master.
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM
2024-09-25 03:00 BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM PG Bug reporting form <noreply@postgresql.org>
@ 2024-09-25 09:28 ` Tender Wang <tndrwang@gmail.com>
2024-09-25 10:19 ` Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM Tender Wang <tndrwang@gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: Tender Wang @ 2024-09-25 09:28 UTC (permalink / raw)
To: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org
PG Bug reporting form <noreply@postgresql.org> 于2024年9月25日周三 13:35写道：
> The following bug has been logged on the website:
>
> Bug reference: 18630
> Logged by: Alexander Lakhin
> Email address: exclusion@gmail.com
> PostgreSQL version: 17rc1
> Operating system: Ubuntu 22.04
> Description:
>
> The following script:
> psql -c "SELECT pg_sleep(5)" &
>
> echo "
> SET lock_timeout = '3s';
> CREATE TABLE t(i int, t text);
> REINDEX TABLE CONCURRENTLY t;
> SELECT pg_sleep(3);
> " | psql
>
> psql -c "VACUUM (PROCESS_MAIN FALSE, FULL TRUE) t;"
>
> produces:
> WARNING: cannot reindex invalid index
> "pg_toast.pg_toast_16384_index_ccnew"
> on TOAST table, skipping
>
> and then a Valgrind-detected error:
> ==00:00:00:10.727 3193327== Invalid read of size 4
> ==00:00:00:10.727 3193327== at 0x5A6D80: list_member_oid (list.c:726)
> ==00:00:00:10.727 3193327== by 0x33FE2F: ReindexIsProcessingIndex
> (index.c:4083)
> ==00:00:00:10.727 3193327== by 0x27B43F: systable_beginscan
> (genam.c:396)
> ==00:00:00:10.727 3193327== by 0x4CE8F9: vac_update_datfrozenxid
> (vacuum.c:1723)
> ==00:00:00:10.727 3193327== by 0x4CCFAB: vacuum (vacuum.c:691)
> ==00:00:00:10.727 3193327== by 0x4CC910: ExecVacuum (vacuum.c:449)
> ==00:00:00:10.727 3193327== by 0x7CE082: standard_ProcessUtility
> (utility.c:859)
> ==00:00:00:10.727 3193327== by 0x7CD61D: ProcessUtility (utility.c:523)
> ==00:00:00:10.727 3193327== by 0x7CBE98: PortalRunUtility
> (pquery.c:1158)
> ==00:00:00:10.727 3193327== by 0x7CC10F: PortalRunMulti (pquery.c:1316)
> ==00:00:00:10.727 3193327== by 0x7CB559: PortalRun (pquery.c:791)
> ==00:00:00:10.727 3193327== by 0x7C3C7A: exec_simple_query
> (postgres.c:1284)
> ==00:00:00:10.727 3193327== Address 0x72f4878 is 7,496 bytes inside a
> recently re-allocated block of size 8,192 alloc'd
> ==00:00:00:10.727 3193327== at 0x4848899: malloc
> (vg_replace_malloc.c:381)
> ==00:00:00:10.727 3193327== by 0x9FDA95: AllocSetContextCreateInternal
> (aset.c:444)
> ==00:00:00:10.727 3193327== by 0x2E0BBC: AtStart_Memory (xact.c:1206)
> ==00:00:00:10.727 3193327== by 0x2E1C56: StartTransaction (xact.c:2143)
> ==00:00:00:10.727 3193327== by 0x2E2CA8: StartTransactionCommand
> (xact.c:3050)
> ==00:00:00:10.727 3193327== by 0x9DF444: InitPostgres (postinit.c:830)
> ==00:00:00:10.727 3193327== by 0x7C8B3A: PostgresMain (postgres.c:4349)
> ==00:00:00:10.727 3193327== by 0x7BF5AE: BackendMain
> (backend_startup.c:107)
> ==00:00:00:10.727 3193327== by 0x6D1E75: postmaster_child_launch
> (launch_backend.c:274)
> ==00:00:00:10.727 3193327== by 0x6D7CE8: BackendStartup
> (postmaster.c:3420)
> ==00:00:00:10.727 3193327== by 0x6D539A: ServerLoop (postmaster.c:1653)
> ==00:00:00:10.727 3193327== by 0x6D4C92: PostmasterMain
> (postmaster.c:1351)
> ==00:00:00:10.727 3193327==
> ...
> 2024-09-25 02:44:16.496 UTC|||66f378f6.30b9b3|LOG: server process (PID
> 3193327) exited with exit code 1
> 2024-09-25 02:44:16.496 UTC|||66f378f6.30b9b3|DETAIL: Failed process was
> running: VACUUM (PROCESS_MAIN FALSE, FULL TRUE) t;
>
> or an assertion failure (when executed without Valgrind):
> TRAP: failed Assert("IsOidList(list)"), File: "list.c", Line: 726, PID:
> 3213057
>
> Reproduced on REL_16_STABLE (starting from 4211fbd84) .. master.
>
>
Thanks for reporting. I can reproduce this issue.
When this statement "REINDEX TABLE CONCURRENTLY t;" failed because of lock
timeout.
If we do vacuum like this case, when we do toast_relid of table t, we will
get two index oids.
pg_toast.pg_toast_16384_index_ccnew is invalid because the REINDEX failed.
Now we only report warings in reindex_relation(). The
pg_toast.pg_toast_16384_index_ccnew is
still on the pendingReindexedIndexes list. After finishing the toast_rel of
table t, the transatiocn committed,
and the memory of pendingReindexedIndexes was reset but not NIL. So it
will trigger assert failure when calling ReindexIsProcessingIndex().
I think we can remove the invalid index oid from the
pendingReindexedIndexes instead of reporting warning.
I try this way, and no assert failure again. See the attached patch.
--
Thanks,
Tender Wang
https://www.openpie.com/
Attachments:
0001-Remove-failed-REINDEX-index-oid-from-the-pending-lis.patch (application/octet-stream, 1.0K) download
From c09e0e08f372595ea8016ca09c11a2ee19952c0d Mon Sep 17 00:00:00 2001
From: Tender Wang <tndrwang@gmail.com>
Date: Wed, 25 Sep 2024 17:19:33 +0800
Subject: [PATCH] Remove failed REINDEX index oid from the pending list.
---
src/backend/catalog/index.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index b2b3ecb524..f149de9225 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -4002,11 +4002,8 @@ reindex_relation(const ReindexStmt *stmt, Oid relid, int flags,
if (IsToastNamespace(indexNamespaceId) &&
!get_index_isvalid(indexOid))
{
- ereport(WARNING,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot reindex invalid index \"%s.%s\" on TOAST table, skipping",
- get_namespace_name(indexNamespaceId),
- get_rel_name(indexOid))));
+ /* Remove the failed REINDEX index from the pending list */
+ RemoveReindexPending(indexOid);
continue;
}
--
2.25.1
* Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM
2024-09-25 03:00 BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM PG Bug reporting form <noreply@postgresql.org>
2024-09-25 09:28 ` Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM Tender Wang <tndrwang@gmail.com>
@ 2024-09-25 10:19 ` Tender Wang <tndrwang@gmail.com>
2024-09-26 07:01 ` Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 6+ messages in thread
From: Tender Wang @ 2024-09-25 10:19 UTC (permalink / raw)
To: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org
Tender Wang <tndrwang@gmail.com> 于2024年9月25日周三 17:28写道：
>
>
> PG Bug reporting form <noreply@postgresql.org> 于2024年9月25日周三 13:35写道：
>
>> The following bug has been logged on the website:
>>
>> Bug reference: 18630
>> Logged by: Alexander Lakhin
>> Email address: exclusion@gmail.com
>> PostgreSQL version: 17rc1
>> Operating system: Ubuntu 22.04
>> Description:
>>
>> The following script:
>> psql -c "SELECT pg_sleep(5)" &
>>
>> echo "
>> SET lock_timeout = '3s';
>> CREATE TABLE t(i int, t text);
>> REINDEX TABLE CONCURRENTLY t;
>> SELECT pg_sleep(3);
>> " | psql
>>
>> psql -c "VACUUM (PROCESS_MAIN FALSE, FULL TRUE) t;"
>>
>> produces:
>> WARNING: cannot reindex invalid index
>> "pg_toast.pg_toast_16384_index_ccnew"
>> on TOAST table, skipping
>>
>> and then a Valgrind-detected error:
>> ==00:00:00:10.727 3193327== Invalid read of size 4
>> ==00:00:00:10.727 3193327== at 0x5A6D80: list_member_oid (list.c:726)
>> ==00:00:00:10.727 3193327== by 0x33FE2F: ReindexIsProcessingIndex
>> (index.c:4083)
>> ==00:00:00:10.727 3193327== by 0x27B43F: systable_beginscan
>> (genam.c:396)
>> ==00:00:00:10.727 3193327== by 0x4CE8F9: vac_update_datfrozenxid
>> (vacuum.c:1723)
>> ==00:00:00:10.727 3193327== by 0x4CCFAB: vacuum (vacuum.c:691)
>> ==00:00:00:10.727 3193327== by 0x4CC910: ExecVacuum (vacuum.c:449)
>> ==00:00:00:10.727 3193327== by 0x7CE082: standard_ProcessUtility
>> (utility.c:859)
>> ==00:00:00:10.727 3193327== by 0x7CD61D: ProcessUtility (utility.c:523)
>> ==00:00:00:10.727 3193327== by 0x7CBE98: PortalRunUtility
>> (pquery.c:1158)
>> ==00:00:00:10.727 3193327== by 0x7CC10F: PortalRunMulti (pquery.c:1316)
>> ==00:00:00:10.727 3193327== by 0x7CB559: PortalRun (pquery.c:791)
>> ==00:00:00:10.727 3193327== by 0x7C3C7A: exec_simple_query
>> (postgres.c:1284)
>> ==00:00:00:10.727 3193327== Address 0x72f4878 is 7,496 bytes inside a
>> recently re-allocated block of size 8,192 alloc'd
>> ==00:00:00:10.727 3193327== at 0x4848899: malloc
>> (vg_replace_malloc.c:381)
>> ==00:00:00:10.727 3193327== by 0x9FDA95: AllocSetContextCreateInternal
>> (aset.c:444)
>> ==00:00:00:10.727 3193327== by 0x2E0BBC: AtStart_Memory (xact.c:1206)
>> ==00:00:00:10.727 3193327== by 0x2E1C56: StartTransaction (xact.c:2143)
>> ==00:00:00:10.727 3193327== by 0x2E2CA8: StartTransactionCommand
>> (xact.c:3050)
>> ==00:00:00:10.727 3193327== by 0x9DF444: InitPostgres (postinit.c:830)
>> ==00:00:00:10.727 3193327== by 0x7C8B3A: PostgresMain (postgres.c:4349)
>> ==00:00:00:10.727 3193327== by 0x7BF5AE: BackendMain
>> (backend_startup.c:107)
>> ==00:00:00:10.727 3193327== by 0x6D1E75: postmaster_child_launch
>> (launch_backend.c:274)
>> ==00:00:00:10.727 3193327== by 0x6D7CE8: BackendStartup
>> (postmaster.c:3420)
>> ==00:00:00:10.727 3193327== by 0x6D539A: ServerLoop (postmaster.c:1653)
>> ==00:00:00:10.727 3193327== by 0x6D4C92: PostmasterMain
>> (postmaster.c:1351)
>> ==00:00:00:10.727 3193327==
>> ...
>> 2024-09-25 02:44:16.496 UTC|||66f378f6.30b9b3|LOG: server process (PID
>> 3193327) exited with exit code 1
>> 2024-09-25 02:44:16.496 UTC|||66f378f6.30b9b3|DETAIL: Failed process was
>> running: VACUUM (PROCESS_MAIN FALSE, FULL TRUE) t;
>>
>> or an assertion failure (when executed without Valgrind):
>> TRAP: failed Assert("IsOidList(list)"), File: "list.c", Line: 726, PID:
>> 3213057
>>
>> Reproduced on REL_16_STABLE (starting from 4211fbd84) .. master.
>>
>>
> Thanks for reporting. I can reproduce this issue.
>
> When this statement "REINDEX TABLE CONCURRENTLY t;" failed because of
> lock timeout.
> If we do vacuum like this case, when we do toast_relid of table t, we will
> get two index oids.
> pg_toast.pg_toast_16384_index_ccnew is invalid because the REINDEX failed.
>
> Now we only report warings in reindex_relation(). The
> pg_toast.pg_toast_16384_index_ccnew is
> still on the pendingReindexedIndexes list. After finishing the toast_rel
> of table t, the transatiocn committed,
> and the memory of pendingReindexedIndexes was reset but not NIL. So it
> will trigger assert failure when calling ReindexIsProcessingIndex().
>
> I think we can remove the invalid index oid from the
> pendingReindexedIndexes instead of reporting warning.
> I try this way, and no assert failure again. See the attached patch.
>
>
Adding if (flags & REINDEX_REL_SUPPRESS_INDEX_USE) check before calling
RemoveReindexPending() may look better.
Any thoughts?
--
Thanks,
Tender Wang
https://www.openpie.com/
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM
2024-09-25 03:00 BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM PG Bug reporting form <noreply@postgresql.org>
2024-09-25 09:28 ` Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM Tender Wang <tndrwang@gmail.com>
2024-09-25 10:19 ` Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM Tender Wang <tndrwang@gmail.com>
@ 2024-09-26 07:01 ` Michael Paquier <michael@paquier.xyz>
2024-09-26 07:24 ` Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM Tender Wang <tndrwang@gmail.com>
0 siblings, 1 reply; 6+ messages in thread
From: Michael Paquier @ 2024-09-26 07:01 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org
On Wed, Sep 25, 2024 at 06:19:20PM +0800, Tender Wang wrote:
> Adding if (flags & REINDEX_REL_SUPPRESS_INDEX_USE) check before calling
> RemoveReindexPending() may look better.
> Any thoughts?
The short answer to that is yes, see below.
Alexander's trick presented upthread is funky. I was first confused
of what was the point until I noticed that this is just a way to make
REINDEX CONCURRENTLY fail and produce an invalid toast index. The
first pg_sleep causes REINDEX to fail because we are waiting for the
first transaction to complete. So this provides a controlled way to
get an index that should never be part of the list reported to
SetReindexPending() to begin with. The VACUUM (PROCESS_MAIN FALSE,
FULL) is then disturbed.
Anyway, PROCESS_MAIN is not related to the failure, as it is just a
flavor grammar introduced for the sake of making toast-only rebuilds
easier for administrators. The same failure reproduces if switching
the test case of upthread to do the REINDEX on the toast table, down
to 12 which is as far as I have tested because that's what we support
around here.
Regarding the fix, we should not disturb the list of indexes in a
relation returned by relcache.c, and invalid indexes are part of it.
So I'd agree with your point to just remove the index from the pending
list because we have to skip invalid toast indexes in the
reindex_relation() path as reindex_index() has to generate a hard
failure because we can never ever have two valid toast indexes, and
that's a guarantee we need to be very careful about.
Now, about REINDEX_REL_SUPPRESS_INDEX_USE. Well, that's only used for
the VACUUM FULL/CLUSTER path which is where the indexes are marked as
being processed, so we have to reomve the invalid toast indexes from
the list only if this flag is used. Hence the answer is yes. I'd
rather keep the informative warning. That's useful for monitoring
purposes.
Something worth noting while I have looked at this code.. Non-toast
invalid indexes are rebuilt but still marked as !indisvalid, actually.
That's a waste and we could just skip them for a CLUSTER/VACUUM FULL.
This cannot be true for a REINDEX TABLE, invalid indexes are switched
to valid in this case.
--
Michael
Attachments:
0001-Remove-invalid-toast-indexes-from-reindex-processing.patch (text/x-diff, 1.1K) download
From 4dc38bc64d96764c4673a73f646c26252bb7fe33 Mon Sep 17 00:00:00 2001
From: Michael Paquier <michael@paquier.xyz>
Date: Thu, 26 Sep 2024 15:59:59 +0900
Subject: [PATCH] Remove invalid toast indexes from reindex processing list in
VACUUM FULL
Blah, blah...
Backpatch-through: 12
---
src/backend/catalog/index.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index d0dcbf318d..3a4f2e260d 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3989,6 +3989,14 @@ reindex_relation(const ReindexStmt *stmt, Oid relid, int flags,
errmsg("cannot reindex invalid index \"%s.%s\" on TOAST table, skipping",
get_namespace_name(indexNamespaceId),
get_rel_name(indexOid))));
+
+ /*
+ * Remove this invalid toast index from the reindex pending list,
+ * as it is skipped here due to the hard failure that would happen
+ * in reindex_index() should we try to process it.
+ */
+ if (flags & REINDEX_REL_SUPPRESS_INDEX_USE)
+ RemoveReindexPending(indexOid);
continue;
}
--
2.45.2
* Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM
2024-09-25 03:00 BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM PG Bug reporting form <noreply@postgresql.org>
2024-09-25 09:28 ` Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM Tender Wang <tndrwang@gmail.com>
2024-09-25 10:19 ` Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM Tender Wang <tndrwang@gmail.com>
2024-09-26 07:01 ` Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM Michael Paquier <michael@paquier.xyz>
@ 2024-09-26 07:24 ` Tender Wang <tndrwang@gmail.com>
2024-09-27 00:45 ` Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM Michael Paquier <michael@paquier.xyz>
0 siblings, 1 reply; 6+ messages in thread
From: Tender Wang @ 2024-09-26 07:24 UTC (permalink / raw)
To: Michael Paquier <michael@paquier.xyz>; +Cc: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org
Michael Paquier <michael@paquier.xyz> 于2024年9月26日周四 15:02写道：
> On Wed, Sep 25, 2024 at 06:19:20PM +0800, Tender Wang wrote:
> > Adding if (flags & REINDEX_REL_SUPPRESS_INDEX_USE) check before calling
> > RemoveReindexPending() may look better.
> > Any thoughts?
>
> The short answer to that is yes, see below.
>
> Alexander's trick presented upthread is funky. I was first confused
> of what was the point until I noticed that this is just a way to make
> REINDEX CONCURRENTLY fail and produce an invalid toast index. The
> first pg_sleep causes REINDEX to fail because we are waiting for the
> first transaction to complete. So this provides a controlled way to
> get an index that should never be part of the list reported to
> SetReindexPending() to begin with. The VACUUM (PROCESS_MAIN FALSE,
> FULL) is then disturbed.
>
> Anyway, PROCESS_MAIN is not related to the failure, as it is just a
> flavor grammar introduced for the sake of making toast-only rebuilds
> easier for administrators. The same failure reproduces if switching
> the test case of upthread to do the REINDEX on the toast table, down
> to 12 which is as far as I have tested because that's what we support
> around here.
>
> Regarding the fix, we should not disturb the list of indexes in a
> relation returned by relcache.c, and invalid indexes are part of it.
> So I'd agree with your point to just remove the index from the pending
> list because we have to skip invalid toast indexes in the
> reindex_relation() path as reindex_index() has to generate a hard
> failure because we can never ever have two valid toast indexes, and
> that's a guarantee we need to be very careful about.
>
> Now, about REINDEX_REL_SUPPRESS_INDEX_USE. Well, that's only used for
> the VACUUM FULL/CLUSTER path which is where the indexes are marked as
> being processed, so we have to reomve the invalid toast indexes from
> the list only if this flag is used. Hence the answer is yes. I'd
> rather keep the informative warning. That's useful for monitoring
> purposes.
>
Agree. The attached patch LGTM.
>
> Something worth noting while I have looked at this code.. Non-toast
> invalid indexes are rebuilt but still marked as !indisvalid, actually.
> That's a waste and we could just skip them for a CLUSTER/VACUUM FULL.
> This cannot be true for a REINDEX TABLE, invalid indexes are switched
> to valid in this case.
>
>
--
Thanks,
Tender Wang
https://www.openpie.com/
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM
2024-09-25 03:00 BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM PG Bug reporting form <noreply@postgresql.org>
2024-09-25 09:28 ` Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM Tender Wang <tndrwang@gmail.com>
2024-09-25 10:19 ` Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM Tender Wang <tndrwang@gmail.com>
2024-09-26 07:01 ` Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM Michael Paquier <michael@paquier.xyz>
2024-09-26 07:24 ` Re: BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM Tender Wang <tndrwang@gmail.com>
@ 2024-09-27 00:45 ` Michael Paquier <michael@paquier.xyz>
0 siblings, 0 replies; 6+ messages in thread
From: Michael Paquier @ 2024-09-27 00:45 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: exclusion@gmail.com; pgsql-bugs@lists.postgresql.org
On Thu, Sep 26, 2024 at 03:24:25PM +0800, Tender Wang wrote:
> Agree. The attached patch LGTM.
I have fixed this issue and backpatched it all the way down for now.
One thing that we still lack is a set of reliable tests. This can be
achieved with injection points, and that's much broader than only this
thread. I'd like to get the coverage with two types of tests,
roughly:
- Isolation, with waits across the steps of REINDEX CONCURRENTLY, and
some concurrent operation. This thread used one.
- SQL tests, where we could force errors in some code paths to get an
incorrect state. Putting ourselves in a corner where there is an
invalid toast index is just one state. CIC with a UNIQUE index gives
this possibility, but this just works for a "new" index, for early
stages of REINDEX CONCURRENTLY.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2024-09-27 00:45 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-09-25 03:00 BUG #18630: Incorrect memory access inside ReindexIsProcessingIndex() on VACUUM PG Bug reporting form <noreply@postgresql.org>
2024-09-25 09:28 ` Tender Wang <tndrwang@gmail.com>
2024-09-25 10:19 ` Tender Wang <tndrwang@gmail.com>
2024-09-26 07:01 ` Michael Paquier <michael@paquier.xyz>
2024-09-26 07:24 ` Tender Wang <tndrwang@gmail.com>
2024-09-27 00:45 ` Michael Paquier <michael@paquier.xyz>
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