public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v6 3/7] Propagate changes to indisclustered to child/parents
5+ messages / 3 participants
[nested] [flat]
* [PATCH v6 3/7] Propagate changes to indisclustered to child/parents
@ 2020-10-07 03:11 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Justin Pryzby @ 2020-10-07 03:11 UTC (permalink / raw)
---
src/backend/commands/cluster.c | 109 ++++++++++++++++----------
src/backend/commands/indexcmds.c | 2 +
src/test/regress/expected/cluster.out | 46 +++++++++++
src/test/regress/sql/cluster.sql | 11 +++
4 files changed, 125 insertions(+), 43 deletions(-)
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 838bcd9e72..078b97fbb9 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -74,6 +74,7 @@ static void rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose);
static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex,
bool verbose, bool *pSwapToastByContent,
TransactionId *pFreezeXid, MultiXactId *pCutoffMulti);
+static void set_indisclustered(Oid indexOid, bool isclustered, Relation pg_index);
static List *get_tables_to_cluster(MemoryContext cluster_context);
static List *get_tables_to_cluster_partitioned(MemoryContext cluster_context,
Oid indexOid);
@@ -510,66 +511,88 @@ check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck, LOCKMOD
index_close(OldIndex, NoLock);
}
+/*
+ * Helper for mark_index_clustered
+ * Mark a single index as clustered or not.
+ * pg_index is passed by caller to avoid repeatedly re-opening it.
+ */
+static void
+set_indisclustered(Oid indexOid, bool isclustered, Relation pg_index)
+{
+ HeapTuple indexTuple;
+ Form_pg_index indexForm;
+
+ indexTuple = SearchSysCacheCopy1(INDEXRELID,
+ ObjectIdGetDatum(indexOid));
+ if (!HeapTupleIsValid(indexTuple))
+ elog(ERROR, "cache lookup failed for index %u", indexOid);
+ indexForm = (Form_pg_index) GETSTRUCT(indexTuple);
+
+ /* this was checked earlier, but let's be real sure */
+ if (isclustered && !indexForm->indisvalid)
+ elog(ERROR, "cannot cluster on invalid index %u", indexOid);
+
+ indexForm->indisclustered = isclustered;
+ CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
+ heap_freetuple(indexTuple);
+}
+
/*
* mark_index_clustered: mark the specified index as the one clustered on
*
- * With indexOid == InvalidOid, will mark all indexes of rel not-clustered.
+ * With indexOid == InvalidOid, mark all indexes of rel not-clustered.
+ * Otherwise, mark children of the clustered index as clustered, and parents of
+ * other indexes as unclustered.
+ * We wish to maintain the following properties:
+ * 1) Only one index on a relation can be marked clustered at once
+ * 2) If a partitioned index is clustered, then all its children must be
+ * clustered.
*/
void
mark_index_clustered(Relation rel, Oid indexOid, bool is_internal)
{
- HeapTuple indexTuple;
- Form_pg_index indexForm;
- Relation pg_index;
- ListCell *index;
-
- /*
- * If the index is already marked clustered, no need to do anything.
- */
- if (OidIsValid(indexOid))
- {
- if (get_index_isclustered(indexOid))
- return;
- }
+ ListCell *lc, *lc2;
+ List *indexes;
+ Relation pg_index = table_open(IndexRelationId, RowExclusiveLock);
+ List *inh = find_all_inheritors(RelationGetRelid(rel), ShareRowExclusiveLock, NULL);
/*
* Check each index of the relation and set/clear the bit as needed.
+ * Iterate over the relation's children rather than the index's children
+ * since we need to unset cluster for indexes on intermediate children,
+ * too.
*/
- pg_index = table_open(IndexRelationId, RowExclusiveLock);
-
- foreach(index, RelationGetIndexList(rel))
+ foreach(lc, inh)
{
- Oid thisIndexOid = lfirst_oid(index);
-
- indexTuple = SearchSysCacheCopy1(INDEXRELID,
- ObjectIdGetDatum(thisIndexOid));
- if (!HeapTupleIsValid(indexTuple))
- elog(ERROR, "cache lookup failed for index %u", thisIndexOid);
- indexForm = (Form_pg_index) GETSTRUCT(indexTuple);
+ Oid inhrelid = lfirst_oid(lc);
+ Relation thisrel = table_open(inhrelid, ShareRowExclusiveLock);
- /*
- * Unset the bit if set. We know it's wrong because we checked this
- * earlier.
- */
- if (indexForm->indisclustered)
+ indexes = RelationGetIndexList(thisrel);
+ foreach (lc2, indexes)
{
- indexForm->indisclustered = false;
- CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
- }
- else if (thisIndexOid == indexOid)
- {
- /* this was checked earlier, but let's be real sure */
- if (!indexForm->indisvalid)
- elog(ERROR, "cannot cluster on invalid index %u", indexOid);
- indexForm->indisclustered = true;
- CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple);
- }
+ bool isclustered;
+ Oid thisIndexOid = lfirst_oid(lc2);
+ List *parentoids = get_rel_relispartition(thisIndexOid) ?
+ get_partition_ancestors(thisIndexOid) : NIL;
- InvokeObjectPostAlterHookArg(IndexRelationId, thisIndexOid, 0,
- InvalidOid, is_internal);
+ /*
+ * A child of the clustered index must be set clustered;
+ * indexes which are not children of the clustered index are
+ * set unclustered
+ */
+ isclustered = (thisIndexOid == indexOid) ||
+ list_member_oid(parentoids, indexOid);
+ Assert(OidIsValid(indexOid) || !isclustered);
+ set_indisclustered(thisIndexOid, isclustered, pg_index);
+
+ InvokeObjectPostAlterHookArg(IndexRelationId, thisIndexOid, 0,
+ InvalidOid, is_internal);
+ }
- heap_freetuple(indexTuple);
+ list_free(indexes);
+ table_close(thisrel, ShareRowExclusiveLock);
}
+ list_free(inh);
table_close(pg_index, RowExclusiveLock);
}
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index f9f3ff3b62..0a65698c28 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -25,6 +25,7 @@
#include "catalog/catalog.h"
#include "catalog/index.h"
#include "catalog/indexing.h"
+#include "catalog/partition.h"
#include "catalog/pg_am.h"
#include "catalog/pg_constraint.h"
#include "catalog/pg_inherits.h"
@@ -32,6 +33,7 @@
#include "catalog/pg_opfamily.h"
#include "catalog/pg_tablespace.h"
#include "catalog/pg_type.h"
+#include "commands/cluster.h"
#include "commands/comment.h"
#include "commands/dbcommands.h"
#include "commands/defrem.h"
diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out
index e4448350e7..a9fb9f1021 100644
--- a/src/test/regress/expected/cluster.out
+++ b/src/test/regress/expected/cluster.out
@@ -495,6 +495,52 @@ Indexes:
"clstrpart_idx" btree (a) CLUSTER
Number of partitions: 3 (Use \d+ to list them.)
+-- Test that it recurses to grandchildren:
+\d clstrpart33
+ Table "public.clstrpart33"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: clstrpart3 DEFAULT
+Indexes:
+ "clstrpart33_a_idx" btree (a) CLUSTER
+
+ALTER TABLE clstrpart SET WITHOUT CLUSTER;
+\d clstrpart33
+ Table "public.clstrpart33"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: clstrpart3 DEFAULT
+Indexes:
+ "clstrpart33_a_idx" btree (a)
+
+ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
+\d clstrpart33
+ Table "public.clstrpart33"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: clstrpart3 DEFAULT
+Indexes:
+ "clstrpart33_a_idx" btree (a) CLUSTER
+
+-- Check that only one child is marked clustered after marking clustered on a different parent
+CREATE INDEX clstrpart1_idx_2 ON clstrpart1(a);
+ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
+ALTER TABLE clstrpart1 CLUSTER ON clstrpart1_idx_2;
+\d clstrpart1
+ Partitioned table "public.clstrpart1"
+ Column | Type | Collation | Nullable | Default
+--------+---------+-----------+----------+---------
+ a | integer | | |
+Partition of: clstrpart FOR VALUES FROM (1) TO (10)
+Partition key: RANGE (a)
+Indexes:
+ "clstrpart1_a_idx" btree (a)
+ "clstrpart1_idx_2" btree (a) CLUSTER
+Number of partitions: 2 (Use \d+ to list them.)
+
-- Test CLUSTER with external tuplesorting
create table clstr_4 as select * from tenk1;
create index cluster_sort on clstr_4 (hundred, thousand, tenthous);
diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql
index 22225dc924..d15bd51496 100644
--- a/src/test/regress/sql/cluster.sql
+++ b/src/test/regress/sql/cluster.sql
@@ -220,6 +220,17 @@ CLUSTER clstrpart1 USING clstrpart1_a_idx; -- partition which is itself partitio
CLUSTER clstrpart12 USING clstrpart12_a_idx; -- partition which is itself partitioned, no childs
CLUSTER clstrpart2 USING clstrpart2_a_idx; -- leaf
\d clstrpart
+-- Test that it recurses to grandchildren:
+\d clstrpart33
+ALTER TABLE clstrpart SET WITHOUT CLUSTER;
+\d clstrpart33
+ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
+\d clstrpart33
+-- Check that only one child is marked clustered after marking clustered on a different parent
+CREATE INDEX clstrpart1_idx_2 ON clstrpart1(a);
+ALTER TABLE clstrpart CLUSTER ON clstrpart_idx;
+ALTER TABLE clstrpart1 CLUSTER ON clstrpart1_idx_2;
+\d clstrpart1
-- Test CLUSTER with external tuplesorting
--
2.17.0
--FsscpQKzF/jJk6ya
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="v6-0004-Invalidate-parent-indexes.patch"
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Is *fast* 32-bit support still important?
@ 2024-07-30 09:06 Aleksander Alekseev <[email protected]>
2024-08-05 12:04 ` Re: Is *fast* 32-bit support still important? Joel Jacobson <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Aleksander Alekseev @ 2024-07-30 09:06 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Joel Jacobson <[email protected]>
Hi Joel,
Here are my two cents.
> 1. Who are the current users of 32-bit PostgreSQL?
Pretty much any embedded system that uses just a few GB of memory may
win from using a 32-bit processor (not necessarily in terms of
performance, maybe in terms of price). Think of WiFi-routers, smart
TVs, 3D printers, etc.
Generally speaking it's hard to give an exact answer due to lack of
"telemetry" in PostgreSQL.
> 2. Among these users, how many are upgrading to new major versions?
I would guess it very much depends on the product and manufacturer. I
wouldn't assume though that users of 32-bit systems don't do major
upgrades. (Not to mention that it's beneficial for us to test the code
on 32-bit systems.)
> 3. For how many of these users is performance critical?
Depends on how you define performance. Performance of a single OLTP
query is important. The performance of the upgrade procedure is
probably not that important. The ability of processing a lot of data
is probably also not extremely important, at least I wouldn't expect a
lot of data and/or fast storage devices on 32-bit systems.
--
Best regards,
Aleksander Alekseev
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Is *fast* 32-bit support still important?
2024-07-30 09:06 Re: Is *fast* 32-bit support still important? Aleksander Alekseev <[email protected]>
@ 2024-08-05 12:04 ` Joel Jacobson <[email protected]>
2024-08-05 12:24 ` Re: Is *fast* 32-bit support still important? Aleksander Alekseev <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Joel Jacobson @ 2024-08-05 12:04 UTC (permalink / raw)
To: Aleksander Alekseev <[email protected]>; pgsql-hackers
On Tue, Jul 30, 2024, at 11:06, Aleksander Alekseev wrote:
> Hi Joel,
>
> Here are my two cents.
>
>> 1. Who are the current users of 32-bit PostgreSQL?
>
> Pretty much any embedded system that uses just a few GB of memory may
> win from using a 32-bit processor (not necessarily in terms of
> performance, maybe in terms of price). Think of WiFi-routers, smart
> TVs, 3D printers, etc.
Thanks for feedback!
Do we know of any such products or users?
I found an OS that runs on many 32-bit chips, FreeRTOS, that seems quite popular.
Couldn't find anything about PostgreSQL and FreeRTOS though.
I've posted a question on their forum. [1] Let's wait and see if we hear from any real user.
I see one i386 and i686 build farm animals runs Debian.
Perhaps it makes sense to try to reach out to the Debian community,
and see if they know of any PostgreSQL users on 32-bit?
> Generally speaking it's hard to give an exact answer due to lack of
> "telemetry" in PostgreSQL.
Could we add a text message that is displayed to a user,
when compiling PostgreSQL on a 32-bit platform?
*****
NOTICE: You are compiling PostgreSQL on a 32-bit platform.
We are interested in learning about your use case.
Please contact us with details about how you are using
PostgreSQL on this platform.
Thank you!
Contact: [email protected]
Subject: Report from 32-bit user
*****
/Joel
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Is *fast* 32-bit support still important?
2024-07-30 09:06 Re: Is *fast* 32-bit support still important? Aleksander Alekseev <[email protected]>
2024-08-05 12:04 ` Re: Is *fast* 32-bit support still important? Joel Jacobson <[email protected]>
@ 2024-08-05 12:24 ` Aleksander Alekseev <[email protected]>
2024-08-05 12:54 ` Re: Is *fast* 32-bit support still important? Joel Jacobson <[email protected]>
0 siblings, 1 reply; 5+ messages in thread
From: Aleksander Alekseev @ 2024-08-05 12:24 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Joel Jacobson <[email protected]>
Hi,
> > Pretty much any embedded system that uses just a few GB of memory may
> > win from using a 32-bit processor (not necessarily in terms of
> > performance, maybe in terms of price). Think of WiFi-routers, smart
> > TVs, 3D printers, etc.
>
> Thanks for feedback!
>
> Do we know of any such products or users?
>
> I found an OS that runs on many 32-bit chips, FreeRTOS, that seems quite popular.
> Couldn't find anything about PostgreSQL and FreeRTOS though.
> I've posted a question on their forum. [1] Let's wait and see if we hear from any real user.
I'm not extremely familiar with FreeRTOS but my humble understanding
is that it's very different from what we would typically call an
"operating system". If I'm not wrong, basically this is a framework
that adds multitasking to STM32 microcontrollers. This is not exactly
a target hardware for PostgreSQL (although running PostgreSQL on STM32
MCUs could be a fun project for someone looking for a challenging
task.)
> I see one i386 and i686 build farm animals runs Debian.
> Perhaps it makes sense to try to reach out to the Debian community,
> and see if they know of any PostgreSQL users on 32-bit?
>
> > Generally speaking it's hard to give an exact answer due to lack of
> > "telemetry" in PostgreSQL.
>
> Could we add a text message that is displayed to a user,
> when compiling PostgreSQL on a 32-bit platform?
What would be actionable items depending on the results? Option A:
someone claims to use PostgreSQL on 32-bit hardware. Option B: no one
admits that they use PostgreSQL on 32-bit hardware (but maybe someone
actually does and/or will in the future). Regardless of the results
you can't drop the support of 32-bit software (until it gets as
difficult and pointless as with AIX that was dropped recently) and it
will not tell you how slow the 32-bit version of PostgreSQL can be.
If there are no actionable items why create a poll?
--
Best regards,
Aleksander Alekseev
^ permalink raw reply [nested|flat] 5+ messages in thread
* Re: Is *fast* 32-bit support still important?
2024-07-30 09:06 Re: Is *fast* 32-bit support still important? Aleksander Alekseev <[email protected]>
2024-08-05 12:04 ` Re: Is *fast* 32-bit support still important? Joel Jacobson <[email protected]>
2024-08-05 12:24 ` Re: Is *fast* 32-bit support still important? Aleksander Alekseev <[email protected]>
@ 2024-08-05 12:54 ` Joel Jacobson <[email protected]>
0 siblings, 0 replies; 5+ messages in thread
From: Joel Jacobson @ 2024-08-05 12:54 UTC (permalink / raw)
To: Aleksander Alekseev <[email protected]>; pgsql-hackers
On Mon, Aug 5, 2024, at 14:24, Aleksander Alekseev wrote:
>> Could we add a text message that is displayed to a user,
>> when compiling PostgreSQL on a 32-bit platform?
>
> What would be actionable items depending on the results? Option A:
> someone claims to use PostgreSQL on 32-bit hardware. Option B: no one
> admits that they use PostgreSQL on 32-bit hardware (but maybe someone
> actually does and/or will in the future). Regardless of the results
> you can't drop the support of 32-bit software (until it gets as
> difficult and pointless as with AIX that was dropped recently) and it
> will not tell you how slow the 32-bit version of PostgreSQL can be.
>
> If there are no actionable items why create a poll?
Never suggested *dropping* 32-bit support; that's a different question.
However, if we were to receive input from 32-bit PostgreSQL users explaining
why they need *high performance*, then I imagine that could affect how we
feel about 32-bit optimizations.
Right now, there doesn't seem to be a consensus on whether we should optimize
for 32-bit or not.
On the one hand, we have commit 5e1f3b9 that says:
"While it adds some space on 32-bit machines, we aren't optimizing
for that case anymore."
On the other hand, we have the ongoing work on optimizing numeric_mul,
where a patch is suggested with extra code to optimize for 32-bit.
I agree, however, that the absence of any comments from such a poll would not
give us any more information.
/Joel
^ permalink raw reply [nested|flat] 5+ messages in thread
end of thread, other threads:[~2024-08-05 12:54 UTC | newest]
Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-10-07 03:11 [PATCH v6 3/7] Propagate changes to indisclustered to child/parents Justin Pryzby <[email protected]>
2024-07-30 09:06 Re: Is *fast* 32-bit support still important? Aleksander Alekseev <[email protected]>
2024-08-05 12:04 ` Re: Is *fast* 32-bit support still important? Joel Jacobson <[email protected]>
2024-08-05 12:24 ` Re: Is *fast* 32-bit support still important? Aleksander Alekseev <[email protected]>
2024-08-05 12:54 ` Re: Is *fast* 32-bit support still important? Joel Jacobson <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox