agora inbox for [email protected]  

help / color / mirror / Atom feed

[PATCH v10 3/8] f! progress reporting..
17+ messages / 2 participants
[nested] [flat]

* [PATCH v10 3/8] f! progress reporting..
@ 2021-02-09 02:45 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Justin Pryzby @ 2021-02-09 02:45 UTC (permalink / raw)

This follows the precedent of pg_stat_progress_create_index, which
simultaneously shows the progress of 1) the leaf partition, and 2) the overall
progress as partitions_done / partitions_total.

This also includes current_child_index_relid, but the create_index view
doesn't, so maybe this shouldn't, either.
---
 doc/src/sgml/monitoring.sgml         | 21 +++++++++++
 src/backend/catalog/system_views.sql |  4 +-
 src/backend/commands/cluster.c       | 55 ++++++++++++++++------------
 src/backend/commands/vacuum.c        |  5 +++
 src/include/commands/progress.h      |  3 ++
 src/test/regress/expected/rules.out  |  4 +-
 6 files changed, 67 insertions(+), 25 deletions(-)

diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 56018745c8..28493c7931 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -6293,6 +6293,27 @@ SELECT pg_stat_get_backend_pid(s.backendid) AS pid,
        is <literal>rebuilding index</literal>.
       </para></entry>
      </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>total_partitions</structfield> <type>bigint</type>
+      </para>
+       When clustering a partitioned table, this column is set to the total
+       number of partitions to be clustered.
+      <para>
+      </para></entry>
+     </row>
+
+     <row>
+      <entry role="catalog_table_entry"><para role="column_definition">
+       <structfield>done_partitions</structfield> <type>bigint</type>
+      </para>
+      <para>
+       When clustering a partitioned table, this column is set to the number
+       of partitions which have been clustered.
+      </para></entry>
+     </row>
+
     </tbody>
    </tgroup>
   </table>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 5f2541d316..0256809f06 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1142,7 +1142,9 @@ CREATE VIEW pg_stat_progress_cluster AS
         S.param5 AS heap_tuples_written,
         S.param6 AS heap_blks_total,
         S.param7 AS heap_blks_scanned,
-        S.param8 AS index_rebuild_count
+        S.param8 AS index_rebuild_count,
+        S.param18 AS partitions_total,
+        S.param19 AS partitions_done
     FROM pg_stat_get_progress_info('CLUSTER') AS S
         LEFT JOIN pg_database D ON S.datid = D.oid;
 
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 0c08ac56dc..7d8d1d8a69 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -77,7 +77,7 @@ static void copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex,
 static List *get_tables_to_cluster(MemoryContext cluster_context);
 static List *get_tables_to_cluster_partitioned(MemoryContext cluster_context,
 		Oid indexOid);
-static void cluster_multiple_rels(List *rvs, int options);
+static void cluster_multiple_rels(List *rvs, int options, bool ispartitioned);
 
 
 /*---------------------------------------------------------------------------
@@ -188,16 +188,26 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
 		/* close relation, keep lock till commit */
 		table_close(rel, ShareUpdateExclusiveLock);
 
+		pgstat_progress_start_command(PROGRESS_COMMAND_CLUSTER, tableOid);
 		if (rel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE)
 		{
 			/* Do the job. */
+			pgstat_progress_update_param(PROGRESS_CLUSTER_COMMAND,
+					PROGRESS_CLUSTER_COMMAND_CLUSTER);
 			cluster_rel(tableOid, indexOid, &params);
+			pgstat_progress_end_command();
 		}
 		else
 		{
 			List	   *rvs;
 			MemoryContext cluster_context;
 
+			int64		progress_val[2];
+			const int	progress_index[2] = {
+				PROGRESS_CLUSTER_COMMAND,
+				PROGRESS_CLUSTER_PARTITIONS_TOTAL,
+			};
+
 			/* Refuse to hold strong locks in a user transaction */
 			PreventInTransactionBlock(isTopLevel, "CLUSTER");
 
@@ -206,7 +216,15 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
 												ALLOCSET_DEFAULT_SIZES);
 
 			rvs = get_tables_to_cluster_partitioned(cluster_context, indexOid);
-			cluster_multiple_rels(rvs, params.options);
+
+			/* Report command and number of partitions */
+			progress_val[0] = PROGRESS_CLUSTER_COMMAND_CLUSTER;
+			/* XXX: rvs includes the parent, which is not a "partition" */
+			progress_val[1] = list_length(rvs);
+
+			pgstat_progress_update_multi_param(2, progress_index, progress_val);
+			cluster_multiple_rels(rvs, params.options, true);
+			pgstat_progress_end_command();
 
 			/* Start a new transaction for the cleanup work. */
 			StartTransactionCommand();
@@ -245,7 +263,11 @@ cluster(ParseState *pstate, ClusterStmt *stmt, bool isTopLevel)
 		 * cluster_context.
 		 */
 		rvs = get_tables_to_cluster(cluster_context);
-		cluster_multiple_rels(rvs, params.options | CLUOPT_RECHECK_ISCLUSTERED);
+
+		pgstat_progress_update_param(PROGRESS_CLUSTER_COMMAND,
+				PROGRESS_CLUSTER_COMMAND_CLUSTER);
+		cluster_multiple_rels(rvs, params.options | CLUOPT_RECHECK_ISCLUSTERED, false);
+		pgstat_progress_end_command();
 
 		/* Start a new transaction for the cleanup work. */
 		StartTransactionCommand();
@@ -282,14 +304,6 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
 	/* Check for user-requested abort. */
 	CHECK_FOR_INTERRUPTS();
 
-	pgstat_progress_start_command(PROGRESS_COMMAND_CLUSTER, tableOid);
-	if (OidIsValid(indexOid))
-		pgstat_progress_update_param(PROGRESS_CLUSTER_COMMAND,
-									 PROGRESS_CLUSTER_COMMAND_CLUSTER);
-	else
-		pgstat_progress_update_param(PROGRESS_CLUSTER_COMMAND,
-									 PROGRESS_CLUSTER_COMMAND_VACUUM_FULL);
-
 	/*
 	 * We grab exclusive access to the target rel and index for the duration
 	 * of the transaction.  (This is redundant for the single-transaction
@@ -300,10 +314,7 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
 
 	/* If the table has gone away, we can skip processing it */
 	if (!OldHeap)
-	{
-		pgstat_progress_end_command();
 		return;
-	}
 
 	/*
 	 * Since we may open a new transaction for each relation, we have to check
@@ -319,7 +330,6 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
 		if (!pg_class_ownercheck(tableOid, GetUserId()))
 		{
 			relation_close(OldHeap, AccessExclusiveLock);
-			pgstat_progress_end_command();
 			return;
 		}
 
@@ -334,7 +344,6 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
 		if (RELATION_IS_OTHER_TEMP(OldHeap))
 		{
 			relation_close(OldHeap, AccessExclusiveLock);
-			pgstat_progress_end_command();
 			return;
 		}
 
@@ -346,7 +355,6 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
 			if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(indexOid)))
 			{
 				relation_close(OldHeap, AccessExclusiveLock);
-				pgstat_progress_end_command();
 				return;
 			}
 
@@ -357,7 +365,6 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
 					!get_index_isclustered(indexOid))
 			{
 				relation_close(OldHeap, AccessExclusiveLock);
-				pgstat_progress_end_command();
 				return;
 			}
 		}
@@ -416,7 +423,6 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
 		!RelationIsPopulated(OldHeap))
 	{
 		relation_close(OldHeap, AccessExclusiveLock);
-		pgstat_progress_end_command();
 		return;
 	}
 
@@ -424,7 +430,6 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
 	if (!RELKIND_HAS_STORAGE(get_rel_relkind(tableOid)))
 	{
 		relation_close(OldHeap, AccessExclusiveLock);
-		pgstat_progress_end_command();
 		return;
 	}
 
@@ -440,8 +445,6 @@ cluster_rel(Oid tableOid, Oid indexOid, ClusterParams *params)
 	rebuild_relation(OldHeap, indexOid, verbose);
 
 	/* NB: rebuild_relation does table_close() on OldHeap */
-
-	pgstat_progress_end_command();
 }
 
 /*
@@ -1632,9 +1635,10 @@ get_tables_to_cluster_partitioned(MemoryContext cluster_context, Oid indexOid)
 
 /* Cluster each relation in a separate transaction */
 static void
-cluster_multiple_rels(List *rvs, int options)
+cluster_multiple_rels(List *rvs, int options, bool ispartitioned)
 {
 	ListCell *lc;
+	off_t	  childnum = 0;
 
 	/* Commit to get out of starting transaction */
 	PopActiveSnapshot();
@@ -1657,6 +1661,11 @@ cluster_multiple_rels(List *rvs, int options)
 		cluster_rel(rvtc->tableOid, rvtc->indexOid,
 					&cluster_params);
 
+		if (ispartitioned)
+			pgstat_progress_update_param(PROGRESS_CLUSTER_PARTITIONS_DONE,
+					++childnum);
+
+
 		PopActiveSnapshot();
 		CommitTransactionCommand();
 	}
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 662aff04b4..1d8a7ca774 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -37,6 +37,7 @@
 #include "catalog/pg_namespace.h"
 #include "commands/cluster.h"
 #include "commands/defrem.h"
+#include "commands/progress.h"
 #include "commands/vacuum.h"
 #include "miscadmin.h"
 #include "nodes/makefuncs.h"
@@ -1937,7 +1938,11 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 			cluster_params.options |= CLUOPT_VERBOSE;
 
 		/* VACUUM FULL is now a variant of CLUSTER; see cluster.c */
+		pgstat_progress_start_command(PROGRESS_COMMAND_CLUSTER, relid);
+		pgstat_progress_update_param(PROGRESS_CLUSTER_COMMAND,
+				PROGRESS_CLUSTER_COMMAND_VACUUM_FULL);
 		cluster_rel(relid, InvalidOid, &cluster_params);
+		pgstat_progress_end_command();
 	}
 	else
 		table_relation_vacuum(onerel, params, vac_strategy);
diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h
index c6b139d57d..d85781ae0b 100644
--- a/src/include/commands/progress.h
+++ b/src/include/commands/progress.h
@@ -60,6 +60,9 @@
 #define PROGRESS_CLUSTER_TOTAL_HEAP_BLKS		5
 #define PROGRESS_CLUSTER_HEAP_BLKS_SCANNED		6
 #define PROGRESS_CLUSTER_INDEX_REBUILD_COUNT	7
+/* Need to avoid the CREATE INDEX params */
+#define PROGRESS_CLUSTER_PARTITIONS_TOTAL	17
+#define PROGRESS_CLUSTER_PARTITIONS_DONE	18
 
 /* Phases of cluster (as advertised via PROGRESS_CLUSTER_PHASE) */
 #define PROGRESS_CLUSTER_PHASE_SEQ_SCAN_HEAP	1
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 9b59a7b4a5..a8bcc599ac 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1943,7 +1943,9 @@ pg_stat_progress_cluster| SELECT s.pid,
     s.param5 AS heap_tuples_written,
     s.param6 AS heap_blks_total,
     s.param7 AS heap_blks_scanned,
-    s.param8 AS index_rebuild_count
+    s.param8 AS index_rebuild_count,
+    s.param18 AS partitions_total,
+    s.param19 AS partitions_done
    FROM (pg_stat_get_progress_info('CLUSTER'::text) s(pid, datid, relid, param1, param2, param3, param4, param5, param6, param7, param8, param9, param10, param11, param12, param13, param14, param15, param16, param17, param18, param19, param20)
      LEFT JOIN pg_database d ON ((s.datid = d.oid)));
 pg_stat_progress_copy| SELECT s.pid,
-- 
2.17.0


--mvpLiMfbWzRoNl4x
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v10-0004-Propagate-changes-to-indisclustered-to-child-par.patch"



^ permalink  raw  reply  [nested|flat] 17+ messages in thread

* [PATCH v2.5 28/30] Temporary: Increase BAS_BULKREAD size
@ 2024-09-01 04:42 Andres Freund <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Andres Freund @ 2024-09-01 04:42 UTC (permalink / raw)

Without this we only can execute very little AIO for sequential scans, as
there's just not enough buffers in the ring.  This isn't the right fix, as
just increasing the ring size can have negative performance implications in
workloads where the kernel has all the data cached.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/storage/buffer/freelist.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 336715b6c63..b72a5957a20 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -555,7 +555,12 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			return NULL;
 
 		case BAS_BULKREAD:
-			ring_size_kb = 256;
+
+			/*
+			 * FIXME: Temporary increase to allow large enough streaming reads
+			 * to actually benefit from AIO. This needs a better solution.
+			 */
+			ring_size_kb = 2 * 1024;
 			break;
 		case BAS_BULKWRITE:
 			ring_size_kb = 16 * 1024;
-- 
2.48.1.76.g4e746b1a31.dirty


--abvteypvk35ocehs
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2.5-0029-WIP-Use-MAP_POPULATE.patch"



^ permalink  raw  reply  [nested|flat] 17+ messages in thread

* [PATCH v2.4 27/29] Temporary: Increase BAS_BULKREAD size
@ 2024-09-01 04:42 Andres Freund <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Andres Freund @ 2024-09-01 04:42 UTC (permalink / raw)

Without this we only can execute very little AIO for sequential scans, as
there's just not enough buffers in the ring.  This isn't the right fix, as
just increasing the ring size can have negative performance implications in
workloads where the kernel has all the data cached.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/storage/buffer/freelist.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 336715b6c63..b72a5957a20 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -555,7 +555,12 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			return NULL;
 
 		case BAS_BULKREAD:
-			ring_size_kb = 256;
+
+			/*
+			 * FIXME: Temporary increase to allow large enough streaming reads
+			 * to actually benefit from AIO. This needs a better solution.
+			 */
+			ring_size_kb = 2 * 1024;
 			break;
 		case BAS_BULKWRITE:
 			ring_size_kb = 16 * 1024;
-- 
2.48.1.76.g4e746b1a31.dirty


--4ckqsto27zwk2eqr
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2.4-0028-WIP-Use-MAP_POPULATE.patch"



^ permalink  raw  reply  [nested|flat] 17+ messages in thread

* [PATCH v2.6 32/34] Temporary: Increase BAS_BULKREAD size
@ 2024-09-01 04:42 Andres Freund <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Andres Freund @ 2024-09-01 04:42 UTC (permalink / raw)

Without this we only can execute very little AIO for sequential scans, as
there's just not enough buffers in the ring.  This isn't the right fix, as
just increasing the ring size can have negative performance implications in
workloads where the kernel has all the data cached.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/storage/buffer/freelist.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 336715b6c63..b72a5957a20 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -555,7 +555,12 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			return NULL;
 
 		case BAS_BULKREAD:
-			ring_size_kb = 256;
+
+			/*
+			 * FIXME: Temporary increase to allow large enough streaming reads
+			 * to actually benefit from AIO. This needs a better solution.
+			 */
+			ring_size_kb = 2 * 1024;
 			break;
 		case BAS_BULKWRITE:
 			ring_size_kb = 16 * 1024;
-- 
2.48.1.76.g4e746b1a31.dirty


--6g5xner6ro2tsnwz
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2.6-0033-WIP-Use-MAP_POPULATE.patch"



^ permalink  raw  reply  [nested|flat] 17+ messages in thread

* [PATCH v2.3 28/30] Temporary: Increase BAS_BULKREAD size
@ 2024-09-01 04:42 Andres Freund <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Andres Freund @ 2024-09-01 04:42 UTC (permalink / raw)

Without this we only can execute very little AIO for sequential scans, as
there's just not enough buffers in the ring.  This isn't the right fix, as
just increasing the ring size can have negative performance implications in
workloads where the kernel has all the data cached.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/storage/buffer/freelist.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 1f757d96f07..ac19fb87433 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -555,7 +555,12 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			return NULL;
 
 		case BAS_BULKREAD:
-			ring_size_kb = 256;
+
+			/*
+			 * FIXME: Temporary increase to allow large enough streaming reads
+			 * to actually benefit from AIO. This needs a better solution.
+			 */
+			ring_size_kb = 2 * 1024;
 			break;
 		case BAS_BULKWRITE:
 			ring_size_kb = 16 * 1024;
-- 
2.48.1.76.g4e746b1a31.dirty


--gbq4ah2rmhae7qhd
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2.3-0029-WIP-Use-MAP_POPULATE.patch"



^ permalink  raw  reply  [nested|flat] 17+ messages in thread

* [PATCH v2.1 20/20] Temporary: Increase BAS_BULKREAD size
@ 2024-09-01 04:42 Andres Freund <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Andres Freund @ 2024-09-01 04:42 UTC (permalink / raw)

Without this we only can execute very little AIO for sequential scans, as
there's just not enough buffers in the ring.  This isn't the right fix, as
just increasing the ring size can have negative performance implications in
workloads where the kernel has all the data cached.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/storage/buffer/freelist.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index dffdd57e9b5..5be8125ad3a 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -555,7 +555,11 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			return NULL;
 
 		case BAS_BULKREAD:
-			ring_size_kb = 256;
+			/*
+			 * FIXME: Temporary increase to allow large enough streaming reads
+			 * to actually benefit from AIO. This needs a better solution.
+			 */
+			ring_size_kb = 2 * 1024;
 			break;
 		case BAS_BULKWRITE:
 			ring_size_kb = 16 * 1024;
-- 
2.45.2.827.g557ae147e6


--ggp33qz5xusicvk6--





^ permalink  raw  reply  [nested|flat] 17+ messages in thread

* [PATCH v2.7 33/35] Temporary: Increase BAS_BULKREAD size
@ 2024-09-01 04:42 Andres Freund <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Andres Freund @ 2024-09-01 04:42 UTC (permalink / raw)

Without this we only can execute very little AIO for sequential scans, as
there's just not enough buffers in the ring.  This isn't the right fix, as
just increasing the ring size can have negative performance implications in
workloads where the kernel has all the data cached.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/storage/buffer/freelist.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 336715b6c63..b72a5957a20 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -555,7 +555,12 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			return NULL;
 
 		case BAS_BULKREAD:
-			ring_size_kb = 256;
+
+			/*
+			 * FIXME: Temporary increase to allow large enough streaming reads
+			 * to actually benefit from AIO. This needs a better solution.
+			 */
+			ring_size_kb = 2 * 1024;
 			break;
 		case BAS_BULKWRITE:
 			ring_size_kb = 16 * 1024;
-- 
2.48.1.76.g4e746b1a31.dirty


--vsphh7g5lukufvxs--





^ permalink  raw  reply  [nested|flat] 17+ messages in thread

* [PATCH v2 19/20] Temporary: Increase BAS_BULKREAD size
@ 2024-09-01 04:42 Andres Freund <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Andres Freund @ 2024-09-01 04:42 UTC (permalink / raw)

Without this we only can execute very little AIO for sequential scans, as
there's just not enough buffers in the ring.  This isn't the right fix, as
just increasing the ring size can have negative performance implications in
workloads where the kernel has all the data cached.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/storage/buffer/freelist.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index dffdd57e9b5..f5795b509c7 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -555,7 +555,12 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			return NULL;
 
 		case BAS_BULKREAD:
-			ring_size_kb = 256;
+
+			/*
+			 * FIXME: Temporary increase to allow large enough streaming reads
+			 * to actually benefit from AIO. This needs a better solution.
+			 */
+			ring_size_kb = 2 * 1024;
 			break;
 		case BAS_BULKWRITE:
 			ring_size_kb = 16 * 1024;
-- 
2.45.2.746.g06e570c0df.dirty


--oy2jwuii6tssbict
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0020-WIP-Use-MAP_POPULATE.patch"



^ permalink  raw  reply  [nested|flat] 17+ messages in thread

* [PATCH v2.8 35/38] Temporary: Increase BAS_BULKREAD size
@ 2024-09-01 04:42 Andres Freund <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Andres Freund @ 2024-09-01 04:42 UTC (permalink / raw)

Without this we only can execute very little AIO for sequential scans, as
there's just not enough buffers in the ring.  This isn't the right fix, as
just increasing the ring size can have negative performance implications in
workloads where the kernel has all the data cached.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/storage/buffer/freelist.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 336715b6c63..b72a5957a20 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -555,7 +555,12 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			return NULL;
 
 		case BAS_BULKREAD:
-			ring_size_kb = 256;
+
+			/*
+			 * FIXME: Temporary increase to allow large enough streaming reads
+			 * to actually benefit from AIO. This needs a better solution.
+			 */
+			ring_size_kb = 2 * 1024;
 			break;
 		case BAS_BULKWRITE:
 			ring_size_kb = 16 * 1024;
-- 
2.48.1.76.g4e746b1a31.dirty


--ow5flh3n247znjrs
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2.8-0036-WIP-Use-MAP_POPULATE.patch"



^ permalink  raw  reply  [nested|flat] 17+ messages in thread

* [PATCH v2.0 17/17] Temporary: Increase BAS_BULKREAD size
@ 2024-09-01 04:42 Andres Freund <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Andres Freund @ 2024-09-01 04:42 UTC (permalink / raw)

Without this we only can execute very little AIO for sequential scans, as
there's just not enough buffers in the ring.  This isn't the right fix, as
just increasing the ring size can have negative performance implications in
workloads where the kernel has all the data cached.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/storage/buffer/freelist.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index dffdd57e9b5..5be8125ad3a 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -555,7 +555,11 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			return NULL;
 
 		case BAS_BULKREAD:
-			ring_size_kb = 256;
+			/*
+			 * FIXME: Temporary increase to allow large enough streaming reads
+			 * to actually benefit from AIO. This needs a better solution.
+			 */
+			ring_size_kb = 2 * 1024;
 			break;
 		case BAS_BULKWRITE:
 			ring_size_kb = 16 * 1024;
-- 
2.45.2.827.g557ae147e6


--ww2auydviafoh7lh--





^ permalink  raw  reply  [nested|flat] 17+ messages in thread

* [PATCH v2.9 27/30] Temporary: Increase BAS_BULKREAD size
@ 2025-03-15 16:29 Andres Freund <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Andres Freund @ 2025-03-15 16:29 UTC (permalink / raw)

Without this we only can execute very little AIO for sequential scans, as
there's just not enough buffers in the ring.  This isn't the right fix, as
just increasing the ring size can have negative performance implications in
workloads where the kernel has all the data cached.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/storage/buffer/freelist.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 336715b6c63..b72a5957a20 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -555,7 +555,12 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			return NULL;
 
 		case BAS_BULKREAD:
-			ring_size_kb = 256;
+
+			/*
+			 * FIXME: Temporary increase to allow large enough streaming reads
+			 * to actually benefit from AIO. This needs a better solution.
+			 */
+			ring_size_kb = 2 * 1024;
 			break;
 		case BAS_BULKWRITE:
 			ring_size_kb = 16 * 1024;
-- 
2.48.1.76.g4e746b1a31.dirty


--23jbdfobqrqxnmx5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2.9-0028-WIP-Use-MAP_POPULATE.patch"



^ permalink  raw  reply  [nested|flat] 17+ messages in thread

* [PATCH v2.14 26/29] Temporary: Increase BAS_BULKREAD size
@ 2025-03-18 18:40 Andres Freund <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Andres Freund @ 2025-03-18 18:40 UTC (permalink / raw)

Without this we only can execute very little AIO for sequential scans, as
there's just not enough buffers in the ring.  This isn't the right fix, as
just increasing the ring size can have negative performance implications in
workloads where the kernel has all the data cached.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/storage/buffer/freelist.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 336715b6c63..b72a5957a20 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -555,7 +555,12 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			return NULL;
 
 		case BAS_BULKREAD:
-			ring_size_kb = 256;
+
+			/*
+			 * FIXME: Temporary increase to allow large enough streaming reads
+			 * to actually benefit from AIO. This needs a better solution.
+			 */
+			ring_size_kb = 2 * 1024;
 			break;
 		case BAS_BULKWRITE:
 			ring_size_kb = 16 * 1024;
-- 
2.48.1.76.g4e746b1a31.dirty


--m2na7lgr3zfazgom
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2.14-0027-WIP-Use-MAP_POPULATE.patch"



^ permalink  raw  reply  [nested|flat] 17+ messages in thread

* [PATCH v2.11 25/27] Temporary: Increase BAS_BULKREAD size
@ 2025-03-18 18:40 Andres Freund <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Andres Freund @ 2025-03-18 18:40 UTC (permalink / raw)

Without this we only can execute very little AIO for sequential scans, as
there's just not enough buffers in the ring.  This isn't the right fix, as
just increasing the ring size can have negative performance implications in
workloads where the kernel has all the data cached.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/storage/buffer/freelist.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 336715b6c63..b72a5957a20 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -555,7 +555,12 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			return NULL;
 
 		case BAS_BULKREAD:
-			ring_size_kb = 256;
+
+			/*
+			 * FIXME: Temporary increase to allow large enough streaming reads
+			 * to actually benefit from AIO. This needs a better solution.
+			 */
+			ring_size_kb = 2 * 1024;
 			break;
 		case BAS_BULKWRITE:
 			ring_size_kb = 16 * 1024;
-- 
2.48.1.76.g4e746b1a31.dirty


--bjnmbpad43bpmfxt
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2.11-0026-WIP-Use-MAP_POPULATE.patch"



^ permalink  raw  reply  [nested|flat] 17+ messages in thread

* [PATCH v2.12 25/28] Temporary: Increase BAS_BULKREAD size
@ 2025-03-18 18:40 Andres Freund <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Andres Freund @ 2025-03-18 18:40 UTC (permalink / raw)

Without this we only can execute very little AIO for sequential scans, as
there's just not enough buffers in the ring.  This isn't the right fix, as
just increasing the ring size can have negative performance implications in
workloads where the kernel has all the data cached.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/storage/buffer/freelist.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 336715b6c63..b72a5957a20 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -555,7 +555,12 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			return NULL;
 
 		case BAS_BULKREAD:
-			ring_size_kb = 256;
+
+			/*
+			 * FIXME: Temporary increase to allow large enough streaming reads
+			 * to actually benefit from AIO. This needs a better solution.
+			 */
+			ring_size_kb = 2 * 1024;
 			break;
 		case BAS_BULKWRITE:
 			ring_size_kb = 16 * 1024;
-- 
2.48.1.76.g4e746b1a31.dirty


--5i73spx2p4vwf7fe
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2.12-0026-WIP-Use-MAP_POPULATE.patch"



^ permalink  raw  reply  [nested|flat] 17+ messages in thread

* [PATCH v2.13 25/28] Temporary: Increase BAS_BULKREAD size
@ 2025-03-18 18:40 Andres Freund <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Andres Freund @ 2025-03-18 18:40 UTC (permalink / raw)

Without this we only can execute very little AIO for sequential scans, as
there's just not enough buffers in the ring.  This isn't the right fix, as
just increasing the ring size can have negative performance implications in
workloads where the kernel has all the data cached.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/storage/buffer/freelist.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 336715b6c63..b72a5957a20 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -555,7 +555,12 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			return NULL;
 
 		case BAS_BULKREAD:
-			ring_size_kb = 256;
+
+			/*
+			 * FIXME: Temporary increase to allow large enough streaming reads
+			 * to actually benefit from AIO. This needs a better solution.
+			 */
+			ring_size_kb = 2 * 1024;
 			break;
 		case BAS_BULKWRITE:
 			ring_size_kb = 16 * 1024;
-- 
2.48.1.76.g4e746b1a31.dirty


--pro7bqageygxfsvg
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2.13-0026-WIP-Use-MAP_POPULATE.patch"



^ permalink  raw  reply  [nested|flat] 17+ messages in thread

* [PATCH v2.15 15/18] Temporary: Increase BAS_BULKREAD size
@ 2025-03-18 18:40 Andres Freund <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Andres Freund @ 2025-03-18 18:40 UTC (permalink / raw)

Without this we only can execute very little AIO for sequential scans, as
there's just not enough buffers in the ring.  This isn't the right fix, as
just increasing the ring size can have negative performance implications in
workloads where the kernel has all the data cached.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/storage/buffer/freelist.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 336715b6c63..b72a5957a20 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -555,7 +555,12 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			return NULL;
 
 		case BAS_BULKREAD:
-			ring_size_kb = 256;
+
+			/*
+			 * FIXME: Temporary increase to allow large enough streaming reads
+			 * to actually benefit from AIO. This needs a better solution.
+			 */
+			ring_size_kb = 2 * 1024;
 			break;
 		case BAS_BULKWRITE:
 			ring_size_kb = 16 * 1024;
-- 
2.48.1.76.g4e746b1a31.dirty


--xevce4sdbnyxplun
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2.15-0016-WIP-Use-MAP_POPULATE.patch"



^ permalink  raw  reply  [nested|flat] 17+ messages in thread

* [PATCH v2.10 24/28] Temporary: Increase BAS_BULKREAD size
@ 2025-03-18 18:40 Andres Freund <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Andres Freund @ 2025-03-18 18:40 UTC (permalink / raw)

Without this we only can execute very little AIO for sequential scans, as
there's just not enough buffers in the ring.  This isn't the right fix, as
just increasing the ring size can have negative performance implications in
workloads where the kernel has all the data cached.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/backend/storage/buffer/freelist.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index 336715b6c63..b72a5957a20 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -555,7 +555,12 @@ GetAccessStrategy(BufferAccessStrategyType btype)
 			return NULL;
 
 		case BAS_BULKREAD:
-			ring_size_kb = 256;
+
+			/*
+			 * FIXME: Temporary increase to allow large enough streaming reads
+			 * to actually benefit from AIO. This needs a better solution.
+			 */
+			ring_size_kb = 2 * 1024;
 			break;
 		case BAS_BULKWRITE:
 			ring_size_kb = 16 * 1024;
-- 
2.48.1.76.g4e746b1a31.dirty


--w6dfit2y42fwvotd
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v2.10-0025-WIP-Use-MAP_POPULATE.patch"



^ permalink  raw  reply  [nested|flat] 17+ messages in thread

end of thread, other threads:[~2025-03-18 18:40 UTC | newest]

Thread overview: 17+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-02-09 02:45 [PATCH v10 3/8] f! progress reporting.. Justin Pryzby <[email protected]>
2024-09-01 04:42 [PATCH v2.5 28/30] Temporary: Increase BAS_BULKREAD size Andres Freund <[email protected]>
2024-09-01 04:42 [PATCH v2.4 27/29] Temporary: Increase BAS_BULKREAD size Andres Freund <[email protected]>
2024-09-01 04:42 [PATCH v2.6 32/34] Temporary: Increase BAS_BULKREAD size Andres Freund <[email protected]>
2024-09-01 04:42 [PATCH v2.3 28/30] Temporary: Increase BAS_BULKREAD size Andres Freund <[email protected]>
2024-09-01 04:42 [PATCH v2.1 20/20] Temporary: Increase BAS_BULKREAD size Andres Freund <[email protected]>
2024-09-01 04:42 [PATCH v2.7 33/35] Temporary: Increase BAS_BULKREAD size Andres Freund <[email protected]>
2024-09-01 04:42 [PATCH v2 19/20] Temporary: Increase BAS_BULKREAD size Andres Freund <[email protected]>
2024-09-01 04:42 [PATCH v2.8 35/38] Temporary: Increase BAS_BULKREAD size Andres Freund <[email protected]>
2024-09-01 04:42 [PATCH v2.0 17/17] Temporary: Increase BAS_BULKREAD size Andres Freund <[email protected]>
2025-03-15 16:29 [PATCH v2.9 27/30] Temporary: Increase BAS_BULKREAD size Andres Freund <[email protected]>
2025-03-18 18:40 [PATCH v2.14 26/29] Temporary: Increase BAS_BULKREAD size Andres Freund <[email protected]>
2025-03-18 18:40 [PATCH v2.11 25/27] Temporary: Increase BAS_BULKREAD size Andres Freund <[email protected]>
2025-03-18 18:40 [PATCH v2.12 25/28] Temporary: Increase BAS_BULKREAD size Andres Freund <[email protected]>
2025-03-18 18:40 [PATCH v2.13 25/28] Temporary: Increase BAS_BULKREAD size Andres Freund <[email protected]>
2025-03-18 18:40 [PATCH v2.15 15/18] Temporary: Increase BAS_BULKREAD size Andres Freund <[email protected]>
2025-03-18 18:40 [PATCH v2.10 24/28] Temporary: Increase BAS_BULKREAD size Andres Freund <[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