public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v24 08/15] Add aggregates support in IVM
18+ messages / 6 participants
[nested] [flat]
* [PATCH v24 08/15] Add aggregates support in IVM
@ 2021-08-02 05:59 Yugo Nagata <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: Yugo Nagata @ 2021-08-02 05:59 UTC (permalink / raw)
Currently, count, sum, avg, min and max are supported.
As a restriction, expressions specified in GROUP BY must appear in
the target list because tuples to be updated in IMMV are identified
by using this group key. However, in the case of aggregates without
GROUP BY, there is only one tuple in the view, so keys are not uses
to identify tuples.
When creating a IMMV, in addition to __ivm_count column, some hidden
columns for each aggregate are added to the target list. For example,
names of these hidden columns are ivm_count_avg and ivm_sum_avg for
the average function, and so on.
In the case of views without aggregate functions, only the number of
tuple multiplicities in __ivm_count__ column are updated at incremental
maintenance. On the other hand, in the case of view with aggregates,
the aggregated values and related hidden columns are also updated. The
way of update depends the kind of aggregate function.=E3=80=80Specifically,
sum and count are updated by simply adding or subtracting delta value
calculated from delta tables. avg is updated by using values of sum
and count stored in views as hidden columns and deltas calculated
from delta tables.
In min or max cases, it becomes more complicated. For an example of min,
when tuples are inserted, the smaller value between the current min value
in the view and the value calculated from the new delta table is used.
When tuples are deleted, if the current min value in the view equals to
the min in the old delta table, we need re-computation the latest min
value from base tables. Otherwise, the current value in the view remains.
As to sum, avg, min, and max (any aggregate functions except count),
NULL in input values is ignored, and this returns a null value when no
rows are selected. To support this specification, the number of not-NULL
input values is counted and stored in views as a hidden column. In the
case of count(), count(x) returns zero when no rows are selected, and
count(*) doesn't ignore NULL input. These specification are also supported.
---
src/backend/commands/createas.c | 299 ++++++++-
src/backend/commands/matview.c | 1016 ++++++++++++++++++++++++++++++-
src/include/commands/createas.h | 1 +
3 files changed, 1281 insertions(+), 35 deletions(-)
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createa=
s.c
index 91888891bf..ce6e6ffe33 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -63,6 +63,7 @@
#include "utils/builtins.h"
#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
+#include "utils/regproc.h"
#include "utils/rel.h"
#include "utils/rls.h"
#include "utils/snapmgr.h"
@@ -80,6 +81,11 @@ typedef struct
BulkInsertState bistate; /* bulk insert state */
} DR_intorel;
=20
+typedef struct
+{
+ bool has_agg;
+} check_ivm_restriction_context;
+
/* utility functions for CTAS definition creation */
static ObjectAddress create_ctas_internal(List *attrList, IntoClause *into=
);
static ObjectAddress create_ctas_nodata(List *tlist, IntoClause *into);
@@ -94,9 +100,10 @@ static void CreateIvmTriggersOnBaseTablesRecurse(Query =
*qry, Node *node, Oid mat
Relids *relids, bool ex_lock);
static void CreateIvmTrigger(Oid relOid, Oid viewOid, int16 type, int16 ti=
ming, bool ex_lock);
static void check_ivm_restriction(Node *node);
-static bool check_ivm_restriction_walker(Node *node, void *context);
+static bool check_ivm_restriction_walker(Node *node, check_ivm_restriction=
_context *context);
static void CreateIndexOnIMMV(Query *query, Relation matviewRel);
static Bitmapset *get_primary_key_attnos_from_query(Query *qry, List **con=
straintList);
+static bool check_aggregate_supports_ivm(Oid aggfnoid);
=20
/*
* create_ctas_internal
@@ -432,6 +439,7 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt=
*stmt,
* rewriteQueryForIMMV -- rewrite view definition query for IMMV
*
* count(*) is added for counting distinct tuples in views.
+ * Also, additional hidden columns are added for aggregate values.
*/
Query *
rewriteQueryForIMMV(Query *query, List *colNames)
@@ -446,14 +454,46 @@ rewriteQueryForIMMV(Query *query, List *colNames)
rewritten =3D copyObject(query);
pstate->p_expr_kind =3D EXPR_KIND_SELECT_TARGET;
=20
- /*
- * Convert DISTINCT to GROUP BY and add count(*) for counting distinct
- * tuples in views.
- */
- if (rewritten->distinctClause)
+ /* group keys must be in targetlist */
+ if (rewritten->groupClause)
{
+ ListCell *lc;
+ foreach(lc, rewritten->groupClause)
+ {
+ SortGroupClause *scl =3D (SortGroupClause *) lfirst(lc);
+ TargetEntry *tle =3D get_sortgroupclause_tle(scl, rewritten->targetList=
);
+
+ if (tle->resjunk)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("GROUP BY expression not appearing in select list is not sup=
ported on incrementally maintainable materialized view")));
+ }
+ }
+ /* Convert DISTINCT to GROUP BY. count(*) will be added afterward. */
+ else if (!rewritten->hasAggs && rewritten->distinctClause)
rewritten->groupClause =3D transformDistinctClause(NULL, &rewritten->tar=
getList, rewritten->sortClause, false);
=20
+ /* Add additional columns for aggregate values */
+ if (rewritten->hasAggs)
+ {
+ ListCell *lc;
+ List *aggs =3D NIL;
+ AttrNumber next_resno =3D list_length(rewritten->targetList) + 1;
+
+ foreach(lc, rewritten->targetList)
+ {
+ TargetEntry *tle =3D (TargetEntry *) lfirst(lc);
+ char *resname =3D (colNames =3D=3D NIL ? tle->resname : strVal(list_nth=
(colNames, tle->resno - 1)));
+
+ if (IsA(tle->expr, Aggref))
+ makeIvmAggColumn(pstate, (Aggref *)tle->expr, resname, &next_resno, &a=
ggs);
+ }
+ rewritten->targetList =3D list_concat(rewritten->targetList, aggs);
+ }
+
+ /* Add count(*) for counting distinct tuples in views */
+ if (rewritten->distinctClause || rewritten->hasAggs)
+ {
fn =3D makeFuncCall(list_make1(makeString("count")), NIL, COERCE_EXPLICI=
T_CALL, -1);
fn->agg_star =3D true;
=20
@@ -470,6 +510,91 @@ rewriteQueryForIMMV(Query *query, List *colNames)
return rewritten;
}
=20
+/*
+ * makeIvmAggColumn -- make additional aggregate columns for IVM
+ *
+ * For an aggregate column specified by aggref, additional aggregate colum=
ns
+ * are added, which are used to calculate the new aggregate value in IMMV.
+ * An additional aggregate columns has a name based on resname
+ * (ex. ivm_count_resname), and resno specified by next_resno. The created
+ * columns are returned to aggs, and the resno for the next column is also
+ * returned to next_resno.
+ *
+ * Currently, an additional count() is created for aggref other than count.
+ * In addition, sum() is created for avg aggregate column.
+ */
+void
+makeIvmAggColumn(ParseState *pstate, Aggref *aggref, char *resname, AttrNu=
mber *next_resno, List **aggs)
+{
+ TargetEntry *tle_count;
+ Node *node;
+ FuncCall *fn;
+ Const *dmy_arg =3D makeConst(INT4OID,
+ -1,
+ InvalidOid,
+ sizeof(int32),
+ Int32GetDatum(1),
+ false,
+ true); /* pass by value */
+ const char *aggname =3D get_func_name(aggref->aggfnoid);
+
+ /*
+ * For aggregate functions except count, add count() func with the same a=
rg parameters.
+ * This count result is used for determining if the aggregate value shoul=
d be NULL or not.
+ * Also, add sum() func for avg because we need to calculate an average v=
alue as sum/count.
+ *
+ * XXX: If there are same expressions explicitly in the target list, we c=
an use this instead
+ * of adding new duplicated one.
+ */
+ if (strcmp(aggname, "count") !=3D 0)
+ {
+ fn =3D makeFuncCall(list_make1(makeString("count")), NIL, COERCE_EXPLICI=
T_CALL, -1);
+
+ /* Make a Func with a dummy arg, and then override this by the original =
agg's args. */
+ node =3D ParseFuncOrColumn(pstate, fn->funcname, list_make1(dmy_arg), NU=
LL, fn, false, -1);
+ ((Aggref *)node)->args =3D aggref->args;
+
+ tle_count =3D makeTargetEntry((Expr *) node,
+ *next_resno,
+ pstrdup(makeObjectName("__ivm_count",resname, "_")),
+ false);
+ *aggs =3D lappend(*aggs, tle_count);
+ (*next_resno)++;
+ }
+ if (strcmp(aggname, "avg") =3D=3D 0)
+ {
+ List *dmy_args =3D NIL;
+ ListCell *lc;
+ foreach(lc, aggref->aggargtypes)
+ {
+ Oid typeid =3D lfirst_oid(lc);
+ Type type =3D typeidType(typeid);
+
+ Const *con =3D makeConst(typeid,
+ -1,
+ typeTypeCollation(type),
+ typeLen(type),
+ (Datum) 0,
+ true,
+ typeByVal(type));
+ dmy_args =3D lappend(dmy_args, con);
+ ReleaseSysCache(type);
+ }
+ fn =3D makeFuncCall(list_make1(makeString("sum")), NIL, COERCE_EXPLICIT_=
CALL, -1);
+
+ /* Make a Func with dummy args, and then override this by the original a=
gg's args. */
+ node =3D ParseFuncOrColumn(pstate, fn->funcname, dmy_args, NULL, fn, fal=
se, -1);
+ ((Aggref *)node)->args =3D aggref->args;
+
+ tle_count =3D makeTargetEntry((Expr *) node,
+ *next_resno,
+ pstrdup(makeObjectName("__ivm_sum",resname, "_")),
+ false);
+ *aggs =3D lappend(*aggs, tle_count);
+ (*next_resno)++;
+ }
+}
+
/*
* GetIntoRelEFlags --- compute executor flags needed for CREATE TABLE AS
*
@@ -923,11 +1048,13 @@ CreateIvmTrigger(Oid relOid, Oid viewOid, int16 type=
, int16 timing, bool ex_lock
static void
check_ivm_restriction(Node *node)
{
- check_ivm_restriction_walker(node, NULL);
+ check_ivm_restriction_context context =3D {false};
+
+ check_ivm_restriction_walker(node, &context);
}
=20
static bool
-check_ivm_restriction_walker(Node *node, void *context)
+check_ivm_restriction_walker(Node *node, check_ivm_restriction_context *co=
ntext)
{
if (node =3D=3D NULL)
return false;
@@ -1008,6 +1135,8 @@ check_ivm_restriction_walker(Node *node, void *contex=
t)
}
}
=20
+ context->has_agg |=3D qry->hasAggs;
+
/* restrictions for rtable */
foreach(lc, qry->rtable)
{
@@ -1056,7 +1185,7 @@ check_ivm_restriction_walker(Node *node, void *contex=
t)
=20
}
=20
- query_tree_walker(qry, check_ivm_restriction_walker, NULL, QTW_IGNORE_=
RANGE_TABLE);
+ query_tree_walker(qry, check_ivm_restriction_walker, (void *) context,=
QTW_IGNORE_RANGE_TABLE);
=20
break;
}
@@ -1067,8 +1196,12 @@ check_ivm_restriction_walker(Node *node, void *conte=
xt)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("column name %s is not supported on incrementally maintain=
able materialized view", tle->resname)));
+ if (context->has_agg && !IsA(tle->expr, Aggref) && contain_aggs_of_lev=
el((Node *) tle->expr, 0))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("expression containing an aggregate in it is not supported =
on incrementally maintainable materialized view")));
=20
- expression_tree_walker(node, check_ivm_restriction_walker, NULL);
+ expression_tree_walker(node, check_ivm_restriction_walker, (void *) co=
ntext);
break;
}
case T_JoinExpr:
@@ -1080,15 +1213,128 @@ check_ivm_restriction_walker(Node *node, void *con=
text)
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("OUTER JOIN is not supported on incrementally maintainable=
materialized view")));
=20
- expression_tree_walker(node, check_ivm_restriction_walker, NULL);
+ expression_tree_walker(node, check_ivm_restriction_walker, (void *) co=
ntext);
+ break;
}
- break;
- expression_tree_walker(node, check_ivm_restriction_walker, NULL);
+ case T_Aggref:
+ {
+ /* Check if this supports IVM */
+ Aggref *aggref =3D (Aggref *) node;
+ const char *aggname =3D format_procedure(aggref->aggfnoid);
+
+ if (aggref->aggfilter !=3D NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("aggregate function with FILTER clause is not supported on =
incrementally maintainable materialized view")));
+
+ if (aggref->aggdistinct !=3D NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("aggregate function with DISTINCT arguments is not supporte=
d on incrementally maintainable materialized view")));
+
+ if (aggref->aggorder !=3D NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("aggregate function with ORDER clause is not supported on i=
ncrementally maintainable materialized view")));
+
+ if (!check_aggregate_supports_ivm(aggref->aggfnoid))
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("aggregate function %s is not supported on incrementally ma=
intainable materialized view", aggname)));
+ break;
+ }
+ default:
+ expression_tree_walker(node, check_ivm_restriction_walker, (void *) con=
text);
break;
}
return false;
}
=20
+/*
+ * check_aggregate_supports_ivm
+ *
+ * Check if the given aggregate function is supporting IVM
+ */
+static bool
+check_aggregate_supports_ivm(Oid aggfnoid)
+{
+ switch (aggfnoid)
+ {
+ /* count */
+ case F_COUNT_ANY:
+ case F_COUNT_:
+
+ /* sum */
+ case F_SUM_INT8:
+ case F_SUM_INT4:
+ case F_SUM_INT2:
+ case F_SUM_FLOAT4:
+ case F_SUM_FLOAT8:
+ case F_SUM_MONEY:
+ case F_SUM_INTERVAL:
+ case F_SUM_NUMERIC:
+
+ /* avg */
+ case F_AVG_INT8:
+ case F_AVG_INT4:
+ case F_AVG_INT2:
+ case F_AVG_NUMERIC:
+ case F_AVG_FLOAT4:
+ case F_AVG_FLOAT8:
+ case F_AVG_INTERVAL:
+
+ /* min */
+ case F_MIN_ANYARRAY:
+ case F_MIN_INT8:
+ case F_MIN_INT4:
+ case F_MIN_INT2:
+ case F_MIN_OID:
+ case F_MIN_FLOAT4:
+ case F_MIN_FLOAT8:
+ case F_MIN_DATE:
+ case F_MIN_TIME:
+ case F_MIN_TIMETZ:
+ case F_MIN_MONEY:
+ case F_MIN_TIMESTAMP:
+ case F_MIN_TIMESTAMPTZ:
+ case F_MIN_INTERVAL:
+ case F_MIN_TEXT:
+ case F_MIN_NUMERIC:
+ case F_MIN_BPCHAR:
+ case F_MIN_TID:
+ case F_MIN_ANYENUM:
+ case F_MIN_INET:
+ case F_MIN_PG_LSN:
+
+ /* max */
+ case F_MAX_ANYARRAY:
+ case F_MAX_INT8:
+ case F_MAX_INT4:
+ case F_MAX_INT2:
+ case F_MAX_OID:
+ case F_MAX_FLOAT4:
+ case F_MAX_FLOAT8:
+ case F_MAX_DATE:
+ case F_MAX_TIME:
+ case F_MAX_TIMETZ:
+ case F_MAX_MONEY:
+ case F_MAX_TIMESTAMP:
+ case F_MAX_TIMESTAMPTZ:
+ case F_MAX_INTERVAL:
+ case F_MAX_TEXT:
+ case F_MAX_NUMERIC:
+ case F_MAX_BPCHAR:
+ case F_MAX_TID:
+ case F_MAX_ANYENUM:
+ case F_MAX_INET:
+ case F_MAX_PG_LSN:
+ return true;
+
+ default:
+ return false;
+ }
+}
+
/*
* CreateIndexOnIMMV
*
@@ -1138,7 +1384,30 @@ CreateIndexOnIMMV(Query *query, Relation matviewRel)
index->concurrent =3D false;
index->if_not_exists =3D false;
=20
- if (qry->distinctClause)
+
+ if (qry->groupClause)
+ {
+ /* create unique constraint on GROUP BY expression columns */
+ foreach(lc, qry->groupClause)
+ {
+ SortGroupClause *scl =3D (SortGroupClause *) lfirst(lc);
+ TargetEntry *tle =3D get_sortgroupclause_tle(scl, qry->targetList);
+ Form_pg_attribute attr =3D TupleDescAttr(matviewRel->rd_att, tle->resno=
- 1);
+ IndexElem *iparam;
+
+ iparam =3D makeNode(IndexElem);
+ iparam->name =3D pstrdup(NameStr(attr->attname));
+ iparam->expr =3D NULL;
+ iparam->indexcolname =3D NULL;
+ iparam->collation =3D NIL;
+ iparam->opclass =3D NIL;
+ iparam->opclassopts =3D NIL;
+ iparam->ordering =3D SORTBY_DEFAULT;
+ iparam->nulls_ordering =3D SORTBY_NULLS_DEFAULT;
+ index->indexParams =3D lappend(index->indexParams, iparam);
+ }
+ }
+ else if (qry->distinctClause)
{
/* create unique constraint on all columns */
foreach(lc, qry->targetList)
@@ -1197,7 +1466,7 @@ CreateIndexOnIMMV(Query *query, Relation matviewRel)
(errmsg("could not create an index on materialized view \"%s\" automa=
tically",
RelationGetRelationName(matviewRel)),
errdetail("This target list does not have all the primary key column=
s, "
- "or this view does not contain DISTINCT clause."),
+ "or this view does not contain GROUP BY or DISTINCT clause."),
errhint("Create an index on the materialized view for efficient incr=
emental maintenance.")));
return;
}
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index a27d23434d..219444571a 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -81,6 +81,32 @@ typedef struct
=20
#define MV_INIT_QUERYHASHSIZE 16
=20
+/* MV query type codes */
+#define MV_PLAN_RECALC 1
+#define MV_PLAN_SET_VALUE 2
+
+/*
+ * MI_QueryKey
+ *
+ * The key identifying a prepared SPI plan in our query hashtable
+ */
+typedef struct MV_QueryKey
+{
+ Oid matview_id; /* OID of materialized view */
+ int32 query_type; /* query type ID, see MV_PLAN_XXX above */
+} MV_QueryKey;
+
+/*
+ * MV_QueryHashEntry
+ *
+ * Hash entry for cached plans used to maintain materialized views.
+ */
+typedef struct MV_QueryHashEntry
+{
+ MV_QueryKey key;
+ SPIPlanPtr plan;
+} MV_QueryHashEntry;
+
/*
* MV_TriggerHashEntry
*
@@ -117,8 +143,16 @@ typedef struct MV_TriggerTable
RangeTblEntry *original_rte; /* the original RTE saved before rewriting q=
uery */
} MV_TriggerTable;
=20
+static HTAB *mv_query_cache =3D NULL;
static HTAB *mv_trigger_info =3D NULL;
=20
+/* kind of IVM operation for the view */
+typedef enum
+{
+ IVM_ADD,
+ IVM_SUB
+} IvmOp;
+
/* ENR name for materialized view delta */
#define NEW_DELTA_ENRNAME "new_delta"
#define OLD_DELTA_ENRNAME "old_delta"
@@ -152,7 +186,7 @@ static RangeTblEntry *get_prestate_rte(RangeTblEntry *r=
te, MV_TriggerTable *tabl
QueryEnvironment *queryEnv);
static RangeTblEntry *union_ENRs(RangeTblEntry *rte, Oid relid, List *enr_=
rtes, const char *prefix,
QueryEnvironment *queryEnv);
-static Query *rewrite_query_for_distinct(Query *query, ParseState *pstate);
+static Query *rewrite_query_for_distinct_and_aggregates(Query *query, Pars=
eState *pstate);
=20
static void calc_delta(MV_TriggerTable *table, int rte_index, Query *query,
DestReceiver *dest_old, DestReceiver *dest_new,
@@ -163,19 +197,48 @@ static Query *rewrite_query_for_postupdate_state(Quer=
y *query, MV_TriggerTable *
static void apply_delta(Oid matviewOid, Tuplestorestate *old_tuplestores, =
Tuplestorestate *new_tuplestores,
TupleDesc tupdesc_old, TupleDesc tupdesc_new,
Query *query, bool use_count, char *count_colname);
+static void append_set_clause_for_count(const char *resname, StringInfo bu=
f_old,
+ StringInfo buf_new,StringInfo aggs_list);
+static void append_set_clause_for_sum(const char *resname, StringInfo buf_=
old,
+ StringInfo buf_new, StringInfo aggs_list);
+static void append_set_clause_for_avg(const char *resname, StringInfo buf_=
old,
+ StringInfo buf_new, StringInfo aggs_list,
+ const char *aggtype);
+static void append_set_clause_for_minmax(const char *resname, StringInfo b=
uf_old,
+ StringInfo buf_new, StringInfo aggs_list,
+ bool is_min);
+static char *get_operation_string(IvmOp op, const char *col, const char *a=
rg1, const char *arg2,
+ const char* count_col, const char *castType);
+static char *get_null_condition_string(IvmOp op, const char *arg1, const c=
har *arg2,
+ const char* count_col);
static void apply_old_delta(const char *matviewname, const char *deltaname=
_old,
List *keys);
static void apply_old_delta_with_count(const char *matviewname, const char=
*deltaname_old,
- List *keys, const char *count_colname);
+ List *keys, StringInfo aggs_list, StringInfo aggs_set,
+ List *minmax_list, List *is_min_list,
+ const char *count_colname,
+ SPITupleTable **tuptable_recalc, uint64 *num_recalc);
static void apply_new_delta(const char *matviewname, const char *deltaname=
_new,
StringInfo target_list);
static void apply_new_delta_with_count(const char *matviewname, const char=
* deltaname_new,
- List *keys, StringInfo target_list, const char* count_colname);
+ List *keys, StringInfo target_list, StringInfo aggs_set,
+ const char* count_colname);
static char *get_matching_condition_string(List *keys);
+static char *get_returning_string(List *minmax_list, List *is_min_list, Li=
st *keys);
+static char *get_minmax_recalc_condition_string(List *minmax_list, List *i=
s_min_list);
+static char *get_select_for_recalc_string(List *keys);
+static void recalc_and_set_values(SPITupleTable *tuptable_recalc, int64 nu=
m_tuples,
+ List *namelist, List *keys, Relation matviewRel);
+static SPIPlanPtr get_plan_for_recalc(Oid matviewOid, List *namelist, List=
*keys, Oid *keyTypes);
+static SPIPlanPtr get_plan_for_set_values(Oid matviewOid, char *matviewnam=
e, List *namelist,
+ Oid *valTypes);
static void generate_equal(StringInfo querybuf, Oid opttype,
const char *leftop, const char *rightop);
=20
static void mv_InitHashTables(void);
+static SPIPlanPtr mv_FetchPreparedPlan(MV_QueryKey *key);
+static void mv_HashPreparedPlan(MV_QueryKey *key, SPIPlanPtr plan);
+static void mv_BuildQueryKey(MV_QueryKey *key, Oid matview_id, int32 query=
_type);
static void clean_up_IVM_hash_entry(MV_TriggerHashEntry *entry);
=20
static List *get_securityQuals(Oid relId, int rt_index, Query *query);
@@ -1441,8 +1504,8 @@ IVM_immediate_maintenance(PG_FUNCTION_ARGS)
rewritten =3D rewrite_query_for_preupdate_state(rewritten, entry->tables,
entry->xid, entry->cid,
pstate);
- /* Rewrite for DISTINCT clause */
- rewritten =3D rewrite_query_for_distinct(rewritten, pstate);
+ /* Rewrite for DISTINCT clause and aggregates functions */
+ rewritten =3D rewrite_query_for_distinct_and_aggregates(rewritten, pstate=
);
=20
/* Create tuplestores to store view deltas */
if (entry->has_old)
@@ -1493,7 +1556,7 @@ IVM_immediate_maintenance(PG_FUNCTION_ARGS)
=20
count_colname =3D pstrdup("__ivm_count__");
=20
- if (query->distinctClause)
+ if (query->hasAggs || query->distinctClause)
use_count =3D true;
=20
/* calculate delta tables */
@@ -1855,17 +1918,34 @@ union_ENRs(RangeTblEntry *rte, Oid relid, List *enr=
_rtes, const char *prefix,
}
=20
/*
- * rewrite_query_for_distinct
+ * rewrite_query_for_distinct_and_aggregates
*
- * Rewrite query for counting DISTINCT clause.
+ * Rewrite query for counting DISTINCT clause and aggregate functions.
*/
static Query *
-rewrite_query_for_distinct(Query *query, ParseState *pstate)
+rewrite_query_for_distinct_and_aggregates(Query *query, ParseState *pstate)
{
TargetEntry *tle_count;
FuncCall *fn;
Node *node;
=20
+ /* For aggregate views */
+ if (query->hasAggs)
+ {
+ ListCell *lc;
+ List *aggs =3D NIL;
+ AttrNumber next_resno =3D list_length(query->targetList) + 1;
+
+ foreach(lc, query->targetList)
+ {
+ TargetEntry *tle =3D (TargetEntry *) lfirst(lc);
+
+ if (IsA(tle->expr, Aggref))
+ makeIvmAggColumn(pstate, (Aggref *)tle->expr, tle->resname, &next_resn=
o, &aggs);
+ }
+ query->targetList =3D list_concat(query->targetList, aggs);
+ }
+
/* Add count(*) for counting distinct tuples in views */
fn =3D makeFuncCall(list_make1(makeString("count")), NIL, COERCE_EXPLICIT=
_CALL, -1);
fn->agg_star =3D true;
@@ -1934,6 +2014,8 @@ rewrite_query_for_postupdate_state(Query *query, MV_T=
riggerTable *table, int rte
return query;
}
=20
+#define IVM_colname(type, col) makeObjectName("__ivm_" type, col, "_")
+
/*
* apply_delta
*
@@ -1947,11 +2029,16 @@ apply_delta(Oid matviewOid, Tuplestorestate *old_tu=
plestores, Tuplestorestate *n
{
StringInfoData querybuf;
StringInfoData target_list_buf;
+ StringInfo aggs_list_buf =3D NULL;
+ StringInfo aggs_set_old =3D NULL;
+ StringInfo aggs_set_new =3D NULL;
Relation matviewRel;
char *matviewname;
ListCell *lc;
int i;
List *keys =3D NIL;
+ List *minmax_list =3D NIL;
+ List *is_min_list =3D NIL;
=20
=20
/*
@@ -1969,6 +2056,15 @@ apply_delta(Oid matviewOid, Tuplestorestate *old_tup=
lestores, Tuplestorestate *n
initStringInfo(&querybuf);
initStringInfo(&target_list_buf);
=20
+ if (query->hasAggs)
+ {
+ if (old_tuplestores && tuplestore_tuple_count(old_tuplestores) > 0)
+ aggs_set_old =3D makeStringInfo();
+ if (new_tuplestores && tuplestore_tuple_count(new_tuplestores) > 0)
+ aggs_set_new =3D makeStringInfo();
+ aggs_list_buf =3D makeStringInfo();
+ }
+
/* build string of target list */
for (i =3D 0; i < matviewRel->rd_att->natts; i++)
{
@@ -1992,7 +2088,65 @@ apply_delta(Oid matviewOid, Tuplestorestate *old_tup=
lestores, Tuplestorestate *n
if (tle->resjunk)
continue;
=20
- keys =3D lappend(keys, resname);
+ /*
+ * For views without aggregates, all attributes are used as keys to iden=
tify a
+ * tuple in a view.
+ */
+ if (!query->hasAggs)
+ keys =3D lappend(keys, attr);
+
+ /* For views with aggregates, we need to build SET clause for updating a=
ggregate
+ * values. */
+ if (query->hasAggs && IsA(tle->expr, Aggref))
+ {
+ Aggref *aggref =3D (Aggref *) tle->expr;
+ const char *aggname =3D get_func_name(aggref->aggfnoid);
+
+ /*
+ * We can use function names here because it is already checked if these
+ * can be used in IMMV by its OID at the definition time.
+ */
+
+ /* count */
+ if (!strcmp(aggname, "count"))
+ append_set_clause_for_count(resname, aggs_set_old, aggs_set_new, aggs_=
list_buf);
+
+ /* sum */
+ else if (!strcmp(aggname, "sum"))
+ append_set_clause_for_sum(resname, aggs_set_old, aggs_set_new, aggs_li=
st_buf);
+
+ /* avg */
+ else if (!strcmp(aggname, "avg"))
+ append_set_clause_for_avg(resname, aggs_set_old, aggs_set_new, aggs_li=
st_buf,
+ format_type_be(aggref->aggtype));
+
+ /* min/max */
+ else if (!strcmp(aggname, "min") || !strcmp(aggname, "max"))
+ {
+ bool is_min =3D (!strcmp(aggname, "min"));
+
+ append_set_clause_for_minmax(resname, aggs_set_old, aggs_set_new, aggs=
_list_buf, is_min);
+
+ /* make a resname list of min and max aggregates */
+ minmax_list =3D lappend(minmax_list, resname);
+ is_min_list =3D lappend_int(is_min_list, is_min);
+ }
+ else
+ elog(ERROR, "unsupported aggregate function: %s", aggname);
+ }
+ }
+
+ /* If we have GROUP BY clause, we use its entries as keys. */
+ if (query->hasAggs && query->groupClause)
+ {
+ foreach (lc, query->groupClause)
+ {
+ SortGroupClause *sgcl =3D (SortGroupClause *) lfirst(lc);
+ TargetEntry *tle =3D get_sortgroupclause_tle(sgcl, query->targetList);
+ Form_pg_attribute attr =3D TupleDescAttr(matviewRel->rd_att, tle->resno=
- 1);
+
+ keys =3D lappend(keys, attr);
+ }
}
=20
/* Start maintaining the materialized view. */
@@ -2006,6 +2160,8 @@ apply_delta(Oid matviewOid, Tuplestorestate *old_tupl=
estores, Tuplestorestate *n
if (old_tuplestores && tuplestore_tuple_count(old_tuplestores) > 0)
{
EphemeralNamedRelation enr =3D palloc(sizeof(EphemeralNamedRelationData)=
);
+ SPITupleTable *tuptable_recalc =3D NULL;
+ uint64 num_recalc;
int rc;
=20
/* convert tuplestores to ENR, and register for SPI */
@@ -2023,10 +2179,19 @@ apply_delta(Oid matviewOid, Tuplestorestate *old_tu=
plestores, Tuplestorestate *n
if (use_count)
/* apply old delta and get rows to be recalculated */
apply_old_delta_with_count(matviewname, OLD_DELTA_ENRNAME,
- keys, count_colname);
+ keys, aggs_list_buf, aggs_set_old,
+ minmax_list, is_min_list,
+ count_colname, &tuptable_recalc, &num_recalc);
else
apply_old_delta(matviewname, OLD_DELTA_ENRNAME, keys);
=20
+ /*
+ * If we have min or max, we might have to recalculate aggregate values =
from base tables
+ * on some tuples. TIDs and keys such tuples are returned as a result of=
the above query.
+ */
+ if (minmax_list && tuptable_recalc)
+ recalc_and_set_values(tuptable_recalc, num_recalc, minmax_list, keys, m=
atviewRel);
+
}
/* For tuple insertion */
if (new_tuplestores && tuplestore_tuple_count(new_tuplestores) > 0)
@@ -2049,7 +2214,7 @@ apply_delta(Oid matviewOid, Tuplestorestate *old_tupl=
estores, Tuplestorestate *n
/* apply new delta */
if (use_count)
apply_new_delta_with_count(matviewname, NEW_DELTA_ENRNAME,
- keys, &target_list_buf, count_colname);
+ keys, aggs_set_new, &target_list_buf, count_colname);
else
apply_new_delta(matviewname, NEW_DELTA_ENRNAME, &target_list_buf);
}
@@ -2064,49 +2229,410 @@ apply_delta(Oid matviewOid, Tuplestorestate *old_t=
uplestores, Tuplestorestate *n
elog(ERROR, "SPI_finish failed");
}
=20
+/*
+ * append_set_clause_for_count
+ *
+ * Append SET clause string for count aggregation to given buffers.
+ * Also, append resnames required for calculating the aggregate value.
+ */
+static void
+append_set_clause_for_count(const char *resname, StringInfo buf_old,
+ StringInfo buf_new,StringInfo aggs_list)
+{
+ /* For tuple deletion */
+ if (buf_old)
+ {
+ /* resname =3D mv.resname - t.resname */
+ appendStringInfo(buf_old,
+ ", %s =3D %s",
+ quote_qualified_identifier(NULL, resname),
+ get_operation_string(IVM_SUB, resname, "mv", "t", NULL, NULL));
+ }
+ /* For tuple insertion */
+ if (buf_new)
+ {
+ /* resname =3D mv.resname + diff.resname */
+ appendStringInfo(buf_new,
+ ", %s =3D %s",
+ quote_qualified_identifier(NULL, resname),
+ get_operation_string(IVM_ADD, resname, "mv", "diff", NULL, NULL));
+ }
+
+ appendStringInfo(aggs_list, ", %s",
+ quote_qualified_identifier("diff", resname)
+ );
+}
+
+/*
+ * append_set_clause_for_sum
+ *
+ * Append SET clause string for sum aggregation to given buffers.
+ * Also, append resnames required for calculating the aggregate value.
+ */
+static void
+append_set_clause_for_sum(const char *resname, StringInfo buf_old,
+ StringInfo buf_new, StringInfo aggs_list)
+{
+ char *count_col =3D IVM_colname("count", resname);
+
+ /* For tuple deletion */
+ if (buf_old)
+ {
+ /* sum =3D mv.sum - t.sum */
+ appendStringInfo(buf_old,
+ ", %s =3D %s",
+ quote_qualified_identifier(NULL, resname),
+ get_operation_string(IVM_SUB, resname, "mv", "t", count_col, NULL)
+ );
+ /* count =3D mv.count - t.count */
+ appendStringInfo(buf_old,
+ ", %s =3D %s",
+ quote_qualified_identifier(NULL, count_col),
+ get_operation_string(IVM_SUB, count_col, "mv", "t", NULL, NULL)
+ );
+ }
+ /* For tuple insertion */
+ if (buf_new)
+ {
+ /* sum =3D mv.sum + diff.sum */
+ appendStringInfo(buf_new,
+ ", %s =3D %s",
+ quote_qualified_identifier(NULL, resname),
+ get_operation_string(IVM_ADD, resname, "mv", "diff", count_col, NULL)
+ );
+ /* count =3D mv.count + diff.count */
+ appendStringInfo(buf_new,
+ ", %s =3D %s",
+ quote_qualified_identifier(NULL, count_col),
+ get_operation_string(IVM_ADD, count_col, "mv", "diff", NULL, NULL)
+ );
+ }
+
+ appendStringInfo(aggs_list, ", %s, %s",
+ quote_qualified_identifier("diff", resname),
+ quote_qualified_identifier("diff", IVM_colname("count", resname))
+ );
+}
+
+/*
+ * append_set_clause_for_avg
+ *
+ * Append SET clause string for avg aggregation to given buffers.
+ * Also, append resnames required for calculating the aggregate value.
+ */
+static void
+append_set_clause_for_avg(const char *resname, StringInfo buf_old,
+ StringInfo buf_new, StringInfo aggs_list,
+ const char *aggtype)
+{
+ char *sum_col =3D IVM_colname("sum", resname);
+ char *count_col =3D IVM_colname("count", resname);
+
+ /* For tuple deletion */
+ if (buf_old)
+ {
+ /* avg =3D (mv.sum - t.sum)::aggtype / (mv.count - t.count) */
+ appendStringInfo(buf_old,
+ ", %s =3D %s OPERATOR(pg_catalog./) %s",
+ quote_qualified_identifier(NULL, resname),
+ get_operation_string(IVM_SUB, sum_col, "mv", "t", count_col, aggtype),
+ get_operation_string(IVM_SUB, count_col, "mv", "t", NULL, NULL)
+ );
+ /* sum =3D mv.sum - t.sum */
+ appendStringInfo(buf_old,
+ ", %s =3D %s",
+ quote_qualified_identifier(NULL, sum_col),
+ get_operation_string(IVM_SUB, sum_col, "mv", "t", count_col, NULL)
+ );
+ /* count =3D mv.count - t.count */
+ appendStringInfo(buf_old,
+ ", %s =3D %s",
+ quote_qualified_identifier(NULL, count_col),
+ get_operation_string(IVM_SUB, count_col, "mv", "t", NULL, NULL)
+ );
+
+ }
+ /* For tuple insertion */
+ if (buf_new)
+ {
+ /* avg =3D (mv.sum + diff.sum)::aggtype / (mv.count + diff.count) */
+ appendStringInfo(buf_new,
+ ", %s =3D %s OPERATOR(pg_catalog./) %s",
+ quote_qualified_identifier(NULL, resname),
+ get_operation_string(IVM_ADD, sum_col, "mv", "diff", count_col, aggtype=
),
+ get_operation_string(IVM_ADD, count_col, "mv", "diff", NULL, NULL)
+ );
+ /* sum =3D mv.sum + diff.sum */
+ appendStringInfo(buf_new,
+ ", %s =3D %s",
+ quote_qualified_identifier(NULL, sum_col),
+ get_operation_string(IVM_ADD, sum_col, "mv", "diff", count_col, NULL)
+ );
+ /* count =3D mv.count + diff.count */
+ appendStringInfo(buf_new,
+ ", %s =3D %s",
+ quote_qualified_identifier(NULL, count_col),
+ get_operation_string(IVM_ADD, count_col, "mv", "diff", NULL, NULL)
+ );
+ }
+
+ appendStringInfo(aggs_list, ", %s, %s, %s",
+ quote_qualified_identifier("diff", resname),
+ quote_qualified_identifier("diff", IVM_colname("sum", resname)),
+ quote_qualified_identifier("diff", IVM_colname("count", resname))
+ );
+}
+
+/*
+ * append_set_clause_for_minmax
+ *
+ * Append SET clause string for min or max aggregation to given buffers.
+ * Also, append resnames required for calculating the aggregate value.
+ * is_min is true if this is min, false if not.
+ */
+static void
+append_set_clause_for_minmax(const char *resname, StringInfo buf_old,
+ StringInfo buf_new, StringInfo aggs_list,
+ bool is_min)
+{
+ char *count_col =3D IVM_colname("count", resname);
+
+ /* For tuple deletion */
+ if (buf_old)
+ {
+ /*
+ * If the new value doesn't became NULL then use the value remaining
+ * in the view although this will be recomputated afterwords.
+ */
+ appendStringInfo(buf_old,
+ ", %s =3D CASE WHEN %s THEN NULL ELSE %s END",
+ quote_qualified_identifier(NULL, resname),
+ get_null_condition_string(IVM_SUB, "mv", "t", count_col),
+ quote_qualified_identifier("mv", resname)
+ );
+ /* count =3D mv.count - t.count */
+ appendStringInfo(buf_old,
+ ", %s =3D %s",
+ quote_qualified_identifier(NULL, count_col),
+ get_operation_string(IVM_SUB, count_col, "mv", "t", NULL, NULL)
+ );
+ }
+ /* For tuple insertion */
+ if (buf_new)
+ {
+ /*
+ * min =3D LEAST(mv.min, diff.min)
+ * max =3D GREATEST(mv.max, diff.max)
+ */
+ appendStringInfo(buf_new,
+ ", %s =3D CASE WHEN %s THEN NULL ELSE %s(%s,%s) END",
+ quote_qualified_identifier(NULL, resname),
+ get_null_condition_string(IVM_ADD, "mv", "diff", count_col),
+
+ is_min ? "LEAST" : "GREATEST",
+ quote_qualified_identifier("mv", resname),
+ quote_qualified_identifier("diff", resname)
+ );
+ /* count =3D mv.count + diff.count */
+ appendStringInfo(buf_new,
+ ", %s =3D %s",
+ quote_qualified_identifier(NULL, count_col),
+ get_operation_string(IVM_ADD, count_col, "mv", "diff", NULL, NULL)
+ );
+ }
+
+ appendStringInfo(aggs_list, ", %s, %s",
+ quote_qualified_identifier("diff", resname),
+ quote_qualified_identifier("diff", IVM_colname("count", resname))
+ );
+}
+
+/*
+ * get_operation_string
+ *
+ * Build a string to calculate the new aggregate values.
+ */
+static char *
+get_operation_string(IvmOp op, const char *col, const char *arg1, const ch=
ar *arg2,
+ const char* count_col, const char *castType)
+{
+ StringInfoData buf;
+ StringInfoData castString;
+ char *col1 =3D quote_qualified_identifier(arg1, col);
+ char *col2 =3D quote_qualified_identifier(arg2, col);
+ char op_char =3D (op =3D=3D IVM_SUB ? '-' : '+');
+
+ initStringInfo(&buf);
+ initStringInfo(&castString);
+
+ if (castType)
+ appendStringInfo(&castString, "::%s", castType);
+
+ if (!count_col)
+ {
+ /*
+ * If the attributes don't have count columns then calc the result
+ * by using the operator simply.
+ */
+ appendStringInfo(&buf, "(%s OPERATOR(pg_catalog.%c) %s)%s",
+ col1, op_char, col2, castString.data);
+ }
+ else
+ {
+ /*
+ * If the attributes have count columns then consider the condition
+ * where the result becomes NULL.
+ */
+ char *null_cond =3D get_null_condition_string(op, arg1, arg2, count_col);
+
+ appendStringInfo(&buf,
+ "(CASE WHEN %s THEN NULL "
+ "WHEN %s IS NULL THEN %s "
+ "WHEN %s IS NULL THEN %s "
+ "ELSE (%s OPERATOR(pg_catalog.%c) %s)%s END)",
+ null_cond,
+ col1, col2,
+ col2, col1,
+ col1, op_char, col2, castString.data
+ );
+ }
+
+ return buf.data;
+}
+
+/*
+ * get_null_condition_string
+ *
+ * Build a predicate string for CASE clause to check if an aggregate value
+ * will became NULL after the given operation is applied.
+ */
+static char *
+get_null_condition_string(IvmOp op, const char *arg1, const char *arg2,
+ const char* count_col)
+{
+ StringInfoData null_cond;
+ initStringInfo(&null_cond);
+
+ switch (op)
+ {
+ case IVM_ADD:
+ appendStringInfo(&null_cond,
+ "%s OPERATOR(pg_catalog.=3D) 0 AND %s OPERATOR(pg_catalog.=3D) 0",
+ quote_qualified_identifier(arg1, count_col),
+ quote_qualified_identifier(arg2, count_col)
+ );
+ break;
+ case IVM_SUB:
+ appendStringInfo(&null_cond,
+ "%s OPERATOR(pg_catalog.=3D) %s",
+ quote_qualified_identifier(arg1, count_col),
+ quote_qualified_identifier(arg2, count_col)
+ );
+ break;
+ default:
+ elog(ERROR,"unknown operation");
+ }
+
+ return null_cond.data;
+}
+
+
/*
* apply_old_delta_with_count
*
* Execute a query for applying a delta table given by deltname_old
* which contains tuples to be deleted from to a materialized view given by
* matviewname. This is used when counting is required, that is, the view
- * has aggregate or distinct.
+ * has aggregate or distinct. Also, when a table in EXISTS sub queries
+ * is modified.
+ *
+ * If the view desn't have aggregates or has GROUP BY, this requires a keys
+ * list to identify a tuple in the view. If the view has aggregates, this
+ * requires strings representing resnames of aggregates and SET clause for
+ * updating aggregate values.
+ *
+ * If the view has min or max aggregate, this requires a list of resnames =
of
+ * min/max aggregates and a list of boolean which represents which entries=
in
+ * minmax_list is min. These are necessary to check if we need to recalcul=
ate
+ * min or max aggregate values. In this case, this query returns TID and k=
eys
+ * of tuples which need to be recalculated. This result and the number of=
rows
+ * are stored in tuptables and num_recalc repectedly.
+ *
*/
static void
apply_old_delta_with_count(const char *matviewname, const char *deltaname_=
old,
- List *keys, const char *count_colname)
+ List *keys, StringInfo aggs_list, StringInfo aggs_set,
+ List *minmax_list, List *is_min_list,
+ const char *count_colname,
+ SPITupleTable **tuptable_recalc, uint64 *num_recalc)
{
StringInfoData querybuf;
char *match_cond;
+ char *updt_returning =3D "";
+ char *select_for_recalc =3D "SELECT";
+ bool agg_without_groupby =3D (list_length(keys) =3D=3D 0);
+
+ Assert(tuptable_recalc !=3D NULL);
+ Assert(num_recalc !=3D NULL);
=20
/* build WHERE condition for searching tuples to be deleted */
match_cond =3D get_matching_condition_string(keys);
=20
+ /*
+ * We need a special RETURNING clause and SELECT statement for min/max to
+ * check which tuple needs re-calculation from base tables.
+ */
+ if (minmax_list)
+ {
+ updt_returning =3D get_returning_string(minmax_list, is_min_list, keys);
+ select_for_recalc =3D get_select_for_recalc_string(keys);
+ }
+
/* Search for matching tuples from the view and update or delete if found=
. */
initStringInfo(&querybuf);
appendStringInfo(&querybuf,
"WITH t AS (" /* collecting tid of target tuples in the view */
"SELECT diff.%s, " /* count column */
- "(diff.%s OPERATOR(pg_catalog.=3D) mv.%s) AS for_dlt, "
+ "(diff.%s OPERATOR(pg_catalog.=3D) mv.%s AND %s) AS for_dlt, "
"mv.ctid "
+ "%s " /* aggregate columns */
"FROM %s AS mv, %s AS diff "
"WHERE %s" /* tuple matching condition */
"), updt AS (" /* update a tuple if this is not to be deleted */
"UPDATE %s AS mv SET %s =3D mv.%s OPERATOR(pg_catalog.-) t.%s "
+ "%s" /* SET clauses for aggregates */
"FROM t WHERE mv.ctid OPERATOR(pg_catalog.=3D) t.ctid AND NOT for_dl=
t "
+ "%s" /* RETURNING clause for recalc infomation */
"), dlt AS (" /* delete a tuple if this is to be deleted */
"DELETE FROM %s AS mv USING t "
"WHERE mv.ctid OPERATOR(pg_catalog.=3D) t.ctid AND for_dlt"
- ")",
+ ") %s", /* SELECT returning which tuples need to be recalculate=
d */
count_colname,
- count_colname, count_colname,
+ count_colname, count_colname, (agg_without_groupby ? "false" : "true"=
),
+ (aggs_list !=3D NULL ? aggs_list->data : ""),
matviewname, deltaname_old,
match_cond,
matviewname, count_colname, count_colname, count_colname,
- matviewname);
+ (aggs_set !=3D NULL ? aggs_set->data : ""),
+ updt_returning,
+ matviewname,
+ select_for_recalc);
=20
- if (SPI_exec(querybuf.data, 0) !=3D SPI_OK_DELETE)
+ if (SPI_exec(querybuf.data, 0) !=3D SPI_OK_SELECT)
elog(ERROR, "SPI_exec failed: %s", querybuf.data);
+
+
+ /* Return tuples to be recalculated. */
+ if (minmax_list)
+ {
+ *tuptable_recalc =3D SPI_tuptable;
+ *num_recalc =3D SPI_processed;
+ }
+ else
+ {
+ *tuptable_recalc =3D NULL;
+ *num_recalc =3D 0;
+ }
}
=20
/*
@@ -2166,10 +2692,15 @@ apply_old_delta(const char *matviewname, const char=
*deltaname_old,
* matviewname. This is used when counting is required, that is, the view
* has aggregate or distinct. Also, when a table in EXISTS sub queries
* is modified.
+ *
+ * If the view desn't have aggregates or has GROUP BY, this requires a keys
+ * list to identify a tuple in the view. If the view has aggregates, this
+ * requires strings representing SET clause for updating aggregate values.
*/
static void
apply_new_delta_with_count(const char *matviewname, const char* deltaname_=
new,
- List *keys, StringInfo target_list, const char* count_colname)
+ List *keys, StringInfo aggs_set, StringInfo target_list,
+ const char* count_colname)
{
StringInfoData querybuf;
StringInfoData returning_keys;
@@ -2200,6 +2731,7 @@ apply_new_delta_with_count(const char *matviewname, c=
onst char* deltaname_new,
appendStringInfo(&querybuf,
"WITH updt AS (" /* update a tuple if this exists in the view */
"UPDATE %s AS mv SET %s =3D mv.%s OPERATOR(pg_catalog.+) diff.%s "
+ "%s " /* SET clauses for aggregates */
"FROM %s AS diff "
"WHERE %s " /* tuple matching condition */
"RETURNING %s" /* returning keys of updated tuples */
@@ -2207,6 +2739,7 @@ apply_new_delta_with_count(const char *matviewname, c=
onst char* deltaname_new,
"SELECT %s FROM %s AS diff "
"WHERE NOT EXISTS (SELECT 1 FROM updt AS mv WHERE %s);",
matviewname, count_colname, count_colname, count_colname,
+ (aggs_set !=3D NULL ? aggs_set->data : ""),
deltaname_new,
match_cond,
returning_keys.data,
@@ -2281,6 +2814,349 @@ get_matching_condition_string(List *keys)
return match_cond.data;
}
=20
+/*
+ * get_returning_string
+ *
+ * Build a string for RETURNING clause of UPDATE used in apply_old_delta_w=
ith_count.
+ * This clause returns ctid and a boolean value that indicates if we need =
to
+ * recalculate min or max value, for each updated row.
+ */
+static char *
+get_returning_string(List *minmax_list, List *is_min_list, List *keys)
+{
+ StringInfoData returning;
+ char *recalc_cond;
+ ListCell *lc;
+
+ Assert(minmax_list !=3D NIL && is_min_list !=3D NIL);
+ recalc_cond =3D get_minmax_recalc_condition_string(minmax_list, is_min_li=
st);
+
+ initStringInfo(&returning);
+
+ appendStringInfo(&returning, "RETURNING mv.ctid AS tid, (%s) AS recalc", =
recalc_cond);
+ foreach (lc, keys)
+ {
+ Form_pg_attribute attr =3D (Form_pg_attribute) lfirst(lc);
+ char *resname =3D NameStr(attr->attname);
+ appendStringInfo(&returning, ", %s", quote_qualified_identifier("mv", re=
sname));
+ }
+
+ return returning.data;
+}
+
+/*
+ * get_minmax_recalc_condition_string
+ *
+ * Build a predicate string for checking if any min/max aggregate
+ * value needs to be recalculated.
+ */
+static char *
+get_minmax_recalc_condition_string(List *minmax_list, List *is_min_list)
+{
+ StringInfoData recalc_cond;
+ ListCell *lc1, *lc2;
+
+ initStringInfo(&recalc_cond);
+
+ Assert (list_length(minmax_list) =3D=3D list_length(is_min_list));
+
+ forboth (lc1, minmax_list, lc2, is_min_list)
+ {
+ char *resname =3D (char *) lfirst(lc1);
+ bool is_min =3D (bool) lfirst_int(lc2);
+ char *op_str =3D (is_min ? ">=3D" : "<=3D");
+
+ appendStringInfo(&recalc_cond, "%s OPERATOR(pg_catalog.%s) %s",
+ quote_qualified_identifier("mv", resname),
+ op_str,
+ quote_qualified_identifier("t", resname)
+ );
+
+ if (lnext(minmax_list, lc1))
+ appendStringInfo(&recalc_cond, " OR ");
+ }
+
+ return recalc_cond.data;
+}
+
+/*
+ * get_select_for_recalc_string
+ *
+ * Build a query to return tid and keys of tuples which need
+ * recalculation. This is used as the result of the query
+ * built by apply_old_delta.
+ */
+static char *
+get_select_for_recalc_string(List *keys)
+{
+ StringInfoData qry;
+ ListCell *lc;
+
+ initStringInfo(&qry);
+
+ appendStringInfo(&qry, "SELECT tid");
+ foreach (lc, keys)
+ {
+ Form_pg_attribute attr =3D (Form_pg_attribute) lfirst(lc);
+ appendStringInfo(&qry, ", %s", NameStr(attr->attname));
+ }
+
+ appendStringInfo(&qry, " FROM updt WHERE recalc");
+
+ return qry.data;
+}
+
+/*
+ * recalc_and_set_values
+ *
+ * Recalculate tuples in a materialized from base tables and update these.
+ * The tuples which needs recalculation are specified by keys, and resnames
+ * of columns to be updated are specified by namelist. TIDs and key values
+ * are given by tuples in tuptable_recalc. Its first attribute must be TID
+ * and key values must be following this.
+ */
+static void
+recalc_and_set_values(SPITupleTable *tuptable_recalc, int64 num_tuples,
+ List *namelist, List *keys, Relation matviewRel)
+{
+ TupleDesc tupdesc_recalc =3D tuptable_recalc->tupdesc;
+ Oid *keyTypes =3D NULL, *types =3D NULL;
+ char *keyNulls =3D NULL, *nulls =3D NULL;
+ Datum *keyVals =3D NULL, *vals =3D NULL;
+ int num_vals =3D list_length(namelist);
+ int num_keys =3D list_length(keys);
+ uint64 i;
+ Oid matviewOid;
+ char *matviewname;
+
+ matviewOid =3D RelationGetRelid(matviewRel);
+ matviewname =3D quote_qualified_identifier(get_namespace_name(RelationGet=
Namespace(matviewRel)),
+ RelationGetRelationName(matviewRel));
+
+ /* If we have keys, initialize arrays for them. */
+ if (keys)
+ {
+ keyTypes =3D palloc(sizeof(Oid) * num_keys);
+ keyNulls =3D palloc(sizeof(char) * num_keys);
+ keyVals =3D palloc(sizeof(Datum) * num_keys);
+ /* a tuple contains keys to be recalculated and ctid to be updated*/
+ Assert(tupdesc_recalc->natts =3D=3D num_keys + 1);
+
+ /* Types of key attributes */
+ for (i =3D 0; i < num_keys; i++)
+ keyTypes[i] =3D TupleDescAttr(tupdesc_recalc, i + 1)->atttypid;
+ }
+
+ /* allocate memory for all attribute names and tid */
+ types =3D palloc(sizeof(Oid) * (num_vals + 1));
+ nulls =3D palloc(sizeof(char) * (num_vals + 1));
+ vals =3D palloc(sizeof(Datum) * (num_vals + 1));
+
+ /* For each tuple which needs recalculation */
+ for (i =3D 0; i < num_tuples; i++)
+ {
+ int j;
+ bool isnull;
+ SPIPlanPtr plan;
+ SPITupleTable *tuptable_newvals;
+ TupleDesc tupdesc_newvals;
+
+ /* Set group key values as parameters if needed. */
+ if (keys)
+ {
+ for (j =3D 0; j < num_keys; j++)
+ {
+ keyVals[j] =3D SPI_getbinval(tuptable_recalc->vals[i], tupdesc_recalc,=
j + 2, &isnull);
+ if (isnull)
+ keyNulls[j] =3D 'n';
+ else
+ keyNulls[j] =3D ' ';
+ }
+ }
+
+ /*
+ * Get recalculated values from base tables. The result must be
+ * only one tuple thich contains the new values for specified keys.
+ */
+ plan =3D get_plan_for_recalc(matviewOid, namelist, keys, keyTypes);
+ if (SPI_execute_plan(plan, keyVals, keyNulls, false, 0) !=3D SPI_OK_SELE=
CT)
+ elog(ERROR, "SPI_execute_plan");
+ if (SPI_processed !=3D 1)
+ elog(ERROR, "SPI_execute_plan returned zero or more than one rows");
+
+ tuptable_newvals =3D SPI_tuptable;
+ tupdesc_newvals =3D tuptable_newvals->tupdesc;
+
+ Assert(tupdesc_newvals->natts =3D=3D num_vals);
+
+ /* Set the new values as parameters */
+ for (j =3D 0; j < tupdesc_newvals->natts; j++)
+ {
+ if (i =3D=3D 0)
+ types[j] =3D TupleDescAttr(tupdesc_newvals, j)->atttypid;
+
+ vals[j] =3D SPI_getbinval(tuptable_newvals->vals[0], tupdesc_newvals, j=
+ 1, &isnull);
+ if (isnull)
+ nulls[j] =3D 'n';
+ else
+ nulls[j] =3D ' ';
+ }
+ /* Set TID of the view tuple to be updated as a parameter */
+ types[j] =3D TIDOID;
+ vals[j] =3D SPI_getbinval(tuptable_recalc->vals[i], tupdesc_recalc, 1, &=
isnull);
+ nulls[j] =3D ' ';
+
+ /* Update the view tuple to the new values */
+ plan =3D get_plan_for_set_values(matviewOid, matviewname, namelist, type=
s);
+ if (SPI_execute_plan(plan, vals, nulls, false, 0) !=3D SPI_OK_UPDATE)
+ elog(ERROR, "SPI_execute_plan");
+ }
+}
+
+
+/*
+ * get_plan_for_recalc
+ *
+ * Create or fetch a plan for recalculating value in the view's target list
+ * from base tables using the definition query of materialized view specif=
ied
+ * by matviewOid. namelist is a list of resnames of values to be recalcula=
ted.
+ *
+ * keys is a list of keys to identify tuples to be recalculated if this is=
not
+ * empty. KeyTypes is an array of types of keys.
+ */
+static SPIPlanPtr
+get_plan_for_recalc(Oid matviewOid, List *namelist, List *keys, Oid *keyTy=
pes)
+{
+ MV_QueryKey hash_key;
+ SPIPlanPtr plan;
+
+ /* Fetch or prepare a saved plan for the recalculation */
+ mv_BuildQueryKey(&hash_key, matviewOid, MV_PLAN_RECALC);
+ if ((plan =3D mv_FetchPreparedPlan(&hash_key)) =3D=3D NULL)
+ {
+ ListCell *lc;
+ StringInfoData str;
+ char *viewdef;
+
+ /* get view definition of matview */
+ viewdef =3D text_to_cstring((text *) DatumGetPointer(
+ DirectFunctionCall1(pg_get_viewdef, ObjectIdGetDatum(matviewOid))));
+ /* get rid of trailing semi-colon */
+ viewdef[strlen(viewdef)-1] =3D '\0';
+
+ /*
+ * Build a query string for recalculating values. This is like
+ *
+ * SELECT x1, x2, x3, ... FROM ( ... view definition query ...) mv
+ * WHERE (key1, key2, ...) =3D ($1, $2, ...);
+ */
+
+ initStringInfo(&str);
+ appendStringInfo(&str, "SELECT ");
+ foreach (lc, namelist)
+ {
+ appendStringInfo(&str, "%s", (char *) lfirst(lc));
+ if (lnext(namelist, lc))
+ appendStringInfoString(&str, ", ");
+ }
+ appendStringInfo(&str, " FROM (%s) mv", viewdef);
+
+ if (keys)
+ {
+ int i =3D 1;
+ char paramname[16];
+
+ appendStringInfo(&str, " WHERE (");
+ foreach (lc, keys)
+ {
+ Form_pg_attribute attr =3D (Form_pg_attribute) lfirst(lc);
+ char *resname =3D NameStr(attr->attname);
+ Oid typid =3D attr->atttypid;
+
+ sprintf(paramname, "$%d", i);
+ appendStringInfo(&str, "(");
+ generate_equal(&str, typid, resname, paramname);
+ appendStringInfo(&str, " OR (%s IS NULL AND %s IS NULL))",
+ resname, paramname);
+
+ if (lnext(keys, lc))
+ appendStringInfoString(&str, " AND ");
+ i++;
+ }
+ appendStringInfo(&str, ")");
+ }
+ else
+ keyTypes =3D NULL;
+
+ plan =3D SPI_prepare(str.data, list_length(keys), keyTypes);
+ if (plan =3D=3D NULL)
+ elog(ERROR, "SPI_prepare returned %s for %s", SPI_result_code_string(SP=
I_result), str.data);
+
+ SPI_keepplan(plan);
+ mv_HashPreparedPlan(&hash_key, plan);
+ }
+
+ return plan;
+}
+
+/*
+ * get_plan_for_set_values
+ *
+ * Create or fetch a plan for applying new values calculated by
+ * get_plan_for_recalc to a materialized view specified by matviewOid.
+ * matviewname is the name of the view. namelist is a list of resnames
+ * of attributes to be updated, and valTypes is an array of types of the
+ * values.
+ */
+static SPIPlanPtr
+get_plan_for_set_values(Oid matviewOid, char *matviewname, List *namelist,
+ Oid *valTypes)
+{
+ MV_QueryKey key;
+ SPIPlanPtr plan;
+
+ /* Fetch or prepare a saved plan for the real check */
+ mv_BuildQueryKey(&key, matviewOid, MV_PLAN_SET_VALUE);
+ if ((plan =3D mv_FetchPreparedPlan(&key)) =3D=3D NULL)
+ {
+ ListCell *lc;
+ StringInfoData str;
+ int i;
+
+ /*
+ * Build a query string for applying min/max values. This is like
+ *
+ * UPDATE matviewname AS mv
+ * SET (x1, x2, x3, x4) =3D ($1, $2, $3, $4)
+ * WHERE ctid =3D $5;
+ */
+
+ initStringInfo(&str);
+ appendStringInfo(&str, "UPDATE %s AS mv SET (", matviewname);
+ foreach (lc, namelist)
+ {
+ appendStringInfo(&str, "%s", (char *) lfirst(lc));
+ if (lnext(namelist, lc))
+ appendStringInfoString(&str, ", ");
+ }
+ appendStringInfo(&str, ") =3D ROW(");
+
+ for (i =3D 1; i <=3D list_length(namelist); i++)
+ appendStringInfo(&str, "%s$%d", (i=3D=3D1 ? "" : ", "), i);
+
+ appendStringInfo(&str, ") WHERE ctid OPERATOR(pg_catalog.=3D) $%d", i);
+
+ plan =3D SPI_prepare(str.data, list_length(namelist) + 1, valTypes);
+ if (plan =3D=3D NULL)
+ elog(ERROR, "SPI_prepare returned %s for %s", SPI_result_code_string(SP=
I_result), str.data);
+
+ SPI_keepplan(plan);
+ mv_HashPreparedPlan(&key, plan);
+ }
+
+ return plan;
+}
+
/*
* generate_equals
*
@@ -2314,6 +3190,13 @@ mv_InitHashTables(void)
{
HASHCTL ctl;
=20
+ memset(&ctl, 0, sizeof(ctl));
+ ctl.keysize =3D sizeof(MV_QueryKey);
+ ctl.entrysize =3D sizeof(MV_QueryHashEntry);
+ mv_query_cache =3D hash_create("MV query cache",
+ MV_INIT_QUERYHASHSIZE,
+ &ctl, HASH_ELEM | HASH_BLOBS);
+
memset(&ctl, 0, sizeof(ctl));
ctl.keysize =3D sizeof(Oid);
ctl.entrysize =3D sizeof(MV_TriggerHashEntry);
@@ -2322,6 +3205,99 @@ mv_InitHashTables(void)
&ctl, HASH_ELEM | HASH_BLOBS);
}
=20
+/*
+ * mv_FetchPreparedPlan
+ */
+static SPIPlanPtr
+mv_FetchPreparedPlan(MV_QueryKey *key)
+{
+ MV_QueryHashEntry *entry;
+ SPIPlanPtr plan;
+
+ /*
+ * On the first call initialize the hashtable
+ */
+ if (!mv_query_cache)
+ mv_InitHashTables();
+
+ /*
+ * Lookup for the key
+ */
+ entry =3D (MV_QueryHashEntry *) hash_search(mv_query_cache,
+ (void *) key,
+ HASH_FIND, NULL);
+ if (entry =3D=3D NULL)
+ return NULL;
+
+ /*
+ * Check whether the plan is still valid. If it isn't, we don't want to
+ * simply rely on plancache.c to regenerate it; rather we should start
+ * from scratch and rebuild the query text too. This is to cover cases
+ * such as table/column renames. We depend on the plancache machinery to
+ * detect possible invalidations, though.
+ *
+ * CAUTION: this check is only trustworthy if the caller has already
+ * locked both materialized views and base tables.
+ */
+ plan =3D entry->plan;
+ if (plan && SPI_plan_is_valid(plan))
+ return plan;
+
+ /*
+ * Otherwise we might as well flush the cached plan now, to free a little
+ * memory space before we make a new one.
+ */
+ entry->plan =3D NULL;
+ if (plan)
+ SPI_freeplan(plan);
+
+ return NULL;
+}
+
+/*
+ * mv_HashPreparedPlan
+ *
+ * Add another plan to our private SPI query plan hashtable.
+ */
+static void
+mv_HashPreparedPlan(MV_QueryKey *key, SPIPlanPtr plan)
+{
+ MV_QueryHashEntry *entry;
+ bool found;
+
+ /*
+ * On the first call initialize the hashtable
+ */
+ if (!mv_query_cache)
+ mv_InitHashTables();
+
+ /*
+ * Add the new plan. We might be overwriting an entry previously found
+ * invalid by mv_FetchPreparedPlan.
+ */
+ entry =3D (MV_QueryHashEntry *) hash_search(mv_query_cache,
+ (void *) key,
+ HASH_ENTER, &found);
+ Assert(!found || entry->plan =3D=3D NULL);
+ entry->plan =3D plan;
+}
+
+/*
+ * mv_BuildQueryKey
+ *
+ * Construct a hashtable key for a prepared SPI plan for IVM.
+ */
+static void
+mv_BuildQueryKey(MV_QueryKey *key, Oid matview_id, int32 query_type)
+{
+ /*
+ * We assume struct MV_QueryKey contains no padding bytes, else we'd need
+ * to use memset to clear them.
+ */
+ key->matview_id =3D matview_id;
+ key->query_type =3D query_type;
+}
+
/*
* AtAbort_IVM
*
diff --git a/src/include/commands/createas.h b/src/include/commands/createa=
s.h
index a57ce463e1..702b097079 100644
--- a/src/include/commands/createas.h
+++ b/src/include/commands/createas.h
@@ -29,6 +29,7 @@ extern ObjectAddress ExecCreateTableAs(ParseState *pstate=
, CreateTableAsStmt *st
extern void CreateIvmTriggersOnBaseTables(Query *qry, Oid matviewOid, bool=
is_create);
=20
extern Query *rewriteQueryForIMMV(Query *query, List *colNames);
+extern void makeIvmAggColumn(ParseState *pstate, Aggref *aggref, char *res=
name, AttrNumber *next_resno, List **aggs);
=20
extern int GetIntoRelEFlags(IntoClause *intoClause);
=20
--=20
2.17.1
--Multipart=_Fri__29_Oct_2021_18_16_28_+0900_jlYRKjywLqhZ7oyk
Content-Type: text/x-diff;
name="v24-0009-Add-regression-tests-for-Incremental-View-Mainte.patch"
Content-Disposition: attachment;
filename="v24-0009-Add-regression-tests-for-Incremental-View-Mainte.patch"
Content-Transfer-Encoding: 7bit
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Track the amount of time waiting due to cost_delay
@ 2024-06-13 11:56 Bertrand Drouvot <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Bertrand Drouvot @ 2024-06-13 11:56 UTC (permalink / raw)
To: Masahiko Sawada <[email protected]>; +Cc: [email protected]; Robert Haas <[email protected]>
Hi,
On Tue, Jun 11, 2024 at 08:26:23AM +0000, Bertrand Drouvot wrote:
> Hi,
>
> On Tue, Jun 11, 2024 at 04:07:05PM +0900, Masahiko Sawada wrote:
>
> > Thank you for the proposal and the patch. I understand the motivation
> > of this patch.
>
> Thanks for looking at it!
>
> > Beside the point Nathan mentioned, I'm slightly worried
> > that massive parallel messages could be sent to the leader process
> > when the cost_limit value is low.
>
> I see, I can/will do some testing in this area and share the numbers.
Here is the result of the test. It has been launched several times and it
produced the same (surprising result) each time.
====================== Context ================================================
The testing has been done with this relation (large_tbl) and its indexes:
postgres=# SELECT n.nspname, c.relname, count(*) AS buffers
FROM pg_buffercache b JOIN pg_class c
ON b.relfilenode = pg_relation_filenode(c.oid) AND
b.reldatabase IN (0, (SELECT oid FROM pg_database
WHERE datname = current_database()))
JOIN pg_namespace n ON n.oid = c.relnamespace
GROUP BY n.nspname, c.relname
ORDER BY 3 DESC
LIMIT 22;
nspname | relname | buffers
---------+--------------------+---------
public | large_tbl | 222280
public | large_tbl_filler13 | 125000
public | large_tbl_filler6 | 125000
public | large_tbl_filler5 | 125000
public | large_tbl_filler3 | 125000
public | large_tbl_filler15 | 125000
public | large_tbl_filler4 | 125000
public | large_tbl_filler20 | 125000
public | large_tbl_filler18 | 125000
public | large_tbl_filler14 | 125000
public | large_tbl_filler8 | 125000
public | large_tbl_filler11 | 125000
public | large_tbl_filler19 | 125000
public | large_tbl_filler7 | 125000
public | large_tbl_filler1 | 125000
public | large_tbl_filler12 | 125000
public | large_tbl_filler9 | 125000
public | large_tbl_filler17 | 125000
public | large_tbl_filler16 | 125000
public | large_tbl_filler10 | 125000
public | large_tbl_filler2 | 125000
public | large_tbl_pkey | 5486
(22 rows)
All of them completly fit in memory (to avoid I/O read latency during the vacuum).
The config, outside of default is:
max_wal_size = 4GB
shared_buffers = 30GB
vacuum_cost_delay = 1
autovacuum_vacuum_cost_delay = 1
max_parallel_maintenance_workers = 8
max_parallel_workers = 10
vacuum_cost_limit = 10
autovacuum_vacuum_cost_limit = 10
My system is not overloaded, has enough resources to run this test and only this
test is running.
====================== Results ================================================
========== With v2 (attached) applied on master
postgres=# VACUUM (PARALLEL 8) large_tbl;
VACUUM
Time: 1146873.016 ms (19:06.873)
The duration is splitted that way:
Vacuum phase: cumulative time (cumulative time delayed)
=======================================================
scanning heap: 00:08:16.414628 (time_delayed: 444370)
vacuuming indexes: 00:14:55.314699 (time_delayed: 2545293)
vacuuming heap: 00:19:06.814617 (time_delayed: 2767540)
I sampled active sessions from pg_stat_activity (one second interval), here is
the summary during the vacuuming indexes phase (ordered by count):
leader_pid | pid | wait_event | count
------------+--------+----------------+-------
452996 | 453225 | VacuumDelay | 366
452996 | 453223 | VacuumDelay | 363
452996 | 453226 | VacuumDelay | 362
452996 | 453224 | VacuumDelay | 361
452996 | 453222 | VacuumDelay | 359
452996 | 453221 | VacuumDelay | 359
| 452996 | VacuumDelay | 331
| 452996 | CPU | 30
452996 | 453224 | WALWriteLock | 23
452996 | 453222 | WALWriteLock | 20
452996 | 453226 | WALWriteLock | 20
452996 | 453221 | WALWriteLock | 19
| 452996 | WalSync | 18
452996 | 453225 | WALWriteLock | 18
452996 | 453223 | WALWriteLock | 16
| 452996 | WALWriteLock | 15
452996 | 453221 | CPU | 14
452996 | 453222 | CPU | 14
452996 | 453223 | CPU | 12
452996 | 453224 | CPU | 10
452996 | 453226 | CPU | 10
452996 | 453225 | CPU | 8
452996 | 453223 | WalSync | 4
452996 | 453221 | WalSync | 2
452996 | 453226 | WalWrite | 2
452996 | 453221 | WalWrite | 1
| 452996 | ParallelFinish | 1
452996 | 453224 | WalSync | 1
452996 | 453225 | WalSync | 1
452996 | 453222 | WalWrite | 1
452996 | 453225 | WalWrite | 1
452996 | 453222 | WalSync | 1
452996 | 453226 | WalSync | 1
========== On master (v2 not applied)
postgres=# VACUUM (PARALLEL 8) large_tbl;
VACUUM
Time: 1322598.087 ms (22:02.598)
Surprisingly it has been longer on master by about 3 minutes.
Let's see how the time is splitted:
Vacuum phase: cumulative time
=============================
scanning heap: 00:08:07.061196
vacuuming indexes: 00:17:50.961228
vacuuming heap: 00:22:02.561199
I sampled active sessions from pg_stat_activity (one second interval), here is
the summary during the vacuuming indexes phase (ordered by count):
leader_pid | pid | wait_event | count
------------+--------+-------------------+-------
468682 | 468858 | VacuumDelay | 548
468682 | 468862 | VacuumDelay | 547
468682 | 468859 | VacuumDelay | 547
468682 | 468860 | VacuumDelay | 545
468682 | 468857 | VacuumDelay | 543
468682 | 468861 | VacuumDelay | 542
| 468682 | VacuumDelay | 378
| 468682 | ParallelFinish | 182
468682 | 468861 | WALWriteLock | 19
468682 | 468857 | WALWriteLock | 19
468682 | 468859 | WALWriteLock | 18
468682 | 468858 | WALWriteLock | 16
468682 | 468860 | WALWriteLock | 15
468682 | 468862 | WALWriteLock | 15
468682 | 468862 | CPU | 12
468682 | 468857 | CPU | 10
468682 | 468859 | CPU | 10
468682 | 468861 | CPU | 10
| 468682 | CPU | 9
468682 | 468860 | CPU | 9
468682 | 468860 | WalSync | 8
| 468682 | WALWriteLock | 7
468682 | 468858 | WalSync | 6
468682 | 468858 | CPU | 6
468682 | 468862 | WalSync | 3
468682 | 468857 | WalSync | 3
468682 | 468861 | WalSync | 3
468682 | 468859 | WalSync | 2
468682 | 468861 | WalWrite | 2
468682 | 468857 | WalWrite | 1
468682 | 468858 | WalWrite | 1
468682 | 468861 | WALBufMappingLock | 1
468682 | 468857 | WALBufMappingLock | 1
| 468682 | WALBufMappingLock | 1
====================== Observations ===========================================
As compare to v2:
1. scanning heap time is about the same
2. vacuuming indexes time is about 3 minutes longer on master
3. vacuuming heap time is about the same
One difference we can see in the sampling, is that on master the "ParallelFinish"
has been sampled about 182 times (so could be _the_ 3 minutes of interest) for
the leader.
On master the vacuum indexes phase has been running between 2024-06-13 10:11:34
and 2024-06-13 10:21:15. If I extract the exact minutes and the counts for the
"ParallelFinish" wait event I get:
minute | wait_event | count
--------+----------------+-------
18 | ParallelFinish | 48
19 | ParallelFinish | 60
20 | ParallelFinish | 60
21 | ParallelFinish | 14
So it's likely that the leader waited on ParallelFinish during about 3 minutes
at the end of the vacuuming indexes phase (as this wait appeared during
consecutives samples).
====================== Conclusion =============================================
1. During the scanning heap and vacuuming heap phases no noticeable performance
degradation has been observed with v2 applied (as compare to master) (cc'ing
Robert as it's also related to his question about noticeable hit when everything
is in-memory in [1]).
2. During the vacuuming indexes phase, v2 has been faster (as compare to master).
The reason is that on master the leader has been waiting during about 3 minutes
on "ParallelFinish" at the end.
====================== Remarks ================================================
As v2 is attached, please find below a summary about the current state of this
thread:
1. v2 implements delay_time as the actual wait time (and not the intended wait
time as proposed in v1).
2. some measurements have been done to check the impact of this new
instrumentation (see this email and [2]): no noticeable performance degradation
has been observed (and surprisingly that's the opposite as mentioned above).
3. there is an ongoing discussion about exposing the number of waits [2].
4. there is an ongoing discussion about exposing the effective cost limit [3].
5. that could be interesting to have a closer look as to why the leader is waiting
during 3 minutes on "ParallelFinish" on master and not with v2 applied (but that's
probably out of scope for this thread).
[1]: https://www.postgresql.org/message-id/CA%2BTgmoZiC%3DzeCDYuMpB%2BGb2yK%3DrTQCGMu0VoxehocKyHxr9Erg%40...
[2]: https://www.postgresql.org/message-id/ZmmOOPwMFIltkdsN%40ip-10-97-1-34.eu-west-3.compute.internal
[3]: https://www.postgresql.org/message-id/Zml9%2Bu37iS7DFkJL%40ip-10-97-1-34.eu-west-3.compute.internal
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v2-0001-Report-the-total-amount-of-time-that-vacuum-has-b.patch (5.6K, ../../[email protected]/2-v2-0001-Report-the-total-amount-of-time-that-vacuum-has-b.patch)
download | inline diff:
From 21eeab61c125a7ca4afccd3bc5961a1f060f0b9a Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Thu, 6 Jun 2024 12:35:57 +0000
Subject: [PATCH v2] Report the total amount of time that vacuum has been
delayed due to cost delay
This commit adds one column: time_delayed to the pg_stat_progress_vacuum system
view to show the total amount of time in milliseconds that vacuum has been
delayed.
This uses the new parallel message type for progress reporting added
by f1889729dd.
Bump catversion because this changes the definition of pg_stat_progress_vacuum.
---
doc/src/sgml/monitoring.sgml | 11 +++++++++++
src/backend/catalog/system_views.sql | 3 ++-
src/backend/commands/vacuum.c | 15 +++++++++++++++
src/include/catalog/catversion.h | 2 +-
src/include/commands/progress.h | 1 +
src/test/regress/expected/rules.out | 3 ++-
6 files changed, 32 insertions(+), 3 deletions(-)
37.5% doc/src/sgml/
42.3% src/backend/commands/
5.9% src/include/catalog/
3.2% src/include/commands/
7.9% src/test/regress/expected/
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 053da8d6e4..cdd0f0e533 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -6290,6 +6290,17 @@ FROM pg_stat_get_backend_idset() AS backendid;
<literal>cleaning up indexes</literal>.
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>time_delayed</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Total amount of time spent in milliseconds waiting due to <varname>vacuum_cost_delay</varname>
+ or <varname>autovacuum_vacuum_cost_delay</varname>. In case of parallel
+ vacuum the reported time is across all the workers and the leader.
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 53047cab5f..1345e99dcb 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1221,7 +1221,8 @@ CREATE VIEW pg_stat_progress_vacuum AS
S.param2 AS heap_blks_total, S.param3 AS heap_blks_scanned,
S.param4 AS heap_blks_vacuumed, S.param5 AS index_vacuum_count,
S.param6 AS max_dead_tuple_bytes, S.param7 AS dead_tuple_bytes,
- S.param8 AS indexes_total, S.param9 AS indexes_processed
+ S.param8 AS indexes_total, S.param9 AS indexes_processed,
+ S.param10 AS time_delayed
FROM pg_stat_get_progress_info('VACUUM') AS S
LEFT JOIN pg_database D ON S.datid = D.oid;
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 48f8eab202..5c40ee6e2c 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -40,6 +40,7 @@
#include "catalog/pg_inherits.h"
#include "commands/cluster.h"
#include "commands/defrem.h"
+#include "commands/progress.h"
#include "commands/vacuum.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
@@ -2380,13 +2381,27 @@ vacuum_delay_point(void)
/* Nap if appropriate */
if (msec > 0)
{
+ instr_time delay_start;
+ instr_time delay_time;
+
if (msec > vacuum_cost_delay * 4)
msec = vacuum_cost_delay * 4;
pgstat_report_wait_start(WAIT_EVENT_VACUUM_DELAY);
+ INSTR_TIME_SET_CURRENT(delay_start);
pg_usleep(msec * 1000);
+ INSTR_TIME_SET_CURRENT(delay_time);
pgstat_report_wait_end();
+ /* Report the amount of time we slept */
+ INSTR_TIME_SUBTRACT(delay_time, delay_start);
+ if (VacuumSharedCostBalance != NULL)
+ pgstat_progress_parallel_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
+ INSTR_TIME_GET_MILLISEC(delay_time));
+ else
+ pgstat_progress_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
+ INSTR_TIME_GET_MILLISEC(delay_time));
+
/*
* We don't want to ignore postmaster death during very long vacuums
* with vacuum_cost_delay configured. We can't use the usual
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index f0809c0e58..40b4f1d1e4 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -57,6 +57,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 202405161
+#define CATALOG_VERSION_NO 202406101
#endif
diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h
index 82a8fe6bd1..1fcefe9436 100644
--- a/src/include/commands/progress.h
+++ b/src/include/commands/progress.h
@@ -27,6 +27,7 @@
#define PROGRESS_VACUUM_DEAD_TUPLE_BYTES 6
#define PROGRESS_VACUUM_INDEXES_TOTAL 7
#define PROGRESS_VACUUM_INDEXES_PROCESSED 8
+#define PROGRESS_VACUUM_TIME_DELAYED 9
/* Phases of vacuum (as advertised via PROGRESS_VACUUM_PHASE) */
#define PROGRESS_VACUUM_PHASE_SCAN_HEAP 1
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index ef658ad740..a499e44df1 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2053,7 +2053,8 @@ pg_stat_progress_vacuum| SELECT s.pid,
s.param6 AS max_dead_tuple_bytes,
s.param7 AS dead_tuple_bytes,
s.param8 AS indexes_total,
- s.param9 AS indexes_processed
+ s.param9 AS indexes_processed,
+ s.param10 AS time_delayed
FROM (pg_stat_get_progress_info('VACUUM'::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_recovery_prefetch| SELECT stats_reset,
--
2.34.1
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Track the amount of time waiting due to cost_delay
@ 2024-06-22 12:48 Bertrand Drouvot <[email protected]>
parent: Bertrand Drouvot <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Bertrand Drouvot @ 2024-06-22 12:48 UTC (permalink / raw)
To: Masahiko Sawada <[email protected]>; +Cc: [email protected]; Robert Haas <[email protected]>
Hi,
On Thu, Jun 13, 2024 at 11:56:26AM +0000, Bertrand Drouvot wrote:
> ====================== Observations ===========================================
>
> As compare to v2:
>
> 1. scanning heap time is about the same
> 2. vacuuming indexes time is about 3 minutes longer on master
> 3. vacuuming heap time is about the same
I had a closer look to understand why the vacuuming indexes time has been about
3 minutes longer on master.
During the vacuuming indexes phase, the leader is helping vacuuming the indexes
until it reaches WaitForParallelWorkersToFinish() (means when all the remaining
indexes are currently handled by the parallel workers, the leader has nothing
more to do and so it is waiting for the parallel workers to finish).
During the time the leader process is involved in indexes vacuuming it is
also subject to wait due to cost delay.
But with v2 applied, the leader may be interrupted by the parallel workers while
it is waiting (due to the new pgstat_progress_parallel_incr_param() calls that
the patch is adding).
So, with v2 applied, the leader is waiting less (as interrupted while waiting)
when being involved in indexes vacuuming and that's why v2 is "faster" than
master.
To put some numbers, I did count the number of times the leader's pg_usleep() has
been interrupted (by counting the number of times the nanosleep() did return a
value < 0 in pg_usleep()). Here they are:
v2: the leader has been interrupted about 342605 times
master: the leader has been interrupted about 36 times
The ones on master are mainly coming from the pgstat_progress_parallel_incr_param()
calls in parallel_vacuum_process_one_index().
The additional ones on v2 are coming from the pgstat_progress_parallel_incr_param()
calls added in vacuum_delay_point().
======== Conclusion ======
1. vacuuming indexes time has been longer on master because with v2, the leader
has been interrupted 342605 times while waiting, then making v2 "faster".
2. the leader being interrupted while waiting is also already happening on master
due to the pgstat_progress_parallel_incr_param() calls in
parallel_vacuum_process_one_index() (that have been added in
46ebdfe164). It has been the case "only" 36 times during my test case.
I think that 2. is less of a concern but I think that 1. is something that needs
to be addressed because the leader process is not honouring its cost delay wait
time in a noticeable way (at least during my test case).
I did not think of a proposal yet, just sharing my investigation as to why
v2 has been faster than master during the vacuuming indexes phase.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Track the amount of time waiting due to cost_delay
@ 2024-06-24 10:50 Bertrand Drouvot <[email protected]>
parent: Bertrand Drouvot <[email protected]>
0 siblings, 2 replies; 18+ messages in thread
From: Bertrand Drouvot @ 2024-06-24 10:50 UTC (permalink / raw)
To: Masahiko Sawada <[email protected]>; +Cc: [email protected]; Robert Haas <[email protected]>
Hi,
On Sat, Jun 22, 2024 at 12:48:33PM +0000, Bertrand Drouvot wrote:
> 1. vacuuming indexes time has been longer on master because with v2, the leader
> has been interrupted 342605 times while waiting, then making v2 "faster".
>
> 2. the leader being interrupted while waiting is also already happening on master
> due to the pgstat_progress_parallel_incr_param() calls in
> parallel_vacuum_process_one_index() (that have been added in
> 46ebdfe164). It has been the case "only" 36 times during my test case.
>
> I think that 2. is less of a concern but I think that 1. is something that needs
> to be addressed because the leader process is not honouring its cost delay wait
> time in a noticeable way (at least during my test case).
>
> I did not think of a proposal yet, just sharing my investigation as to why
> v2 has been faster than master during the vacuuming indexes phase.
I think that a reasonable approach is to make the reporting from the parallel
workers to the leader less aggressive (means occur less frequently).
Please find attached v3, that:
- ensures that there is at least 1 second between 2 reports, per parallel worker,
to the leader.
- ensures that the reported delayed time is still correct (keep track of the
delayed time between 2 reports).
- does not add any extra pg_clock_gettime_ns() calls (as compare to v2).
Remarks:
1. Having a time based only approach to throttle the reporting of the parallel
workers sounds reasonable. I don't think that the number of parallel workers has
to come into play as:
1.1) the more parallel workers is used, the less the impact of the leader on
the vacuum index phase duration/workload is (because the repartition is done
on more processes).
1.2) the less parallel workers is, the less the leader will be interrupted (
less parallel workers would report their delayed time).
2. The throttling is not based on the cost limit as that value is distributed
proportionally among the parallel workers (so we're back to the previous point).
3. The throttling is not based on the actual cost delay value because the leader
could be interrupted at the beginning, the midle or whatever part of the wait and
we are more interested about the frequency of the interrupts.
3. A 1 second reporting "throttling" looks a reasonable threshold as:
3.1 the idea is to have a significant impact when the leader could have been
interrupted say hundred/thousand times per second.
3.2 it does not make that much sense for any tools to sample pg_stat_progress_vacuum
multiple times per second (so a one second reporting granularity seems ok).
With this approach in place, v3 attached applied, during my test case:
- the leader has been interrupted about 2500 times (instead of about 345000
times with v2)
- the vacuum index phase duration is very close to the master one (it has been
4 seconds faster (over a 8 minutes 40 seconds duration time), instead of 3
minutes faster with v2).
Thoughts?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v3-0001-Report-the-total-amount-of-time-that-vacuum-has-b.patch (8.4K, ../../ZnlPZZZJCRu%[email protected]/2-v3-0001-Report-the-total-amount-of-time-that-vacuum-has-b.patch)
download | inline diff:
From 99f417c0bcd7c29e126fdccdd6214ea37db67379 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Mon, 24 Jun 2024 08:43:26 +0000
Subject: [PATCH v3] Report the total amount of time that vacuum has been
delayed due to cost delay
This commit adds one column: time_delayed to the pg_stat_progress_vacuum system
view to show the total amount of time in milliseconds that vacuum has been
delayed.
This uses the new parallel message type for progress reporting added
by f1889729dd.
In case of parallel worker, to avoid the leader to be interrupted too frequently
(while it might be sleeping for cost delay), the report is done only if the last
report has been done more than 1 second ago.
Having a time based only approach to throttle the reporting of the parallel
workers sounds reasonable.
Indeed when deciding about the throttling:
1. The number of parallel workers should not come into play:
1.1) the more parallel workers is used, the less the impact of the leader on
the vacuum index phase duration/workload is (because the repartition is done
on more processes).
1.2) the less parallel workers is, the less the leader will be interrupted (
less parallel workers would report their delayed time).
2. The cost limit should not come into play as that value is distributed
proportionally among the parallel workers (so we're back to the previous point).
3. The cost delay does not come into play as the leader could be interrupted at
the beginning, the midle or whatever part of the wait and we are more interested
about the frequency of the interrupts.
3. A 1 second reporting "throttling" looks a reasonable threshold as:
3.1 the idea is to have a significant impact when the leader could have been
interrupted say hundred/thousand times per second.
3.2 it does not make that much sense for any tools to sample pg_stat_progress_vacuum
multiple times per second (so a one second reporting granularity seems ok).
Bump catversion because this changes the definition of pg_stat_progress_vacuum.
---
doc/src/sgml/monitoring.sgml | 11 +++++++
src/backend/catalog/system_views.sql | 2 +-
src/backend/commands/vacuum.c | 49 ++++++++++++++++++++++++++++
src/include/catalog/catversion.h | 2 +-
src/include/commands/progress.h | 1 +
src/test/regress/expected/rules.out | 3 +-
6 files changed, 65 insertions(+), 3 deletions(-)
19.7% doc/src/sgml/
4.4% src/backend/catalog/
66.6% src/backend/commands/
3.1% src/include/catalog/
4.2% src/test/regress/expected/
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index b2ad9b446f..e9608fb6fe 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -6299,6 +6299,17 @@ FROM pg_stat_get_backend_idset() AS backendid;
<literal>cleaning up indexes</literal>.
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>time_delayed</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Total amount of time spent in milliseconds waiting due to <varname>vacuum_cost_delay</varname>
+ or <varname>autovacuum_vacuum_cost_delay</varname>. In case of parallel
+ vacuum the reported time is across all the workers and the leader.
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index efb29adeb3..74b2ef12af 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1222,7 +1222,7 @@ CREATE VIEW pg_stat_progress_vacuum AS
S.param4 AS heap_blks_vacuumed, S.param5 AS index_vacuum_count,
S.param6 AS max_dead_tuple_bytes, S.param7 AS dead_tuple_bytes,
S.param8 AS num_dead_item_ids, S.param9 AS indexes_total,
- S.param10 AS indexes_processed
+ S.param10 AS indexes_processed, S.param11 AS time_delayed
FROM pg_stat_get_progress_info('VACUUM') AS S
LEFT JOIN pg_database D ON S.datid = D.oid;
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 48f8eab202..03470a450f 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -40,6 +40,7 @@
#include "catalog/pg_inherits.h"
#include "commands/cluster.h"
#include "commands/defrem.h"
+#include "commands/progress.h"
#include "commands/vacuum.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
@@ -60,6 +61,12 @@
#include "utils/snapmgr.h"
#include "utils/syscache.h"
+/*
+ * Minimum amount of time (in ms) between two reports of the delayed time from a
+ * parallel worker to the leader. The goal is to avoid the leader to be
+ * interrupted too frequently while it might be sleeping for cost delay.
+ */
+#define WORKER_REPORT_DELAY_INTERVAL 1000
/*
* GUC parameters
@@ -103,6 +110,16 @@ pg_atomic_uint32 *VacuumSharedCostBalance = NULL;
pg_atomic_uint32 *VacuumActiveNWorkers = NULL;
int VacuumCostBalanceLocal = 0;
+/*
+ * In case of parallel workers, the last time the delay has been reported to
+ * the leader.
+ * We assume this initializes to zero.
+ */
+static instr_time last_report_time;
+
+/* total nap time between two reports */
+double nap_time_since_last_report = 0;
+
/* non-export function prototypes */
static List *expand_vacuum_rel(VacuumRelation *vrel,
MemoryContext vac_context, int options);
@@ -2380,13 +2397,45 @@ vacuum_delay_point(void)
/* Nap if appropriate */
if (msec > 0)
{
+ instr_time delay_start;
+ instr_time delay_end;
+ instr_time delayed_time;
+
if (msec > vacuum_cost_delay * 4)
msec = vacuum_cost_delay * 4;
pgstat_report_wait_start(WAIT_EVENT_VACUUM_DELAY);
+ INSTR_TIME_SET_CURRENT(delay_start);
pg_usleep(msec * 1000);
+ INSTR_TIME_SET_CURRENT(delay_end);
pgstat_report_wait_end();
+ /* Report the amount of time we slept */
+ INSTR_TIME_SET_ZERO(delayed_time);
+ INSTR_TIME_ACCUM_DIFF(delayed_time, delay_end, delay_start);
+
+ /* Parallel worker */
+ if (VacuumSharedCostBalance != NULL)
+ {
+ instr_time time_since_last_report;
+
+ INSTR_TIME_SET_ZERO(time_since_last_report);
+ INSTR_TIME_ACCUM_DIFF(time_since_last_report, delay_end,
+ last_report_time);
+ nap_time_since_last_report += INSTR_TIME_GET_MILLISEC(delayed_time);
+
+ if (INSTR_TIME_GET_MILLISEC(time_since_last_report) > WORKER_REPORT_DELAY_INTERVAL)
+ {
+ pgstat_progress_parallel_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
+ nap_time_since_last_report);
+ nap_time_since_last_report = 0;
+ last_report_time = delay_end;
+ }
+ }
+ else
+ pgstat_progress_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
+ INSTR_TIME_GET_MILLISEC(delayed_time));
+
/*
* We don't want to ignore postmaster death during very long vacuums
* with vacuum_cost_delay configured. We can't use the usual
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index b3322e8d67..752473a44e 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -57,6 +57,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 202406171
+#define CATALOG_VERSION_NO 202406241
#endif
diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h
index 5616d64523..9a0c2358c6 100644
--- a/src/include/commands/progress.h
+++ b/src/include/commands/progress.h
@@ -28,6 +28,7 @@
#define PROGRESS_VACUUM_NUM_DEAD_ITEM_IDS 7
#define PROGRESS_VACUUM_INDEXES_TOTAL 8
#define PROGRESS_VACUUM_INDEXES_PROCESSED 9
+#define PROGRESS_VACUUM_TIME_DELAYED 10
/* Phases of vacuum (as advertised via PROGRESS_VACUUM_PHASE) */
#define PROGRESS_VACUUM_PHASE_SCAN_HEAP 1
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 13178e2b3d..54c8d9d042 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2054,7 +2054,8 @@ pg_stat_progress_vacuum| SELECT s.pid,
s.param7 AS dead_tuple_bytes,
s.param8 AS num_dead_item_ids,
s.param9 AS indexes_total,
- s.param10 AS indexes_processed
+ s.param10 AS indexes_processed,
+ s.param11 AS time_delayed
FROM (pg_stat_get_progress_info('VACUUM'::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_recovery_prefetch| SELECT stats_reset,
--
2.34.1
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Track the amount of time waiting due to cost_delay
@ 2024-06-25 01:12 Imseih (AWS), Sami <[email protected]>
parent: Bertrand Drouvot <[email protected]>
1 sibling, 2 replies; 18+ messages in thread
From: Imseih (AWS), Sami @ 2024-06-25 01:12 UTC (permalink / raw)
To: Bertrand Drouvot <[email protected]>; Masahiko Sawada <[email protected]>; +Cc: [email protected] <[email protected]>; Robert Haas <[email protected]>
>> 2. the leader being interrupted while waiting is also already happening on master
>> due to the pgstat_progress_parallel_incr_param() calls in
>> parallel_vacuum_process_one_index() (that have been added in
>> 46ebdfe164). It has been the case "only" 36 times during my test case.
46ebdfe164 will interrupt the leaders sleep every time a parallel workers reports
progress, and we currently don't handle interrupts by restarting the sleep with
the remaining time. nanosleep does provide the ability to restart with the remaining
time [1], but I don't think it's worth the effort to ensure more accurate
vacuum delays for the leader process.
> 1. Having a time based only approach to throttle
I do agree with a time based approach overall.
> 1.1) the more parallel workers is used, the less the impact of the leader on
> the vacuum index phase duration/workload is (because the repartition is done
> on more processes).
Did you mean " because the vacuum is done on more processes"?
When a leader is operating on a large index(s) during the entirety
of the vacuum operation, wouldn't more parallel workers end up
interrupting the leader more often? This is why I think reporting even more
often than 1 second (more below) will be better.
> 3. A 1 second reporting "throttling" looks a reasonable threshold as:
> 3.1 the idea is to have a significant impact when the leader could have been
> interrupted say hundred/thousand times per second.
> 3.2 it does not make that much sense for any tools to sample pg_stat_progress_vacuum
> multiple times per second (so a one second reporting granularity seems ok).
I feel 1 second may still be too frequent.
What about 10 seconds ( or 30 seconds )?
I think this metric in particular will be mainly useful for vacuum runs that are
running for minutes or more, making reporting every 10 or 30 seconds
still useful.
It just occurred to me also that pgstat_progress_parallel_incr_param
should have a code comment that it will interrupt a leader process and
cause activity such as a sleep to end early.
Regards,
Sami Imseih
Amazon Web Services (AWS)
[1] https://man7.org/linux/man-pages/man2/nanosleep.2.html
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Track the amount of time waiting due to cost_delay
@ 2024-06-25 08:29 Bertrand Drouvot <[email protected]>
parent: Imseih (AWS), Sami <[email protected]>
1 sibling, 0 replies; 18+ messages in thread
From: Bertrand Drouvot @ 2024-06-25 08:29 UTC (permalink / raw)
To: Imseih (AWS), Sami <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; [email protected] <[email protected]>; Robert Haas <[email protected]>
Hi,
On Tue, Jun 25, 2024 at 01:12:16AM +0000, Imseih (AWS), Sami wrote:
Thanks for the feedback!
> >> 2. the leader being interrupted while waiting is also already happening on master
> >> due to the pgstat_progress_parallel_incr_param() calls in
> >> parallel_vacuum_process_one_index() (that have been added in
> >> 46ebdfe164). It has been the case "only" 36 times during my test case.
>
> 46ebdfe164 will interrupt the leaders sleep every time a parallel workers reports
> progress, and we currently don't handle interrupts by restarting the sleep with
> the remaining time. nanosleep does provide the ability to restart with the remaining
> time [1], but I don't think it's worth the effort to ensure more accurate
> vacuum delays for the leader process.
+1. I don't think it's necessary to have a 100% accurate delay for all the
times the delay is involded. I think that's an heuristic parameter (among
with cost limit). What matters at the end is by how much you've been able to
pause the whole vacuum (and not by a sleep by sleep basis)).
> > 1. Having a time based only approach to throttle
>
> I do agree with a time based approach overall.
>
>
> > 1.1) the more parallel workers is used, the less the impact of the leader on
> > the vacuum index phase duration/workload is (because the repartition is done
> > on more processes).
>
> Did you mean " because the vacuum is done on more processes"?
Yes.
> When a leader is operating on a large index(s) during the entirety
> of the vacuum operation, wouldn't more parallel workers end up
> interrupting the leader more often?
That's right but my point was about the impact on the "whole" duration time and
"whole" workload (leader + workers included) and not about the number of times the
leader is interrupted. If there is say 100 workers then interrupting the leader
(1 process out of 101) is probably less of an issue as it means that there is a
lot of work to be done to have those 100 workers busy. I don't think the size of
the index the leader is vacuuming has an impact. I think that having the leader
vacuuming a 100 GB index or 100 x 1GB indexes is the same (as long as all the
other workers are actives during all that time).
> > 3. A 1 second reporting "throttling" looks a reasonable threshold as:
>
> > 3.1 the idea is to have a significant impact when the leader could have been
> > interrupted say hundred/thousand times per second.
>
> > 3.2 it does not make that much sense for any tools to sample pg_stat_progress_vacuum
> > multiple times per second (so a one second reporting granularity seems ok).
>
> I feel 1 second may still be too frequent.
Maybe we'll need more measurements but this is what my test case made of:
vacuum_cost_delay = 1
vacuum_cost_limit = 10
8 parallel workers, 1 leader
21 indexes (about 1GB each, one 40MB), all in memory
lead to:
With 1 second reporting frequency, the leader has been interruped about 2500
times over 8m39s leading to about the same time as on master (8m43s).
> What about 10 seconds ( or 30 seconds )?
I'm not sure (may need more measurements) but it would probably complicate the
reporting a bit (as with the current v3 we'd miss reporting the indexes that
take less time than the threshold to complete).
> I think this metric in particular will be mainly useful for vacuum runs that are
> running for minutes or more, making reporting every 10 or 30 seconds
> still useful.
Agree. OTOH, one could be interested to diagnose what happened during a say 5
seconds peak on I/O resource consumption/latency. Sampling pg_stat_progress_vacuum
at 1 second interval and see by how much the vaccum has been paused during that
time could help too (specially if it is made of a lot of parallel workers that
could lead to a lot of I/O). But it would miss data if we are reporting at a
larger rate.
> It just occurred to me also that pgstat_progress_parallel_incr_param
> should have a code comment that it will interrupt a leader process and
> cause activity such as a sleep to end early.
Good point, I'll add a comment for it.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Track the amount of time waiting due to cost_delay
@ 2024-06-27 05:17 Masahiko Sawada <[email protected]>
parent: Bertrand Drouvot <[email protected]>
1 sibling, 0 replies; 18+ messages in thread
From: Masahiko Sawada @ 2024-06-27 05:17 UTC (permalink / raw)
To: Bertrand Drouvot <[email protected]>; +Cc: [email protected]; Robert Haas <[email protected]>
On Mon, Jun 24, 2024 at 7:50 PM Bertrand Drouvot
<[email protected]> wrote:
>
> Hi,
>
> On Sat, Jun 22, 2024 at 12:48:33PM +0000, Bertrand Drouvot wrote:
> > 1. vacuuming indexes time has been longer on master because with v2, the leader
> > has been interrupted 342605 times while waiting, then making v2 "faster".
> >
> > 2. the leader being interrupted while waiting is also already happening on master
> > due to the pgstat_progress_parallel_incr_param() calls in
> > parallel_vacuum_process_one_index() (that have been added in
> > 46ebdfe164). It has been the case "only" 36 times during my test case.
> >
> > I think that 2. is less of a concern but I think that 1. is something that needs
> > to be addressed because the leader process is not honouring its cost delay wait
> > time in a noticeable way (at least during my test case).
> >
> > I did not think of a proposal yet, just sharing my investigation as to why
> > v2 has been faster than master during the vacuuming indexes phase.
Thank you for the benchmarking and analyzing the results! I agree with
your analysis and was surprised by the fact that the more times
workers go to sleep, the more times the leader wakes up.
>
> I think that a reasonable approach is to make the reporting from the parallel
> workers to the leader less aggressive (means occur less frequently).
>
> Please find attached v3, that:
>
> - ensures that there is at least 1 second between 2 reports, per parallel worker,
> to the leader.
>
> - ensures that the reported delayed time is still correct (keep track of the
> delayed time between 2 reports).
>
> - does not add any extra pg_clock_gettime_ns() calls (as compare to v2).
>
Sounds good to me. I think it's better to keep the logic for
throttling the reporting the delay message simple. It's an important
consideration but executing parallel vacuum with delays would be less
likely to be used in practice.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Track the amount of time waiting due to cost_delay
@ 2024-06-28 20:07 Imseih (AWS), Sami <[email protected]>
parent: Imseih (AWS), Sami <[email protected]>
1 sibling, 1 reply; 18+ messages in thread
From: Imseih (AWS), Sami @ 2024-06-28 20:07 UTC (permalink / raw)
To: Bertrand Drouvot <[email protected]>; Masahiko Sawada <[email protected]>; +Cc: [email protected] <[email protected]>; Robert Haas <[email protected]>
> 46ebdfe164 will interrupt the leaders sleep every time a parallel workers reports
> progress, and we currently don't handle interrupts by restarting the sleep with
> the remaining time. nanosleep does provide the ability to restart with the remaining
> time [1], but I don't think it's worth the effort to ensure more accurate
> vacuum delays for the leader process.
After discussing offline with Bertrand, it may be better to have
a solution to deal with the interrupts and allows the sleep to continue to
completion. This will simplify this patch and will be useful
for other cases in which parallel workers need to send a message
to the leader. This is the thread [1] for that discussion.
[1] https://www.postgresql.org/message-id/01000190606e3d2a-116ead16-84d2-4449-8d18-5053da66b1f4-000000%4...
Regards,
Sami
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Track the amount of time waiting due to cost_delay
@ 2024-07-01 04:59 Bertrand Drouvot <[email protected]>
parent: Imseih (AWS), Sami <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Bertrand Drouvot @ 2024-07-01 04:59 UTC (permalink / raw)
To: Imseih (AWS), Sami <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; [email protected] <[email protected]>; Robert Haas <[email protected]>
Hi,
On Fri, Jun 28, 2024 at 08:07:39PM +0000, Imseih (AWS), Sami wrote:
> > 46ebdfe164 will interrupt the leaders sleep every time a parallel workers reports
> > progress, and we currently don't handle interrupts by restarting the sleep with
> > the remaining time. nanosleep does provide the ability to restart with the remaining
> > time [1], but I don't think it's worth the effort to ensure more accurate
> > vacuum delays for the leader process.
>
> After discussing offline with Bertrand, it may be better to have
> a solution to deal with the interrupts and allows the sleep to continue to
> completion. This will simplify this patch and will be useful
> for other cases in which parallel workers need to send a message
> to the leader. This is the thread [1] for that discussion.
>
> [1] https://www.postgresql.org/message-id/01000190606e3d2a-116ead16-84d2-4449-8d18-5053da66b1f4-000000%4...
>
Yeah, I think it would make sense to put this thread on hold until we know more
about [1] (you mentioned above) outcome.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Track the amount of time waiting due to cost_delay
@ 2024-08-20 12:48 Bertrand Drouvot <[email protected]>
parent: Bertrand Drouvot <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Bertrand Drouvot @ 2024-08-20 12:48 UTC (permalink / raw)
To: Imseih (AWS), Sami <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; [email protected] <[email protected]>; Robert Haas <[email protected]>
Hi,
On Mon, Jul 01, 2024 at 04:59:25AM +0000, Bertrand Drouvot wrote:
> Hi,
>
> On Fri, Jun 28, 2024 at 08:07:39PM +0000, Imseih (AWS), Sami wrote:
> > > 46ebdfe164 will interrupt the leaders sleep every time a parallel workers reports
> > > progress, and we currently don't handle interrupts by restarting the sleep with
> > > the remaining time. nanosleep does provide the ability to restart with the remaining
> > > time [1], but I don't think it's worth the effort to ensure more accurate
> > > vacuum delays for the leader process.
> >
> > After discussing offline with Bertrand, it may be better to have
> > a solution to deal with the interrupts and allows the sleep to continue to
> > completion. This will simplify this patch and will be useful
> > for other cases in which parallel workers need to send a message
> > to the leader. This is the thread [1] for that discussion.
> >
> > [1] https://www.postgresql.org/message-id/01000190606e3d2a-116ead16-84d2-4449-8d18-5053da66b1f4-000000%4...
> >
>
> Yeah, I think it would make sense to put this thread on hold until we know more
> about [1] (you mentioned above) outcome.
As it looks like we have a consensus not to wait on [0] (as reducing the number
of interrupts makes sense on its own), then please find attached v4, a rebase
version (that also makes clear in the doc that that new field might show slightly
old values, as mentioned in [1]).
[0]: https://www.postgresql.org/message-id/flat/01000190606e3d2a-116ead16-84d2-4449-8d18-5053da66b1f4-000...
[1]: https://www.postgresql.org/message-id/ZruMe-ppopQX4uP8%40nathan
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v4-0001-Report-the-total-amount-of-time-that-vacuum-has-b.patch (8.5K, ../../[email protected]/2-v4-0001-Report-the-total-amount-of-time-that-vacuum-has-b.patch)
download | inline diff:
From 90196125d1262095d02f0df74bb6cab0d03c75ff Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Mon, 24 Jun 2024 08:43:26 +0000
Subject: [PATCH v4] Report the total amount of time that vacuum has been
delayed due to cost delay
This commit adds one column: time_delayed to the pg_stat_progress_vacuum system
view to show the total amount of time in milliseconds that vacuum has been
delayed.
This uses the new parallel message type for progress reporting added
by f1889729dd.
In case of parallel worker, to avoid the leader to be interrupted too frequently
(while it might be sleeping for cost delay), the report is done only if the last
report has been done more than 1 second ago.
Having a time based only approach to throttle the reporting of the parallel
workers sounds reasonable.
Indeed when deciding about the throttling:
1. The number of parallel workers should not come into play:
1.1) the more parallel workers is used, the less the impact of the leader on
the vacuum index phase duration/workload is (because the repartition is done
on more processes).
1.2) the less parallel workers is, the less the leader will be interrupted (
less parallel workers would report their delayed time).
2. The cost limit should not come into play as that value is distributed
proportionally among the parallel workers (so we're back to the previous point).
3. The cost delay does not come into play as the leader could be interrupted at
the beginning, the midle or whatever part of the wait and we are more interested
about the frequency of the interrupts.
3. A 1 second reporting "throttling" looks a reasonable threshold as:
3.1 the idea is to have a significant impact when the leader could have been
interrupted say hundred/thousand times per second.
3.2 it does not make that much sense for any tools to sample pg_stat_progress_vacuum
multiple times per second (so a one second reporting granularity seems ok).
Bump catversion because this changes the definition of pg_stat_progress_vacuum.
---
doc/src/sgml/monitoring.sgml | 13 ++++++++
src/backend/catalog/system_views.sql | 2 +-
src/backend/commands/vacuum.c | 49 ++++++++++++++++++++++++++++
src/include/catalog/catversion.h | 2 +-
src/include/commands/progress.h | 1 +
src/test/regress/expected/rules.out | 3 +-
6 files changed, 67 insertions(+), 3 deletions(-)
23.5% doc/src/sgml/
4.2% src/backend/catalog/
63.4% src/backend/commands/
4.6% src/include/
4.0% src/test/regress/expected/
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 55417a6fa9..d87604331a 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -6307,6 +6307,19 @@ FROM pg_stat_get_backend_idset() AS backendid;
<literal>cleaning up indexes</literal>.
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>time_delayed</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Total amount of time spent in milliseconds waiting due to <varname>vacuum_cost_delay</varname>
+ or <varname>autovacuum_vacuum_cost_delay</varname>. In case of parallel
+ vacuum the reported time is across all the workers and the leader. This
+ column is updated at a 1 Hz frequency (one time per second) so could show
+ slightly old values.
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 19cabc9a47..875df7d0e4 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1218,7 +1218,7 @@ CREATE VIEW pg_stat_progress_vacuum AS
S.param4 AS heap_blks_vacuumed, S.param5 AS index_vacuum_count,
S.param6 AS max_dead_tuple_bytes, S.param7 AS dead_tuple_bytes,
S.param8 AS num_dead_item_ids, S.param9 AS indexes_total,
- S.param10 AS indexes_processed
+ S.param10 AS indexes_processed, S.param11 AS time_delayed
FROM pg_stat_get_progress_info('VACUUM') AS S
LEFT JOIN pg_database D ON S.datid = D.oid;
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 7d8e9d2045..5bf2e37d3f 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -40,6 +40,7 @@
#include "catalog/pg_inherits.h"
#include "commands/cluster.h"
#include "commands/defrem.h"
+#include "commands/progress.h"
#include "commands/vacuum.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
@@ -60,6 +61,12 @@
#include "utils/snapmgr.h"
#include "utils/syscache.h"
+/*
+ * Minimum amount of time (in ms) between two reports of the delayed time from a
+ * parallel worker to the leader. The goal is to avoid the leader to be
+ * interrupted too frequently while it might be sleeping for cost delay.
+ */
+#define WORKER_REPORT_DELAY_INTERVAL 1000
/*
* GUC parameters
@@ -103,6 +110,16 @@ pg_atomic_uint32 *VacuumSharedCostBalance = NULL;
pg_atomic_uint32 *VacuumActiveNWorkers = NULL;
int VacuumCostBalanceLocal = 0;
+/*
+ * In case of parallel workers, the last time the delay has been reported to
+ * the leader.
+ * We assume this initializes to zero.
+ */
+static instr_time last_report_time;
+
+/* total nap time between two reports */
+double nap_time_since_last_report = 0;
+
/* non-export function prototypes */
static List *expand_vacuum_rel(VacuumRelation *vrel,
MemoryContext vac_context, int options);
@@ -2377,13 +2394,45 @@ vacuum_delay_point(void)
/* Nap if appropriate */
if (msec > 0)
{
+ instr_time delay_start;
+ instr_time delay_end;
+ instr_time delayed_time;
+
if (msec > vacuum_cost_delay * 4)
msec = vacuum_cost_delay * 4;
pgstat_report_wait_start(WAIT_EVENT_VACUUM_DELAY);
+ INSTR_TIME_SET_CURRENT(delay_start);
pg_usleep(msec * 1000);
+ INSTR_TIME_SET_CURRENT(delay_end);
pgstat_report_wait_end();
+ /* Report the amount of time we slept */
+ INSTR_TIME_SET_ZERO(delayed_time);
+ INSTR_TIME_ACCUM_DIFF(delayed_time, delay_end, delay_start);
+
+ /* Parallel worker */
+ if (IsParallelWorker())
+ {
+ instr_time time_since_last_report;
+
+ INSTR_TIME_SET_ZERO(time_since_last_report);
+ INSTR_TIME_ACCUM_DIFF(time_since_last_report, delay_end,
+ last_report_time);
+ nap_time_since_last_report += INSTR_TIME_GET_MILLISEC(delayed_time);
+
+ if (INSTR_TIME_GET_MILLISEC(time_since_last_report) > WORKER_REPORT_DELAY_INTERVAL)
+ {
+ pgstat_progress_parallel_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
+ nap_time_since_last_report);
+ nap_time_since_last_report = 0;
+ last_report_time = delay_end;
+ }
+ }
+ else
+ pgstat_progress_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
+ INSTR_TIME_GET_MILLISEC(delayed_time));
+
/*
* We don't want to ignore postmaster death during very long vacuums
* with vacuum_cost_delay configured. We can't use the usual
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 9a0ae27823..ec1f13748f 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -57,6 +57,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 202408122
+#define CATALOG_VERSION_NO 202408201
#endif
diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h
index 5616d64523..9a0c2358c6 100644
--- a/src/include/commands/progress.h
+++ b/src/include/commands/progress.h
@@ -28,6 +28,7 @@
#define PROGRESS_VACUUM_NUM_DEAD_ITEM_IDS 7
#define PROGRESS_VACUUM_INDEXES_TOTAL 8
#define PROGRESS_VACUUM_INDEXES_PROCESSED 9
+#define PROGRESS_VACUUM_TIME_DELAYED 10
/* Phases of vacuum (as advertised via PROGRESS_VACUUM_PHASE) */
#define PROGRESS_VACUUM_PHASE_SCAN_HEAP 1
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 862433ee52..2bef31a66d 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2052,7 +2052,8 @@ pg_stat_progress_vacuum| SELECT s.pid,
s.param7 AS dead_tuple_bytes,
s.param8 AS num_dead_item_ids,
s.param9 AS indexes_total,
- s.param10 AS indexes_processed
+ s.param10 AS indexes_processed,
+ s.param11 AS time_delayed
FROM (pg_stat_get_progress_info('VACUUM'::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_recovery_prefetch| SELECT stats_reset,
--
2.34.1
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Track the amount of time waiting due to cost_delay
@ 2024-09-02 05:11 Bertrand Drouvot <[email protected]>
parent: Bertrand Drouvot <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Bertrand Drouvot @ 2024-09-02 05:11 UTC (permalink / raw)
To: Imseih (AWS), Sami <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; [email protected] <[email protected]>; Robert Haas <[email protected]>
Hi,
On Tue, Aug 20, 2024 at 12:48:29PM +0000, Bertrand Drouvot wrote:
> As it looks like we have a consensus not to wait on [0] (as reducing the number
> of interrupts makes sense on its own), then please find attached v4, a rebase
> version (that also makes clear in the doc that that new field might show slightly
> old values, as mentioned in [1]).
Please find attached v5, a mandatory rebase.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v5-0001-Report-the-total-amount-of-time-that-vacuum-has-b.patch (8.5K, ../../[email protected]/2-v5-0001-Report-the-total-amount-of-time-that-vacuum-has-b.patch)
download | inline diff:
From 1a14b708e0ee74c2f38835968d828c54022a5526 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Mon, 24 Jun 2024 08:43:26 +0000
Subject: [PATCH v5] Report the total amount of time that vacuum has been
delayed due to cost delay
This commit adds one column: time_delayed to the pg_stat_progress_vacuum system
view to show the total amount of time in milliseconds that vacuum has been
delayed.
This uses the new parallel message type for progress reporting added
by f1889729dd.
In case of parallel worker, to avoid the leader to be interrupted too frequently
(while it might be sleeping for cost delay), the report is done only if the last
report has been done more than 1 second ago.
Having a time based only approach to throttle the reporting of the parallel
workers sounds reasonable.
Indeed when deciding about the throttling:
1. The number of parallel workers should not come into play:
1.1) the more parallel workers is used, the less the impact of the leader on
the vacuum index phase duration/workload is (because the repartition is done
on more processes).
1.2) the less parallel workers is, the less the leader will be interrupted (
less parallel workers would report their delayed time).
2. The cost limit should not come into play as that value is distributed
proportionally among the parallel workers (so we're back to the previous point).
3. The cost delay does not come into play as the leader could be interrupted at
the beginning, the midle or whatever part of the wait and we are more interested
about the frequency of the interrupts.
3. A 1 second reporting "throttling" looks a reasonable threshold as:
3.1 the idea is to have a significant impact when the leader could have been
interrupted say hundred/thousand times per second.
3.2 it does not make that much sense for any tools to sample pg_stat_progress_vacuum
multiple times per second (so a one second reporting granularity seems ok).
Bump catversion because this changes the definition of pg_stat_progress_vacuum.
---
doc/src/sgml/monitoring.sgml | 13 ++++++++
src/backend/catalog/system_views.sql | 2 +-
src/backend/commands/vacuum.c | 49 ++++++++++++++++++++++++++++
src/include/catalog/catversion.h | 2 +-
src/include/commands/progress.h | 1 +
src/test/regress/expected/rules.out | 3 +-
6 files changed, 67 insertions(+), 3 deletions(-)
23.5% doc/src/sgml/
4.2% src/backend/catalog/
63.4% src/backend/commands/
4.6% src/include/
4.0% src/test/regress/expected/
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 55417a6fa9..d87604331a 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -6307,6 +6307,19 @@ FROM pg_stat_get_backend_idset() AS backendid;
<literal>cleaning up indexes</literal>.
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>time_delayed</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Total amount of time spent in milliseconds waiting due to <varname>vacuum_cost_delay</varname>
+ or <varname>autovacuum_vacuum_cost_delay</varname>. In case of parallel
+ vacuum the reported time is across all the workers and the leader. This
+ column is updated at a 1 Hz frequency (one time per second) so could show
+ slightly old values.
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 19cabc9a47..875df7d0e4 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1218,7 +1218,7 @@ CREATE VIEW pg_stat_progress_vacuum AS
S.param4 AS heap_blks_vacuumed, S.param5 AS index_vacuum_count,
S.param6 AS max_dead_tuple_bytes, S.param7 AS dead_tuple_bytes,
S.param8 AS num_dead_item_ids, S.param9 AS indexes_total,
- S.param10 AS indexes_processed
+ S.param10 AS indexes_processed, S.param11 AS time_delayed
FROM pg_stat_get_progress_info('VACUUM') AS S
LEFT JOIN pg_database D ON S.datid = D.oid;
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 7d8e9d2045..5bf2e37d3f 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -40,6 +40,7 @@
#include "catalog/pg_inherits.h"
#include "commands/cluster.h"
#include "commands/defrem.h"
+#include "commands/progress.h"
#include "commands/vacuum.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
@@ -60,6 +61,12 @@
#include "utils/snapmgr.h"
#include "utils/syscache.h"
+/*
+ * Minimum amount of time (in ms) between two reports of the delayed time from a
+ * parallel worker to the leader. The goal is to avoid the leader to be
+ * interrupted too frequently while it might be sleeping for cost delay.
+ */
+#define WORKER_REPORT_DELAY_INTERVAL 1000
/*
* GUC parameters
@@ -103,6 +110,16 @@ pg_atomic_uint32 *VacuumSharedCostBalance = NULL;
pg_atomic_uint32 *VacuumActiveNWorkers = NULL;
int VacuumCostBalanceLocal = 0;
+/*
+ * In case of parallel workers, the last time the delay has been reported to
+ * the leader.
+ * We assume this initializes to zero.
+ */
+static instr_time last_report_time;
+
+/* total nap time between two reports */
+double nap_time_since_last_report = 0;
+
/* non-export function prototypes */
static List *expand_vacuum_rel(VacuumRelation *vrel,
MemoryContext vac_context, int options);
@@ -2377,13 +2394,45 @@ vacuum_delay_point(void)
/* Nap if appropriate */
if (msec > 0)
{
+ instr_time delay_start;
+ instr_time delay_end;
+ instr_time delayed_time;
+
if (msec > vacuum_cost_delay * 4)
msec = vacuum_cost_delay * 4;
pgstat_report_wait_start(WAIT_EVENT_VACUUM_DELAY);
+ INSTR_TIME_SET_CURRENT(delay_start);
pg_usleep(msec * 1000);
+ INSTR_TIME_SET_CURRENT(delay_end);
pgstat_report_wait_end();
+ /* Report the amount of time we slept */
+ INSTR_TIME_SET_ZERO(delayed_time);
+ INSTR_TIME_ACCUM_DIFF(delayed_time, delay_end, delay_start);
+
+ /* Parallel worker */
+ if (IsParallelWorker())
+ {
+ instr_time time_since_last_report;
+
+ INSTR_TIME_SET_ZERO(time_since_last_report);
+ INSTR_TIME_ACCUM_DIFF(time_since_last_report, delay_end,
+ last_report_time);
+ nap_time_since_last_report += INSTR_TIME_GET_MILLISEC(delayed_time);
+
+ if (INSTR_TIME_GET_MILLISEC(time_since_last_report) > WORKER_REPORT_DELAY_INTERVAL)
+ {
+ pgstat_progress_parallel_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
+ nap_time_since_last_report);
+ nap_time_since_last_report = 0;
+ last_report_time = delay_end;
+ }
+ }
+ else
+ pgstat_progress_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
+ INSTR_TIME_GET_MILLISEC(delayed_time));
+
/*
* We don't want to ignore postmaster death during very long vacuums
* with vacuum_cost_delay configured. We can't use the usual
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 1980d492c3..fbee0db2eb 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -57,6 +57,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 202408301
+#define CATALOG_VERSION_NO 202409021
#endif
diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h
index 5616d64523..9a0c2358c6 100644
--- a/src/include/commands/progress.h
+++ b/src/include/commands/progress.h
@@ -28,6 +28,7 @@
#define PROGRESS_VACUUM_NUM_DEAD_ITEM_IDS 7
#define PROGRESS_VACUUM_INDEXES_TOTAL 8
#define PROGRESS_VACUUM_INDEXES_PROCESSED 9
+#define PROGRESS_VACUUM_TIME_DELAYED 10
/* Phases of vacuum (as advertised via PROGRESS_VACUUM_PHASE) */
#define PROGRESS_VACUUM_PHASE_SCAN_HEAP 1
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 862433ee52..2bef31a66d 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2052,7 +2052,8 @@ pg_stat_progress_vacuum| SELECT s.pid,
s.param7 AS dead_tuple_bytes,
s.param8 AS num_dead_item_ids,
s.param9 AS indexes_total,
- s.param10 AS indexes_processed
+ s.param10 AS indexes_processed,
+ s.param11 AS time_delayed
FROM (pg_stat_get_progress_info('VACUUM'::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_recovery_prefetch| SELECT stats_reset,
--
2.34.1
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Track the amount of time waiting due to cost_delay
@ 2024-09-05 04:59 Bertrand Drouvot <[email protected]>
parent: Bertrand Drouvot <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Bertrand Drouvot @ 2024-09-05 04:59 UTC (permalink / raw)
To: Imseih (AWS), Sami <[email protected]>; +Cc: Masahiko Sawada <[email protected]>; [email protected] <[email protected]>; Robert Haas <[email protected]>
Hi,
On Mon, Sep 02, 2024 at 05:11:36AM +0000, Bertrand Drouvot wrote:
> Hi,
>
> On Tue, Aug 20, 2024 at 12:48:29PM +0000, Bertrand Drouvot wrote:
> > As it looks like we have a consensus not to wait on [0] (as reducing the number
> > of interrupts makes sense on its own), then please find attached v4, a rebase
> > version (that also makes clear in the doc that that new field might show slightly
> > old values, as mentioned in [1]).
>
> Please find attached v5, a mandatory rebase.
Please find attached v6, a mandatory rebase due to catversion bump conflict.
I'm removing the catversion bump from the patch as it generates too frequent
conflicts (just mention it needs to be done in the commit message).
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v6-0001-Report-the-total-amount-of-time-that-vacuum-has-b.patch (8.1K, ../../[email protected]/2-v6-0001-Report-the-total-amount-of-time-that-vacuum-has-b.patch)
download | inline diff:
From 45be7dfd86948415962696128a17a68e49c9a773 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Mon, 24 Jun 2024 08:43:26 +0000
Subject: [PATCH v6] Report the total amount of time that vacuum has been
delayed due to cost delay
This commit adds one column: time_delayed to the pg_stat_progress_vacuum system
view to show the total amount of time in milliseconds that vacuum has been
delayed.
This uses the new parallel message type for progress reporting added
by f1889729dd.
In case of parallel worker, to avoid the leader to be interrupted too frequently
(while it might be sleeping for cost delay), the report is done only if the last
report has been done more than 1 second ago.
Having a time based only approach to throttle the reporting of the parallel
workers sounds reasonable.
Indeed when deciding about the throttling:
1. The number of parallel workers should not come into play:
1.1) the more parallel workers is used, the less the impact of the leader on
the vacuum index phase duration/workload is (because the repartition is done
on more processes).
1.2) the less parallel workers is, the less the leader will be interrupted (
less parallel workers would report their delayed time).
2. The cost limit should not come into play as that value is distributed
proportionally among the parallel workers (so we're back to the previous point).
3. The cost delay does not come into play as the leader could be interrupted at
the beginning, the midle or whatever part of the wait and we are more interested
about the frequency of the interrupts.
3. A 1 second reporting "throttling" looks a reasonable threshold as:
3.1 the idea is to have a significant impact when the leader could have been
interrupted say hundred/thousand times per second.
3.2 it does not make that much sense for any tools to sample pg_stat_progress_vacuum
multiple times per second (so a one second reporting granularity seems ok).
Would need to bump catversion because this changes the definition of
pg_stat_progress_vacuum.
---
doc/src/sgml/monitoring.sgml | 13 ++++++++
src/backend/catalog/system_views.sql | 2 +-
src/backend/commands/vacuum.c | 49 ++++++++++++++++++++++++++++
src/include/commands/progress.h | 1 +
src/test/regress/expected/rules.out | 3 +-
5 files changed, 66 insertions(+), 2 deletions(-)
24.2% doc/src/sgml/
4.3% src/backend/catalog/
65.4% src/backend/commands/
4.2% src/test/regress/expected/
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 933de6fe07..64b0604e04 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -6380,6 +6380,19 @@ FROM pg_stat_get_backend_idset() AS backendid;
<literal>cleaning up indexes</literal>.
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>time_delayed</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Total amount of time spent in milliseconds waiting due to <varname>vacuum_cost_delay</varname>
+ or <varname>autovacuum_vacuum_cost_delay</varname>. In case of parallel
+ vacuum the reported time is across all the workers and the leader. This
+ column is updated at a 1 Hz frequency (one time per second) so could show
+ slightly old values.
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 7fd5d256a1..a40888ef2a 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1218,7 +1218,7 @@ CREATE VIEW pg_stat_progress_vacuum AS
S.param4 AS heap_blks_vacuumed, S.param5 AS index_vacuum_count,
S.param6 AS max_dead_tuple_bytes, S.param7 AS dead_tuple_bytes,
S.param8 AS num_dead_item_ids, S.param9 AS indexes_total,
- S.param10 AS indexes_processed
+ S.param10 AS indexes_processed, S.param11 AS time_delayed
FROM pg_stat_get_progress_info('VACUUM') AS S
LEFT JOIN pg_database D ON S.datid = D.oid;
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 7d8e9d2045..5bf2e37d3f 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -40,6 +40,7 @@
#include "catalog/pg_inherits.h"
#include "commands/cluster.h"
#include "commands/defrem.h"
+#include "commands/progress.h"
#include "commands/vacuum.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
@@ -60,6 +61,12 @@
#include "utils/snapmgr.h"
#include "utils/syscache.h"
+/*
+ * Minimum amount of time (in ms) between two reports of the delayed time from a
+ * parallel worker to the leader. The goal is to avoid the leader to be
+ * interrupted too frequently while it might be sleeping for cost delay.
+ */
+#define WORKER_REPORT_DELAY_INTERVAL 1000
/*
* GUC parameters
@@ -103,6 +110,16 @@ pg_atomic_uint32 *VacuumSharedCostBalance = NULL;
pg_atomic_uint32 *VacuumActiveNWorkers = NULL;
int VacuumCostBalanceLocal = 0;
+/*
+ * In case of parallel workers, the last time the delay has been reported to
+ * the leader.
+ * We assume this initializes to zero.
+ */
+static instr_time last_report_time;
+
+/* total nap time between two reports */
+double nap_time_since_last_report = 0;
+
/* non-export function prototypes */
static List *expand_vacuum_rel(VacuumRelation *vrel,
MemoryContext vac_context, int options);
@@ -2377,13 +2394,45 @@ vacuum_delay_point(void)
/* Nap if appropriate */
if (msec > 0)
{
+ instr_time delay_start;
+ instr_time delay_end;
+ instr_time delayed_time;
+
if (msec > vacuum_cost_delay * 4)
msec = vacuum_cost_delay * 4;
pgstat_report_wait_start(WAIT_EVENT_VACUUM_DELAY);
+ INSTR_TIME_SET_CURRENT(delay_start);
pg_usleep(msec * 1000);
+ INSTR_TIME_SET_CURRENT(delay_end);
pgstat_report_wait_end();
+ /* Report the amount of time we slept */
+ INSTR_TIME_SET_ZERO(delayed_time);
+ INSTR_TIME_ACCUM_DIFF(delayed_time, delay_end, delay_start);
+
+ /* Parallel worker */
+ if (IsParallelWorker())
+ {
+ instr_time time_since_last_report;
+
+ INSTR_TIME_SET_ZERO(time_since_last_report);
+ INSTR_TIME_ACCUM_DIFF(time_since_last_report, delay_end,
+ last_report_time);
+ nap_time_since_last_report += INSTR_TIME_GET_MILLISEC(delayed_time);
+
+ if (INSTR_TIME_GET_MILLISEC(time_since_last_report) > WORKER_REPORT_DELAY_INTERVAL)
+ {
+ pgstat_progress_parallel_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
+ nap_time_since_last_report);
+ nap_time_since_last_report = 0;
+ last_report_time = delay_end;
+ }
+ }
+ else
+ pgstat_progress_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
+ INSTR_TIME_GET_MILLISEC(delayed_time));
+
/*
* We don't want to ignore postmaster death during very long vacuums
* with vacuum_cost_delay configured. We can't use the usual
diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h
index 5616d64523..9a0c2358c6 100644
--- a/src/include/commands/progress.h
+++ b/src/include/commands/progress.h
@@ -28,6 +28,7 @@
#define PROGRESS_VACUUM_NUM_DEAD_ITEM_IDS 7
#define PROGRESS_VACUUM_INDEXES_TOTAL 8
#define PROGRESS_VACUUM_INDEXES_PROCESSED 9
+#define PROGRESS_VACUUM_TIME_DELAYED 10
/* Phases of vacuum (as advertised via PROGRESS_VACUUM_PHASE) */
#define PROGRESS_VACUUM_PHASE_SCAN_HEAP 1
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index a1626f3fae..af3a92f882 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2052,7 +2052,8 @@ pg_stat_progress_vacuum| SELECT s.pid,
s.param7 AS dead_tuple_bytes,
s.param8 AS num_dead_item_ids,
s.param9 AS indexes_total,
- s.param10 AS indexes_processed
+ s.param10 AS indexes_processed,
+ s.param11 AS time_delayed
FROM (pg_stat_get_progress_info('VACUUM'::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_recovery_prefetch| SELECT stats_reset,
--
2.34.1
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Track the amount of time waiting due to cost_delay
@ 2024-09-18 21:04 Nathan Bossart <[email protected]>
parent: Bertrand Drouvot <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Nathan Bossart @ 2024-09-18 21:04 UTC (permalink / raw)
To: Bertrand Drouvot <[email protected]>; +Cc: Imseih (AWS), Sami <[email protected]>; Masahiko Sawada <[email protected]>; [email protected] <[email protected]>; Robert Haas <[email protected]>
On Thu, Sep 05, 2024 at 04:59:54AM +0000, Bertrand Drouvot wrote:
> Please find attached v6, a mandatory rebase due to catversion bump conflict.
> I'm removing the catversion bump from the patch as it generates too frequent
> conflicts (just mention it needs to be done in the commit message).
v6 looks generally reasonable to me. I think the
nap_time_since_last_report variable needs to be marked static, though.
One thing that occurs to me is that this information may not be
particularly useful when parallel workers are used. Without parallelism,
it's easy enough to figure out the percentage of time that your VACUUM is
spending asleep, but when there are parallel workers, it may be hard to
deduce much of anything from the value. I'm not sure that this is a
deal-breaker for the patch, though, if for no other reason than it'll most
likely be used for autovacuum, which doesn't use parallel vacuum yet.
If there are no other concerns, I'll plan on committing this one soon after
a bit of editorialization.
--
nathan
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Track the amount of time waiting due to cost_delay
@ 2024-09-19 07:54 Bertrand Drouvot <[email protected]>
parent: Nathan Bossart <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Bertrand Drouvot @ 2024-09-19 07:54 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Imseih (AWS), Sami <[email protected]>; Masahiko Sawada <[email protected]>; [email protected] <[email protected]>; Robert Haas <[email protected]>
Hi,
On Wed, Sep 18, 2024 at 04:04:53PM -0500, Nathan Bossart wrote:
> On Thu, Sep 05, 2024 at 04:59:54AM +0000, Bertrand Drouvot wrote:
> > Please find attached v6, a mandatory rebase due to catversion bump conflict.
> > I'm removing the catversion bump from the patch as it generates too frequent
> > conflicts (just mention it needs to be done in the commit message).
>
> v6 looks generally reasonable to me.
Thanks for looking at it!
> I think the
> nap_time_since_last_report variable needs to be marked static, though.
Agree.
> One thing that occurs to me is that this information may not be
> particularly useful when parallel workers are used. Without parallelism,
> it's easy enough to figure out the percentage of time that your VACUUM is
> spending asleep, but when there are parallel workers, it may be hard to
> deduce much of anything from the value.
I think that if the number of parallel workers being used are the same across
runs then one can measure "accurately" the impact of some changes (set
vacuum_cost_delay=... for example) on the delay. Without the patch one could just
guess as many others factors could impact the vacuum duration (load on the system,
i/o latency,...).
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Track the amount of time waiting due to cost_delay
@ 2024-10-11 17:12 Bertrand Drouvot <[email protected]>
parent: Bertrand Drouvot <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Bertrand Drouvot @ 2024-10-11 17:12 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Imseih (AWS), Sami <[email protected]>; Masahiko Sawada <[email protected]>; [email protected] <[email protected]>; Robert Haas <[email protected]>
Hi,
On Thu, Sep 19, 2024 at 07:54:21AM +0000, Bertrand Drouvot wrote:
> Hi,
>
> On Wed, Sep 18, 2024 at 04:04:53PM -0500, Nathan Bossart wrote:
> > On Thu, Sep 05, 2024 at 04:59:54AM +0000, Bertrand Drouvot wrote:
> > > Please find attached v6, a mandatory rebase due to catversion bump conflict.
> > > I'm removing the catversion bump from the patch as it generates too frequent
> > > conflicts (just mention it needs to be done in the commit message).
> >
> > v6 looks generally reasonable to me.
>
> Thanks for looking at it!
>
> > I think the
> > nap_time_since_last_report variable needs to be marked static, though.
>
> Agree.
Please find attached v7 where nap_time_since_last_report is declared as static.
That's the only change as compared to v6.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v7-0001-Report-the-total-amount-of-time-that-vacuum-has-b.patch (8.1K, ../../[email protected]/2-v7-0001-Report-the-total-amount-of-time-that-vacuum-has-b.patch)
download | inline diff:
From 7470ac76d5f3a9165d6d0e5b8b20a0fe16ce4b6a Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Mon, 24 Jun 2024 08:43:26 +0000
Subject: [PATCH v7] Report the total amount of time that vacuum has been
delayed due to cost delay
This commit adds one column: time_delayed to the pg_stat_progress_vacuum system
view to show the total amount of time in milliseconds that vacuum has been
delayed.
This uses the new parallel message type for progress reporting added
by f1889729dd.
In case of parallel worker, to avoid the leader to be interrupted too frequently
(while it might be sleeping for cost delay), the report is done only if the last
report has been done more than 1 second ago.
Having a time based only approach to throttle the reporting of the parallel
workers sounds reasonable.
Indeed when deciding about the throttling:
1. The number of parallel workers should not come into play:
1.1) the more parallel workers is used, the less the impact of the leader on
the vacuum index phase duration/workload is (because the repartition is done
on more processes).
1.2) the less parallel workers is, the less the leader will be interrupted (
less parallel workers would report their delayed time).
2. The cost limit should not come into play as that value is distributed
proportionally among the parallel workers (so we're back to the previous point).
3. The cost delay does not come into play as the leader could be interrupted at
the beginning, the midle or whatever part of the wait and we are more interested
about the frequency of the interrupts.
3. A 1 second reporting "throttling" looks a reasonable threshold as:
3.1 the idea is to have a significant impact when the leader could have been
interrupted say hundred/thousand times per second.
3.2 it does not make that much sense for any tools to sample pg_stat_progress_vacuum
multiple times per second (so a one second reporting granularity seems ok).
Would need to bump catversion because this changes the definition of
pg_stat_progress_vacuum.
---
doc/src/sgml/monitoring.sgml | 13 ++++++++
src/backend/catalog/system_views.sql | 2 +-
src/backend/commands/vacuum.c | 49 ++++++++++++++++++++++++++++
src/include/commands/progress.h | 1 +
src/test/regress/expected/rules.out | 3 +-
5 files changed, 66 insertions(+), 2 deletions(-)
24.1% doc/src/sgml/
4.3% src/backend/catalog/
65.4% src/backend/commands/
4.2% src/test/regress/expected/
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 331315f8d3..8b6330830b 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -6410,6 +6410,19 @@ FROM pg_stat_get_backend_idset() AS backendid;
<literal>cleaning up indexes</literal>.
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>time_delayed</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Total amount of time spent in milliseconds waiting due to <varname>vacuum_cost_delay</varname>
+ or <varname>autovacuum_vacuum_cost_delay</varname>. In case of parallel
+ vacuum the reported time is across all the workers and the leader. This
+ column is updated at a 1 Hz frequency (one time per second) so could show
+ slightly old values.
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 3456b821bc..9ed8cfce70 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1220,7 +1220,7 @@ CREATE VIEW pg_stat_progress_vacuum AS
S.param4 AS heap_blks_vacuumed, S.param5 AS index_vacuum_count,
S.param6 AS max_dead_tuple_bytes, S.param7 AS dead_tuple_bytes,
S.param8 AS num_dead_item_ids, S.param9 AS indexes_total,
- S.param10 AS indexes_processed
+ S.param10 AS indexes_processed, S.param11 AS time_delayed
FROM pg_stat_get_progress_info('VACUUM') AS S
LEFT JOIN pg_database D ON S.datid = D.oid;
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index ac8f5d9c25..d56ab66d50 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -40,6 +40,7 @@
#include "catalog/pg_inherits.h"
#include "commands/cluster.h"
#include "commands/defrem.h"
+#include "commands/progress.h"
#include "commands/vacuum.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
@@ -60,6 +61,12 @@
#include "utils/snapmgr.h"
#include "utils/syscache.h"
+/*
+ * Minimum amount of time (in ms) between two reports of the delayed time from a
+ * parallel worker to the leader. The goal is to avoid the leader to be
+ * interrupted too frequently while it might be sleeping for cost delay.
+ */
+#define WORKER_REPORT_DELAY_INTERVAL 1000
/*
* GUC parameters
@@ -103,6 +110,16 @@ pg_atomic_uint32 *VacuumSharedCostBalance = NULL;
pg_atomic_uint32 *VacuumActiveNWorkers = NULL;
int VacuumCostBalanceLocal = 0;
+/*
+ * In case of parallel workers, the last time the delay has been reported to
+ * the leader.
+ * We assume this initializes to zero.
+ */
+static instr_time last_report_time;
+
+/* total nap time between two reports */
+static double nap_time_since_last_report = 0;
+
/* non-export function prototypes */
static List *expand_vacuum_rel(VacuumRelation *vrel,
MemoryContext vac_context, int options);
@@ -2402,13 +2419,45 @@ vacuum_delay_point(void)
/* Nap if appropriate */
if (msec > 0)
{
+ instr_time delay_start;
+ instr_time delay_end;
+ instr_time delayed_time;
+
if (msec > vacuum_cost_delay * 4)
msec = vacuum_cost_delay * 4;
pgstat_report_wait_start(WAIT_EVENT_VACUUM_DELAY);
+ INSTR_TIME_SET_CURRENT(delay_start);
pg_usleep(msec * 1000);
+ INSTR_TIME_SET_CURRENT(delay_end);
pgstat_report_wait_end();
+ /* Report the amount of time we slept */
+ INSTR_TIME_SET_ZERO(delayed_time);
+ INSTR_TIME_ACCUM_DIFF(delayed_time, delay_end, delay_start);
+
+ /* Parallel worker */
+ if (IsParallelWorker())
+ {
+ instr_time time_since_last_report;
+
+ INSTR_TIME_SET_ZERO(time_since_last_report);
+ INSTR_TIME_ACCUM_DIFF(time_since_last_report, delay_end,
+ last_report_time);
+ nap_time_since_last_report += INSTR_TIME_GET_MILLISEC(delayed_time);
+
+ if (INSTR_TIME_GET_MILLISEC(time_since_last_report) > WORKER_REPORT_DELAY_INTERVAL)
+ {
+ pgstat_progress_parallel_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
+ nap_time_since_last_report);
+ nap_time_since_last_report = 0;
+ last_report_time = delay_end;
+ }
+ }
+ else
+ pgstat_progress_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
+ INSTR_TIME_GET_MILLISEC(delayed_time));
+
/*
* We don't want to ignore postmaster death during very long vacuums
* with vacuum_cost_delay configured. We can't use the usual
diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h
index 5616d64523..9a0c2358c6 100644
--- a/src/include/commands/progress.h
+++ b/src/include/commands/progress.h
@@ -28,6 +28,7 @@
#define PROGRESS_VACUUM_NUM_DEAD_ITEM_IDS 7
#define PROGRESS_VACUUM_INDEXES_TOTAL 8
#define PROGRESS_VACUUM_INDEXES_PROCESSED 9
+#define PROGRESS_VACUUM_TIME_DELAYED 10
/* Phases of vacuum (as advertised via PROGRESS_VACUUM_PHASE) */
#define PROGRESS_VACUUM_PHASE_SCAN_HEAP 1
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 2b47013f11..a7baa04441 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2054,7 +2054,8 @@ pg_stat_progress_vacuum| SELECT s.pid,
s.param7 AS dead_tuple_bytes,
s.param8 AS num_dead_item_ids,
s.param9 AS indexes_total,
- s.param10 AS indexes_processed
+ s.param10 AS indexes_processed,
+ s.param11 AS time_delayed
FROM (pg_stat_get_progress_info('VACUUM'::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_recovery_prefetch| SELECT stats_reset,
--
2.34.1
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Track the amount of time waiting due to cost_delay
@ 2024-12-05 08:51 Masahiro Ikeda <[email protected]>
parent: Bertrand Drouvot <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Masahiro Ikeda @ 2024-12-05 08:51 UTC (permalink / raw)
To: Bertrand Drouvot <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Imseih (AWS), Sami <[email protected]>; Masahiko Sawada <[email protected]>; [email protected]; Robert Haas <[email protected]>
Hi,
I recently encountered a case where having this feature would have been
very helpful.
Thank you for developing it! I have a few questions and comments.
Here are questions:
After this patch is merged, are you considering adding delayed_time
information to the logs output by log_autovacuum_min_duration?
In the case I experienced, it would have been great to easily understand
how much of the total execution time was spent on timed delays from the
already executed VACUUM logs.
Recently, this thread has not been active. Is the reason to wait for
[1]?
[1] Vacuum statistics: https://commitfest.postgresql.org/50/5012/
Here are minor comments on the v7 patch:
+ Total amount of time spent in milliseconds waiting due to
<varname>vacuum_cost_delay</varname>
+ or <varname>autovacuum_vacuum_cost_delay</varname>. In case of
parallel
Why not use the <xref> element, for example, <xref
linkend="guc-autovacuum-vacuum-cost-delay"/>,
as in the max_dead_tuple_bytes column?
+ vacuum the reported time is across all the workers and the
leader. This
+ column is updated at a 1 Hz frequency (one time per second) so
could show
+ slightly old values.
I wonder if "Hz frequency" is the best term for the context, as I
couldn’t
find similar usage in other documents, though I’m not a native English
speaker.
FWIW, the document contains a similar description.
* not more frequently than once per PGSTAT_MIN_INTERVAL milliseconds
IIUC, only the worker updates the column at a 1 Hz frequency. Would it
be
better to rephrase the following?"
* The workers update the column no more frequently than once per second,
so it could show slightly old values.
+ if (INSTR_TIME_GET_MILLISEC(time_since_last_report) >
WORKER_REPORT_DELAY_INTERVAL)
+ {
+ pgstat_progress_parallel_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
+ nap_time_since_last_report);
+ nap_time_since_last_report = 0;
+ last_report_time = delay_end;
+ }
IIUC, unsent delayed_time will disappear when the parallel workers exit
because they are not considered in parallel_vacuum_end(). I assume this
is intentional behavior, as it is an acceptable error for the use cases.
I didn't see any comments regarding this, so I wanted to confirm.
Regards,
--
Masahiro Ikeda
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Track the amount of time waiting due to cost_delay
@ 2024-12-05 10:43 Bertrand Drouvot <[email protected]>
parent: Masahiro Ikeda <[email protected]>
0 siblings, 1 reply; 18+ messages in thread
From: Bertrand Drouvot @ 2024-12-05 10:43 UTC (permalink / raw)
To: Masahiro Ikeda <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Imseih (AWS), Sami <[email protected]>; Masahiko Sawada <[email protected]>; [email protected]; Robert Haas <[email protected]>
Hi,
On Thu, Dec 05, 2024 at 05:51:11PM +0900, Masahiro Ikeda wrote:
> Hi,
>
> I recently encountered a case where having this feature would have been very
> helpful.
Oh great, thanks for the feedback!
> Thank you for developing it! I have a few questions and comments.
>
> Here are questions:
>
> After this patch is merged, are you considering adding delayed_time
> information to the logs output by log_autovacuum_min_duration?
> In the case I experienced, it would have been great to easily understand
> how much of the total execution time was spent on timed delays from the
> already executed VACUUM logs.
That's a good point. We already discussed adding some information in a dedicated
view ([1]) (and that's an idea I keep in mind). I also think your idea is worth
it and that it would make sense to start a dedicated thread once this one is
merged.
> Recently, this thread has not been active.
I think than Nathan wants to give time to others to interact on it like you
do ;-) (Nathan please correct me if I'm wrong).
> Here are minor comments on the v7 patch:
Thanks!
> Why not use the <xref> element, for example, <xref
> linkend="guc-autovacuum-vacuum-cost-delay"/>,
> as in the max_dead_tuple_bytes column?
There is multiple places where "<varname>vacuum_cost_delay</varname>" is
being used but I agree that's better to be consistent with how it is done for
this view. Done in v8 attached.
> IIUC, only the worker updates the column at a 1 Hz frequency. Would it be
> better to rephrase the following?"
> * The workers update the column no more frequently than once per second,
> so it could show slightly old values.
Yeah I like the re-wording, done that way in v8.
> + if (INSTR_TIME_GET_MILLISEC(time_since_last_report) >
> WORKER_REPORT_DELAY_INTERVAL)
> + {
> + pgstat_progress_parallel_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
> + nap_time_since_last_report);
> + nap_time_since_last_report = 0;
> + last_report_time = delay_end;
> + }
>
> IIUC, unsent delayed_time will disappear when the parallel workers exit
> because they are not considered in parallel_vacuum_end(). I assume this
> is intentional behavior, as it is an acceptable error for the use cases.
Yeah, people would likely use this new field to monitor long running vacuum.
Long enough that this error should be acceptable. Do you agree?
> I didn't see any comments regarding this, so I wanted to confirm.
Added a comment to make it clear, thanks!
[1]: https://www.postgresql.org/message-id/CAD21AoDOu%3DDZcC%2BPemYmCNGSwbgL1s-5OZkZ1Spd5pSxofWNCw%40mail...
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v8-0001-Report-the-total-amount-of-time-that-vacuum-has-b.patch (8.3K, ../../[email protected]/2-v8-0001-Report-the-total-amount-of-time-that-vacuum-has-b.patch)
download | inline diff:
From fc4b761a917804e6e7de46868d388a41735d8cca Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Mon, 24 Jun 2024 08:43:26 +0000
Subject: [PATCH v8] Report the total amount of time that vacuum has been
delayed due to cost delay
This commit adds one column: time_delayed to the pg_stat_progress_vacuum system
view to show the total amount of time in milliseconds that vacuum has been
delayed.
This uses the new parallel message type for progress reporting added
by f1889729dd.
In case of parallel worker, to avoid the leader to be interrupted too frequently
(while it might be sleeping for cost delay), the report is done only if the last
report has been done more than 1 second ago.
Having a time based only approach to throttle the reporting of the parallel
workers sounds reasonable.
Indeed when deciding about the throttling:
1. The number of parallel workers should not come into play:
1.1) the more parallel workers is used, the less the impact of the leader on
the vacuum index phase duration/workload is (because the repartition is done
on more processes).
1.2) the less parallel workers is, the less the leader will be interrupted (
less parallel workers would report their delayed time).
2. The cost limit should not come into play as that value is distributed
proportionally among the parallel workers (so we're back to the previous point).
3. The cost delay does not come into play as the leader could be interrupted at
the beginning, the midle or whatever part of the wait and we are more interested
about the frequency of the interrupts.
3. A 1 second reporting "throttling" looks a reasonable threshold as:
3.1 the idea is to have a significant impact when the leader could have been
interrupted say hundred/thousand times per second.
3.2 it does not make that much sense for any tools to sample pg_stat_progress_vacuum
multiple times per second (so a one second reporting granularity seems ok).
Would need to bump catversion because this changes the definition of
pg_stat_progress_vacuum.
---
doc/src/sgml/monitoring.sgml | 13 +++++++
src/backend/catalog/system_views.sql | 2 +-
src/backend/commands/vacuum.c | 53 ++++++++++++++++++++++++++++
src/include/commands/progress.h | 1 +
src/test/regress/expected/rules.out | 3 +-
5 files changed, 70 insertions(+), 2 deletions(-)
22.8% doc/src/sgml/
4.0% src/backend/catalog/
67.6% src/backend/commands/
3.8% src/test/regress/expected/
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 840d7f8161..7386f7333d 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -6428,6 +6428,19 @@ FROM pg_stat_get_backend_idset() AS backendid;
<literal>cleaning up indexes</literal>.
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>time_delayed</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Total amount of time spent in milliseconds waiting due to <xref linkend="guc-vacuum-cost-delay"/>
+ or <xref linkend="guc-autovacuum-vacuum-cost-delay"/>. In case of parallel
+ vacuum the reported time is across all the workers and the leader. The
+ workers update the column no more frequently than once per second, so it
+ could show slightly old values.
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index da9a8fe99f..013bd06222 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1222,7 +1222,7 @@ CREATE VIEW pg_stat_progress_vacuum AS
S.param4 AS heap_blks_vacuumed, S.param5 AS index_vacuum_count,
S.param6 AS max_dead_tuple_bytes, S.param7 AS dead_tuple_bytes,
S.param8 AS num_dead_item_ids, S.param9 AS indexes_total,
- S.param10 AS indexes_processed
+ S.param10 AS indexes_processed, S.param11 AS time_delayed
FROM pg_stat_get_progress_info('VACUUM') AS S
LEFT JOIN pg_database D ON S.datid = D.oid;
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index bb639ef51f..6f9e515f56 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -39,6 +39,7 @@
#include "catalog/pg_inherits.h"
#include "commands/cluster.h"
#include "commands/defrem.h"
+#include "commands/progress.h"
#include "commands/vacuum.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
@@ -59,6 +60,16 @@
#include "utils/snapmgr.h"
#include "utils/syscache.h"
+/*
+ * Minimum amount of time (in ms) between two reports of the delayed time from a
+ * parallel worker to the leader. The goal is to avoid the leader to be
+ * interrupted too frequently while it might be sleeping for cost delay.
+ *
+ * Note that unsent delayed_time will disappear when the parallel workers exit
+ * because they are not considered in parallel_vacuum_end(). That's an acceptable
+ * error for the use cases.
+ */
+#define WORKER_REPORT_DELAY_INTERVAL 1000
/*
* GUC parameters
@@ -102,6 +113,16 @@ pg_atomic_uint32 *VacuumSharedCostBalance = NULL;
pg_atomic_uint32 *VacuumActiveNWorkers = NULL;
int VacuumCostBalanceLocal = 0;
+/*
+ * In case of parallel workers, the last time the delay has been reported to
+ * the leader.
+ * We assume this initializes to zero.
+ */
+static instr_time last_report_time;
+
+/* total nap time between two reports */
+static double nap_time_since_last_report = 0;
+
/* non-export function prototypes */
static List *expand_vacuum_rel(VacuumRelation *vrel,
MemoryContext vac_context, int options);
@@ -2402,13 +2423,45 @@ vacuum_delay_point(void)
/* Nap if appropriate */
if (msec > 0)
{
+ instr_time delay_start;
+ instr_time delay_end;
+ instr_time delayed_time;
+
if (msec > vacuum_cost_delay * 4)
msec = vacuum_cost_delay * 4;
pgstat_report_wait_start(WAIT_EVENT_VACUUM_DELAY);
+ INSTR_TIME_SET_CURRENT(delay_start);
pg_usleep(msec * 1000);
+ INSTR_TIME_SET_CURRENT(delay_end);
pgstat_report_wait_end();
+ /* Report the amount of time we slept */
+ INSTR_TIME_SET_ZERO(delayed_time);
+ INSTR_TIME_ACCUM_DIFF(delayed_time, delay_end, delay_start);
+
+ /* Parallel worker */
+ if (IsParallelWorker())
+ {
+ instr_time time_since_last_report;
+
+ INSTR_TIME_SET_ZERO(time_since_last_report);
+ INSTR_TIME_ACCUM_DIFF(time_since_last_report, delay_end,
+ last_report_time);
+ nap_time_since_last_report += INSTR_TIME_GET_MILLISEC(delayed_time);
+
+ if (INSTR_TIME_GET_MILLISEC(time_since_last_report) > WORKER_REPORT_DELAY_INTERVAL)
+ {
+ pgstat_progress_parallel_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
+ nap_time_since_last_report);
+ nap_time_since_last_report = 0;
+ last_report_time = delay_end;
+ }
+ }
+ else
+ pgstat_progress_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
+ INSTR_TIME_GET_MILLISEC(delayed_time));
+
/*
* We don't want to ignore postmaster death during very long vacuums
* with vacuum_cost_delay configured. We can't use the usual
diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h
index 5616d64523..9a0c2358c6 100644
--- a/src/include/commands/progress.h
+++ b/src/include/commands/progress.h
@@ -28,6 +28,7 @@
#define PROGRESS_VACUUM_NUM_DEAD_ITEM_IDS 7
#define PROGRESS_VACUUM_INDEXES_TOTAL 8
#define PROGRESS_VACUUM_INDEXES_PROCESSED 9
+#define PROGRESS_VACUUM_TIME_DELAYED 10
/* Phases of vacuum (as advertised via PROGRESS_VACUUM_PHASE) */
#define PROGRESS_VACUUM_PHASE_SCAN_HEAP 1
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 3014d047fe..8b1154efac 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2056,7 +2056,8 @@ pg_stat_progress_vacuum| SELECT s.pid,
s.param7 AS dead_tuple_bytes,
s.param8 AS num_dead_item_ids,
s.param9 AS indexes_total,
- s.param10 AS indexes_processed
+ s.param10 AS indexes_processed,
+ s.param11 AS time_delayed
FROM (pg_stat_get_progress_info('VACUUM'::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_recovery_prefetch| SELECT stats_reset,
--
2.34.1
^ permalink raw reply [nested|flat] 18+ messages in thread
* Re: Track the amount of time waiting due to cost_delay
@ 2024-12-06 09:31 Bertrand Drouvot <[email protected]>
parent: Bertrand Drouvot <[email protected]>
0 siblings, 0 replies; 18+ messages in thread
From: Bertrand Drouvot @ 2024-12-06 09:31 UTC (permalink / raw)
To: Masahiro Ikeda <[email protected]>; +Cc: Nathan Bossart <[email protected]>; Imseih (AWS), Sami <[email protected]>; Masahiko Sawada <[email protected]>; [email protected]; Robert Haas <[email protected]>
Hi,
On Thu, Dec 05, 2024 at 10:43:51AM +0000, Bertrand Drouvot wrote:
> Yeah, people would likely use this new field to monitor long running vacuum.
> Long enough that this error should be acceptable. Do you agree?
OTOH, adding the 100% accuracy looks as simple as v9-0002 attached (0001 is
same as for v8), so I think we should provide it.
Thoughts?
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v9-0001-Report-the-total-amount-of-time-that-vacuum-has-b.patch (8.3K, ../../[email protected]/2-v9-0001-Report-the-total-amount-of-time-that-vacuum-has-b.patch)
download | inline diff:
From 8be8a71eb3c010d51bd6749dce33794f763c8572 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Mon, 24 Jun 2024 08:43:26 +0000
Subject: [PATCH v9 1/2] Report the total amount of time that vacuum has been
delayed due to cost delay
This commit adds one column: time_delayed to the pg_stat_progress_vacuum system
view to show the total amount of time in milliseconds that vacuum has been
delayed.
This uses the new parallel message type for progress reporting added
by f1889729dd.
In case of parallel worker, to avoid the leader to be interrupted too frequently
(while it might be sleeping for cost delay), the report is done only if the last
report has been done more than 1 second ago.
Having a time based only approach to throttle the reporting of the parallel
workers sounds reasonable.
Indeed when deciding about the throttling:
1. The number of parallel workers should not come into play:
1.1) the more parallel workers is used, the less the impact of the leader on
the vacuum index phase duration/workload is (because the repartition is done
on more processes).
1.2) the less parallel workers is, the less the leader will be interrupted (
less parallel workers would report their delayed time).
2. The cost limit should not come into play as that value is distributed
proportionally among the parallel workers (so we're back to the previous point).
3. The cost delay does not come into play as the leader could be interrupted at
the beginning, the midle or whatever part of the wait and we are more interested
about the frequency of the interrupts.
3. A 1 second reporting "throttling" looks a reasonable threshold as:
3.1 the idea is to have a significant impact when the leader could have been
interrupted say hundred/thousand times per second.
3.2 it does not make that much sense for any tools to sample pg_stat_progress_vacuum
multiple times per second (so a one second reporting granularity seems ok).
Would need to bump catversion because this changes the definition of
pg_stat_progress_vacuum.
---
doc/src/sgml/monitoring.sgml | 13 +++++++
src/backend/catalog/system_views.sql | 2 +-
src/backend/commands/vacuum.c | 53 ++++++++++++++++++++++++++++
src/include/commands/progress.h | 1 +
src/test/regress/expected/rules.out | 3 +-
5 files changed, 70 insertions(+), 2 deletions(-)
22.8% doc/src/sgml/
4.0% src/backend/catalog/
67.6% src/backend/commands/
3.8% src/test/regress/expected/
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 840d7f8161..7386f7333d 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -6428,6 +6428,19 @@ FROM pg_stat_get_backend_idset() AS backendid;
<literal>cleaning up indexes</literal>.
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>time_delayed</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Total amount of time spent in milliseconds waiting due to <xref linkend="guc-vacuum-cost-delay"/>
+ or <xref linkend="guc-autovacuum-vacuum-cost-delay"/>. In case of parallel
+ vacuum the reported time is across all the workers and the leader. The
+ workers update the column no more frequently than once per second, so it
+ could show slightly old values.
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index da9a8fe99f..013bd06222 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1222,7 +1222,7 @@ CREATE VIEW pg_stat_progress_vacuum AS
S.param4 AS heap_blks_vacuumed, S.param5 AS index_vacuum_count,
S.param6 AS max_dead_tuple_bytes, S.param7 AS dead_tuple_bytes,
S.param8 AS num_dead_item_ids, S.param9 AS indexes_total,
- S.param10 AS indexes_processed
+ S.param10 AS indexes_processed, S.param11 AS time_delayed
FROM pg_stat_get_progress_info('VACUUM') AS S
LEFT JOIN pg_database D ON S.datid = D.oid;
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index bb639ef51f..6f9e515f56 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -39,6 +39,7 @@
#include "catalog/pg_inherits.h"
#include "commands/cluster.h"
#include "commands/defrem.h"
+#include "commands/progress.h"
#include "commands/vacuum.h"
#include "miscadmin.h"
#include "nodes/makefuncs.h"
@@ -59,6 +60,16 @@
#include "utils/snapmgr.h"
#include "utils/syscache.h"
+/*
+ * Minimum amount of time (in ms) between two reports of the delayed time from a
+ * parallel worker to the leader. The goal is to avoid the leader to be
+ * interrupted too frequently while it might be sleeping for cost delay.
+ *
+ * Note that unsent delayed_time will disappear when the parallel workers exit
+ * because they are not considered in parallel_vacuum_end(). That's an acceptable
+ * error for the use cases.
+ */
+#define WORKER_REPORT_DELAY_INTERVAL 1000
/*
* GUC parameters
@@ -102,6 +113,16 @@ pg_atomic_uint32 *VacuumSharedCostBalance = NULL;
pg_atomic_uint32 *VacuumActiveNWorkers = NULL;
int VacuumCostBalanceLocal = 0;
+/*
+ * In case of parallel workers, the last time the delay has been reported to
+ * the leader.
+ * We assume this initializes to zero.
+ */
+static instr_time last_report_time;
+
+/* total nap time between two reports */
+static double nap_time_since_last_report = 0;
+
/* non-export function prototypes */
static List *expand_vacuum_rel(VacuumRelation *vrel,
MemoryContext vac_context, int options);
@@ -2402,13 +2423,45 @@ vacuum_delay_point(void)
/* Nap if appropriate */
if (msec > 0)
{
+ instr_time delay_start;
+ instr_time delay_end;
+ instr_time delayed_time;
+
if (msec > vacuum_cost_delay * 4)
msec = vacuum_cost_delay * 4;
pgstat_report_wait_start(WAIT_EVENT_VACUUM_DELAY);
+ INSTR_TIME_SET_CURRENT(delay_start);
pg_usleep(msec * 1000);
+ INSTR_TIME_SET_CURRENT(delay_end);
pgstat_report_wait_end();
+ /* Report the amount of time we slept */
+ INSTR_TIME_SET_ZERO(delayed_time);
+ INSTR_TIME_ACCUM_DIFF(delayed_time, delay_end, delay_start);
+
+ /* Parallel worker */
+ if (IsParallelWorker())
+ {
+ instr_time time_since_last_report;
+
+ INSTR_TIME_SET_ZERO(time_since_last_report);
+ INSTR_TIME_ACCUM_DIFF(time_since_last_report, delay_end,
+ last_report_time);
+ nap_time_since_last_report += INSTR_TIME_GET_MILLISEC(delayed_time);
+
+ if (INSTR_TIME_GET_MILLISEC(time_since_last_report) > WORKER_REPORT_DELAY_INTERVAL)
+ {
+ pgstat_progress_parallel_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
+ nap_time_since_last_report);
+ nap_time_since_last_report = 0;
+ last_report_time = delay_end;
+ }
+ }
+ else
+ pgstat_progress_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
+ INSTR_TIME_GET_MILLISEC(delayed_time));
+
/*
* We don't want to ignore postmaster death during very long vacuums
* with vacuum_cost_delay configured. We can't use the usual
diff --git a/src/include/commands/progress.h b/src/include/commands/progress.h
index 5616d64523..9a0c2358c6 100644
--- a/src/include/commands/progress.h
+++ b/src/include/commands/progress.h
@@ -28,6 +28,7 @@
#define PROGRESS_VACUUM_NUM_DEAD_ITEM_IDS 7
#define PROGRESS_VACUUM_INDEXES_TOTAL 8
#define PROGRESS_VACUUM_INDEXES_PROCESSED 9
+#define PROGRESS_VACUUM_TIME_DELAYED 10
/* Phases of vacuum (as advertised via PROGRESS_VACUUM_PHASE) */
#define PROGRESS_VACUUM_PHASE_SCAN_HEAP 1
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 3014d047fe..8b1154efac 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -2056,7 +2056,8 @@ pg_stat_progress_vacuum| SELECT s.pid,
s.param7 AS dead_tuple_bytes,
s.param8 AS num_dead_item_ids,
s.param9 AS indexes_total,
- s.param10 AS indexes_processed
+ s.param10 AS indexes_processed,
+ s.param11 AS time_delayed
FROM (pg_stat_get_progress_info('VACUUM'::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_recovery_prefetch| SELECT stats_reset,
--
2.34.1
[text/x-diff] v9-0002-Report-the-amount-of-time-we-slept-before-exiting.patch (2.8K, ../../[email protected]/3-v9-0002-Report-the-amount-of-time-we-slept-before-exiting.patch)
download | inline diff:
From 5b382029eb8330bf9daef50093bb483d8e48e6c2 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Fri, 6 Dec 2024 07:39:19 +0000
Subject: [PATCH v9 2/2] Report the amount of time we slept before exiting
parallel workers
or we might get incomplete data due to WORKER_REPORT_DELAY_INTERVAL
---
src/backend/commands/vacuum.c | 6 +-----
src/backend/commands/vacuumparallel.c | 7 +++++++
src/include/commands/vacuum.h | 1 +
3 files changed, 9 insertions(+), 5 deletions(-)
90.3% src/backend/commands/
9.6% src/include/commands/
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 6f9e515f56..7ad42a9507 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -64,10 +64,6 @@
* Minimum amount of time (in ms) between two reports of the delayed time from a
* parallel worker to the leader. The goal is to avoid the leader to be
* interrupted too frequently while it might be sleeping for cost delay.
- *
- * Note that unsent delayed_time will disappear when the parallel workers exit
- * because they are not considered in parallel_vacuum_end(). That's an acceptable
- * error for the use cases.
*/
#define WORKER_REPORT_DELAY_INTERVAL 1000
@@ -121,7 +117,7 @@ int VacuumCostBalanceLocal = 0;
static instr_time last_report_time;
/* total nap time between two reports */
-static double nap_time_since_last_report = 0;
+double nap_time_since_last_report = 0;
/* non-export function prototypes */
static List *expand_vacuum_rel(VacuumRelation *vrel,
diff --git a/src/backend/commands/vacuumparallel.c b/src/backend/commands/vacuumparallel.c
index 67cba17a56..a09b655a13 100644
--- a/src/backend/commands/vacuumparallel.c
+++ b/src/backend/commands/vacuumparallel.c
@@ -1087,6 +1087,13 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc)
InstrEndParallelQuery(&buffer_usage[ParallelWorkerNumber],
&wal_usage[ParallelWorkerNumber]);
+ /*
+ * Report the amount of time we slept (or we might get incomplete data due
+ * to WORKER_REPORT_DELAY_INTERVAL).
+ */
+ pgstat_progress_parallel_incr_param(PROGRESS_VACUUM_TIME_DELAYED,
+ nap_time_since_last_report);
+
TidStoreDetach(dead_items);
/* Pop the error context stack */
diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h
index 759f9a87d3..7a3ff07ec0 100644
--- a/src/include/commands/vacuum.h
+++ b/src/include/commands/vacuum.h
@@ -312,6 +312,7 @@ extern PGDLLIMPORT int VacuumCostBalanceLocal;
extern PGDLLIMPORT bool VacuumFailsafeActive;
extern PGDLLIMPORT double vacuum_cost_delay;
extern PGDLLIMPORT int vacuum_cost_limit;
+extern PGDLLIMPORT double nap_time_since_last_report;
/* in commands/vacuum.c */
extern void ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel);
--
2.34.1
^ permalink raw reply [nested|flat] 18+ messages in thread
end of thread, other threads:[~2024-12-06 09:31 UTC | newest]
Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-08-02 05:59 [PATCH v24 08/15] Add aggregates support in IVM Yugo Nagata <[email protected]>
2024-06-13 11:56 Re: Track the amount of time waiting due to cost_delay Bertrand Drouvot <[email protected]>
2024-06-22 12:48 ` Re: Track the amount of time waiting due to cost_delay Bertrand Drouvot <[email protected]>
2024-06-24 10:50 ` Re: Track the amount of time waiting due to cost_delay Bertrand Drouvot <[email protected]>
2024-06-25 01:12 ` Re: Track the amount of time waiting due to cost_delay Imseih (AWS), Sami <[email protected]>
2024-06-25 08:29 ` Re: Track the amount of time waiting due to cost_delay Bertrand Drouvot <[email protected]>
2024-06-28 20:07 ` Re: Track the amount of time waiting due to cost_delay Imseih (AWS), Sami <[email protected]>
2024-07-01 04:59 ` Re: Track the amount of time waiting due to cost_delay Bertrand Drouvot <[email protected]>
2024-08-20 12:48 ` Re: Track the amount of time waiting due to cost_delay Bertrand Drouvot <[email protected]>
2024-09-02 05:11 ` Re: Track the amount of time waiting due to cost_delay Bertrand Drouvot <[email protected]>
2024-09-05 04:59 ` Re: Track the amount of time waiting due to cost_delay Bertrand Drouvot <[email protected]>
2024-09-18 21:04 ` Re: Track the amount of time waiting due to cost_delay Nathan Bossart <[email protected]>
2024-09-19 07:54 ` Re: Track the amount of time waiting due to cost_delay Bertrand Drouvot <[email protected]>
2024-10-11 17:12 ` Re: Track the amount of time waiting due to cost_delay Bertrand Drouvot <[email protected]>
2024-12-05 08:51 ` Re: Track the amount of time waiting due to cost_delay Masahiro Ikeda <[email protected]>
2024-12-05 10:43 ` Re: Track the amount of time waiting due to cost_delay Bertrand Drouvot <[email protected]>
2024-12-06 09:31 ` Re: Track the amount of time waiting due to cost_delay Bertrand Drouvot <[email protected]>
2024-06-27 05:17 ` Re: Track the amount of time waiting due to cost_delay Masahiko Sawada <[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