public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v9 1/1] shared detoast datum
12+ messages / 3 participants
[nested] [flat]
* [PATCH v9 1/1] shared detoast datum
@ 2024-03-03 05:48 yizhi.fzh <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: yizhi.fzh @ 2024-03-03 05:48 UTC (permalink / raw)
See the overall design & alternative design & testing in
shared_detoast_datum.org
---
src/backend/access/common/detoast.c | 68 +-
src/backend/access/common/toast_compression.c | 10 +-
src/backend/executor/execExpr.c | 60 +-
src/backend/executor/execExprInterp.c | 179 ++++++
src/backend/executor/execTuples.c | 127 ++++
src/backend/executor/execUtils.c | 2 +
src/backend/executor/nodeHashjoin.c | 2 +
src/backend/executor/nodeMergejoin.c | 2 +
src/backend/executor/nodeNestloop.c | 1 +
src/backend/executor/shared_detoast_datum.org | 203 ++++++
src/backend/jit/llvm/llvmjit_expr.c | 26 +-
src/backend/jit/llvm/llvmjit_types.c | 1 +
src/backend/optimizer/plan/createplan.c | 107 +++-
src/backend/optimizer/plan/setrefs.c | 590 +++++++++++++++---
src/include/access/detoast.h | 3 +
src/include/access/toast_compression.h | 4 +-
src/include/executor/execExpr.h | 12 +
src/include/executor/tuptable.h | 14 +
src/include/nodes/execnodes.h | 14 +
src/include/nodes/plannodes.h | 53 ++
src/tools/pgindent/typedefs.list | 2 +
21 files changed, 1342 insertions(+), 138 deletions(-)
create mode 100644 src/backend/executor/shared_detoast_datum.org
diff --git a/src/backend/access/common/detoast.c b/src/backend/access/common/detoast.c
index 3547cdba56..acc9644689 100644
--- a/src/backend/access/common/detoast.c
+++ b/src/backend/access/common/detoast.c
@@ -22,11 +22,11 @@
#include "utils/expandeddatum.h"
#include "utils/rel.h"
-static struct varlena *toast_fetch_datum(struct varlena *attr);
+static struct varlena *toast_fetch_datum(struct varlena *attr, MemoryContext ctx);
static struct varlena *toast_fetch_datum_slice(struct varlena *attr,
int32 sliceoffset,
int32 slicelength);
-static struct varlena *toast_decompress_datum(struct varlena *attr);
+static struct varlena *toast_decompress_datum(struct varlena *attr, MemoryContext ctx);
static struct varlena *toast_decompress_datum_slice(struct varlena *attr, int32 slicelength);
/* ----------
@@ -42,7 +42,7 @@ static struct varlena *toast_decompress_datum_slice(struct varlena *attr, int32
* ----------
*/
struct varlena *
-detoast_external_attr(struct varlena *attr)
+detoast_external_attr_ext(struct varlena *attr, MemoryContext ctx)
{
struct varlena *result;
@@ -51,7 +51,7 @@ detoast_external_attr(struct varlena *attr)
/*
* This is an external stored plain value
*/
- result = toast_fetch_datum(attr);
+ result = toast_fetch_datum(attr, ctx);
}
else if (VARATT_IS_EXTERNAL_INDIRECT(attr))
{
@@ -68,13 +68,13 @@ detoast_external_attr(struct varlena *attr)
/* recurse if value is still external in some other way */
if (VARATT_IS_EXTERNAL(attr))
- return detoast_external_attr(attr);
+ return detoast_external_attr_ext(attr, ctx);
/*
* Copy into the caller's memory context, in case caller tries to
* pfree the result.
*/
- result = (struct varlena *) palloc(VARSIZE_ANY(attr));
+ result = (struct varlena *) MemoryContextAlloc(ctx, VARSIZE_ANY(attr));
memcpy(result, attr, VARSIZE_ANY(attr));
}
else if (VARATT_IS_EXTERNAL_EXPANDED(attr))
@@ -87,7 +87,7 @@ detoast_external_attr(struct varlena *attr)
eoh = DatumGetEOHP(PointerGetDatum(attr));
resultsize = EOH_get_flat_size(eoh);
- result = (struct varlena *) palloc(resultsize);
+ result = (struct varlena *) MemoryContextAlloc(ctx, resultsize);
EOH_flatten_into(eoh, (void *) result, resultsize);
}
else
@@ -101,32 +101,45 @@ detoast_external_attr(struct varlena *attr)
return result;
}
+struct varlena *
+detoast_external_attr(struct varlena *attr)
+{
+ return detoast_external_attr_ext(attr, CurrentMemoryContext);
+}
+
/* ----------
- * detoast_attr -
+ * detoast_attr_ext -
*
* Public entry point to get back a toasted value from compression
* or external storage. The result is always non-extended varlena form.
*
+ * ctx: The memory context which the final value belongs to.
+ *
* Note some callers assume that if the input is an EXTERNAL or COMPRESSED
* datum, the result will be a pfree'able chunk.
* ----------
*/
-struct varlena *
-detoast_attr(struct varlena *attr)
+
+extern struct varlena *
+detoast_attr_ext(struct varlena *attr, MemoryContext ctx)
{
if (VARATT_IS_EXTERNAL_ONDISK(attr))
{
/*
* This is an externally stored datum --- fetch it back from there
*/
- attr = toast_fetch_datum(attr);
+ attr = toast_fetch_datum(attr, ctx);
/* If it's compressed, decompress it */
if (VARATT_IS_COMPRESSED(attr))
{
struct varlena *tmp = attr;
- attr = toast_decompress_datum(tmp);
+ attr = toast_decompress_datum(tmp, ctx);
+ /*
+ * XXX: this pfree block us from using BumpContext directly
+ * we need some extra effort to make it happen at least.
+ */
pfree(tmp);
}
}
@@ -144,14 +157,14 @@ detoast_attr(struct varlena *attr)
Assert(!VARATT_IS_EXTERNAL_INDIRECT(attr));
/* recurse in case value is still extended in some other way */
- attr = detoast_attr(attr);
+ attr = detoast_attr_ext(attr, ctx);
/* if it isn't, we'd better copy it */
if (attr == (struct varlena *) redirect.pointer)
{
struct varlena *result;
- result = (struct varlena *) palloc(VARSIZE_ANY(attr));
+ result = (struct varlena *) MemoryContextAlloc(ctx, VARSIZE_ANY(attr));
memcpy(result, attr, VARSIZE_ANY(attr));
attr = result;
}
@@ -161,7 +174,7 @@ detoast_attr(struct varlena *attr)
/*
* This is an expanded-object pointer --- get flat format
*/
- attr = detoast_external_attr(attr);
+ attr = detoast_external_attr_ext(attr, ctx);
/* flatteners are not allowed to produce compressed/short output */
Assert(!VARATT_IS_EXTENDED(attr));
}
@@ -170,7 +183,7 @@ detoast_attr(struct varlena *attr)
/*
* This is a compressed value inside of the main tuple
*/
- attr = toast_decompress_datum(attr);
+ attr = toast_decompress_datum(attr, ctx);
}
else if (VARATT_IS_SHORT(attr))
{
@@ -181,7 +194,7 @@ detoast_attr(struct varlena *attr)
Size new_size = data_size + VARHDRSZ;
struct varlena *new_attr;
- new_attr = (struct varlena *) palloc(new_size);
+ new_attr = (struct varlena *) MemoryContextAlloc(ctx, new_size);
SET_VARSIZE(new_attr, new_size);
memcpy(VARDATA(new_attr), VARDATA_SHORT(attr), data_size);
attr = new_attr;
@@ -190,6 +203,11 @@ detoast_attr(struct varlena *attr)
return attr;
}
+struct varlena *
+detoast_attr(struct varlena *attr)
+{
+ return detoast_attr_ext(attr, CurrentMemoryContext);
+}
/* ----------
* detoast_attr_slice -
@@ -262,7 +280,7 @@ detoast_attr_slice(struct varlena *attr,
preslice = toast_fetch_datum_slice(attr, 0, max_size);
}
else
- preslice = toast_fetch_datum(attr);
+ preslice = toast_fetch_datum(attr, CurrentMemoryContext);
}
else if (VARATT_IS_EXTERNAL_INDIRECT(attr))
{
@@ -294,7 +312,7 @@ detoast_attr_slice(struct varlena *attr,
if (slicelimit >= 0)
preslice = toast_decompress_datum_slice(tmp, slicelimit);
else
- preslice = toast_decompress_datum(tmp);
+ preslice = toast_decompress_datum(tmp, CurrentMemoryContext);
if (tmp != attr)
pfree(tmp);
@@ -340,7 +358,7 @@ detoast_attr_slice(struct varlena *attr,
* ----------
*/
static struct varlena *
-toast_fetch_datum(struct varlena *attr)
+toast_fetch_datum(struct varlena *attr, MemoryContext ctx)
{
Relation toastrel;
struct varlena *result;
@@ -355,7 +373,7 @@ toast_fetch_datum(struct varlena *attr)
attrsize = VARATT_EXTERNAL_GET_EXTSIZE(toast_pointer);
- result = (struct varlena *) palloc(attrsize + VARHDRSZ);
+ result = (struct varlena *) MemoryContextAlloc(ctx, attrsize + VARHDRSZ);
if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer))
SET_VARSIZE_COMPRESSED(result, attrsize + VARHDRSZ);
@@ -468,7 +486,7 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset,
* Decompress a compressed version of a varlena datum
*/
static struct varlena *
-toast_decompress_datum(struct varlena *attr)
+toast_decompress_datum(struct varlena *attr, MemoryContext ctx)
{
ToastCompressionId cmid;
@@ -482,9 +500,9 @@ toast_decompress_datum(struct varlena *attr)
switch (cmid)
{
case TOAST_PGLZ_COMPRESSION_ID:
- return pglz_decompress_datum(attr);
+ return pglz_decompress_datum(attr, ctx);
case TOAST_LZ4_COMPRESSION_ID:
- return lz4_decompress_datum(attr);
+ return lz4_decompress_datum(attr, ctx);
default:
elog(ERROR, "invalid compression method id %d", cmid);
return NULL; /* keep compiler quiet */
@@ -515,7 +533,7 @@ toast_decompress_datum_slice(struct varlena *attr, int32 slicelength)
* more than the data's true decompressed size.
*/
if ((uint32) slicelength >= TOAST_COMPRESS_EXTSIZE(attr))
- return toast_decompress_datum(attr);
+ return toast_decompress_datum(attr, CurrentMemoryContext);
/*
* Fetch the compression method id stored in the compression header and
diff --git a/src/backend/access/common/toast_compression.c b/src/backend/access/common/toast_compression.c
index 09d05d97c5..323cf013da 100644
--- a/src/backend/access/common/toast_compression.c
+++ b/src/backend/access/common/toast_compression.c
@@ -81,13 +81,13 @@ pglz_compress_datum(const struct varlena *value)
* Decompress a varlena that was compressed using PGLZ.
*/
struct varlena *
-pglz_decompress_datum(const struct varlena *value)
+pglz_decompress_datum(const struct varlena *value, MemoryContext ctx)
{
struct varlena *result;
int32 rawsize;
/* allocate memory for the uncompressed data */
- result = (struct varlena *) palloc(VARDATA_COMPRESSED_GET_EXTSIZE(value) + VARHDRSZ);
+ result = (struct varlena *) MemoryContextAlloc(ctx, VARDATA_COMPRESSED_GET_EXTSIZE(value) + VARHDRSZ);
/* decompress the data */
rawsize = pglz_decompress((char *) value + VARHDRSZ_COMPRESSED,
@@ -181,7 +181,7 @@ lz4_compress_datum(const struct varlena *value)
* Decompress a varlena that was compressed using LZ4.
*/
struct varlena *
-lz4_decompress_datum(const struct varlena *value)
+lz4_decompress_datum(const struct varlena *value, MemoryContext ctx)
{
#ifndef USE_LZ4
NO_LZ4_SUPPORT();
@@ -191,7 +191,7 @@ lz4_decompress_datum(const struct varlena *value)
struct varlena *result;
/* allocate memory for the uncompressed data */
- result = (struct varlena *) palloc(VARDATA_COMPRESSED_GET_EXTSIZE(value) + VARHDRSZ);
+ result = (struct varlena *) MemoryContextAlloc(ctx, VARDATA_COMPRESSED_GET_EXTSIZE(value) + VARHDRSZ);
/* decompress the data */
rawsize = LZ4_decompress_safe((char *) value + VARHDRSZ_COMPRESSED,
@@ -225,7 +225,7 @@ lz4_decompress_datum_slice(const struct varlena *value, int32 slicelength)
/* slice decompression not supported prior to 1.8.3 */
if (LZ4_versionNumber() < 10803)
- return lz4_decompress_datum(value);
+ return lz4_decompress_datum(value, CurrentMemoryContext);
/* allocate memory for the uncompressed data */
result = (struct varlena *) palloc(slicelength + VARHDRSZ);
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 3181b1136a..45c2c625b2 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -932,22 +932,76 @@ ExecInitExprRec(Expr *node, ExprState *state,
}
else
{
+ int attnum;
+ Plan *plan = state->parent ? state->parent->plan : NULL;
+
/* regular user column */
scratch.d.var.attnum = variable->varattno - 1;
scratch.d.var.vartype = variable->vartype;
+ attnum = scratch.d.var.attnum;
+
switch (variable->varno)
{
case INNER_VAR:
- scratch.opcode = EEOP_INNER_VAR;
+
+ if (is_join_plan(plan) &&
+ bms_is_member(attnum,
+ ((JoinState *) state->parent)->inner_pre_detoast_attrs))
+ {
+ scratch.opcode = EEOP_INNER_VAR_TOAST;
+#ifdef DEBUG_PRE_DETOAST_DATUM
+ elog(INFO,
+ "EEOP_INNER_VAR_TOAST: flags = %d costs=%.2f..%.2f, attnum: %d",
+ state->flags,
+ plan->startup_cost,
+ plan->total_cost,
+ attnum);
+#endif
+ }
+ else
+ {
+ scratch.opcode = EEOP_INNER_VAR;
+ }
break;
case OUTER_VAR:
- scratch.opcode = EEOP_OUTER_VAR;
+ if (is_join_plan(plan) &&
+ bms_is_member(attnum,
+ ((JoinState *) state->parent)->outer_pre_detoast_attrs))
+ {
+ scratch.opcode = EEOP_OUTER_VAR_TOAST;
+#ifdef DEBUG_PRE_DETOAST_DATUM
+ elog(INFO,
+ "EEOP_OUTER_VAR_TOAST: flags = %u costs=%.2f..%.2f, attnum: %d",
+ state->flags,
+ plan->startup_cost,
+ plan->total_cost,
+ attnum);
+#endif
+ }
+ else
+ scratch.opcode = EEOP_OUTER_VAR;
break;
/* INDEX_VAR is handled by default case */
default:
- scratch.opcode = EEOP_SCAN_VAR;
+ if (is_scan_plan(plan) && bms_is_member(
+ attnum,
+ ((ScanState *) state->parent)->scan_pre_detoast_attrs))
+ {
+ scratch.opcode = EEOP_SCAN_VAR_TOAST;
+#ifdef DEBUG_PRE_DETOAST_DATUM
+ elog(INFO,
+ "EEOP_SCAN_VAR_TOAST: flags = %u costs=%.2f..%.2f, scanId: %d, attnum: %d",
+ state->flags,
+ plan->startup_cost,
+ plan->total_cost,
+ ((Scan *) plan)->scanrelid,
+ attnum);
+#endif
+ }
+ else
+ scratch.opcode = EEOP_SCAN_VAR;
break;
}
}
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index 3f20f1dd31..7ebaca36bb 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -57,6 +57,7 @@
#include "postgres.h"
#include "access/heaptoast.h"
+#include "access/detoast.h"
#include "catalog/pg_type.h"
#include "commands/sequence.h"
#include "executor/execExpr.h"
@@ -158,6 +159,9 @@ static void ExecEvalRowNullInt(ExprState *state, ExprEvalStep *op,
static Datum ExecJustInnerVar(ExprState *state, ExprContext *econtext, bool *isnull);
static Datum ExecJustOuterVar(ExprState *state, ExprContext *econtext, bool *isnull);
static Datum ExecJustScanVar(ExprState *state, ExprContext *econtext, bool *isnull);
+static Datum ExecJustInnerVarToast(ExprState *state, ExprContext *econtext, bool *isnull);
+static Datum ExecJustOuterVarToast(ExprState *state, ExprContext *econtext, bool *isnull);
+static Datum ExecJustScanVarToast(ExprState *state, ExprContext *econtext, bool *isnull);
static Datum ExecJustAssignInnerVar(ExprState *state, ExprContext *econtext, bool *isnull);
static Datum ExecJustAssignOuterVar(ExprState *state, ExprContext *econtext, bool *isnull);
static Datum ExecJustAssignScanVar(ExprState *state, ExprContext *econtext, bool *isnull);
@@ -166,6 +170,9 @@ static Datum ExecJustConst(ExprState *state, ExprContext *econtext, bool *isnull
static Datum ExecJustInnerVarVirt(ExprState *state, ExprContext *econtext, bool *isnull);
static Datum ExecJustOuterVarVirt(ExprState *state, ExprContext *econtext, bool *isnull);
static Datum ExecJustScanVarVirt(ExprState *state, ExprContext *econtext, bool *isnull);
+static Datum ExecJustInnerVarVirtToast(ExprState *state, ExprContext *econtext, bool *isnull);
+static Datum ExecJustOuterVarVirtToast(ExprState *state, ExprContext *econtext, bool *isnull);
+static Datum ExecJustScanVarVirtToast(ExprState *state, ExprContext *econtext, bool *isnull);
static Datum ExecJustAssignInnerVarVirt(ExprState *state, ExprContext *econtext, bool *isnull);
static Datum ExecJustAssignOuterVarVirt(ExprState *state, ExprContext *econtext, bool *isnull);
static Datum ExecJustAssignScanVarVirt(ExprState *state, ExprContext *econtext, bool *isnull);
@@ -181,6 +188,42 @@ static pg_attribute_always_inline void ExecAggPlainTransByRef(AggState *aggstate
AggStatePerGroup pergroup,
ExprContext *aggcontext,
int setno);
+static inline void
+ExecSlotDetoastDatum(TupleTableSlot *slot, int attnum)
+{
+ if (!slot->tts_isnull[attnum] &&
+ VARATT_IS_EXTENDED(slot->tts_values[attnum]))
+ {
+ if (unlikely(slot->tts_data_mctx == NULL))
+ slot->tts_data_mctx = GenerationContextCreate(slot->tts_mcxt,
+ "tts_value_ctx",
+ ALLOCSET_START_SMALL_SIZES);
+ slot->tts_values[attnum] = PointerGetDatum(detoast_attr_ext(
+ (struct varlena *) slot->tts_values[attnum],
+ /* save the detoast value to the given MemoryContext. */
+ slot->tts_data_mctx));
+ Assert(slot->tts_nvalid > attnum);
+ }
+}
+
+/* JIT requires a non-static (and external?) function */
+void
+ExecSlotDetoastDatumExternal(TupleTableSlot *slot, int attnum)
+{
+ return ExecSlotDetoastDatum(slot, attnum);
+}
+
+
+static inline void
+ExecEvalToastVar(TupleTableSlot *slot,
+ ExprEvalStep *op,
+ int attnum)
+{
+ ExecSlotDetoastDatum(slot, attnum);
+
+ *op->resvalue = slot->tts_values[attnum];
+ *op->resnull = slot->tts_isnull[attnum];
+}
/*
* ScalarArrayOpExprHashEntry
@@ -296,6 +339,24 @@ ExecReadyInterpretedExpr(ExprState *state)
state->evalfunc_private = (void *) ExecJustScanVar;
return;
}
+ if (step0 == EEOP_INNER_FETCHSOME &&
+ step1 == EEOP_INNER_VAR_TOAST)
+ {
+ state->evalfunc_private = (void *) ExecJustInnerVarToast;
+ return;
+ }
+ else if (step0 == EEOP_OUTER_FETCHSOME &&
+ step1 == EEOP_OUTER_VAR_TOAST)
+ {
+ state->evalfunc_private = (void *) ExecJustOuterVarToast;
+ return;
+ }
+ else if (step0 == EEOP_SCAN_FETCHSOME &&
+ step1 == EEOP_SCAN_VAR_TOAST)
+ {
+ state->evalfunc_private = (void *) ExecJustScanVarToast;
+ return;
+ }
else if (step0 == EEOP_INNER_FETCHSOME &&
step1 == EEOP_ASSIGN_INNER_VAR)
{
@@ -346,6 +407,21 @@ ExecReadyInterpretedExpr(ExprState *state)
state->evalfunc_private = (void *) ExecJustScanVarVirt;
return;
}
+ else if (step0 == EEOP_INNER_VAR_TOAST)
+ {
+ state->evalfunc_private = (void *) ExecJustInnerVarVirtToast;
+ return;
+ }
+ else if (step0 == EEOP_OUTER_VAR_TOAST)
+ {
+ state->evalfunc_private = (void *) ExecJustOuterVarVirtToast;
+ return;
+ }
+ else if (step0 == EEOP_SCAN_VAR_TOAST)
+ {
+ state->evalfunc_private = (void *) ExecJustScanVarVirtToast;
+ return;
+ }
else if (step0 == EEOP_ASSIGN_INNER_VAR)
{
state->evalfunc_private = (void *) ExecJustAssignInnerVarVirt;
@@ -413,6 +489,9 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
&&CASE_EEOP_INNER_VAR,
&&CASE_EEOP_OUTER_VAR,
&&CASE_EEOP_SCAN_VAR,
+ &&CASE_EEOP_INNER_VAR_TOAST,
+ &&CASE_EEOP_OUTER_VAR_TOAST,
+ &&CASE_EEOP_SCAN_VAR_TOAST,
&&CASE_EEOP_INNER_SYSVAR,
&&CASE_EEOP_OUTER_SYSVAR,
&&CASE_EEOP_SCAN_SYSVAR,
@@ -597,6 +676,25 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
Assert(attnum >= 0 && attnum < scanslot->tts_nvalid);
*op->resvalue = scanslot->tts_values[attnum];
*op->resnull = scanslot->tts_isnull[attnum];
+ EEO_NEXT();
+ }
+
+ EEO_CASE(EEOP_INNER_VAR_TOAST)
+ {
+ ExecEvalToastVar(innerslot, op, op->d.var.attnum);
+ EEO_NEXT();
+ }
+
+ EEO_CASE(EEOP_OUTER_VAR_TOAST)
+ {
+ ExecEvalToastVar(outerslot, op, op->d.var.attnum);
+
+ EEO_NEXT();
+ }
+
+ EEO_CASE(EEOP_SCAN_VAR_TOAST)
+ {
+ ExecEvalToastVar(scanslot, op, op->d.var.attnum);
EEO_NEXT();
}
@@ -2137,6 +2235,42 @@ ExecJustScanVar(ExprState *state, ExprContext *econtext, bool *isnull)
return ExecJustVarImpl(state, econtext->ecxt_scantuple, isnull);
}
+static pg_attribute_always_inline Datum
+ExecJustVarImplToast(ExprState *state, TupleTableSlot *slot, bool *isnull)
+{
+ ExprEvalStep *op = &state->steps[1];
+ int attnum = op->d.var.attnum;
+
+ CheckOpSlotCompatibility(&state->steps[0], slot);
+
+ slot_getattr(slot, attnum + 1, isnull);
+
+ ExecSlotDetoastDatum(slot, attnum);
+
+ return slot->tts_values[attnum];
+}
+
+/* Simple reference to inner Var */
+static Datum
+ExecJustInnerVarToast(ExprState *state, ExprContext *econtext, bool *isnull)
+{
+ return ExecJustVarImplToast(state, econtext->ecxt_innertuple, isnull);
+}
+
+/* Simple reference to outer Var */
+static Datum
+ExecJustOuterVarToast(ExprState *state, ExprContext *econtext, bool *isnull)
+{
+ return ExecJustVarImplToast(state, econtext->ecxt_outertuple, isnull);
+}
+
+/* Simple reference to scan Var */
+static Datum
+ExecJustScanVarToast(ExprState *state, ExprContext *econtext, bool *isnull)
+{
+ return ExecJustVarImplToast(state, econtext->ecxt_scantuple, isnull);
+}
+
/* implementation of ExecJustAssign(Inner|Outer|Scan)Var */
static pg_attribute_always_inline Datum
ExecJustAssignVarImpl(ExprState *state, TupleTableSlot *inslot, bool *isnull)
@@ -2275,6 +2409,51 @@ ExecJustScanVarVirt(ExprState *state, ExprContext *econtext, bool *isnull)
return ExecJustVarVirtImpl(state, econtext->ecxt_scantuple, isnull);
}
+/* implementation of ExecJust(Inner|Outer|Scan)VarVirt */
+static pg_attribute_always_inline Datum
+ExecJustVarVirtImplToast(ExprState *state, TupleTableSlot *slot, bool *isnull)
+{
+ ExprEvalStep *op = &state->steps[0];
+ int attnum = op->d.var.attnum;
+
+ /*
+ * As it is guaranteed that a virtual slot is used, there never is a need
+ * to perform tuple deforming (nor would it be possible). Therefore
+ * execExpr.c has not emitted an EEOP_*_FETCHSOME step. Verify, as much as
+ * possible, that that determination was accurate.
+ */
+ Assert(TTS_IS_VIRTUAL(slot));
+ Assert(TTS_FIXED(slot));
+ Assert(attnum >= 0 && attnum < slot->tts_nvalid);
+
+ *isnull = slot->tts_isnull[attnum];
+
+ ExecSlotDetoastDatum(slot, attnum);
+
+ return slot->tts_values[attnum];
+}
+
+/* Like ExecJustInnerVar, optimized for virtual slots */
+static Datum
+ExecJustInnerVarVirtToast(ExprState *state, ExprContext *econtext, bool *isnull)
+{
+ return ExecJustVarVirtImplToast(state, econtext->ecxt_innertuple, isnull);
+}
+
+/* Like ExecJustOuterVar, optimized for virtual slots */
+static Datum
+ExecJustOuterVarVirtToast(ExprState *state, ExprContext *econtext, bool *isnull)
+{
+ return ExecJustVarVirtImplToast(state, econtext->ecxt_outertuple, isnull);
+}
+
+/* Like ExecJustScanVar, optimized for virtual slots */
+static Datum
+ExecJustScanVarVirtToast(ExprState *state, ExprContext *econtext, bool *isnull)
+{
+ return ExecJustVarVirtImplToast(state, econtext->ecxt_scantuple, isnull);
+}
+
/* implementation of ExecJustAssign(Inner|Outer|Scan)VarVirt */
static pg_attribute_always_inline Datum
ExecJustAssignVarVirtImpl(ExprState *state, TupleTableSlot *inslot, bool *isnull)
diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c
index a7aa2ee02b..584f9f1fd1 100644
--- a/src/backend/executor/execTuples.c
+++ b/src/backend/executor/execTuples.c
@@ -79,6 +79,9 @@ static inline void tts_buffer_heap_store_tuple(TupleTableSlot *slot,
bool transfer_pin);
static void tts_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, bool shouldFree);
+static Bitmapset *cal_final_pre_detoast_attrs(Bitmapset *reference_attrs,
+ TupleDesc tupleDesc,
+ List *forbid_pre_detoast_vars);
const TupleTableSlotOps TTSOpsVirtual;
const TupleTableSlotOps TTSOpsHeapTuple;
@@ -392,6 +395,12 @@ tts_heap_materialize(TupleTableSlot *slot)
slot->tts_flags |= TTS_FLAG_SHOULDFREE;
MemoryContextSwitchTo(oldContext);
+
+ /*
+ * tts_values is treated invalidated since tts_nvalid will is set to 0,
+ * so let's free the pre-detoast datum.
+ */
+ ExecFreePreDetoastDatum(slot);
}
static void
@@ -449,6 +458,9 @@ tts_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, bool shouldFree)
tts_heap_clear(slot);
+ /* slot_nvalid = 0 */
+ ExecFreePreDetoastDatum(slot);
+
slot->tts_nvalid = 0;
hslot->tuple = tuple;
hslot->off = 0;
@@ -535,6 +547,7 @@ tts_minimal_materialize(TupleTableSlot *slot)
oldContext = MemoryContextSwitchTo(slot->tts_mcxt);
+
/*
* Have to deform from scratch, otherwise tts_values[] entries could point
* into the non-materialized tuple (which might be gone when accessed).
@@ -567,6 +580,9 @@ tts_minimal_materialize(TupleTableSlot *slot)
mslot->minhdr.t_data = (HeapTupleHeader) ((char *) mslot->mintuple - MINIMAL_TUPLE_OFFSET);
MemoryContextSwitchTo(oldContext);
+
+ /* slot_nvalid = 0 */
+ ExecFreePreDetoastDatum(slot);
}
static void
@@ -626,6 +642,9 @@ tts_minimal_store_tuple(TupleTableSlot *slot, MinimalTuple mtup, bool shouldFree
Assert(TTS_EMPTY(slot));
slot->tts_flags &= ~TTS_FLAG_EMPTY;
+
+ /* tts_nvalid = 0 */
+ ExecFreePreDetoastDatum(slot);
slot->tts_nvalid = 0;
mslot->off = 0;
@@ -733,6 +752,10 @@ tts_buffer_heap_materialize(TupleTableSlot *slot)
* into the non-materialized tuple (which might be gone when accessed).
*/
bslot->base.off = 0;
+
+ /* slot_nvalid = 0 */
+ ExecFreePreDetoastDatum(slot);
+
slot->tts_nvalid = 0;
if (!bslot->base.tuple)
@@ -870,6 +893,10 @@ tts_buffer_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple,
}
slot->tts_flags &= ~TTS_FLAG_EMPTY;
+
+ /* tts_nvalid = 0 */
+ ExecFreePreDetoastDatum(slot);
+
slot->tts_nvalid = 0;
bslot->base.tuple = tuple;
bslot->base.off = 0;
@@ -1137,6 +1164,7 @@ MakeTupleTableSlot(TupleDesc tupleDesc,
slot->tts_flags |= TTS_FLAG_FIXED;
slot->tts_tupleDescriptor = tupleDesc;
slot->tts_mcxt = CurrentMemoryContext;
+ slot->tts_data_mctx = NULL;
slot->tts_nvalid = 0;
if (tupleDesc != NULL)
@@ -1215,6 +1243,8 @@ ExecResetTupleTable(List *tupleTable, /* tuple table */
if (slot->tts_isnull)
pfree(slot->tts_isnull);
}
+ if (slot->tts_data_mctx != NULL)
+ MemoryContextDelete(slot->tts_data_mctx);
pfree(slot);
}
}
@@ -1265,6 +1295,9 @@ ExecDropSingleTupleTableSlot(TupleTableSlot *slot)
if (slot->tts_isnull)
pfree(slot->tts_isnull);
}
+ if (slot->tts_data_mctx != NULL)
+ MemoryContextDelete(slot->tts_data_mctx);
+
pfree(slot);
}
@@ -1810,12 +1843,26 @@ void
ExecInitScanTupleSlot(EState *estate, ScanState *scanstate,
TupleDesc tupledesc, const TupleTableSlotOps *tts_ops)
{
+ Scan *splan = (Scan *) scanstate->ps.plan;
+
scanstate->ss_ScanTupleSlot = ExecAllocTableSlot(&estate->es_tupleTable,
tupledesc, tts_ops);
scanstate->ps.scandesc = tupledesc;
scanstate->ps.scanopsfixed = tupledesc != NULL;
scanstate->ps.scanops = tts_ops;
scanstate->ps.scanopsset = true;
+
+ if (is_scan_plan((Plan *) splan))
+ {
+ /*
+ * We may run detoast in Qual or Projection, but all of them happen at
+ * the ss_ScanTupleSlot rather than ps_ResultTupleSlot. So we can only
+ * take care of the ss_ScanTupleSlot.
+ */
+ scanstate->scan_pre_detoast_attrs = cal_final_pre_detoast_attrs(splan->reference_attrs,
+ tupledesc,
+ splan->plan.forbid_pre_detoast_vars);
+ }
}
/* ----------------
@@ -2336,3 +2383,83 @@ end_tup_output(TupOutputState *tstate)
ExecDropSingleTupleTableSlot(tstate->slot);
pfree(tstate);
}
+
+/*
+ * cal_final_pre_detoast_attrs
+ * Calculate the final attributes which pre-detoast be helpful.
+ *
+ * reference_attrs: the attributes which will be detoast at this plan level.
+ * due to the implementation issue, some non-toast attribute may be included
+ * which should be filtered out with tupleDesc.
+ *
+ * forbid_pre_detoast_vars: the vars which should not be pre-detoast as the
+ * small_tlist reason.
+ */
+static Bitmapset *
+cal_final_pre_detoast_attrs(Bitmapset *reference_attrs,
+ TupleDesc tupleDesc,
+ List *forbid_pre_detoast_vars)
+{
+ Bitmapset *final = NULL,
+ *toast_attrs = NULL,
+ *forbid_pre_detoast_attrs = NULL;
+
+ int i;
+ ListCell *lc;
+
+ if (bms_is_empty(reference_attrs))
+ return NULL;
+
+ /*
+ * there is no exact data type in create_plan or set_plan_refs stage, so
+ * reference_attrs may have some attribute which is not toast attrs at
+ * all, which should be removed.
+ */
+ for (i = 0; i < tupleDesc->natts; i++)
+ {
+ Form_pg_attribute attr = TupleDescAttr(tupleDesc, i);
+
+ if (attr->attlen == -1 && attr->attstorage != TYPSTORAGE_PLAIN)
+ toast_attrs = bms_add_member(toast_attrs, attr->attnum - 1);
+ }
+
+ /* Filter out the non-toastable attributes. */
+ final = bms_intersect(reference_attrs, toast_attrs);
+
+ /*
+ * Due to the fact of detoast-datum will make the tuple bigger which is
+ * bad for some nodes like Sort/Hash, to avoid performance regression,
+ * such attribute should be removed as well.
+ */
+ foreach(lc, forbid_pre_detoast_vars)
+ {
+ Var *var = lfirst_node(Var, lc);
+
+ forbid_pre_detoast_attrs = bms_add_member(forbid_pre_detoast_attrs, var->varattno - 1);
+ }
+
+ final = bms_del_members(final, forbid_pre_detoast_attrs);
+
+ bms_free(toast_attrs);
+ bms_free(forbid_pre_detoast_attrs);
+
+ return final;
+}
+
+
+void
+SetPredetoastAttrsForJoin(JoinState *j)
+{
+ PlanState *outerstate = outerPlanState(j);
+ PlanState *innerstate = innerPlanState(j);
+
+ j->outer_pre_detoast_attrs = cal_final_pre_detoast_attrs(
+ ((Join *) j->ps.plan)->outer_reference_attrs,
+ outerstate->ps_ResultTupleDesc,
+ outerstate->plan->forbid_pre_detoast_vars);
+
+ j->inner_pre_detoast_attrs = cal_final_pre_detoast_attrs(
+ ((Join *) j->ps.plan)->inner_reference_attrs,
+ innerstate->ps_ResultTupleDesc,
+ innerstate->plan->forbid_pre_detoast_vars);
+}
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index cff5dc723e..a8646ded02 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -572,6 +572,8 @@ ExecConditionalAssignProjectionInfo(PlanState *planstate, TupleDesc inputDesc,
planstate->resultopsset = planstate->scanopsset;
planstate->resultopsfixed = planstate->scanopsfixed;
planstate->resultops = planstate->scanops;
+
+ Assert(planstate->ps_ResultTupleDesc != NULL);
}
else
{
diff --git a/src/backend/executor/nodeHashjoin.c b/src/backend/executor/nodeHashjoin.c
index 1cbec4647c..19a05ed624 100644
--- a/src/backend/executor/nodeHashjoin.c
+++ b/src/backend/executor/nodeHashjoin.c
@@ -756,6 +756,8 @@ ExecInitHashJoin(HashJoin *node, EState *estate, int eflags)
innerPlanState(hjstate) = ExecInitNode((Plan *) hashNode, estate, eflags);
innerDesc = ExecGetResultType(innerPlanState(hjstate));
+ SetPredetoastAttrsForJoin((JoinState *) hjstate);
+
/*
* Initialize result slot, type and projection.
*/
diff --git a/src/backend/executor/nodeMergejoin.c b/src/backend/executor/nodeMergejoin.c
index c1a8ca2464..be7cbd7f30 100644
--- a/src/backend/executor/nodeMergejoin.c
+++ b/src/backend/executor/nodeMergejoin.c
@@ -1497,6 +1497,8 @@ ExecInitMergeJoin(MergeJoin *node, EState *estate, int eflags)
(eflags | EXEC_FLAG_MARK));
innerDesc = ExecGetResultType(innerPlanState(mergestate));
+ SetPredetoastAttrsForJoin((JoinState *) mergestate);
+
/*
* For certain types of inner child nodes, it is advantageous to issue
* MARK every time we advance past an inner tuple we will never return to.
diff --git a/src/backend/executor/nodeNestloop.c b/src/backend/executor/nodeNestloop.c
index 06fa0a9b31..2d40d19192 100644
--- a/src/backend/executor/nodeNestloop.c
+++ b/src/backend/executor/nodeNestloop.c
@@ -306,6 +306,7 @@ ExecInitNestLoop(NestLoop *node, EState *estate, int eflags)
*/
ExecInitResultTupleSlotTL(&nlstate->js.ps, &TTSOpsVirtual);
ExecAssignProjectionInfo(&nlstate->js.ps, NULL);
+ SetPredetoastAttrsForJoin((JoinState *) nlstate);
/*
* initialize child expressions
diff --git a/src/backend/executor/shared_detoast_datum.org b/src/backend/executor/shared_detoast_datum.org
new file mode 100644
index 0000000000..f63392ef4f
--- /dev/null
+++ b/src/backend/executor/shared_detoast_datum.org
@@ -0,0 +1,203 @@
+The problem:
+-------------
+
+In the current expression engine, a toasted datum is detoasted when
+required, but the result is discarded immediately, either by pfree it
+immediately or leave it for ResetExprContext. Arguments for which one to
+use exists sometimes. More serious problem is detoasting is expensive,
+especially for the data types like jsonb or array, which the value might
+be very huge. In the blow example, the detoasting happens twice.
+
+SELECT jb_col->'a', jb_col->'b' FROM t;
+
+Within the shared-detoast-datum, we just need to detoast once for each
+tuple, and discard it immediately when the tuple is not needed any
+more. FWIW this issue may existing for small numeric, text as well
+because of SHORT_TOAST feature where the toast's len using 1 byte rather
+than 4 bytes.
+
+Current Design
+--------------
+
+The high level design is let createplan.c and setref.c decide which
+Vars can use this feature, and then the executor save the detoast
+datum back slot->to tts_values[*] during the ExprEvalStep of
+EEOP_{INNER|OUTER|SCAN}_VAR_TOAST. The reasons includes:
+
+- The existing expression engine read datum from tts_values[*], no any
+ extra work need to be done.
+- Reuse the lifespan of TupleTableSlot system to manage memory. It
+ is natural to think the detoast datum is a tts_value just that it is
+ in a detoast format. Since we have a clear lifespan for TupleTableSlot
+ already, like ExecClearTuple, ExecCopySlot et. We are easy to reuse
+ them for managing the datoast datum's memory.
+- The existing projection method can copy the datoasted datum (int64)
+ automatically to the next node's slot, but keeping the ownership
+ unchanged, so only the slot where the detoast really happen take the
+ charge of it's lifespan.
+
+Assuming which Var should use this feature has been decided in
+createplan.c and setref.c already. The 3 new ExprEvalSteps
+EEOP_{INNER,OUTER,SCAN}_VAR_TOAST as used. During the evaluating these
+steps, the below code is used.
+
+static inline void
+ExecSlotDetoastDatum(TupleTableSlot *slot, int attnum)
+{
+ if (!slot->tts_isnull[attnum] &&
+ VARATT_IS_EXTENDED(slot->tts_values[attnum]))
+ {
+ if (unlikely(slot->tts_data_mctx == NULL))
+ slot->tts_data_mctx = GenerationContextCreate(slot->tts_mcxt,
+ "tts_value_ctx",
+ ALLOCSET_START_SMALL_SIZES);
+ slot->tts_values[attnum] = PointerGetDatum(detoast_attr_ext((struct varlena *) slot->tts_values[attnum],
+ /* save the detoast value to the given MemoryContext. */
+ slot->tts_data_mctx));
+ }
+}
+
+Since I don't want to the run-time extra check to see if is a detoast
+should happen, so introducing 3 new steps.
+
+When to free the detoast datum? It depends on when the slot's
+tts_values[*] is invalidated, ExecClearTuple is the clear one, but any
+TupleTableSlotOps which set the tts_nvalid = 0 tells us no one will use
+the datum in tts_values[*] so it is time to release them, this is an
+important part for memory usage consideration. since we used
+dedicated MemoryContext for it, so what we just need to do it:
+
+/*
+ * ExecFreePreDetoastDatum - free the memory which is allocated in tts_data_mcxt.
+ */
+static inline void
+ExecFreePreDetoastDatum(TupleTableSlot *slot)
+{
+ if (slot->tts_data_mctx)
+ MemoryContextResetOnly(slot->tts_data_mctx);
+}
+
+
+Now comes to the createplan.c/setref.c part, which decides which Vars
+should use the shared detoast feature. The guideline of this is:
+
+1. It needs a detoast for a given expression in the previous logic.
+2. It should not breaks the CP_SMALL_TLIST design. Since we saved the
+ detoast datum back to tts_values[*], which make tuple bigger. if we
+ do this blindly, it would be harmful to the ORDER / HASH style nodes.
+
+A high level data flow is:
+
+1. at the createplan.c, we walk the plan tree go gather the
+ CP_SMALL_TLIST because of SORT/HASH style nodes, information and save
+ it to Plan.forbid_pre_detoast_vars via the function
+ set_plan_forbid_pre_detoast_vars_recurse.
+
+2. at the setrefs.c, fix_{scan|join}_expr will recurse to Var for each
+ expression, so it is a good time to track the attribute number and
+ see if the Var is directly or indirectly accessed. Usually the
+ indirectly access a Var means a detoast would happens, for
+ example an expression like a > 3. However some known expressions is
+ ignored. for example: NullTest, pg_column_compression which needs the
+ raw datum, start_with/sub_string which needs a slice
+ detoasting. Currently there is some hard code here, we may needs a
+ pg_proc.detoasting_requirement flags to make this generic. The
+ output is {Scan|Join}.xxx_reference_attrs;
+
+Note that here I used '_reference_' rather than '_detoast_' is because
+at this part, I still don't know if it is a toastable attrbiute, which
+is known at the MakeTupleTableSlot stage.
+
+3. At the InitPlan Stage, we calculate the final xxx_pre_detoast_attrs
+ in ScanState & JoinState, which will be passed into expression
+ engine in the ExecInitExprRec stage and EEOP_{INNER|OUTER|SCAN}
+ _VAR_TOAST steps are generated finally then everything is connected
+ with ExecSlotDetoastDatum!
+
+
+Testing
+-------
+
+Case 1: small numeric testing.
+===============================
+
+create table t (a numeric);
+insert into t select i from generate_series(1, 100000)i;
+
+cat 1.sql
+
+select * from t where a > 0;
+
+In this test, the current master run detoast twice for each datum. one
+in numeric_gt, one in numeric_out. this feature makes the detoast once.
+
+pgbench -f 1.sql -n postgres -T 10 -M prepared
+
+master: 30.218 ms
+patched: 26.957 ms
+
+
+Case 2: Big jsonbs test:
+=============================
+
+
+create table b(blog jsonb);
+
+INSERT INTO b
+SELECT jsonb_build_object(
+ 'title', 'title ' || s.i::text,
+ 'content', substring(repeat(md5(random()::text), 100), 1, 3000),
+ 'subscriber', (random() * 100)::int,
+ 'reader', (random() * 100)::int
+ )
+FROM generate_series(1, 10000) s(i);
+
+
+explain analyze
+select blog from b
+where cast(blog->'reader' as numeric) > 10 and
+cast(blog->'subscriber' as numeric) > 0;
+
+Dump and restore the above data into the current master:
+
+master: 24.588 ms
+patched: 17.664 ms
+
+Memory usage test:
+
+I run the workload of tpch scale 10 on against both master and patched
+versions, the memory usage looks stable and the performance doesn't have
+noticeable improvement and regression as well.
+
+A alternative design: toast cache
+---------------------------------
+
+This method is provided by Tomas during the review process. IIUC, this
+method would maintain a local HTAB which map a toast datum to a detoast
+datum and the entry is maintained / used in detoast_attr
+function. Within this method, the overall design is pretty clear and the
+code modification can be controlled in toasting system only.
+
+I assumed that releasing all of the memory at the end of executor once
+is not an option since it may consumed too many memory. Then, when and
+which entry to release becomes a trouble for me. For example:
+
+ QUERY PLAN
+------------------------------
+ Nested Loop
+ Join Filter: (t1.a = t2.a)
+ -> Seq Scan on t1
+ -> Seq Scan on t2
+(4 rows)
+
+In this case t1.a needs a longer lifespan than t2.a since it is
+in outer relation. Without the help from slot's life-cycle system, I
+can't think out a answer for the above question.
+
+Another difference between the 2 methods is my method have many
+modification on createplan.c/setref.c/execExpr.c/execExprInterp.c, but
+it can save some run-time effort like hash_search find / enter run-time
+in method 2 since I put them directly into tts_values[*].
+
+I'm not sure the factor 2 makes some real measurable difference in real
+case, so my current concern mainly comes from factor 1.
diff --git a/src/backend/jit/llvm/llvmjit_expr.c b/src/backend/jit/llvm/llvmjit_expr.c
index 0c448422e2..74563c3454 100644
--- a/src/backend/jit/llvm/llvmjit_expr.c
+++ b/src/backend/jit/llvm/llvmjit_expr.c
@@ -396,30 +396,52 @@ llvm_compile_expr(ExprState *state)
case EEOP_INNER_VAR:
case EEOP_OUTER_VAR:
case EEOP_SCAN_VAR:
+ case EEOP_INNER_VAR_TOAST:
+ case EEOP_OUTER_VAR_TOAST:
+ case EEOP_SCAN_VAR_TOAST:
{
LLVMValueRef value,
isnull;
LLVMValueRef v_attnum;
LLVMValueRef v_values;
LLVMValueRef v_nulls;
+ LLVMValueRef v_slot;
- if (opcode == EEOP_INNER_VAR)
+ if (opcode == EEOP_INNER_VAR || opcode == EEOP_INNER_VAR_TOAST)
{
+ v_slot = v_innerslot;
v_values = v_innervalues;
v_nulls = v_innernulls;
}
- else if (opcode == EEOP_OUTER_VAR)
+ else if (opcode == EEOP_OUTER_VAR || opcode == EEOP_OUTER_VAR_TOAST)
{
+ v_slot = v_outerslot;
v_values = v_outervalues;
v_nulls = v_outernulls;
}
else
{
+ v_slot = v_scanslot;
v_values = v_scanvalues;
v_nulls = v_scannulls;
}
v_attnum = l_int32_const(lc, op->d.var.attnum);
+
+ if (opcode == EEOP_INNER_VAR_TOAST ||
+ opcode == EEOP_OUTER_VAR_TOAST ||
+ opcode == EEOP_SCAN_VAR_TOAST)
+ {
+ LLVMValueRef params[2];
+
+ params[0] = v_slot;
+ params[1] = l_int32_const(lc, op->d.var.attnum);
+ l_call(b,
+ llvm_pg_var_func_type("ExecSlotDetoastDatumExternal"),
+ llvm_pg_func(mod, "ExecSlotDetoastDatumExternal"),
+ params, lengthof(params), "");
+ }
+
value = l_load_gep1(b, TypeSizeT, v_values, v_attnum, "");
isnull = l_load_gep1(b, TypeStorageBool, v_nulls, v_attnum, "");
LLVMBuildStore(b, value, v_resvaluep);
diff --git a/src/backend/jit/llvm/llvmjit_types.c b/src/backend/jit/llvm/llvmjit_types.c
index 47c9daf402..1dcf0c2fd8 100644
--- a/src/backend/jit/llvm/llvmjit_types.c
+++ b/src/backend/jit/llvm/llvmjit_types.c
@@ -178,4 +178,5 @@ void *referenced_functions[] =
strlen,
varsize_any,
ExecInterpExprStillValid,
+ ExecSlotDetoastDatumExternal,
};
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 610f4a56d6..8acb48240e 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -314,7 +314,9 @@ static ModifyTable *make_modifytable(PlannerInfo *root, Plan *subplan,
List *mergeActionLists, int epqParam);
static GatherMerge *create_gather_merge_plan(PlannerInfo *root,
GatherMergePath *best_path);
-
+static void set_plan_forbid_pre_detoast_vars_recurse(Plan *plan,
+ List *small_tlist);
+static void set_plan_not_pre_detoast_vars(Plan *plan, List *small_tlist);
/*
* create_plan
@@ -346,6 +348,12 @@ create_plan(PlannerInfo *root, Path *best_path)
/* Recursively process the path tree, demanding the correct tlist result */
plan = create_plan_recurse(root, best_path, CP_EXACT_TLIST);
+ /*
+ * After the plan tree is built completed, we start to walk for which
+ * expressions should not used the shared-detoast feature.
+ */
+ set_plan_forbid_pre_detoast_vars_recurse(plan, NIL);
+
/*
* Make sure the topmost plan node's targetlist exposes the original
* column names and other decorative info. Targetlists generated within
@@ -378,6 +386,101 @@ create_plan(PlannerInfo *root, Path *best_path)
return plan;
}
+/*
+ * set_plan_forbid_pre_detoast_vars_recurse
+ * Walking the Plan tree in the top-down manner to gather the vars which
+ * should be as small as possible and record them in Plan.forbid_pre_detoast_vars
+ *
+ * plan: the plan node to walk right now.
+ * small_tlist: a list of nodes which its subplan should provide them as
+ * small as possible.
+ */
+static void
+set_plan_forbid_pre_detoast_vars_recurse(Plan *plan, List *small_tlist)
+{
+ if (plan == NULL)
+ return;
+
+ set_plan_not_pre_detoast_vars(plan, small_tlist);
+
+ /* Recurse to its subplan.. */
+ if (IsA(plan, Sort) || IsA(plan, Memoize) || IsA(plan, WindowAgg) ||
+ IsA(plan, Hash) || IsA(plan, Material) || IsA(plan, IncrementalSort))
+ {
+ List *small_tlist = get_tlist_exprs(plan->lefttree->targetlist, true);
+
+ /*
+ * For the sort-like nodes, we want the output of its subplan as small
+ * as possible, but the subplan's other expressions like Qual doesn't
+ * have this restriction since they are not output to the upper nodes.
+ * so we set the small_tlist to the subplan->targetlist.
+ */
+ set_plan_forbid_pre_detoast_vars_recurse(plan->lefttree, small_tlist);
+ }
+ else if (IsA(plan, HashJoin) && castNode(HashJoin, plan)->left_small_tlist)
+ {
+ List *small_tlist = get_tlist_exprs(plan->lefttree->targetlist, true);
+
+ /*
+ * If the left_small_tlist wants a as small as possible tlist, set it
+ * in a way like sort for the left node.
+ */
+ set_plan_forbid_pre_detoast_vars_recurse(plan->lefttree, small_tlist);
+
+ /*
+ * The righttree is a Hash node, it can be set with its own rule, so
+ * the small_tlist provided is not important, we just need to recuse
+ * to its subplan.
+ */
+ set_plan_forbid_pre_detoast_vars_recurse(plan->righttree, plan->forbid_pre_detoast_vars);
+ }
+ else
+ {
+ /*
+ * Recurse to its children, just push down the forbid_pre_detoast_vars
+ * to its children.
+ */
+ set_plan_forbid_pre_detoast_vars_recurse(plan->lefttree, plan->forbid_pre_detoast_vars);
+ set_plan_forbid_pre_detoast_vars_recurse(plan->righttree, plan->forbid_pre_detoast_vars);
+ }
+}
+
+/*
+ * set_plan_not_pre_detoast_vars
+ *
+ * Set the Plan.forbid_pre_detoast_vars according the small_tlist information.
+ *
+ * small_tlist = NIL means nothing is forbidden, or else if a Var belongs to the
+ * small_tlist, then it must not be pre-detoasted.
+ */
+static void
+set_plan_not_pre_detoast_vars(Plan *plan, List *small_tlist)
+{
+ ListCell *lc;
+ Var *var;
+
+ /*
+ * fast path, if we don't have a small_tlist, the var in targetlist is
+ * impossible member of it. and this case might be a pretty common case.
+ */
+ if (small_tlist == NIL)
+ return;
+
+ foreach(lc, plan->targetlist)
+ {
+ TargetEntry *te = lfirst_node(TargetEntry, lc);
+
+ if (!IsA(te->expr, Var))
+ continue;
+ var = castNode(Var, te->expr);
+ if (var->varattno <= 0)
+ continue;
+ if (list_member(small_tlist, var))
+ /* pass the recheck */
+ plan->forbid_pre_detoast_vars = lappend(plan->forbid_pre_detoast_vars, var);
+ }
+}
+
/*
* create_plan_recurse
* Recursive guts of create_plan().
@@ -4893,6 +4996,8 @@ create_hashjoin_plan(PlannerInfo *root,
copy_generic_path_info(&join_plan->join.plan, &best_path->jpath.path);
+ join_plan->left_small_tlist = (best_path->num_batches > 1);
+
return join_plan;
}
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 22a1fa29f3..9b9e2b4345 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -27,6 +27,7 @@
#include "optimizer/tlist.h"
#include "parser/parse_relation.h"
#include "tcop/utility.h"
+#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
#include "utils/syscache.h"
@@ -55,11 +56,48 @@ typedef struct
tlist_vinfo vars[FLEXIBLE_ARRAY_MEMBER]; /* has num_vars entries */
} indexed_tlist;
+/*
+ * Decide which attrs are detoasted in a expressions level, this is judged
+ * at the fix_scan/join_expr stage. The recursed level is tracked when we
+ * walk to a Var, if the level is greater than 1, then it means the
+ * var needs an detoast in this expression list, there are some exceptions
+ * here, see increase_level_for_pre_detoast for details.
+ */
+typedef struct
+{
+ /* if the level is added during a certain walk. */
+ bool level_added;
+ /* the current level during the walk. */
+ int level;
+} intermediate_level_context;
+
+/*
+ * Context to hold the detoast attribute within a expression.
+ *
+ * XXX: this design was intent to avoid the pre-detoast-logic if the var
+ * only need to be detoasted *once*, but for now, this context is only
+ * maintained at the expression level rather than plan tree level, so it
+ * can't detect if a Var will be detoasted 2+ time at the plan level.
+ * Recording the times of a Var is detoasted in the plan tree level is
+ * complex, so before we decide it is a must, I am not willing to do too
+ * many changes here.
+ */
+typedef struct
+{
+ /* var is accessed for the first time. */
+ Bitmapset *existing_attrs;
+ /* var is accessed for the 2+ times. */
+ Bitmapset **final_ref_attrs;
+} intermediate_var_ref_context;
+
+
typedef struct
{
PlannerInfo *root;
int rtoffset;
double num_exec;
+ intermediate_level_context level_ctx;
+ intermediate_var_ref_context scan_reference_attrs;
} fix_scan_expr_context;
typedef struct
@@ -71,6 +109,9 @@ typedef struct
int rtoffset;
NullingRelsMatch nrm_match;
double num_exec;
+ intermediate_level_context level_ctx;
+ intermediate_var_ref_context outer_reference_attrs;
+ intermediate_var_ref_context inner_reference_attrs;
} fix_join_expr_context;
typedef struct
@@ -127,8 +168,8 @@ typedef struct
(((con)->consttype == REGCLASSOID || (con)->consttype == OIDOID) && \
!(con)->constisnull)
-#define fix_scan_list(root, lst, rtoffset, num_exec) \
- ((List *) fix_scan_expr(root, (Node *) (lst), rtoffset, num_exec))
+#define fix_scan_list(root, lst, rtoffset, num_exec, pre_detoast_attrs) \
+ ((List *) fix_scan_expr(root, (Node *) (lst), rtoffset, num_exec, pre_detoast_attrs))
static void add_rtes_to_flat_rtable(PlannerInfo *root, bool recursing);
static void flatten_unplanned_rtes(PlannerGlobal *glob, RangeTblEntry *rte);
@@ -158,7 +199,8 @@ static Plan *set_mergeappend_references(PlannerInfo *root,
static void set_hash_references(PlannerInfo *root, Plan *plan, int rtoffset);
static Relids offset_relid_set(Relids relids, int rtoffset);
static Node *fix_scan_expr(PlannerInfo *root, Node *node,
- int rtoffset, double num_exec);
+ int rtoffset, double num_exec,
+ Bitmapset **scan_reference_attrs);
static Node *fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context);
static bool fix_scan_expr_walker(Node *node, fix_scan_expr_context *context);
static void set_join_references(PlannerInfo *root, Join *join, int rtoffset);
@@ -190,7 +232,10 @@ static List *fix_join_expr(PlannerInfo *root,
Index acceptable_rel,
int rtoffset,
NullingRelsMatch nrm_match,
- double num_exec);
+ double num_exec,
+ Bitmapset **outer_reference_attrs,
+ Bitmapset **inner_reference_attrs);
+
static Node *fix_join_expr_mutator(Node *node,
fix_join_expr_context *context);
static Node *fix_upper_expr(PlannerInfo *root,
@@ -211,6 +256,38 @@ static List *set_windowagg_runcondition_references(PlannerInfo *root,
List *runcondition,
Plan *plan);
+/*
+ * func_use_slice_detoast
+ * Check if a func just needs a pg_detoast_datum_slice, if so we should
+ * not pre detoast it. For now, the known function ID is hard-coded. but
+ * it'd be good that pg_proc can have a attribute like 'detoast_requirement'
+ * the value can be either of:
+ * - full
+ * - first_n (0 ~ N, the current slice method).
+ * - any. (for the incoming of partial detoast feature.
+ *
+ * I think adding this attribute to pg_proc has a stronger reason if partial
+ * detoast patch is accepted.
+ */
+static inline bool
+func_use_slice_detoast(Oid funcOid)
+{
+ /* hard code for now, and it is not used in a hot path yet. */
+ const Oid oids[] = {F_STARTS_WITH,
+
+ F_OVERLAY_BYTEA_BYTEA_INT4, F_OVERLAY_BYTEA_BYTEA_INT4_INT4,
+ F_OVERLAY_TEXT_TEXT_INT4, F_OVERLAY_TEXT_TEXT_INT4_INT4,
+
+ F_SUBSTRING_BYTEA_INT4, F_SUBSTRING_BYTEA_INT4_INT4,
+ F_SUBSTRING_TEXT_INT4, F_SUBSTRING_TEXT_INT4_INT4};
+
+ for (int i = 0; i < sizeof(oids) /sizeof(Oid); i++)
+ {
+ if (funcOid == oids[i])
+ return true;
+ }
+ return false;
+}
/*****************************************************************************
*
@@ -628,10 +705,16 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
splan->scan.scanrelid += rtoffset;
splan->scan.plan.targetlist =
fix_scan_list(root, splan->scan.plan.targetlist,
- rtoffset, NUM_EXEC_TLIST(plan));
+ rtoffset, NUM_EXEC_TLIST(plan),
+ &splan->scan.reference_attrs);
splan->scan.plan.qual =
fix_scan_list(root, splan->scan.plan.qual,
- rtoffset, NUM_EXEC_QUAL(plan));
+ rtoffset, NUM_EXEC_QUAL(plan),
+ &splan->scan.reference_attrs);
+
+ splan->scan.plan.forbid_pre_detoast_vars =
+ fix_scan_list(root, splan->scan.plan.forbid_pre_detoast_vars,
+ rtoffset, NUM_EXEC_TLIST(plan), NULL);
}
break;
case T_SampleScan:
@@ -641,13 +724,20 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
splan->scan.scanrelid += rtoffset;
splan->scan.plan.targetlist =
fix_scan_list(root, splan->scan.plan.targetlist,
- rtoffset, NUM_EXEC_TLIST(plan));
+ rtoffset, NUM_EXEC_TLIST(plan),
+ &splan->scan.reference_attrs
+ );
splan->scan.plan.qual =
fix_scan_list(root, splan->scan.plan.qual,
- rtoffset, NUM_EXEC_QUAL(plan));
+ rtoffset, NUM_EXEC_QUAL(plan),
+ &splan->scan.reference_attrs);
splan->tablesample = (TableSampleClause *)
fix_scan_expr(root, (Node *) splan->tablesample,
- rtoffset, 1);
+ rtoffset, 1,
+ &splan->scan.reference_attrs);
+ splan->scan.plan.forbid_pre_detoast_vars =
+ fix_scan_list(root, splan->scan.plan.forbid_pre_detoast_vars,
+ rtoffset, NUM_EXEC_TLIST(plan), NULL);
}
break;
case T_IndexScan:
@@ -657,28 +747,40 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
splan->scan.scanrelid += rtoffset;
splan->scan.plan.targetlist =
fix_scan_list(root, splan->scan.plan.targetlist,
- rtoffset, NUM_EXEC_TLIST(plan));
+ rtoffset, NUM_EXEC_TLIST(plan),
+ &splan->scan.reference_attrs);
+
splan->scan.plan.qual =
fix_scan_list(root, splan->scan.plan.qual,
- rtoffset, NUM_EXEC_QUAL(plan));
+ rtoffset, NUM_EXEC_QUAL(plan),
+ &splan->scan.reference_attrs);
+
splan->indexqual =
fix_scan_list(root, splan->indexqual,
- rtoffset, 1);
+ rtoffset, 1, &splan->scan.reference_attrs);
splan->indexqualorig =
fix_scan_list(root, splan->indexqualorig,
- rtoffset, NUM_EXEC_QUAL(plan));
+ rtoffset, NUM_EXEC_QUAL(plan),
+ &splan->scan.reference_attrs);
splan->indexorderby =
fix_scan_list(root, splan->indexorderby,
- rtoffset, 1);
+ rtoffset, 1, &splan->scan.reference_attrs);
splan->indexorderbyorig =
fix_scan_list(root, splan->indexorderbyorig,
- rtoffset, NUM_EXEC_QUAL(plan));
+ rtoffset, NUM_EXEC_QUAL(plan), &splan->scan.reference_attrs);
+ splan->scan.plan.forbid_pre_detoast_vars =
+ fix_scan_list(root, splan->scan.plan.forbid_pre_detoast_vars,
+ rtoffset, NUM_EXEC_TLIST(plan), NULL);
}
break;
case T_IndexOnlyScan:
{
IndexOnlyScan *splan = (IndexOnlyScan *) plan;
+ splan->scan.plan.forbid_pre_detoast_vars =
+ fix_scan_list(root, splan->scan.plan.forbid_pre_detoast_vars,
+ rtoffset, NUM_EXEC_TLIST(plan), NULL);
+
return set_indexonlyscan_references(root, splan, rtoffset);
}
break;
@@ -691,10 +793,15 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
Assert(splan->scan.plan.targetlist == NIL);
Assert(splan->scan.plan.qual == NIL);
splan->indexqual =
- fix_scan_list(root, splan->indexqual, rtoffset, 1);
+ fix_scan_list(root, splan->indexqual, rtoffset, 1,
+ &splan->scan.reference_attrs);
splan->indexqualorig =
fix_scan_list(root, splan->indexqualorig,
- rtoffset, NUM_EXEC_QUAL(plan));
+ rtoffset, NUM_EXEC_QUAL(plan),
+ &splan->scan.reference_attrs);
+ splan->scan.plan.forbid_pre_detoast_vars =
+ fix_scan_list(root, splan->scan.plan.forbid_pre_detoast_vars,
+ rtoffset, NUM_EXEC_TLIST(plan), NULL);
}
break;
case T_BitmapHeapScan:
@@ -704,13 +811,20 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
splan->scan.scanrelid += rtoffset;
splan->scan.plan.targetlist =
fix_scan_list(root, splan->scan.plan.targetlist,
- rtoffset, NUM_EXEC_TLIST(plan));
+ rtoffset, NUM_EXEC_TLIST(plan),
+ &splan->scan.reference_attrs);
splan->scan.plan.qual =
fix_scan_list(root, splan->scan.plan.qual,
- rtoffset, NUM_EXEC_QUAL(plan));
+ rtoffset, NUM_EXEC_QUAL(plan),
+ &splan->scan.reference_attrs);
splan->bitmapqualorig =
fix_scan_list(root, splan->bitmapqualorig,
- rtoffset, NUM_EXEC_QUAL(plan));
+ rtoffset, NUM_EXEC_QUAL(plan),
+ &splan->scan.reference_attrs);
+ splan->scan.plan.forbid_pre_detoast_vars =
+ fix_scan_list(root, splan->scan.plan.forbid_pre_detoast_vars,
+ rtoffset, NUM_EXEC_TLIST(plan),
+ NULL);
}
break;
case T_TidScan:
@@ -720,13 +834,20 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
splan->scan.scanrelid += rtoffset;
splan->scan.plan.targetlist =
fix_scan_list(root, splan->scan.plan.targetlist,
- rtoffset, NUM_EXEC_TLIST(plan));
+ rtoffset, NUM_EXEC_TLIST(plan),
+ &splan->scan.reference_attrs);
splan->scan.plan.qual =
fix_scan_list(root, splan->scan.plan.qual,
- rtoffset, NUM_EXEC_QUAL(plan));
+ rtoffset, NUM_EXEC_QUAL(plan),
+ &splan->scan.reference_attrs);
splan->tidquals =
fix_scan_list(root, splan->tidquals,
- rtoffset, 1);
+ rtoffset, 1,
+ &splan->scan.reference_attrs);
+ splan->scan.plan.forbid_pre_detoast_vars =
+ fix_scan_list(root, splan->scan.plan.forbid_pre_detoast_vars,
+ rtoffset, NUM_EXEC_TLIST(plan),
+ NULL);
}
break;
case T_TidRangeScan:
@@ -736,13 +857,20 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
splan->scan.scanrelid += rtoffset;
splan->scan.plan.targetlist =
fix_scan_list(root, splan->scan.plan.targetlist,
- rtoffset, NUM_EXEC_TLIST(plan));
+ rtoffset, NUM_EXEC_TLIST(plan),
+ &splan->scan.reference_attrs);
splan->scan.plan.qual =
fix_scan_list(root, splan->scan.plan.qual,
- rtoffset, NUM_EXEC_QUAL(plan));
+ rtoffset, NUM_EXEC_QUAL(plan),
+ &splan->scan.reference_attrs);
splan->tidrangequals =
fix_scan_list(root, splan->tidrangequals,
- rtoffset, 1);
+ rtoffset, 1,
+ &splan->scan.reference_attrs);
+ splan->scan.plan.forbid_pre_detoast_vars =
+ fix_scan_list(root, splan->scan.plan.forbid_pre_detoast_vars,
+ rtoffset, NUM_EXEC_TLIST(plan),
+ NULL);
}
break;
case T_SubqueryScan:
@@ -757,12 +885,16 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
splan->scan.scanrelid += rtoffset;
splan->scan.plan.targetlist =
fix_scan_list(root, splan->scan.plan.targetlist,
- rtoffset, NUM_EXEC_TLIST(plan));
+ rtoffset, NUM_EXEC_TLIST(plan),
+ &splan->scan.reference_attrs);
splan->scan.plan.qual =
fix_scan_list(root, splan->scan.plan.qual,
- rtoffset, NUM_EXEC_QUAL(plan));
+ rtoffset, NUM_EXEC_QUAL(plan),
+ &splan->scan.reference_attrs);
splan->functions =
- fix_scan_list(root, splan->functions, rtoffset, 1);
+ fix_scan_list(root, splan->functions, rtoffset, 1,
+ &splan->scan.reference_attrs);
+
}
break;
case T_TableFuncScan:
@@ -772,13 +904,17 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
splan->scan.scanrelid += rtoffset;
splan->scan.plan.targetlist =
fix_scan_list(root, splan->scan.plan.targetlist,
- rtoffset, NUM_EXEC_TLIST(plan));
+ rtoffset, NUM_EXEC_TLIST(plan),
+ &splan->scan.reference_attrs);
splan->scan.plan.qual =
fix_scan_list(root, splan->scan.plan.qual,
- rtoffset, NUM_EXEC_QUAL(plan));
+ rtoffset, NUM_EXEC_QUAL(plan),
+ &splan->scan.reference_attrs);
+
splan->tablefunc = (TableFunc *)
fix_scan_expr(root, (Node *) splan->tablefunc,
- rtoffset, 1);
+ rtoffset, 1,
+ &splan->scan.reference_attrs);
}
break;
case T_ValuesScan:
@@ -788,13 +924,16 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
splan->scan.scanrelid += rtoffset;
splan->scan.plan.targetlist =
fix_scan_list(root, splan->scan.plan.targetlist,
- rtoffset, NUM_EXEC_TLIST(plan));
+ rtoffset, NUM_EXEC_TLIST(plan),
+ &splan->scan.reference_attrs);
splan->scan.plan.qual =
fix_scan_list(root, splan->scan.plan.qual,
- rtoffset, NUM_EXEC_QUAL(plan));
+ rtoffset, NUM_EXEC_QUAL(plan),
+ &splan->scan.reference_attrs);
splan->values_lists =
fix_scan_list(root, splan->values_lists,
- rtoffset, 1);
+ rtoffset, 1,
+ &splan->scan.reference_attrs);
}
break;
case T_CteScan:
@@ -804,10 +943,16 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
splan->scan.scanrelid += rtoffset;
splan->scan.plan.targetlist =
fix_scan_list(root, splan->scan.plan.targetlist,
- rtoffset, NUM_EXEC_TLIST(plan));
+ rtoffset, NUM_EXEC_TLIST(plan),
+ &splan->scan.reference_attrs);
splan->scan.plan.qual =
fix_scan_list(root, splan->scan.plan.qual,
- rtoffset, NUM_EXEC_QUAL(plan));
+ rtoffset, NUM_EXEC_QUAL(plan),
+ &splan->scan.reference_attrs);
+ splan->scan.plan.forbid_pre_detoast_vars =
+ fix_scan_list(root, splan->scan.plan.forbid_pre_detoast_vars,
+ rtoffset, NUM_EXEC_TLIST(plan),
+ NULL);
}
break;
case T_NamedTuplestoreScan:
@@ -817,10 +962,12 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
splan->scan.scanrelid += rtoffset;
splan->scan.plan.targetlist =
fix_scan_list(root, splan->scan.plan.targetlist,
- rtoffset, NUM_EXEC_TLIST(plan));
+ rtoffset, NUM_EXEC_TLIST(plan),
+ &splan->scan.reference_attrs);
splan->scan.plan.qual =
fix_scan_list(root, splan->scan.plan.qual,
- rtoffset, NUM_EXEC_QUAL(plan));
+ rtoffset, NUM_EXEC_QUAL(plan),
+ &splan->scan.reference_attrs);
}
break;
case T_WorkTableScan:
@@ -830,10 +977,12 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
splan->scan.scanrelid += rtoffset;
splan->scan.plan.targetlist =
fix_scan_list(root, splan->scan.plan.targetlist,
- rtoffset, NUM_EXEC_TLIST(plan));
+ rtoffset, NUM_EXEC_TLIST(plan),
+ &splan->scan.reference_attrs);
splan->scan.plan.qual =
fix_scan_list(root, splan->scan.plan.qual,
- rtoffset, NUM_EXEC_QUAL(plan));
+ rtoffset, NUM_EXEC_QUAL(plan),
+ &splan->scan.reference_attrs);
}
break;
case T_ForeignScan:
@@ -873,7 +1022,8 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
mplan->param_exprs = fix_scan_list(root, mplan->param_exprs,
rtoffset,
- NUM_EXEC_TLIST(plan));
+ NUM_EXEC_TLIST(plan),
+ NULL);
break;
}
@@ -933,9 +1083,9 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
Assert(splan->plan.qual == NIL);
splan->limitOffset =
- fix_scan_expr(root, splan->limitOffset, rtoffset, 1);
+ fix_scan_expr(root, splan->limitOffset, rtoffset, 1, NULL);
splan->limitCount =
- fix_scan_expr(root, splan->limitCount, rtoffset, 1);
+ fix_scan_expr(root, splan->limitCount, rtoffset, 1, NULL);
}
break;
case T_Agg:
@@ -988,17 +1138,17 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
* variable refs, so fix_scan_expr works for them.
*/
wplan->startOffset =
- fix_scan_expr(root, wplan->startOffset, rtoffset, 1);
+ fix_scan_expr(root, wplan->startOffset, rtoffset, 1, NULL);
wplan->endOffset =
- fix_scan_expr(root, wplan->endOffset, rtoffset, 1);
+ fix_scan_expr(root, wplan->endOffset, rtoffset, 1, NULL);
wplan->runCondition = fix_scan_list(root,
wplan->runCondition,
rtoffset,
- NUM_EXEC_TLIST(plan));
+ NUM_EXEC_TLIST(plan), NULL);
wplan->runConditionOrig = fix_scan_list(root,
wplan->runConditionOrig,
rtoffset,
- NUM_EXEC_TLIST(plan));
+ NUM_EXEC_TLIST(plan), NULL);
}
break;
case T_Result:
@@ -1038,14 +1188,14 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
splan->plan.targetlist =
fix_scan_list(root, splan->plan.targetlist,
- rtoffset, NUM_EXEC_TLIST(plan));
+ rtoffset, NUM_EXEC_TLIST(plan), NULL);
splan->plan.qual =
fix_scan_list(root, splan->plan.qual,
- rtoffset, NUM_EXEC_QUAL(plan));
+ rtoffset, NUM_EXEC_QUAL(plan), NULL);
}
/* resconstantqual can't contain any subplan variable refs */
splan->resconstantqual =
- fix_scan_expr(root, splan->resconstantqual, rtoffset, 1);
+ fix_scan_expr(root, splan->resconstantqual, rtoffset, 1, NULL);
}
break;
case T_ProjectSet:
@@ -1061,7 +1211,7 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
splan->withCheckOptionLists =
fix_scan_list(root, splan->withCheckOptionLists,
- rtoffset, 1);
+ rtoffset, 1, NULL);
if (splan->returningLists)
{
@@ -1118,18 +1268,20 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
fix_join_expr(root, splan->onConflictSet,
NULL, itlist,
linitial_int(splan->resultRelations),
- rtoffset, NRM_EQUAL, NUM_EXEC_QUAL(plan));
+ rtoffset, NRM_EQUAL, NUM_EXEC_QUAL(plan),
+ NULL, NULL);
splan->onConflictWhere = (Node *)
fix_join_expr(root, (List *) splan->onConflictWhere,
NULL, itlist,
linitial_int(splan->resultRelations),
- rtoffset, NRM_EQUAL, NUM_EXEC_QUAL(plan));
+ rtoffset, NRM_EQUAL, NUM_EXEC_QUAL(plan),
+ NULL, NULL);
pfree(itlist);
splan->exclRelTlist =
- fix_scan_list(root, splan->exclRelTlist, rtoffset, 1);
+ fix_scan_list(root, splan->exclRelTlist, rtoffset, 1, NULL);
}
/*
@@ -1182,7 +1334,8 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
resultrel,
rtoffset,
NRM_EQUAL,
- NUM_EXEC_TLIST(plan));
+ NUM_EXEC_TLIST(plan),
+ NULL, NULL);
/* Fix quals too. */
action->qual = (Node *) fix_join_expr(root,
@@ -1191,7 +1344,8 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
resultrel,
rtoffset,
NRM_EQUAL,
- NUM_EXEC_QUAL(plan));
+ NUM_EXEC_QUAL(plan),
+ NULL, NULL);
}
}
}
@@ -1356,13 +1510,16 @@ set_indexonlyscan_references(PlannerInfo *root,
NUM_EXEC_QUAL((Plan *) plan));
/* indexqual is already transformed to reference index columns */
plan->indexqual = fix_scan_list(root, plan->indexqual,
- rtoffset, 1);
+ rtoffset, 1,
+ &plan->scan.reference_attrs);
/* indexorderby is already transformed to reference index columns */
plan->indexorderby = fix_scan_list(root, plan->indexorderby,
- rtoffset, 1);
+ rtoffset, 1,
+ &plan->scan.reference_attrs);
/* indextlist must NOT be transformed to reference index columns */
plan->indextlist = fix_scan_list(root, plan->indextlist,
- rtoffset, NUM_EXEC_TLIST((Plan *) plan));
+ rtoffset, NUM_EXEC_TLIST((Plan *) plan),
+ &plan->scan.reference_attrs);
pfree(index_itlist);
@@ -1409,10 +1566,10 @@ set_subqueryscan_references(PlannerInfo *root,
plan->scan.scanrelid += rtoffset;
plan->scan.plan.targetlist =
fix_scan_list(root, plan->scan.plan.targetlist,
- rtoffset, NUM_EXEC_TLIST((Plan *) plan));
+ rtoffset, NUM_EXEC_TLIST((Plan *) plan), NULL);
plan->scan.plan.qual =
fix_scan_list(root, plan->scan.plan.qual,
- rtoffset, NUM_EXEC_QUAL((Plan *) plan));
+ rtoffset, NUM_EXEC_QUAL((Plan *) plan), NULL);
result = (Plan *) plan;
}
@@ -1612,7 +1769,7 @@ set_foreignscan_references(PlannerInfo *root,
/* fdw_scan_tlist itself just needs fix_scan_list() adjustments */
fscan->fdw_scan_tlist =
fix_scan_list(root, fscan->fdw_scan_tlist,
- rtoffset, NUM_EXEC_TLIST((Plan *) fscan));
+ rtoffset, NUM_EXEC_TLIST((Plan *) fscan), NULL);
}
else
{
@@ -1622,16 +1779,16 @@ set_foreignscan_references(PlannerInfo *root,
*/
fscan->scan.plan.targetlist =
fix_scan_list(root, fscan->scan.plan.targetlist,
- rtoffset, NUM_EXEC_TLIST((Plan *) fscan));
+ rtoffset, NUM_EXEC_TLIST((Plan *) fscan), NULL);
fscan->scan.plan.qual =
fix_scan_list(root, fscan->scan.plan.qual,
- rtoffset, NUM_EXEC_QUAL((Plan *) fscan));
+ rtoffset, NUM_EXEC_QUAL((Plan *) fscan), NULL);
fscan->fdw_exprs =
fix_scan_list(root, fscan->fdw_exprs,
- rtoffset, NUM_EXEC_QUAL((Plan *) fscan));
+ rtoffset, NUM_EXEC_QUAL((Plan *) fscan), NULL);
fscan->fdw_recheck_quals =
fix_scan_list(root, fscan->fdw_recheck_quals,
- rtoffset, NUM_EXEC_QUAL((Plan *) fscan));
+ rtoffset, NUM_EXEC_QUAL((Plan *) fscan), NULL);
}
fscan->fs_relids = offset_relid_set(fscan->fs_relids, rtoffset);
@@ -1690,20 +1847,20 @@ set_customscan_references(PlannerInfo *root,
/* custom_scan_tlist itself just needs fix_scan_list() adjustments */
cscan->custom_scan_tlist =
fix_scan_list(root, cscan->custom_scan_tlist,
- rtoffset, NUM_EXEC_TLIST((Plan *) cscan));
+ rtoffset, NUM_EXEC_TLIST((Plan *) cscan), NULL);
}
else
{
/* Adjust tlist, qual, custom_exprs in the standard way */
cscan->scan.plan.targetlist =
fix_scan_list(root, cscan->scan.plan.targetlist,
- rtoffset, NUM_EXEC_TLIST((Plan *) cscan));
+ rtoffset, NUM_EXEC_TLIST((Plan *) cscan), NULL);
cscan->scan.plan.qual =
fix_scan_list(root, cscan->scan.plan.qual,
- rtoffset, NUM_EXEC_QUAL((Plan *) cscan));
+ rtoffset, NUM_EXEC_QUAL((Plan *) cscan), NULL);
cscan->custom_exprs =
fix_scan_list(root, cscan->custom_exprs,
- rtoffset, NUM_EXEC_QUAL((Plan *) cscan));
+ rtoffset, NUM_EXEC_QUAL((Plan *) cscan), NULL);
}
/* Adjust child plan-nodes recursively, if needed */
@@ -2111,6 +2268,102 @@ fix_alternative_subplan(PlannerInfo *root, AlternativeSubPlan *asplan,
return (Node *) bestplan;
}
+
+static inline void
+setup_intermediate_level_ctx(intermediate_level_context *ctx)
+{
+ ctx->level = 0;
+ ctx->level_added = false;
+}
+
+static inline void
+setup_intermediate_var_ref_ctx(intermediate_var_ref_context *ctx, Bitmapset **final_ref_attrs)
+{
+ ctx->existing_attrs = NULL;
+ ctx->final_ref_attrs = final_ref_attrs;
+}
+
+/*
+ * increase_level_for_pre_detoast
+ * Check if the given Expr could detoast a Var directly, if yes,
+ * increase the level and return true. otherwise return false;
+ */
+static inline void
+increase_level_for_pre_detoast(Node *node, intermediate_level_context *ctx)
+{
+ /* The following nodes is impossible to detoast a Var directly. */
+ if (IsA(node, List) || IsA(node, TargetEntry) || IsA(node, NullTest))
+ {
+ ctx->level_added = false;
+ }
+ else if (IsA(node, FuncExpr))
+ {
+ Oid funcOid = castNode(FuncExpr, node)->funcid;
+
+ if (funcOid == F_PG_COLUMN_COMPRESSION || func_use_slice_detoast(funcOid))
+ ctx->level_added = false;
+ else
+ {
+ ctx->level_added = true;
+ ctx->level += 1;
+ }
+ }
+ else
+ {
+ ctx->level_added = true;
+ ctx->level += 1;
+ }
+}
+
+static inline void
+decreased_level_for_pre_detoast(intermediate_level_context *ctx)
+{
+ if (ctx->level_added)
+ ctx->level -= 1;
+
+ ctx->level_added = false;
+}
+
+/*
+ * add_pre_detoast_vars
+ * add the var's information into pre_detoast_attrs when the check is pass.
+ */
+static inline void
+add_pre_detoast_vars(intermediate_level_context *level_ctx,
+ intermediate_var_ref_context *ctx,
+ Var *var)
+{
+ int attno;
+
+ if (level_ctx->level <= 1 || ctx->final_ref_attrs == NULL || var->varattno <= 0)
+ return;
+
+ attno = var->varattno - 1;
+ if (bms_is_member(attno, ctx->existing_attrs))
+ {
+ /* not the first time to access it, add it to final result. */
+ *ctx->final_ref_attrs = bms_add_member(*ctx->final_ref_attrs, attno);
+ }
+ else
+ {
+ /* first time. */
+ ctx->existing_attrs = bms_add_member(ctx->existing_attrs, attno);
+
+ /*
+ * XXX:
+ *
+ * The above strategy doesn't help to detect if a Var is detoast
+ * twice. Reasons are: 1. the context is not maintain in Plan node
+ * level. so if it is detoast at targetlist and qual, we can't detect
+ * it. 2. even we can make it at plan node, it still doesn't help for
+ * the among-nodes case.
+ *
+ * So for now, I just disable it.
+ */
+ *ctx->final_ref_attrs = bms_add_member(*ctx->final_ref_attrs, attno);
+ }
+}
+
/*
* fix_scan_expr
* Do set_plan_references processing on a scan-level expression
@@ -2125,18 +2378,23 @@ fix_alternative_subplan(PlannerInfo *root, AlternativeSubPlan *asplan,
* 'node': the expression to be modified
* 'rtoffset': how much to increment varnos by
* 'num_exec': estimated number of executions of expression
+ * 'scan_reference_attrs': gather which vars are potential to run the detoast
+ * on this expr, NULL means the caller doesn't have interests on this.
*
* The expression tree is either copied-and-modified, or modified in-place
* if that seems safe.
*/
static Node *
-fix_scan_expr(PlannerInfo *root, Node *node, int rtoffset, double num_exec)
+fix_scan_expr(PlannerInfo *root, Node *node, int rtoffset,
+ double num_exec, Bitmapset **scan_reference_attrs)
{
fix_scan_expr_context context;
context.root = root;
context.rtoffset = rtoffset;
context.num_exec = num_exec;
+ setup_intermediate_level_ctx(&context.level_ctx);
+ setup_intermediate_var_ref_ctx(&context.scan_reference_attrs, scan_reference_attrs);
if (rtoffset != 0 ||
root->multiexpr_params != NIL ||
@@ -2167,8 +2425,13 @@ fix_scan_expr(PlannerInfo *root, Node *node, int rtoffset, double num_exec)
static Node *
fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context)
{
+ Node *n;
+
if (node == NULL)
return NULL;
+
+ increase_level_for_pre_detoast(node, &context->level_ctx);
+
if (IsA(node, Var))
{
Var *var = copyVar((Var *) node);
@@ -2186,10 +2449,16 @@ fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context)
var->varno += context->rtoffset;
if (var->varnosyn > 0)
var->varnosyn += context->rtoffset;
+
+ add_pre_detoast_vars(&context->level_ctx, &context->scan_reference_attrs, var);
+ decreased_level_for_pre_detoast(&context->level_ctx);
return (Node *) var;
}
if (IsA(node, Param))
+ {
+ decreased_level_for_pre_detoast(&context->level_ctx);
return fix_param_node(context->root, (Param *) node);
+ }
if (IsA(node, Aggref))
{
Aggref *aggref = (Aggref *) node;
@@ -2199,8 +2468,10 @@ fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context)
aggparam = find_minmax_agg_replacement_param(context->root, aggref);
if (aggparam != NULL)
{
+ decreased_level_for_pre_detoast(&context->level_ctx);
/* Make a copy of the Param for paranoia's sake */
return (Node *) copyObject(aggparam);
+
}
/* If no match, just fall through to process it normally */
}
@@ -2210,6 +2481,7 @@ fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context)
Assert(!IS_SPECIAL_VARNO(cexpr->cvarno));
cexpr->cvarno += context->rtoffset;
+ decreased_level_for_pre_detoast(&context->level_ctx);
return (Node *) cexpr;
}
if (IsA(node, PlaceHolderVar))
@@ -2218,29 +2490,52 @@ fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context)
PlaceHolderVar *phv = (PlaceHolderVar *) node;
/* XXX can we assert something about phnullingrels? */
- return fix_scan_expr_mutator((Node *) phv->phexpr, context);
+ Node *n2 = fix_scan_expr_mutator((Node *) phv->phexpr, context);
+
+ decreased_level_for_pre_detoast(&context->level_ctx);
+ return n2;
}
if (IsA(node, AlternativeSubPlan))
- return fix_scan_expr_mutator(fix_alternative_subplan(context->root,
- (AlternativeSubPlan *) node,
- context->num_exec),
- context);
+ {
+ Node *n2 = fix_scan_expr_mutator(fix_alternative_subplan(context->root,
+ (AlternativeSubPlan *) node,
+ context->num_exec),
+ context);
+
+ decreased_level_for_pre_detoast(&context->level_ctx);
+ return n2;
+ }
fix_expr_common(context->root, node);
- return expression_tree_mutator(node, fix_scan_expr_mutator,
- (void *) context);
+ n = expression_tree_mutator(node, fix_scan_expr_mutator, (void *) context);
+ decreased_level_for_pre_detoast(&context->level_ctx);
+ return n;
}
static bool
fix_scan_expr_walker(Node *node, fix_scan_expr_context *context)
{
+ bool ret;
+
if (node == NULL)
return false;
+
+ increase_level_for_pre_detoast(node, &context->level_ctx);
+
+ if (IsA(node, Var))
+ {
+ add_pre_detoast_vars(&context->level_ctx,
+ &context->scan_reference_attrs,
+ castNode(Var, node));
+ }
Assert(!(IsA(node, Var) && ((Var *) node)->varno == ROWID_VAR));
Assert(!IsA(node, PlaceHolderVar));
Assert(!IsA(node, AlternativeSubPlan));
fix_expr_common(context->root, node);
- return expression_tree_walker(node, fix_scan_expr_walker,
- (void *) context);
+ ret = expression_tree_walker(node, fix_scan_expr_walker,
+ (void *) context);
+
+ decreased_level_for_pre_detoast(&context->level_ctx);
+ return ret;
}
/*
@@ -2276,7 +2571,10 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset)
(Index) 0,
rtoffset,
NRM_EQUAL,
- NUM_EXEC_QUAL((Plan *) join));
+ NUM_EXEC_QUAL((Plan *) join),
+ &join->outer_reference_attrs,
+ &join->inner_reference_attrs
+ );
/* Now do join-type-specific stuff */
if (IsA(join, NestLoop))
@@ -2323,7 +2621,9 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset)
(Index) 0,
rtoffset,
NRM_EQUAL,
- NUM_EXEC_QUAL((Plan *) join));
+ NUM_EXEC_QUAL((Plan *) join),
+ &join->outer_reference_attrs,
+ &join->inner_reference_attrs);
}
else if (IsA(join, HashJoin))
{
@@ -2336,7 +2636,9 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset)
(Index) 0,
rtoffset,
NRM_EQUAL,
- NUM_EXEC_QUAL((Plan *) join));
+ NUM_EXEC_QUAL((Plan *) join),
+ &join->outer_reference_attrs,
+ &join->inner_reference_attrs);
/*
* HashJoin's hashkeys are used to look for matching tuples from its
@@ -2368,7 +2670,9 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset)
(Index) 0,
rtoffset,
(join->jointype == JOIN_INNER ? NRM_EQUAL : NRM_SUPERSET),
- NUM_EXEC_TLIST((Plan *) join));
+ NUM_EXEC_TLIST((Plan *) join),
+ &join->outer_reference_attrs,
+ &join->inner_reference_attrs);
join->plan.qual = fix_join_expr(root,
join->plan.qual,
outer_itlist,
@@ -2376,8 +2680,20 @@ set_join_references(PlannerInfo *root, Join *join, int rtoffset)
(Index) 0,
rtoffset,
(join->jointype == JOIN_INNER ? NRM_EQUAL : NRM_SUPERSET),
- NUM_EXEC_QUAL((Plan *) join));
-
+ NUM_EXEC_QUAL((Plan *) join),
+ &join->outer_reference_attrs,
+ &join->inner_reference_attrs);
+
+ join->plan.forbid_pre_detoast_vars = fix_join_expr(root,
+ join->plan.forbid_pre_detoast_vars,
+ outer_itlist,
+ inner_itlist,
+ (Index) 0,
+ rtoffset,
+ (join->jointype == JOIN_INNER ? NRM_EQUAL : NRM_SUPERSET),
+ NUM_EXEC_TLIST((Plan *) join),
+ NULL,
+ NULL);
pfree(outer_itlist);
pfree(inner_itlist);
}
@@ -3010,9 +3326,12 @@ fix_join_expr(PlannerInfo *root,
Index acceptable_rel,
int rtoffset,
NullingRelsMatch nrm_match,
- double num_exec)
+ double num_exec,
+ Bitmapset **outer_reference_attrs,
+ Bitmapset **inner_reference_attrs)
{
fix_join_expr_context context;
+ List *ret;
context.root = root;
context.outer_itlist = outer_itlist;
@@ -3021,16 +3340,30 @@ fix_join_expr(PlannerInfo *root,
context.rtoffset = rtoffset;
context.nrm_match = nrm_match;
context.num_exec = num_exec;
- return (List *) fix_join_expr_mutator((Node *) clauses, &context);
+
+ setup_intermediate_level_ctx(&context.level_ctx);
+ setup_intermediate_var_ref_ctx(&context.outer_reference_attrs, outer_reference_attrs);
+ setup_intermediate_var_ref_ctx(&context.inner_reference_attrs, inner_reference_attrs);
+
+ ret = (List *) fix_join_expr_mutator((Node *) clauses, &context);
+
+ bms_free(context.outer_reference_attrs.existing_attrs);
+ bms_free(context.inner_reference_attrs.existing_attrs);
+
+ return ret;
}
static Node *
fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
{
Var *newvar;
+ Node *ret_node;
if (node == NULL)
return NULL;
+
+ increase_level_for_pre_detoast(node, &context->level_ctx);
+
if (IsA(node, Var))
{
Var *var = (Var *) node;
@@ -3044,7 +3377,13 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
context->rtoffset,
context->nrm_match);
if (newvar)
+ {
+ add_pre_detoast_vars(&context->level_ctx,
+ &context->outer_reference_attrs,
+ newvar);
+ decreased_level_for_pre_detoast(&context->level_ctx);
return (Node *) newvar;
+ }
}
/* then in the inner. */
@@ -3056,7 +3395,13 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
context->rtoffset,
context->nrm_match);
if (newvar)
+ {
+ add_pre_detoast_vars(&context->level_ctx,
+ &context->inner_reference_attrs,
+ newvar);
+ decreased_level_for_pre_detoast(&context->level_ctx);
return (Node *) newvar;
+ }
}
/* If it's for acceptable_rel, adjust and return it */
@@ -3066,6 +3411,9 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
var->varno += context->rtoffset;
if (var->varnosyn > 0)
var->varnosyn += context->rtoffset;
+ /* XXX acceptable_rel? we can ignore it for safety. */
+ decreased_level_for_pre_detoast(&context->level_ctx);
+
return (Node *) var;
}
@@ -3084,22 +3432,38 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
OUTER_VAR,
context->nrm_match);
if (newvar)
+ {
+ add_pre_detoast_vars(&context->level_ctx,
+ &context->outer_reference_attrs,
+ newvar);
+ decreased_level_for_pre_detoast(&context->level_ctx);
return (Node *) newvar;
+ }
}
if (context->inner_itlist && context->inner_itlist->has_ph_vars)
{
+
newvar = search_indexed_tlist_for_phv(phv,
context->inner_itlist,
INNER_VAR,
context->nrm_match);
if (newvar)
+ {
+ add_pre_detoast_vars(&context->level_ctx,
+ &context->inner_reference_attrs,
+ newvar);
+ decreased_level_for_pre_detoast(&context->level_ctx);
return (Node *) newvar;
+ }
}
/* If not supplied by input plans, evaluate the contained expr */
/* XXX can we assert something about phnullingrels? */
- return fix_join_expr_mutator((Node *) phv->phexpr, context);
+ ret_node = fix_join_expr_mutator((Node *) phv->phexpr, context);
+ decreased_level_for_pre_detoast(&context->level_ctx);
+ return ret_node;
}
+
/* Try matching more complex expressions too, if tlists have any */
if (context->outer_itlist && context->outer_itlist->has_non_vars)
{
@@ -3107,7 +3471,13 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
context->outer_itlist,
OUTER_VAR);
if (newvar)
+ {
+ add_pre_detoast_vars(&context->level_ctx,
+ &context->outer_reference_attrs,
+ newvar);
+ decreased_level_for_pre_detoast(&context->level_ctx);
return (Node *) newvar;
+ }
}
if (context->inner_itlist && context->inner_itlist->has_non_vars)
{
@@ -3115,20 +3485,36 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
context->inner_itlist,
INNER_VAR);
if (newvar)
+ {
+ add_pre_detoast_vars(&context->level_ctx,
+ &context->inner_reference_attrs,
+ newvar);
+ decreased_level_for_pre_detoast(&context->level_ctx);
return (Node *) newvar;
+ }
}
/* Special cases (apply only AFTER failing to match to lower tlist) */
if (IsA(node, Param))
- return fix_param_node(context->root, (Param *) node);
+ {
+ ret_node = fix_param_node(context->root, (Param *) node);
+ decreased_level_for_pre_detoast(&context->level_ctx);
+ return ret_node;
+ }
if (IsA(node, AlternativeSubPlan))
- return fix_join_expr_mutator(fix_alternative_subplan(context->root,
- (AlternativeSubPlan *) node,
- context->num_exec),
- context);
+ {
+ ret_node = fix_join_expr_mutator(fix_alternative_subplan(context->root,
+ (AlternativeSubPlan *) node,
+ context->num_exec),
+ context);
+ decreased_level_for_pre_detoast(&context->level_ctx);
+ return ret_node;
+ }
fix_expr_common(context->root, node);
- return expression_tree_mutator(node,
- fix_join_expr_mutator,
- (void *) context);
+ ret_node = expression_tree_mutator(node,
+ fix_join_expr_mutator,
+ (void *) context);
+ decreased_level_for_pre_detoast(&context->level_ctx);
+ return ret_node;
}
/*
@@ -3163,7 +3549,8 @@ fix_join_expr_mutator(Node *node, fix_join_expr_context *context)
* varno = newvarno, varattno = resno of corresponding targetlist element.
* The original tree is not modified.
*/
-static Node *
+static Node * /* XXX: shall I care about this for shared
+ * detoast optimization? */
fix_upper_expr(PlannerInfo *root,
Node *node,
indexed_tlist *subplan_itlist,
@@ -3318,7 +3705,10 @@ set_returning_clause_references(PlannerInfo *root,
resultRelation,
rtoffset,
NRM_EQUAL,
- NUM_EXEC_TLIST(topplan));
+ NUM_EXEC_TLIST(topplan),
+ NULL,
+ NULL
+ );
pfree(itlist);
diff --git a/src/include/access/detoast.h b/src/include/access/detoast.h
index 12d8cdb356..9ddc05604e 100644
--- a/src/include/access/detoast.h
+++ b/src/include/access/detoast.h
@@ -42,6 +42,7 @@ do { \
* ----------
*/
extern struct varlena *detoast_external_attr(struct varlena *attr);
+extern struct varlena *detoast_external_attr_ext(struct varlena *attr, MemoryContext ctx);
/* ----------
* detoast_attr() -
@@ -51,6 +52,8 @@ extern struct varlena *detoast_external_attr(struct varlena *attr);
* ----------
*/
extern struct varlena *detoast_attr(struct varlena *attr);
+extern struct varlena *detoast_attr_ext(struct varlena *attr, MemoryContext ctx);
+
/* ----------
* detoast_attr_slice() -
diff --git a/src/include/access/toast_compression.h b/src/include/access/toast_compression.h
index 64d5e079fa..00ea153e4e 100644
--- a/src/include/access/toast_compression.h
+++ b/src/include/access/toast_compression.h
@@ -55,13 +55,13 @@ typedef enum ToastCompressionId
/* pglz compression/decompression routines */
extern struct varlena *pglz_compress_datum(const struct varlena *value);
-extern struct varlena *pglz_decompress_datum(const struct varlena *value);
+extern struct varlena *pglz_decompress_datum(const struct varlena *value, MemoryContext ctx);
extern struct varlena *pglz_decompress_datum_slice(const struct varlena *value,
int32 slicelength);
/* lz4 compression/decompression routines */
extern struct varlena *lz4_compress_datum(const struct varlena *value);
-extern struct varlena *lz4_decompress_datum(const struct varlena *value);
+extern struct varlena *lz4_decompress_datum(const struct varlena *value, MemoryContext ctx);
extern struct varlena *lz4_decompress_datum_slice(const struct varlena *value,
int32 slicelength);
diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h
index a28ddcdd77..9304786bb2 100644
--- a/src/include/executor/execExpr.h
+++ b/src/include/executor/execExpr.h
@@ -78,6 +78,17 @@ typedef enum ExprEvalOp
EEOP_OUTER_VAR,
EEOP_SCAN_VAR,
+ /*
+ * compute non-system Var value with shared-detoast-datum logic, use some
+ * dedicated steps rather than add extra logic to existing steps is for
+ * performance aspect, within this way, we just decide if the extra logic
+ * is needed at ExecInitExpr stage once rather than every time of
+ * ExecInterpExpr.
+ */
+ EEOP_INNER_VAR_TOAST,
+ EEOP_OUTER_VAR_TOAST,
+ EEOP_SCAN_VAR_TOAST,
+
/* compute system Var value */
EEOP_INNER_SYSVAR,
EEOP_OUTER_SYSVAR,
@@ -830,5 +841,6 @@ extern void ExecEvalAggOrderedTransDatum(ExprState *state, ExprEvalStep *op,
ExprContext *econtext);
extern void ExecEvalAggOrderedTransTuple(ExprState *state, ExprEvalStep *op,
ExprContext *econtext);
+extern void ExecSlotDetoastDatumExternal(TupleTableSlot *slot, int attnum);
#endif /* EXEC_EXPR_H */
diff --git a/src/include/executor/tuptable.h b/src/include/executor/tuptable.h
index 6133dbcd0a..9edb843b15 100644
--- a/src/include/executor/tuptable.h
+++ b/src/include/executor/tuptable.h
@@ -19,6 +19,7 @@
#include "access/sysattr.h"
#include "access/tupdesc.h"
#include "storage/buf.h"
+#include "utils/memutils.h"
/*----------
* The executor stores tuples in a "tuple table" which is a List of
@@ -126,6 +127,7 @@ typedef struct TupleTableSlot
#define FIELDNO_TUPLETABLESLOT_ISNULL 6
bool *tts_isnull; /* current per-attribute isnull flags */
MemoryContext tts_mcxt; /* slot itself is in this context */
+ MemoryContext tts_data_mctx; /* The external content of tts_values[*] */
ItemPointerData tts_tid; /* stored tuple's tid */
Oid tts_tableOid; /* table oid of tuple */
} TupleTableSlot;
@@ -426,12 +428,24 @@ slot_getsysattr(TupleTableSlot *slot, int attnum, bool *isnull)
return slot->tts_ops->getsysattr(slot, attnum, isnull);
}
+/*
+ * ExecFreePreDetoastDatum - free the memory which is allocated in tts_data_mcxt.
+ */
+static inline void
+ExecFreePreDetoastDatum(TupleTableSlot *slot)
+{
+ if (slot->tts_data_mctx)
+ MemoryContextResetOnly(slot->tts_data_mctx);
+}
+
/*
* ExecClearTuple - clear the slot's contents
*/
static inline TupleTableSlot *
ExecClearTuple(TupleTableSlot *slot)
{
+ ExecFreePreDetoastDatum(slot);
+
slot->tts_ops->clear(slot);
return slot;
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 444a5f0fd5..30fdb37d1c 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1481,6 +1481,12 @@ typedef struct ScanState
Relation ss_currentRelation;
struct TableScanDescData *ss_currentScanDesc;
TupleTableSlot *ss_ScanTupleSlot;
+
+ /*
+ * The final attributes which should apply the pre-detoast-attrs logic on
+ * the Scan nodes.
+ */
+ Bitmapset *scan_pre_detoast_attrs;
} ScanState;
/* ----------------
@@ -2010,6 +2016,13 @@ typedef struct JoinState
bool single_match; /* True if we should skip to next outer tuple
* after finding one inner match */
ExprState *joinqual; /* JOIN quals (in addition to ps.qual) */
+
+ /*
+ * The final attributes which should apply the pre-detoast-attrs logic on
+ * the join nodes.
+ */
+ Bitmapset *outer_pre_detoast_attrs;
+ Bitmapset *inner_pre_detoast_attrs;
} JoinState;
/* ----------------
@@ -2771,4 +2784,5 @@ typedef struct LimitState
TupleTableSlot *last_slot; /* slot for evaluation of ties */
} LimitState;
+extern void SetPredetoastAttrsForJoin(JoinState *joinstate);
#endif /* EXECNODES_H */
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index b4ef6bc44c..ea5033aaa0 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -169,6 +169,13 @@ typedef struct Plan
*/
Bitmapset *extParam;
Bitmapset *allParam;
+
+ /*
+ * A list of Vars which should not apply the shared-detoast-datum logic
+ * since the upper nodes like Sort/Hash wants them as small as possible.
+ * It's a subset of targetlist in each Plan node.
+ */
+ List *forbid_pre_detoast_vars;
} Plan;
/* ----------------
@@ -385,6 +392,16 @@ typedef struct Scan
Plan plan;
Index scanrelid; /* relid is index into the range table */
+
+ /*
+ * Records of var's varattno - 1 where the Var is accessed indirectly by
+ * any expression, like a > 3. However a IS [NOT] NULL is not included
+ * since it doesn't access the tts_values[*] at all.
+ *
+ * This is a essential information to figure out which attrs should use
+ * the pre-detoast-attrs logic.
+ */
+ Bitmapset *reference_attrs;
} Scan;
/* ----------------
@@ -789,6 +806,17 @@ typedef struct Join
JoinType jointype;
bool inner_unique;
List *joinqual; /* JOIN quals (in addition to plan.qual) */
+
+ /*
+ * Records of var's varattno - 1 where the Var is accessed indirectly by
+ * any expression, like a > 3. However a IS [NOT] NULL is not included
+ * since it doesn't access the tts_values[*] at all.
+ *
+ * This is a essential information to figure out which attrs should use
+ * the pre-detoast-attrs logic.
+ */
+ Bitmapset *outer_reference_attrs;
+ Bitmapset *inner_reference_attrs;
} Join;
/* ----------------
@@ -869,6 +897,11 @@ typedef struct HashJoin
* perform lookups in the hashtable over the inner plan.
*/
List *hashkeys;
+
+ /*
+ * Whether the left plan tree should use a SMALL_TLIST.
+ */
+ bool left_small_tlist;
} HashJoin;
/* ----------------
@@ -1588,4 +1621,24 @@ typedef enum MonotonicFunction
MONOTONICFUNC_BOTH = MONOTONICFUNC_INCREASING | MONOTONICFUNC_DECREASING,
} MonotonicFunction;
+static inline bool
+is_join_plan(Plan *plan)
+{
+ return (plan != NULL) && (IsA(plan, NestLoop) || IsA(plan, HashJoin) || IsA(plan, MergeJoin));
+}
+
+static inline bool
+is_scan_plan(Plan *plan)
+{
+ return (plan != NULL) &&
+ (IsA(plan, SeqScan) ||
+ IsA(plan, SampleScan) ||
+ IsA(plan, IndexScan) ||
+ IsA(plan, IndexOnlyScan) ||
+ IsA(plan, BitmapIndexScan) ||
+ IsA(plan, BitmapHeapScan) ||
+ IsA(plan, TidScan) ||
+ IsA(plan, SubqueryScan));
+}
+
#endif /* PLANNODES_H */
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index ee40a341d3..2335000e18 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -4048,6 +4048,8 @@ cb_cleanup_dir
cb_options
cb_tablespace
cb_tablespace_mapping
+intermediate_var_ref_context
+intermediate_level_context
manifest_data
manifest_writer
rfile
--
2.34.1
--=-=-=--
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: per backend I/O statistics
@ 2024-11-25 07:12 Bertrand Drouvot <[email protected]>
2024-11-25 07:18 ` Re: per backend I/O statistics[ Michael Paquier <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Bertrand Drouvot @ 2024-11-25 07:12 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Nazir Bilal Yavuz <[email protected]>; Alvaro Herrera <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
Hi,
On Mon, Nov 25, 2024 at 10:06:44AM +0900, Michael Paquier wrote:
> On Fri, Nov 22, 2024 at 07:49:58AM +0000, Bertrand Drouvot wrote:
> > On Fri, Nov 22, 2024 at 10:36:29AM +0900, Michael Paquier wrote:
> >> Hmm. created_entry only matters for pgstat_init_function_usage().
> >> All the other callers of pgstat_prep_pending_entry() pass a NULL
> >> value.
> >
> > I meant to say all the calls that passe "create" as true in pgstat_get_entry_ref().
>
> Ah, OK, I think that I see your point here.
>
> I am wondering how much this would matter as well for custom stats,
> but we're not there yet without at least one release out and folks try
> new things with these APIs and variable-numbered kinds.
Not sure here, could custom stats start incrementing before the database system
is ready to accept connections?
> pgstat_prep_pending_entry() to return NULL even if "create" is true
> may be a good thing, at the end, because that's the only way I can see
> based on the current APIs where we could say "Sorry, but the stats
> have not been loaded yet, so you cannot try to do anything related to
> the dshash".
Yeah, same here.
> From my view having a kind of barrier would be cleaner in the long
> run, but it's true that it may not be mandatory, as well. pg_stat_io
> is currently OK to be called because the stats are loaded for
> auxiliary processes because it uses fixed-numbered stats in shmem.
> And it means we already have early calls that add stats getting
> overwritten once the stats are loaded from the on-disk file (Am I
> getting this part right?).
Yeah, we can already see that, for example, the background writer could enter
pgstat_io_flush_cb() before the stats are reset or restored.
> Anyway, do we really require that for the sake of this thread? We
> know that there's only one of each auxiliary process at a time, and
> they keep a footprint in pg_stat_io already. So we could just limit
> outselves to live database backends, WAL senders and autovacuum
> workers, everything that's not auxiliary and spawned on request?
I think that's a fair starting point and that we will not lose any informations
doing so (as you said there is only one of each auxiliary process at a time,
so that one could already see their stats from pg_stat_io).
The only cons that I can see is that we will not be able to merge the flush cb
but I don't think that's a blocker (the flush are done in shared memory so the
impact on performance should not be that much of an issue).
I'll come back with a new version implementing the above.
[1]: https://www.postgresql.org/message-id/Zz9sno%2BJJbWqdXhQ%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
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: per backend I/O statistics[
2024-11-25 07:12 Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
@ 2024-11-25 07:18 ` Michael Paquier <[email protected]>
2024-11-25 15:47 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Michael Paquier @ 2024-11-25 07:18 UTC (permalink / raw)
To: Bertrand Drouvot <[email protected]>; +Cc: Nazir Bilal Yavuz <[email protected]>; Alvaro Herrera <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On Mon, Nov 25, 2024 at 07:12:56AM +0000, Bertrand Drouvot wrote:
> Not sure here, could custom stats start incrementing before the database system
> is ready to accept connections?
In theory, that could be possible. Like pg_stat_io currently, I am
ready to assume that it likely won't matter much.
> The only cons that I can see is that we will not be able to merge the flush cb
> but I don't think that's a blocker (the flush are done in shared memory so the
> impact on performance should not be that much of an issue).
The backend and I/O stats could begin diverging as a result of a new
implementation detail, and the performance of the flushes don't worry
me knowing at which frequency they happen on a live system.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: per backend I/O statistics
2024-11-25 07:12 Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-11-25 07:18 ` Re: per backend I/O statistics[ Michael Paquier <[email protected]>
@ 2024-11-25 15:47 ` Bertrand Drouvot <[email protected]>
2024-11-27 06:33 ` Re: per backend I/O statistics Michael Paquier <[email protected]>
2024-12-12 04:52 ` Re: per backend I/O statistics Michael Paquier <[email protected]>
0 siblings, 2 replies; 12+ messages in thread
From: Bertrand Drouvot @ 2024-11-25 15:47 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Nazir Bilal Yavuz <[email protected]>; Alvaro Herrera <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
Hi,
On Mon, Nov 25, 2024 at 04:18:54PM +0900, Michael Paquier wrote:
> On Mon, Nov 25, 2024 at 07:12:56AM +0000, Bertrand Drouvot wrote:
> > Not sure here, could custom stats start incrementing before the database system
> > is ready to accept connections?
>
> In theory, that could be possible. Like pg_stat_io currently, I am
> ready to assume that it likely won't matter much.
Yeah right, agree.
> > The only cons that I can see is that we will not be able to merge the flush cb
> > but I don't think that's a blocker (the flush are done in shared memory so the
> > impact on performance should not be that much of an issue).
>
> The backend and I/O stats could begin diverging as a result of a new
> implementation detail, and the performance of the flushes don't worry
> me knowing at which frequency they happen on a live system.
Same here.
Please find attached v6 that enables per-backend I/O stats for the
B_AUTOVAC_WORKER, B_BACKEND, B_BG_WORKER, B_STANDALONE_BACKEND, B_SLOTSYNC_WORKER
and B_WAL_SENDER backend types.
It also takes care of most of the comments that you have made in [1], meaning
that it:
- removes the backend type from PgStat_Backend and look for the backend type
at "display" time.
- creates PgStat_BackendPendingIO and PgStat_PendingIO now refers to it (I
used PgStat_BackendPendingIO and not PgStat_BackendPending because this is what
it is after all).
- adds the missing comment related to the PID in the doc.
- merges 0004 with 0001 (so that pg_stat_get_backend_io() is now part of 0001).
- creates its own pgstat_backend.c file.
=== Remarks
R1: as compared to v5, v6 removes the per-backend I/O stats reset from
pg_stat_reset_shared(). I think it makes more sense that way, since we are
adding pg_stat_reset_single_backend_io_counters(). The per-backend I/O stats
behaves then as the subscription stats as far the reset is concerned.
R2: as we can't merge the flush cb anymore, only the patches related to
the stats_fetch_consistency/'snapshot' are missing in v6 (as compared to v5).
I propose to re-submit them, re-start the discussion once 0001 goes in.
[1]: https://www.postgresql.org/message-id/ZzWZV9LdyZ9aFSWs%40paquier.xyz
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v6-0001-per-backend-I-O-statistics.patch (41.7K, ../../[email protected]/2-v6-0001-per-backend-I-O-statistics.patch)
download | inline diff:
From 21d773ae2b64571b09a77d6de4c955338cf979ae Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Mon, 28 Oct 2024 12:50:32 +0000
Subject: [PATCH v6] per backend I/O statistics
While pg_stat_io provides cluster-wide I/O statistics, this commit adds the
ability to track and display per backend I/O statistics.
It adds a new statistics kind, a new pg_my_stat_io view to display "my" backend
I/O statistics, and 2 new functions:
- pg_stat_reset_single_backend_io_counters() to be able to reset the I/O stats
for a given backend pid.
- pg_stat_get_backend_io() to retrieve I/O statistics for a given backend pid.
The new KIND is named PGSTAT_KIND_PER_BACKEND as it could be used in the future
to store other statistics (than the I/O ones) per backend. The new KIND is
a variable-numbered one and has an automatic cap on the maximum number of
entries (as its hash key contains the proc number).
There is no need to write the per backend I/O stats to disk (no point to
see stats for backends that do not exist anymore after a re-start), so using
"write_to_file = false".
Note that per backend I/O statistics are not collected for the checkpointer,
the background writer, the startup process and the autovacuum launcher as those
are already visible in pg_stat_io and there is only one of those.
XXX: Bump catalog version needs to be done.
---
doc/src/sgml/config.sgml | 4 +-
doc/src/sgml/monitoring.sgml | 63 ++++++
src/backend/catalog/system_functions.sql | 2 +
src/backend/catalog/system_views.sql | 22 +++
src/backend/utils/activity/Makefile | 1 +
src/backend/utils/activity/backend_status.c | 4 +
src/backend/utils/activity/meson.build | 1 +
src/backend/utils/activity/pgstat.c | 19 +-
src/backend/utils/activity/pgstat_backend.c | 176 +++++++++++++++++
src/backend/utils/activity/pgstat_io.c | 37 +++-
src/backend/utils/adt/pgstatfuncs.c | 208 ++++++++++++++++++++
src/include/catalog/pg_proc.dat | 14 ++
src/include/pgstat.h | 32 ++-
src/include/utils/pgstat_internal.h | 12 ++
src/test/regress/expected/rules.out | 19 ++
src/test/regress/expected/stats.out | 85 +++++++-
src/test/regress/sql/stats.sql | 47 ++++-
src/tools/pgindent/typedefs.list | 3 +
18 files changed, 729 insertions(+), 20 deletions(-)
11.6% doc/src/sgml/
27.0% src/backend/utils/activity/
22.4% src/backend/utils/adt/
4.0% src/include/catalog/
5.9% src/include/
14.9% src/test/regress/expected/
11.4% src/test/regress/sql/
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index a84e60c09b..20a0862661 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8365,7 +8365,9 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
displayed in <link linkend="monitoring-pg-stat-database-view">
<structname>pg_stat_database</structname></link>,
<link linkend="monitoring-pg-stat-io-view">
- <structname>pg_stat_io</structname></link>, in the output of
+ <structname>pg_stat_io</structname></link>,
+ <link linkend="monitoring-pg-my-stat-io-view">
+ <structname>pg_my_stat_io</structname></link>, in the output of
<xref linkend="sql-explain"/> when the <literal>BUFFERS</literal> option
is used, in the output of <xref linkend="sql-vacuum"/> when
the <literal>VERBOSE</literal> option is used, by autovacuum
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 840d7f8161..8e7a7a0f36 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -488,6 +488,16 @@ postgres 27093 0.0 0.0 30096 2752 ? Ss 11:34 0:00 postgres: ser
</entry>
</row>
+ <row>
+ <entry><structname>pg_my_stat_io</structname><indexterm><primary>pg_my_stat_io</primary></indexterm></entry>
+ <entry>
+ One row for each combination of context and target object containing
+ my backend I/O statistics.
+ See <link linkend="monitoring-pg-my-stat-io-view">
+ <structname>pg_my_stat_io</structname></link> for details.
+ </entry>
+ </row>
+
<row>
<entry><structname>pg_stat_replication_slots</structname><indexterm><primary>pg_stat_replication_slots</primary></indexterm></entry>
<entry>One row per replication slot, showing statistics about the
@@ -2946,7 +2956,23 @@ description | Waiting for a newly initialized WAL file to reach durable storage
</para>
</note>
+ </sect2>
+
+ <sect2 id="monitoring-pg-my-stat-io-view">
+ <title><structname>pg_my_stat_io</structname></title>
+
+ <indexterm>
+ <primary>pg_my_stat_io</primary>
+ </indexterm>
+ <para>
+ The <structname>pg_my_stat_io</structname> view will contain one row for each
+ combination of target I/O object and I/O context, showing
+ my backend I/O statistics. Combinations which do not make sense are
+ omitted. The fields are exactly the same as the ones in the <link
+ linkend="monitoring-pg-stat-io-view"> <structname>pg_stat_io</structname></link>
+ view.
+ </para>
</sect2>
@@ -4790,6 +4816,25 @@ description | Waiting for a newly initialized WAL file to reach durable storage
</para></entry>
</row>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>pg_stat_get_backend_io</primary>
+ </indexterm>
+ <function>pg_stat_get_backend_io</function> ( <type>integer</type> )
+ <returnvalue>setof record</returnvalue>
+ </para>
+ <para>
+ Returns I/O statistics about the backend with the specified
+ process ID. The output fields are exactly the same as the ones in the
+ <link linkend="monitoring-pg-stat-io-view"> <structname>pg_stat_io</structname></link>
+ view. The function does not return I/O statistics for the checkpointer,
+ the background writer, the startup process and the autovacuum launcher
+ as they are already visible in the <link linkend="monitoring-pg-stat-io-view"> <structname>pg_stat_io</structname></link>
+ view and there is only one of those.
+ </para></entry>
+ </row>
+
<row>
<entry role="func_table_entry"><para role="func_signature">
<indexterm>
@@ -4971,6 +5016,24 @@ description | Waiting for a newly initialized WAL file to reach durable storage
</para></entry>
</row>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>pg_stat_reset_single_backend_io_counters</primary>
+ </indexterm>
+ <function>pg_stat_reset_single_backend_io_counters</function> ( <type>integer</type> )
+ <returnvalue>void</returnvalue>
+ </para>
+ <para>
+ Resets I/O statistics for a single backend with the specified process ID
+ to zero.
+ </para>
+ <para>
+ This function is restricted to superusers by default, but other users
+ can be granted EXECUTE to run the function.
+ </para></entry>
+ </row>
+
<row>
<entry role="func_table_entry"><para role="func_signature">
<indexterm>
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index c51dfca802..1b21b242cd 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -711,6 +711,8 @@ REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_table_counters(oid) FROM public;
REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_function_counters(oid) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_backend_io_counters(integer) FROM public;
+
REVOKE EXECUTE ON FUNCTION pg_stat_reset_replication_slot(text) FROM public;
REVOKE EXECUTE ON FUNCTION pg_stat_have_stats(text, oid, int8) FROM public;
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index da9a8fe99f..fce3b791bb 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1172,6 +1172,28 @@ SELECT
b.stats_reset
FROM pg_stat_get_io() b;
+CREATE VIEW pg_my_stat_io AS
+SELECT
+ b.backend_type,
+ b.object,
+ b.context,
+ b.reads,
+ b.read_time,
+ b.writes,
+ b.write_time,
+ b.writebacks,
+ b.writeback_time,
+ b.extends,
+ b.extend_time,
+ b.op_bytes,
+ b.hits,
+ b.evictions,
+ b.reuses,
+ b.fsyncs,
+ b.fsync_time,
+ b.stats_reset
+FROM pg_stat_get_backend_io(NULL) b;
+
CREATE VIEW pg_stat_wal AS
SELECT
w.wal_records,
diff --git a/src/backend/utils/activity/Makefile b/src/backend/utils/activity/Makefile
index b9fd66ea17..24b64a2742 100644
--- a/src/backend/utils/activity/Makefile
+++ b/src/backend/utils/activity/Makefile
@@ -20,6 +20,7 @@ OBJS = \
backend_status.o \
pgstat.o \
pgstat_archiver.o \
+ pgstat_backend.o \
pgstat_bgwriter.o \
pgstat_checkpointer.o \
pgstat_database.o \
diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c
index bdb3a296ca..249439b990 100644
--- a/src/backend/utils/activity/backend_status.c
+++ b/src/backend/utils/activity/backend_status.c
@@ -249,6 +249,10 @@ pgstat_beinit(void)
Assert(MyProcNumber >= 0 && MyProcNumber < NumBackendStatSlots);
MyBEEntry = &BackendStatusArray[MyProcNumber];
+ /* Create the per-backend statistics entry */
+ if (pgstat_tracks_per_backend_bktype(MyBackendType))
+ pgstat_create_backend_stat(MyProcNumber);
+
/* Set up a process-exit hook to clean up */
on_shmem_exit(pgstat_beshutdown_hook, 0);
}
diff --git a/src/backend/utils/activity/meson.build b/src/backend/utils/activity/meson.build
index f73c22905c..380d3dd70c 100644
--- a/src/backend/utils/activity/meson.build
+++ b/src/backend/utils/activity/meson.build
@@ -5,6 +5,7 @@ backend_sources += files(
'backend_status.c',
'pgstat.c',
'pgstat_archiver.c',
+ 'pgstat_backend.c',
'pgstat_bgwriter.c',
'pgstat_checkpointer.c',
'pgstat_database.c',
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 6f8d237826..1449d6a3f4 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -77,6 +77,7 @@
*
* Each statistics kind is handled in a dedicated file:
* - pgstat_archiver.c
+ * - pgstat_backend.c
* - pgstat_bgwriter.c
* - pgstat_checkpointer.c
* - pgstat_database.c
@@ -358,6 +359,22 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
.reset_timestamp_cb = pgstat_subscription_reset_timestamp_cb,
},
+ [PGSTAT_KIND_PER_BACKEND] = {
+ .name = "per-backend",
+
+ .fixed_amount = false,
+ .write_to_file = false,
+
+ .accessed_across_databases = true,
+
+ .shared_size = sizeof(PgStatShared_Backend),
+ .shared_data_off = offsetof(PgStatShared_Backend, stats),
+ .shared_data_len = sizeof(((PgStatShared_Backend *) 0)->stats),
+ .pending_size = sizeof(PgStat_BackendPendingIO),
+
+ .flush_pending_cb = pgstat_per_backend_flush_cb,
+ .reset_timestamp_cb = pgstat_backend_reset_timestamp_cb,
+ },
/* stats for fixed-numbered (mostly 1) objects */
@@ -768,7 +785,7 @@ pgstat_report_stat(bool force)
partial_flush = false;
- /* flush database / relation / function / ... stats */
+ /* flush database / relation / function / backend / ... stats */
partial_flush |= pgstat_flush_pending_entries(nowait);
/* flush of fixed-numbered stats */
diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c
new file mode 100644
index 0000000000..f079f15ff2
--- /dev/null
+++ b/src/backend/utils/activity/pgstat_backend.c
@@ -0,0 +1,176 @@
+/* -------------------------------------------------------------------------
+ *
+ * pgstat_backend.c
+ * Implementation of per-backend statistics.
+ *
+ * This file contains the implementation of per-backend statistics. It is kept
+ * separate from pgstat.c to enforce the line between the statistics access /
+ * storage implementation and the details about individual types of statistics.
+ *
+ * Copyright (c) 2024, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/backend/utils/activity/pgstat_backend.c
+ * -------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "utils/pgstat_internal.h"
+
+/*
+ * Returns my backend IO stats.
+ */
+PgStat_Backend *
+pgstat_fetch_my_stat_io(void)
+{
+ return (PgStat_Backend *)
+ pgstat_fetch_entry(PGSTAT_KIND_PER_BACKEND, InvalidOid, MyProcNumber);
+}
+
+/*
+ * Returns other backend's IO stats.
+ */
+PgStat_Backend *
+pgstat_fetch_proc_stat_io(ProcNumber procNumber)
+{
+ PgStat_Backend *backend_entry;
+
+ backend_entry = (PgStat_Backend *) pgstat_fetch_entry(PGSTAT_KIND_PER_BACKEND,
+ InvalidOid, procNumber);
+
+ return backend_entry;
+}
+
+/*
+ * Flush out locally pending backend statistics
+ *
+ * If no stats have been recorded, this function returns false.
+ */
+bool
+pgstat_per_backend_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
+{
+ PgStatShared_Backend *shbackendioent;
+ PgStat_BackendPendingIO *pendingent;
+ PgStat_BktypeIO *bktype_shstats;
+
+ if (!pgstat_lock_entry(entry_ref, nowait))
+ return false;
+
+ shbackendioent = (PgStatShared_Backend *) entry_ref->shared_stats;
+ bktype_shstats = &shbackendioent->stats.stats;
+ pendingent = (PgStat_BackendPendingIO *) entry_ref->pending;
+
+ for (int io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)
+ {
+ for (int io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++)
+ {
+ for (int io_op = 0; io_op < IOOP_NUM_TYPES; io_op++)
+ {
+ instr_time time;
+
+ bktype_shstats->counts[io_object][io_context][io_op] +=
+ pendingent->counts[io_object][io_context][io_op];
+
+ time = pendingent->pending_times[io_object][io_context][io_op];
+
+ bktype_shstats->times[io_object][io_context][io_op] +=
+ INSTR_TIME_GET_MICROSEC(time);
+ }
+ }
+ }
+
+ pgstat_unlock_entry(entry_ref);
+
+ return true;
+}
+
+/*
+ * Create the per-backend statistics entry for procnum.
+ */
+void
+pgstat_create_backend_stat(ProcNumber procnum)
+{
+ PgStat_EntryRef *entry_ref;
+ PgStatShared_Backend *shstatent;
+
+ entry_ref = pgstat_prep_pending_entry(PGSTAT_KIND_PER_BACKEND, InvalidOid,
+ procnum, NULL);
+
+ shstatent = (PgStatShared_Backend *) entry_ref->shared_stats;
+
+ /*
+ * NB: need to accept that there might be stats from an older backend,
+ * e.g. if we previously used this proc number.
+ */
+ memset(&shstatent->stats, 0, sizeof(shstatent->stats));
+}
+
+/*
+ * Find or create a local PgStat_BackendPendingIO entry for procnum.
+ */
+PgStat_BackendPendingIO *
+pgstat_prep_per_backend_pending(ProcNumber procnum)
+{
+ PgStat_EntryRef *entry_ref;
+
+ entry_ref = pgstat_prep_pending_entry(PGSTAT_KIND_PER_BACKEND, InvalidOid,
+ procnum, NULL);
+
+ return entry_ref->pending;
+}
+
+/*
+ * per-backend statistics are not collected for all BackendTypes.
+ *
+ * The following BackendTypes do not participate in the per-backend stats
+ * subsystem:
+ * - The same and for the same reasons as in pgstat_tracks_io_bktype().
+ * - B_BG_WRITER, B_CHECKPOINTER, B_STARTUP and B_AUTOVAC_LAUNCHER because their
+ * I/O stats are already visible in pg_stat_io and there is only one of those.
+ *
+ * Function returns true if BackendType participates in the per-backend stats
+ * subsystem for IO and false if it does not.
+ *
+ * When adding a new BackendType, also consider adding relevant restrictions to
+ * pgstat_tracks_io_object() and pgstat_tracks_io_op().
+ */
+bool
+pgstat_tracks_per_backend_bktype(BackendType bktype)
+{
+ /*
+ * List every type so that new backend types trigger a warning about
+ * needing to adjust this switch.
+ */
+ switch (bktype)
+ {
+ case B_INVALID:
+ case B_AUTOVAC_LAUNCHER:
+ case B_DEAD_END_BACKEND:
+ case B_ARCHIVER:
+ case B_LOGGER:
+ case B_WAL_RECEIVER:
+ case B_WAL_WRITER:
+ case B_WAL_SUMMARIZER:
+ case B_BG_WRITER:
+ case B_CHECKPOINTER:
+ case B_STARTUP:
+ return false;
+
+ case B_AUTOVAC_WORKER:
+ case B_BACKEND:
+ case B_BG_WORKER:
+ case B_STANDALONE_BACKEND:
+ case B_SLOTSYNC_WORKER:
+ case B_WAL_SENDER:
+ return true;
+ }
+
+ return false;
+}
+
+void
+pgstat_backend_reset_timestamp_cb(PgStatShared_Common *header, TimestampTz ts)
+{
+ ((PgStatShared_Backend *) header)->stats.stat_reset_timestamp = ts;
+}
diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c
index f9883af2b3..1af5b75df6 100644
--- a/src/backend/utils/activity/pgstat_io.c
+++ b/src/backend/utils/activity/pgstat_io.c
@@ -20,13 +20,7 @@
#include "storage/bufmgr.h"
#include "utils/pgstat_internal.h"
-
-typedef struct PgStat_PendingIO
-{
- PgStat_Counter counts[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
- instr_time pending_times[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
-} PgStat_PendingIO;
-
+typedef PgStat_BackendPendingIO PgStat_PendingIO;
static PgStat_PendingIO PendingIOStats;
static bool have_iostats = false;
@@ -87,6 +81,14 @@ pgstat_count_io_op_n(IOObject io_object, IOContext io_context, IOOp io_op, uint3
Assert((unsigned int) io_op < IOOP_NUM_TYPES);
Assert(pgstat_tracks_io_op(MyBackendType, io_object, io_context, io_op));
+ if (pgstat_tracks_per_backend_bktype(MyBackendType))
+ {
+ PgStat_PendingIO *entry_ref;
+
+ entry_ref = pgstat_prep_per_backend_pending(MyProcNumber);
+ entry_ref->counts[io_object][io_context][io_op] += cnt;
+ }
+
PendingIOStats.counts[io_object][io_context][io_op] += cnt;
have_iostats = true;
@@ -122,6 +124,7 @@ void
pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op,
instr_time start_time, uint32 cnt)
{
+
if (track_io_timing)
{
instr_time io_time;
@@ -148,6 +151,15 @@ pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op,
INSTR_TIME_ADD(PendingIOStats.pending_times[io_object][io_context][io_op],
io_time);
+
+ if (pgstat_tracks_per_backend_bktype(MyBackendType))
+ {
+ PgStat_PendingIO *entry_ref;
+
+ entry_ref = pgstat_prep_per_backend_pending(MyProcNumber);
+ INSTR_TIME_ADD(entry_ref->pending_times[io_object][io_context][io_op],
+ io_time);
+ }
}
pgstat_count_io_op_n(io_object, io_context, io_op, cnt);
@@ -171,12 +183,21 @@ pgstat_io_have_pending_cb(void)
}
/*
- * Simpler wrapper of pgstat_io_flush_cb()
+ * Simpler wrapper of pgstat_io_flush_cb() and pgstat_per_backend_flush_cb().
*/
void
pgstat_flush_io(bool nowait)
{
+ PgStat_EntryRef *entry_ref;
+
(void) pgstat_io_flush_cb(nowait);
+
+ if (pgstat_tracks_per_backend_bktype(MyBackendType))
+ {
+ entry_ref = pgstat_get_entry_ref(PGSTAT_KIND_PER_BACKEND, InvalidOid,
+ MyProcNumber, false, NULL);
+ (void) pgstat_per_backend_flush_cb(entry_ref, nowait);
+ }
}
/*
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 60a397dc56..0302970246 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1480,6 +1480,165 @@ pg_stat_get_io(PG_FUNCTION_ARGS)
return (Datum) 0;
}
+Datum
+pg_stat_get_backend_io(PG_FUNCTION_ARGS)
+{
+ ReturnSetInfo *rsinfo;
+ PgStat_Backend *backend_stats;
+ Datum bktype_desc;
+ PgStat_BktypeIO *bktype_stats;
+ BackendType bktype;
+ Datum reset_time;
+ int num_backends = pgstat_fetch_stat_numbackends();
+ int curr_backend;
+ int pid;
+
+ /* just to keep compiler quiet */
+ bktype = B_INVALID;
+
+ InitMaterializedSRF(fcinfo, 0);
+ rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+
+ if (PG_ARGISNULL(0) || PG_GETARG_INT32(0) == MyProcPid)
+ {
+ backend_stats = pgstat_fetch_my_stat_io();
+ pid = MyProcPid;
+ bktype = MyBackendType;
+ }
+ else
+ {
+ PGPROC *proc;
+ ProcNumber procNumber;
+
+ pid = PG_GETARG_INT32(0);
+ proc = BackendPidGetProc(pid);
+
+ /*
+ * Maybe an auxiliary process? That should not be possible, due to
+ * pgstat_tracks_per_backend_bktype() though.
+ */
+ if (proc == NULL)
+ proc = AuxiliaryPidGetProc(pid);
+
+ if (proc != NULL)
+ {
+ procNumber = GetNumberFromPGProc(proc);
+ backend_stats = pgstat_fetch_proc_stat_io(procNumber);
+
+ if (!backend_stats)
+ return (Datum) 0;
+ }
+ else
+ return (Datum) 0;
+ }
+
+ /* Look for the backend type */
+ for (curr_backend = 1; curr_backend <= num_backends; curr_backend++)
+ {
+ LocalPgBackendStatus *local_beentry;
+ PgBackendStatus *beentry;
+
+ /* Get the next one in the list */
+ local_beentry = pgstat_get_local_beentry_by_index(curr_backend);
+ beentry = &local_beentry->backendStatus;
+
+ /* looking for specific PID, ignore all the others */
+ if (beentry->st_procpid != pid)
+ continue;
+
+ bktype = beentry->st_backendType;
+ break;
+ }
+
+ bktype_desc = CStringGetTextDatum(GetBackendTypeDesc(bktype));
+ bktype_stats = &backend_stats->stats;
+ reset_time = TimestampTzGetDatum(backend_stats->stat_reset_timestamp);
+
+ /*
+ * In Assert builds, we can afford an extra loop through all of the
+ * counters checking that only expected stats are non-zero, since it keeps
+ * the non-Assert code cleaner.
+ */
+ Assert(pgstat_bktype_io_stats_valid(bktype_stats, bktype));
+
+ for (int io_obj = 0; io_obj < IOOBJECT_NUM_TYPES; io_obj++)
+ {
+ const char *obj_name = pgstat_get_io_object_name(io_obj);
+
+ for (int io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++)
+ {
+ const char *context_name = pgstat_get_io_context_name(io_context);
+
+ Datum values[IO_NUM_COLUMNS] = {0};
+ bool nulls[IO_NUM_COLUMNS] = {0};
+
+ /*
+ * Some combinations of BackendType, IOObject, and IOContext are
+ * not valid for any type of IOOp. In such cases, omit the entire
+ * row from the view.
+ */
+ if (!pgstat_tracks_io_object(bktype, io_obj, io_context))
+ continue;
+
+ values[IO_COL_BACKEND_TYPE] = bktype_desc;
+ values[IO_COL_CONTEXT] = CStringGetTextDatum(context_name);
+ values[IO_COL_OBJECT] = CStringGetTextDatum(obj_name);
+ if (backend_stats->stat_reset_timestamp != 0)
+ values[IO_COL_RESET_TIME] = reset_time;
+ else
+ nulls[IO_COL_RESET_TIME] = true;
+
+ /*
+ * Hard-code this to the value of BLCKSZ for now. Future values
+ * could include XLOG_BLCKSZ, once WAL IO is tracked, and constant
+ * multipliers, once non-block-oriented IO (e.g. temporary file
+ * IO) is tracked.
+ */
+ values[IO_COL_CONVERSION] = Int64GetDatum(BLCKSZ);
+
+ for (int io_op = 0; io_op < IOOP_NUM_TYPES; io_op++)
+ {
+ int op_idx = pgstat_get_io_op_index(io_op);
+ int time_idx = pgstat_get_io_time_index(io_op);
+
+ /*
+ * Some combinations of BackendType and IOOp, of IOContext and
+ * IOOp, and of IOObject and IOOp are not tracked. Set these
+ * cells in the view NULL.
+ */
+ if (pgstat_tracks_io_op(bktype, io_obj, io_context, io_op))
+ {
+ PgStat_Counter count =
+ bktype_stats->counts[io_obj][io_context][io_op];
+
+ values[op_idx] = Int64GetDatum(count);
+ }
+ else
+ nulls[op_idx] = true;
+
+ /* not every operation is timed */
+ if (time_idx == IO_COL_INVALID)
+ continue;
+
+ if (!nulls[op_idx])
+ {
+ PgStat_Counter time =
+ bktype_stats->times[io_obj][io_context][io_op];
+
+ values[time_idx] = Float8GetDatum(pg_stat_us_to_ms(time));
+ }
+ else
+ nulls[time_idx] = true;
+ }
+
+ tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc,
+ values, nulls);
+ }
+ }
+
+ return (Datum) 0;
+}
+
/*
* Returns statistics of WAL activity
*/
@@ -1785,6 +1944,55 @@ pg_stat_reset_single_function_counters(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+Datum
+pg_stat_reset_single_backend_io_counters(PG_FUNCTION_ARGS)
+{
+ PGPROC *proc;
+ BackendType bktype;
+ int backend_pid = PG_GETARG_INT32(0);
+ int num_backends = pgstat_fetch_stat_numbackends();
+ int curr_backend;
+
+ proc = BackendPidGetProc(backend_pid);
+
+ /*
+ * Maybe an auxiliary process? That should not be possible, due to
+ * pgstat_tracks_per_backend_bktype() though.
+ */
+ if (proc == NULL)
+ proc = AuxiliaryPidGetProc(backend_pid);
+
+ /* Check that IO stats are collected for this backend type */
+ if (proc)
+ {
+ for (curr_backend = 1; curr_backend <= num_backends; curr_backend++)
+ {
+ LocalPgBackendStatus *local_beentry;
+ PgBackendStatus *beentry;
+
+ /* Get the next one in the list */
+ local_beentry = pgstat_get_local_beentry_by_index(curr_backend);
+ beentry = &local_beentry->backendStatus;
+
+ /* looking for specific PID, ignore all the others */
+ if (beentry->st_procpid != backend_pid)
+ continue;
+
+ bktype = beentry->st_backendType;
+
+ if (!pgstat_tracks_per_backend_bktype(bktype))
+ PG_RETURN_VOID();
+ else
+ {
+ pgstat_reset(PGSTAT_KIND_PER_BACKEND, InvalidOid, GetNumberFromPGProc(proc));
+ break;
+ }
+ }
+ }
+
+ PG_RETURN_VOID();
+}
+
/* Reset SLRU counters (a specific one or all of them). */
Datum
pg_stat_reset_slru(PG_FUNCTION_ARGS)
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index cbbe8acd38..6e5c2082a7 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5913,6 +5913,15 @@
proargnames => '{backend_type,object,context,reads,read_time,writes,write_time,writebacks,writeback_time,extends,extend_time,op_bytes,hits,evictions,reuses,fsyncs,fsync_time,stats_reset}',
prosrc => 'pg_stat_get_io' },
+{ oid => '8806', descr => 'statistics: per backend IO statistics',
+ proname => 'pg_stat_get_backend_io', prorows => '5', proretset => 't',
+ provolatile => 'v', proparallel => 'r', prorettype => 'record',
+ proargtypes => 'int4', proisstrict => 'f',
+ proallargtypes => '{int4,text,text,text,int8,float8,int8,float8,int8,float8,int8,float8,int8,int8,int8,int8,int8,float8,timestamptz}',
+ proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}',
+ proargnames => '{backend_pid,backend_type,object,context,reads,read_time,writes,write_time,writebacks,writeback_time,extends,extend_time,op_bytes,hits,evictions,reuses,fsyncs,fsync_time,stats_reset}',
+ prosrc => 'pg_stat_get_backend_io' },
+
{ oid => '1136', descr => 'statistics: information about WAL activity',
proname => 'pg_stat_get_wal', proisstrict => 'f', provolatile => 's',
proparallel => 'r', prorettype => 'record', proargtypes => '',
@@ -6052,6 +6061,11 @@
proname => 'pg_stat_reset_single_function_counters', provolatile => 'v',
prorettype => 'void', proargtypes => 'oid',
prosrc => 'pg_stat_reset_single_function_counters' },
+{ oid => '9987',
+ descr => 'statistics: reset collected IO statistics for a single backend',
+ proname => 'pg_stat_reset_single_backend_io_counters', provolatile => 'v',
+ prorettype => 'void', proargtypes => 'int4',
+ prosrc => 'pg_stat_reset_single_backend_io_counters' },
{ oid => '2307',
descr => 'statistics: reset collected statistics for a single SLRU',
proname => 'pg_stat_reset_slru', proisstrict => 'f', provolatile => 'v',
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 59c28b4aca..c5151c8e6f 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -49,14 +49,15 @@
#define PGSTAT_KIND_FUNCTION 3 /* per-function statistics */
#define PGSTAT_KIND_REPLSLOT 4 /* per-slot statistics */
#define PGSTAT_KIND_SUBSCRIPTION 5 /* per-subscription statistics */
+#define PGSTAT_KIND_PER_BACKEND 6
/* stats for fixed-numbered objects */
-#define PGSTAT_KIND_ARCHIVER 6
-#define PGSTAT_KIND_BGWRITER 7
-#define PGSTAT_KIND_CHECKPOINTER 8
-#define PGSTAT_KIND_IO 9
-#define PGSTAT_KIND_SLRU 10
-#define PGSTAT_KIND_WAL 11
+#define PGSTAT_KIND_ARCHIVER 7
+#define PGSTAT_KIND_BGWRITER 8
+#define PGSTAT_KIND_CHECKPOINTER 9
+#define PGSTAT_KIND_IO 10
+#define PGSTAT_KIND_SLRU 11
+#define PGSTAT_KIND_WAL 12
#define PGSTAT_KIND_BUILTIN_MIN PGSTAT_KIND_DATABASE
#define PGSTAT_KIND_BUILTIN_MAX PGSTAT_KIND_WAL
@@ -347,12 +348,23 @@ typedef struct PgStat_BktypeIO
PgStat_Counter times[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
} PgStat_BktypeIO;
+typedef struct PgStat_BackendPendingIO
+{
+ PgStat_Counter counts[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
+ instr_time pending_times[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
+} PgStat_BackendPendingIO;
+
typedef struct PgStat_IO
{
TimestampTz stat_reset_timestamp;
PgStat_BktypeIO stats[BACKEND_NUM_TYPES];
} PgStat_IO;
+typedef struct PgStat_Backend
+{
+ TimestampTz stat_reset_timestamp;
+ PgStat_BktypeIO stats;
+} PgStat_Backend;
typedef struct PgStat_StatDBEntry
{
@@ -534,6 +546,14 @@ extern bool pgstat_have_entry(PgStat_Kind kind, Oid dboid, uint64 objid);
extern void pgstat_report_archiver(const char *xlog, bool failed);
extern PgStat_ArchiverStats *pgstat_fetch_stat_archiver(void);
+/*
+ * Functions in pgstat_backend.c
+ */
+
+extern PgStat_Backend *pgstat_fetch_my_stat_io(void);
+extern PgStat_Backend *pgstat_fetch_proc_stat_io(ProcNumber procNumber);
+extern bool pgstat_tracks_per_backend_bktype(BackendType bktype);
+extern void pgstat_create_backend_stat(ProcNumber procnum);
/*
* Functions in pgstat_bgwriter.c
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index 7338bc1e28..625bb55864 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -450,6 +450,11 @@ typedef struct PgStatShared_ReplSlot
PgStat_StatReplSlotEntry stats;
} PgStatShared_ReplSlot;
+typedef struct PgStatShared_Backend
+{
+ PgStatShared_Common header;
+ PgStat_Backend stats;
+} PgStatShared_Backend;
/*
* Central shared memory entry for the cumulative stats system.
@@ -604,6 +609,13 @@ extern void pgstat_archiver_init_shmem_cb(void *stats);
extern void pgstat_archiver_reset_all_cb(TimestampTz ts);
extern void pgstat_archiver_snapshot_cb(void);
+/*
+ * Functions in pgstat_backend.c
+ */
+
+extern PgStat_BackendPendingIO *pgstat_prep_per_backend_pending(ProcNumber procnum);
+extern bool pgstat_per_backend_flush_cb(PgStat_EntryRef *entry_ref, bool nowait);
+extern void pgstat_backend_reset_timestamp_cb(PgStatShared_Common *header, TimestampTz ts);
/*
* Functions in pgstat_bgwriter.c
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index 3014d047fe..3922675bbf 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1398,6 +1398,25 @@ pg_matviews| SELECT n.nspname AS schemaname,
LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace)))
LEFT JOIN pg_tablespace t ON ((t.oid = c.reltablespace)))
WHERE (c.relkind = 'm'::"char");
+pg_my_stat_io| SELECT backend_type,
+ object,
+ context,
+ reads,
+ read_time,
+ writes,
+ write_time,
+ writebacks,
+ writeback_time,
+ extends,
+ extend_time,
+ op_bytes,
+ hits,
+ evictions,
+ reuses,
+ fsyncs,
+ fsync_time,
+ stats_reset
+ FROM pg_stat_get_backend_io(NULL::integer) b(backend_type, object, context, reads, read_time, writes, write_time, writebacks, writeback_time, extends, extend_time, op_bytes, hits, evictions, reuses, fsyncs, fsync_time, stats_reset);
pg_policies| SELECT n.nspname AS schemaname,
c.relname AS tablename,
pol.polname AS policyname,
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 56771f83ed..7f2fd873b9 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -1249,7 +1249,7 @@ SELECT pg_stat_get_subscription_stats(NULL);
(1 row)
--- Test that the following operations are tracked in pg_stat_io:
+-- Test that the following operations are tracked in pg_[my_]stat_io:
-- - reads of target blocks into shared buffers
-- - writes of shared buffers to permanent storage
-- - extends of relations using shared buffers
@@ -1259,11 +1259,24 @@ SELECT pg_stat_get_subscription_stats(NULL);
-- be sure of the state of shared buffers at the point the test is run.
-- Create a regular table and insert some data to generate IOCONTEXT_NORMAL
-- extends.
+SELECT pid AS checkpointer_pid FROM pg_stat_activity
+ WHERE backend_type = 'checkpointer' \gset
SELECT sum(extends) AS io_sum_shared_before_extends
FROM pg_stat_io WHERE context = 'normal' AND object = 'relation' \gset
+SELECT sum(extends) AS my_io_sum_shared_before_extends
+ FROM pg_my_stat_io WHERE context = 'normal' AND object = 'relation' \gset
+SELECT sum(extends) AS backend_io_sum_shared_before_extends
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE context = 'normal' AND object = 'relation' \gset
SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
FROM pg_stat_io
WHERE object = 'relation' \gset io_sum_shared_before_
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+ FROM pg_my_stat_io
+ WHERE object = 'relation' \gset my_io_sum_shared_before_
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE object = 'relation' \gset backend_io_sum_shared_before_
CREATE TABLE test_io_shared(a int);
INSERT INTO test_io_shared SELECT i FROM generate_series(1,100)i;
SELECT pg_stat_force_next_flush();
@@ -1280,8 +1293,25 @@ SELECT :io_sum_shared_after_extends > :io_sum_shared_before_extends;
t
(1 row)
+SELECT sum(extends) AS my_io_sum_shared_after_extends
+ FROM pg_my_stat_io WHERE context = 'normal' AND object = 'relation' \gset
+SELECT :my_io_sum_shared_after_extends > :my_io_sum_shared_before_extends;
+ ?column?
+----------
+ t
+(1 row)
+
+SELECT sum(extends) AS backend_io_sum_shared_after_extends
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE context = 'normal' AND object = 'relation' \gset
+SELECT :backend_io_sum_shared_after_extends > :backend_io_sum_shared_before_extends;
+ ?column?
+----------
+ t
+(1 row)
+
-- After a checkpoint, there should be some additional IOCONTEXT_NORMAL writes
--- and fsyncs.
+-- and fsyncs in the global stats (not for the backend).
-- See comment above for rationale for two explicit CHECKPOINTs.
CHECKPOINT;
CHECKPOINT;
@@ -1301,6 +1331,31 @@ SELECT current_setting('fsync') = 'off'
t
(1 row)
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+ FROM pg_my_stat_io
+ WHERE object = 'relation' \gset my_io_sum_shared_after_
+SELECT :my_io_sum_shared_after_writes >= :my_io_sum_shared_before_writes;
+ ?column?
+----------
+ t
+(1 row)
+
+SELECT current_setting('fsync') = 'off'
+ OR (:my_io_sum_shared_after_fsyncs = :my_io_sum_shared_before_fsyncs
+ AND :my_io_sum_shared_after_fsyncs= 0);
+ ?column?
+----------
+ t
+(1 row)
+
+-- Don't return any rows if querying other backend's stats that are excluded
+-- from the per-backend stats collection (like the checkpointer).
+SELECT count(1) = 0 FROM pg_stat_get_backend_io(:checkpointer_pid);
+ ?column?
+----------
+ t
+(1 row)
+
-- Change the tablespace so that the table is rewritten directly, then SELECT
-- from it to cause it to be read back into shared buffers.
SELECT sum(reads) AS io_sum_shared_before_reads
@@ -1521,6 +1576,8 @@ SELECT pg_stat_have_stats('io', 0, 0);
SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS io_stats_pre_reset
FROM pg_stat_io \gset
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_pre_reset
+ FROM pg_my_stat_io \gset
SELECT pg_stat_reset_shared('io');
pg_stat_reset_shared
----------------------
@@ -1535,6 +1592,30 @@ SELECT :io_stats_post_reset < :io_stats_pre_reset;
t
(1 row)
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_post_reset
+ FROM pg_my_stat_io \gset
+-- pg_stat_reset_shared() did not reset backend IO stats
+SELECT :my_io_stats_pre_reset <= :my_io_stats_post_reset;
+ ?column?
+----------
+ t
+(1 row)
+
+-- but pg_stat_reset_single_backend_io_counters() does
+SELECT pg_stat_reset_single_backend_io_counters(pg_backend_pid());
+ pg_stat_reset_single_backend_io_counters
+------------------------------------------
+
+(1 row)
+
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_post_backend_reset
+ FROM pg_my_stat_io \gset
+SELECT :my_io_stats_pre_reset > :my_io_stats_post_backend_reset;
+ ?column?
+----------
+ t
+(1 row)
+
-- test BRIN index doesn't block HOT update
CREATE TABLE brin_hot (
id integer PRIMARY KEY,
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index 7147cc2f89..497b2b438a 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -595,7 +595,7 @@ SELECT pg_stat_get_replication_slot(NULL);
SELECT pg_stat_get_subscription_stats(NULL);
--- Test that the following operations are tracked in pg_stat_io:
+-- Test that the following operations are tracked in pg_[my_]stat_io:
-- - reads of target blocks into shared buffers
-- - writes of shared buffers to permanent storage
-- - extends of relations using shared buffers
@@ -607,20 +607,40 @@ SELECT pg_stat_get_subscription_stats(NULL);
-- Create a regular table and insert some data to generate IOCONTEXT_NORMAL
-- extends.
+SELECT pid AS checkpointer_pid FROM pg_stat_activity
+ WHERE backend_type = 'checkpointer' \gset
SELECT sum(extends) AS io_sum_shared_before_extends
FROM pg_stat_io WHERE context = 'normal' AND object = 'relation' \gset
+SELECT sum(extends) AS my_io_sum_shared_before_extends
+ FROM pg_my_stat_io WHERE context = 'normal' AND object = 'relation' \gset
+SELECT sum(extends) AS backend_io_sum_shared_before_extends
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE context = 'normal' AND object = 'relation' \gset
SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
FROM pg_stat_io
WHERE object = 'relation' \gset io_sum_shared_before_
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+ FROM pg_my_stat_io
+ WHERE object = 'relation' \gset my_io_sum_shared_before_
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE object = 'relation' \gset backend_io_sum_shared_before_
CREATE TABLE test_io_shared(a int);
INSERT INTO test_io_shared SELECT i FROM generate_series(1,100)i;
SELECT pg_stat_force_next_flush();
SELECT sum(extends) AS io_sum_shared_after_extends
FROM pg_stat_io WHERE context = 'normal' AND object = 'relation' \gset
SELECT :io_sum_shared_after_extends > :io_sum_shared_before_extends;
+SELECT sum(extends) AS my_io_sum_shared_after_extends
+ FROM pg_my_stat_io WHERE context = 'normal' AND object = 'relation' \gset
+SELECT :my_io_sum_shared_after_extends > :my_io_sum_shared_before_extends;
+SELECT sum(extends) AS backend_io_sum_shared_after_extends
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE context = 'normal' AND object = 'relation' \gset
+SELECT :backend_io_sum_shared_after_extends > :backend_io_sum_shared_before_extends;
-- After a checkpoint, there should be some additional IOCONTEXT_NORMAL writes
--- and fsyncs.
+-- and fsyncs in the global stats (not for the backend).
-- See comment above for rationale for two explicit CHECKPOINTs.
CHECKPOINT;
CHECKPOINT;
@@ -631,6 +651,18 @@ SELECT :io_sum_shared_after_writes > :io_sum_shared_before_writes;
SELECT current_setting('fsync') = 'off'
OR :io_sum_shared_after_fsyncs > :io_sum_shared_before_fsyncs;
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+ FROM pg_my_stat_io
+ WHERE object = 'relation' \gset my_io_sum_shared_after_
+SELECT :my_io_sum_shared_after_writes >= :my_io_sum_shared_before_writes;
+SELECT current_setting('fsync') = 'off'
+ OR (:my_io_sum_shared_after_fsyncs = :my_io_sum_shared_before_fsyncs
+ AND :my_io_sum_shared_after_fsyncs= 0);
+
+-- Don't return any rows if querying other backend's stats that are excluded
+-- from the per-backend stats collection (like the checkpointer).
+SELECT count(1) = 0 FROM pg_stat_get_backend_io(:checkpointer_pid);
+
-- Change the tablespace so that the table is rewritten directly, then SELECT
-- from it to cause it to be read back into shared buffers.
SELECT sum(reads) AS io_sum_shared_before_reads
@@ -762,10 +794,21 @@ SELECT :io_sum_bulkwrite_strategy_extends_after > :io_sum_bulkwrite_strategy_ext
SELECT pg_stat_have_stats('io', 0, 0);
SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS io_stats_pre_reset
FROM pg_stat_io \gset
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_pre_reset
+ FROM pg_my_stat_io \gset
SELECT pg_stat_reset_shared('io');
SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS io_stats_post_reset
FROM pg_stat_io \gset
SELECT :io_stats_post_reset < :io_stats_pre_reset;
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_post_reset
+ FROM pg_my_stat_io \gset
+-- pg_stat_reset_shared() did not reset backend IO stats
+SELECT :my_io_stats_pre_reset <= :my_io_stats_post_reset;
+-- but pg_stat_reset_single_backend_io_counters() does
+SELECT pg_stat_reset_single_backend_io_counters(pg_backend_pid());
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_post_backend_reset
+ FROM pg_my_stat_io \gset
+SELECT :my_io_stats_pre_reset > :my_io_stats_post_backend_reset;
-- test BRIN index doesn't block HOT update
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index b54428b38c..ea676361a5 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2120,6 +2120,7 @@ PgFdwSamplingMethod
PgFdwScanState
PgIfAddrCallback
PgStatShared_Archiver
+PgStatShared_Backend
PgStatShared_BgWriter
PgStatShared_Checkpointer
PgStatShared_Common
@@ -2135,6 +2136,8 @@ PgStatShared_SLRU
PgStatShared_Subscription
PgStatShared_Wal
PgStat_ArchiverStats
+PgStat_Backend
+PgStat_BackendPendingIO
PgStat_BackendSubEntry
PgStat_BgWriterStats
PgStat_BktypeIO
--
2.34.1
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: per backend I/O statistics
2024-11-25 07:12 Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-11-25 07:18 ` Re: per backend I/O statistics[ Michael Paquier <[email protected]>
2024-11-25 15:47 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
@ 2024-11-27 06:33 ` Michael Paquier <[email protected]>
2024-11-27 16:00 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
1 sibling, 1 reply; 12+ messages in thread
From: Michael Paquier @ 2024-11-27 06:33 UTC (permalink / raw)
To: Bertrand Drouvot <[email protected]>; +Cc: Nazir Bilal Yavuz <[email protected]>; Alvaro Herrera <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On Mon, Nov 25, 2024 at 03:47:59PM +0000, Bertrand Drouvot wrote:
> It also takes care of most of the comments that you have made in [1], meaning
> that it:
>
> - removes the backend type from PgStat_Backend and look for the backend type
> at "display" time.
> - creates PgStat_BackendPendingIO and PgStat_PendingIO now refers to it (I
> used PgStat_BackendPendingIO and not PgStat_BackendPending because this is what
> it is after all).
> - adds the missing comment related to the PID in the doc.
> - merges 0004 with 0001 (so that pg_stat_get_backend_io() is now part of 0001).
> - creates its own pgstat_backend.c file.
I have begun studying the patch, and I have one question.
+void
+pgstat_create_backend_stat(ProcNumber procnum)
[...]
+ /* Create the per-backend statistics entry */
+ if (pgstat_tracks_per_backend_bktype(MyBackendType))
+ pgstat_create_backend_stat(MyProcNumber);
Perhaps that's a very stupid question, but I was looking at this part
of the patch, and wondered why we don't use init_backend_cb? I
vaguely recalled that MyProcNumber and MyBackendType would be set
before we go through the pgstat initialization step. MyDatabaseId is
filled after the pgstat initialization, but we don't care about this
information for such backend stats.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: per backend I/O statistics
2024-11-25 07:12 Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-11-25 07:18 ` Re: per backend I/O statistics[ Michael Paquier <[email protected]>
2024-11-25 15:47 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-11-27 06:33 ` Re: per backend I/O statistics Michael Paquier <[email protected]>
@ 2024-11-27 16:00 ` Bertrand Drouvot <[email protected]>
2024-12-12 04:11 ` Re: per backend I/O statistics Michael Paquier <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Bertrand Drouvot @ 2024-11-27 16:00 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Nazir Bilal Yavuz <[email protected]>; Alvaro Herrera <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
Hi,
On Wed, Nov 27, 2024 at 03:33:38PM +0900, Michael Paquier wrote:
> On Mon, Nov 25, 2024 at 03:47:59PM +0000, Bertrand Drouvot wrote:
> > It also takes care of most of the comments that you have made in [1], meaning
> > that it:
> >
> > - removes the backend type from PgStat_Backend and look for the backend type
> > at "display" time.
> > - creates PgStat_BackendPendingIO and PgStat_PendingIO now refers to it (I
> > used PgStat_BackendPendingIO and not PgStat_BackendPending because this is what
> > it is after all).
> > - adds the missing comment related to the PID in the doc.
> > - merges 0004 with 0001 (so that pg_stat_get_backend_io() is now part of 0001).
> > - creates its own pgstat_backend.c file.
>
> I have begun studying the patch, and I have one question.
>
> +void
> +pgstat_create_backend_stat(ProcNumber procnum)
> [...]
> + /* Create the per-backend statistics entry */
> + if (pgstat_tracks_per_backend_bktype(MyBackendType))
> + pgstat_create_backend_stat(MyProcNumber);
>
> Perhaps that's a very stupid question, but I was looking at this part
> of the patch, and wondered why we don't use init_backend_cb? I
> vaguely recalled that MyProcNumber and MyBackendType would be set
> before we go through the pgstat initialization step. MyDatabaseId is
> filled after the pgstat initialization, but we don't care about this
> information for such backend stats.
Using init_backend_cb (and moving the pgstat_tracks_per_backend_bktype() check
in it) is currently producing a failed assertion:
TRAP: failed Assert("pgstat_is_initialized && !pgstat_is_shutdown"), File: "pgstat.c", Line: 1567, PID: 2080000
postgres: postgres postgres [local] initializing(ExceptionalCondition+0xbb)[0x5fd0c69afaed]
postgres: postgres postgres [local] initializing(pgstat_assert_is_up+0x3f)[0x5fd0c67d1b77]
postgres: postgres postgres [local] initializing(pgstat_get_entry_ref+0x95)[0x5fd0c67db068]
postgres: postgres postgres [local] initializing(pgstat_prep_pending_entry+0xaa)[0x5fd0c67d1181]
postgres: postgres postgres [local] initializing(pgstat_create_backend_stat+0x96)[0x5fd0c67d3986]
But even if we, say move the "pgstat_is_initialized = true" before the init_backend_cb()
call in pgstat_initialize() (not saying that makes sense, just trying out), then
we are back to the restore/reset issue but this time during initdb:
TRAP: failed Assert("!pgstat_entry_ref_hash_lookup(pgStatEntryRefHash, shent->key)"), File: "pgstat_shmem.c", Line: 852, PID: 2109086
/home/postgres/postgresql/pg_installed/pg18/bin/postgres(ExceptionalCondition+0xbb)[0x621723499c5a]
/home/postgres/postgresql/pg_installed/pg18/bin/postgres(+0x70dd2b)[0x6217232c5d2b]
/home/postgres/postgresql/pg_installed/pg18/bin/postgres(pgstat_drop_all_entries+0x61)[0x6217232c60b5]
/home/postgres/postgresql/pg_installed/pg18/bin/postgres(+0x705356)[0x6217232bd356]
/home/postgres/postgresql/pg_installed/pg18/bin/postgres(+0x70462a)[0x6217232bc62a]
/home/postgres/postgresql/pg_installed/pg18/bin/postgres(pgstat_restore_stats+0x1c)[0x6217232b9f01]
/home/postgres/postgresql/pg_installed/pg18/bin/postgres(StartupXLOG+0x693)[0x621722da9697]
/home/postgres/postgresql/pg_installed/pg18/bin/postgres(InitPostgres+0x1d8)[0x6217234b40df]
/home/postgres/postgresql/pg_installed/pg18/bin/postgres(BootstrapModeMain+0x532)[0x621722dd79bf]
where "PID: 2109086" is a B_STANDALONE_BACKEND.
This is due to the fact that, during the initdb bootstrap, init_backend_cb() is
called before StartupXLOG().
One option could be to move B_STANDALONE_BACKEND in the "false" section of
pgstat_tracks_per_backend_bktype() and ensure that we set pgstat_is_initialized
to true before init_backend_cb() is called (but I don't think that makes that much
sense).
I'd vote to just keep the pgstat_create_backend_stat() call in pgstat_beinit().
That said, we probably should document that init_backend_cb() should not call
pgstat_get_entry_ref(), but that's probably worth another thread.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: per backend I/O statistics
2024-11-25 07:12 Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-11-25 07:18 ` Re: per backend I/O statistics[ Michael Paquier <[email protected]>
2024-11-25 15:47 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-11-27 06:33 ` Re: per backend I/O statistics Michael Paquier <[email protected]>
2024-11-27 16:00 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
@ 2024-12-12 04:11 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: Michael Paquier @ 2024-12-12 04:11 UTC (permalink / raw)
To: Bertrand Drouvot <[email protected]>; +Cc: Nazir Bilal Yavuz <[email protected]>; Alvaro Herrera <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On Wed, Nov 27, 2024 at 04:00:14PM +0000, Bertrand Drouvot wrote:
> I'd vote to just keep the pgstat_create_backend_stat() call in pgstat_beinit().
>
> That said, we probably should document that init_backend_cb() should not call
> pgstat_get_entry_ref(), but that's probably worth another thread.
Hmm, yeah. I have considered this point and your approach does not
seem that bad to me after thinking much more about it, and you are
putting the call in pgstat_beinit(), which is about initializing the
stats for the backend. So I'm OK with what the patch does here in
terms of the location where pgstat_create_backend_stat() is called.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: per backend I/O statistics
2024-11-25 07:12 Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-11-25 07:18 ` Re: per backend I/O statistics[ Michael Paquier <[email protected]>
2024-11-25 15:47 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
@ 2024-12-12 04:52 ` Michael Paquier <[email protected]>
2024-12-12 14:02 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
1 sibling, 1 reply; 12+ messages in thread
From: Michael Paquier @ 2024-12-12 04:52 UTC (permalink / raw)
To: Bertrand Drouvot <[email protected]>; +Cc: Nazir Bilal Yavuz <[email protected]>; Alvaro Herrera <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On Mon, Nov 25, 2024 at 03:47:59PM +0000, Bertrand Drouvot wrote:
> === Remarks
>
> R1: as compared to v5, v6 removes the per-backend I/O stats reset from
> pg_stat_reset_shared(). I think it makes more sense that way, since we are
> adding pg_stat_reset_single_backend_io_counters(). The per-backend I/O stats
> behaves then as the subscription stats as far the reset is concerned.
Makes sense to me to not include that in pg_stat_reset_shared().
> R2: as we can't merge the flush cb anymore, only the patches related to
> the stats_fetch_consistency/'snapshot' are missing in v6 (as compared to v5).
> I propose to re-submit them, re-start the discussion once 0001 goes in.
Yeah, thanks. I should think more about this part, but I'm still kind
of unconvinced. Let's do things step by step. For now, I have looked
at v6.
+ view. The function does not return I/O statistics for the checkpointer,
+ the background writer, the startup process and the autovacuum launcher
+ as they are already visible in the <link linkend="monitoring-pg-stat-io-view"> <structname>pg_stat_io</structname></link>
+ view and there is only one of those.
This last sentence seems unnecessary? The function is named
"backend", and well, all these processes are not backends.
+ /*
+ * Maybe an auxiliary process? That should not be possible, due to
+ * pgstat_tracks_per_backend_bktype() though.
+ */
+ if (proc == NULL)
+ proc = AuxiliaryPidGetProc(backend_pid);
[...]
+ /*
+ * Maybe an auxiliary process? That should not be possible, due to
+ * pgstat_tracks_per_backend_bktype() though.
+ */
+ if (proc == NULL)
+ proc = AuxiliaryPidGetProc(pid);
This does not seem right. Shouldn't we return immediately if
BackendPidGetProc() finds nothing matching with the PID?
+ /* Look for the backend type */
+ for (curr_backend = 1; curr_backend <= num_backends; curr_backend++)
+ {
+ LocalPgBackendStatus *local_beentry;
+ PgBackendStatus *beentry;
+
+ /* Get the next one in the list */
+ local_beentry = pgstat_get_local_beentry_by_index(curr_backend);
+ beentry = &local_beentry->backendStatus;
+
+ /* looking for specific PID, ignore all the others */
+ if (beentry->st_procpid != pid)
+ continue;
+
+ bktype = beentry->st_backendType;
+ break;
+ }
Sounds to me that the backend type is not strictly required in this
function call if pg_stat_activity can tell already that?
+ (void) pgstat_per_backend_flush_cb(entry_ref, nowait);
I'd recommend to not directly call the callback, use a wrapper
function instead if need be.
pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op,
instr_time start_time, uint32 cnt)
{
+
if (track_io_timing)
Noise diff.
/*
- * Simpler wrapper of pgstat_io_flush_cb()
+ * Simpler wrapper of pgstat_io_flush_cb() and pgstat_per_backend_flush_cb().
*/
void
pgstat_flush_io(bool nowait)
This is also called in the checkpointer and the bgwriter and the
walwriter via pgstat_report_wal(), which is kind of useless. Perhaps
just use a different, separate function instead and use that where it
makes sense (per se also the argument of upthread that backend stats
may not be only IO-related..).
Sounds to me that PgStat_BackendPendingIO should be
PgStat_BackendPendingStats?
+{ oid => '8806', descr => 'statistics: per backend IO statistics',
+ proname => 'pg_stat_get_backend_io', prorows => '5', proretset => 't',
Similarly, s/pg_stat_get_backend_io/pg_stat_get_backend_stats/?
+ descr => 'statistics: reset collected IO statistics for a single backend',
+ proname => 'pg_stat_reset_single_backend_io_counters', provolatile => 'v',
And here, pg_stat_reset_backend_stats?
#define PGSTAT_KIND_SUBSCRIPTION 5 /* per-subscription statistics */
+#define PGSTAT_KIND_PER_BACKEND 6
Missing one comment here.
FWIW, I'm so-so about the addition of pg_my_stat_io, knowing that
pg_stat_get_backend_io(NULL/pg_backend_pid()) does the same job. I
would just add a note in the docs with a query showing how to use it
with pg_stat_activity. An example with LATERAL, doing the same work:
select a.pid, s.* from pg_stat_activity as a,
lateral pg_stat_get_backend_io(a.pid) as s
where pid = pg_backend_pid();
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: per backend I/O statistics
2024-11-25 07:12 Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-11-25 07:18 ` Re: per backend I/O statistics[ Michael Paquier <[email protected]>
2024-11-25 15:47 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-12-12 04:52 ` Re: per backend I/O statistics Michael Paquier <[email protected]>
@ 2024-12-12 14:02 ` Bertrand Drouvot <[email protected]>
2024-12-13 02:02 ` Re: per backend I/O statistics Michael Paquier <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Bertrand Drouvot @ 2024-12-12 14:02 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Nazir Bilal Yavuz <[email protected]>; Alvaro Herrera <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
Hi,
On Thu, Dec 12, 2024 at 01:52:03PM +0900, Michael Paquier wrote:
> On Mon, Nov 25, 2024 at 03:47:59PM +0000, Bertrand Drouvot wrote:
> + view. The function does not return I/O statistics for the checkpointer,
> + the background writer, the startup process and the autovacuum launcher
> + as they are already visible in the <link linkend="monitoring-pg-stat-io-view"> <structname>pg_stat_io</structname></link>
> + view and there is only one of those.
>
> This last sentence seems unnecessary? The function is named
> "backend", and well, all these processes are not backends.
Yeah, but you can find their pid through pg_stat_activity for which the pid field
description is "Process ID of this backend". Also we can find "backend_type" in
both pg_stat_activity and pg_stat_io, so I think this last sentence could help
to avoid any confusion though.
> + /*
> + * Maybe an auxiliary process? That should not be possible, due to
> + * pgstat_tracks_per_backend_bktype() though.
> + */
> + if (proc == NULL)
> + proc = AuxiliaryPidGetProc(backend_pid);
> [...]
> + /*
> + * Maybe an auxiliary process? That should not be possible, due to
> + * pgstat_tracks_per_backend_bktype() though.
> + */
> + if (proc == NULL)
> + proc = AuxiliaryPidGetProc(pid);
>
> This does not seem right. Shouldn't we return immediately if
> BackendPidGetProc() finds nothing matching with the PID?
Yeah, that would work. I was keeping the AuxiliaryPidGetProc() calls just in case
we want to add the Aux processes back in the future. Replaced with a comment
in v7 attached instead.
>
> + /* Look for the backend type */
> + for (curr_backend = 1; curr_backend <= num_backends; curr_backend++)
> + {
> + LocalPgBackendStatus *local_beentry;
> + PgBackendStatus *beentry;
> +
> + /* Get the next one in the list */
> + local_beentry = pgstat_get_local_beentry_by_index(curr_backend);
> + beentry = &local_beentry->backendStatus;
> +
> + /* looking for specific PID, ignore all the others */
> + if (beentry->st_procpid != pid)
> + continue;
> +
> + bktype = beentry->st_backendType;
> + break;
> + }
>
> Sounds to me that the backend type is not strictly required in this
> function call if pg_stat_activity can tell already that?
Yeah, but it's needed in pg_stat_get_backend_io() for the stats filtering (to
display only those linked to this backend type), later in the function here:
+ /*
+ * Some combinations of BackendType, IOObject, and IOContext are
+ * not valid for any type of IOOp. In such cases, omit the entire
+ * row from the view.
+ */
+ if (!pgstat_tracks_io_object(bktype, io_obj, io_context))
+ continue;
OTOH, now that we get rid of the AuxiliaryPidGetProc() call in
pg_stat_reset_single_backend_io_counters() we can also remove the need to
look for the backend type in this function.
>
> + (void) pgstat_per_backend_flush_cb(entry_ref, nowait);
>
> I'd recommend to not directly call the callback, use a wrapper
> function instead if need be.
Makes sense, created its own callback in pgstat_backend.c. Bonus point, it avoids
unnecessary pgstat_tracks_per_backend_bktype() checks in :
pgstat_report_bgwriter()
pgstat_report_checkpointer()
pgstat_report_wal()
as there is no attempt to call the callback anymore in those places.
> /*
> - * Simpler wrapper of pgstat_io_flush_cb()
> + * Simpler wrapper of pgstat_io_flush_cb() and pgstat_per_backend_flush_cb().
> */
> void
> pgstat_flush_io(bool nowait)
>
> This is also called in the checkpointer and the bgwriter and the
> walwriter via pgstat_report_wal(), which is kind of useless. Perhaps
> just use a different, separate function instead and use that where it
> makes sense (per se also the argument of upthread that backend stats
> may not be only IO-related..).
Yeah, done that way.
BTW, not related to this particular patch but I realized that pgstat_flush_io()
is called for the walwriter. Indeed, it's coming from the pgstat_report_wal()
call in WalWriterMain(). That can not report any I/O stats activity (as the
walwriter is not part of the I/O stats tracking, see pgstat_tracks_io_bktype()).
So it looks like, we could move pgstat_flush_io() outside of pgstat_report_wal()
and add the pgstat_flush_io() calls only where they need to be made (and so, not
in WalWriterMain()).
Maybe a dedicated thread is worth it for that, thoughts?
> Sounds to me that PgStat_BackendPendingIO should be
> PgStat_BackendPendingStats?
Yeah, can do that as that's what is being used for the pending_size for
example.
OTOH, it's clear that this one in pgstat_io.c has to be "linked" to an "IO"
related one:
"
typedef PgStat_BackendPendingStats PgStat_PendingIO;
"
The right way would probably be to do something like:
"
typedef struct PgStat_BackendPendingStats {
PgStat_BackendPendingIO pendingio;
} PgStat_BackendPendingStats;
"
But I'm not sure that's worth it until we don't have a need to add more
pending stats per-backend, thoughts?
> +{ oid => '8806', descr => 'statistics: per backend IO statistics',
> + proname => 'pg_stat_get_backend_io', prorows => '5', proretset => 't',
>
> Similarly, s/pg_stat_get_backend_io/pg_stat_get_backend_stats/?
I think that's fine to keep pg_stat_get_backend_io(). If we add more per-backend
stats in the future then we could add "dedicated" get functions too and a generic
one retrieving all of them.
> + descr => 'statistics: reset collected IO statistics for a single backend',
> + proname => 'pg_stat_reset_single_backend_io_counters', provolatile => 'v',
>
> And here, pg_stat_reset_backend_stats?
Same as above, we could imagine that in the future the backend would get mutiple
stats and that one would want to reset only the I/O ones for example.
> #define PGSTAT_KIND_SUBSCRIPTION 5 /* per-subscription statistics */
> +#define PGSTAT_KIND_PER_BACKEND 6
>
> Missing one comment here.
Yeap.
> FWIW, I'm so-so about the addition of pg_my_stat_io, knowing that
> pg_stat_get_backend_io(NULL/pg_backend_pid()) does the same job.
Okay, I don't have a strong opinion about that. Removed in v7.
> I
> would just add a note in the docs with a query showing how to use it
> with pg_stat_activity. An example with LATERAL, doing the same work:
> select a.pid, s.* from pg_stat_activity as a,
> lateral pg_stat_get_backend_io(a.pid) as s
> where pid = pg_backend_pid();
I'm not sure it's worth it. I think that's clear that to get our own stats
then we need to provide our own backend pid. For example pg_stat_get_activity()
does not provide such an example using pg_stat_activity or using something like
pg_stat_get_activity(pg_backend_pid()).
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v7-0001-per-backend-I-O-statistics.patch (37.0K, ../../Z1rs%[email protected]/2-v7-0001-per-backend-I-O-statistics.patch)
download | inline diff:
From 6662a78036e92035bd4899e4a92af42f3aaae944 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Mon, 28 Oct 2024 12:50:32 +0000
Subject: [PATCH v7] per backend I/O statistics
While pg_stat_io provides cluster-wide I/O statistics, this commit adds the
ability to track and display per backend I/O statistics.
It adds a new statistics kind and 2 new functions:
- pg_stat_reset_single_backend_io_counters() to be able to reset the I/O stats
for a given backend pid.
- pg_stat_get_backend_io() to retrieve I/O statistics for a given backend pid.
The new KIND is named PGSTAT_KIND_PER_BACKEND as it could be used in the future
to store other statistics (than the I/O ones) per backend. The new KIND is
a variable-numbered one and has an automatic cap on the maximum number of
entries (as its hash key contains the proc number).
There is no need to write the per backend I/O stats to disk (no point to
see stats for backends that do not exist anymore after a re-start), so using
"write_to_file = false".
Note that per backend I/O statistics are not collected for the checkpointer,
the background writer, the startup process and the autovacuum launcher as those
are already visible in pg_stat_io and there is only one of those.
XXX: Bump catalog version needs to be done.
---
doc/src/sgml/config.sgml | 8 +-
doc/src/sgml/monitoring.sgml | 37 ++++
src/backend/catalog/system_functions.sql | 2 +
src/backend/utils/activity/Makefile | 1 +
src/backend/utils/activity/backend_status.c | 4 +
src/backend/utils/activity/meson.build | 1 +
src/backend/utils/activity/pgstat.c | 19 +-
src/backend/utils/activity/pgstat_backend.c | 182 +++++++++++++++++++
src/backend/utils/activity/pgstat_io.c | 25 ++-
src/backend/utils/activity/pgstat_relation.c | 2 +
src/backend/utils/adt/pgstatfuncs.c | 166 +++++++++++++++++
src/include/catalog/pg_proc.dat | 14 ++
src/include/pgstat.h | 31 +++-
src/include/utils/pgstat_internal.h | 14 ++
src/test/regress/expected/stats.out | 72 +++++++-
src/test/regress/sql/stats.sql | 38 +++-
src/tools/pgindent/typedefs.list | 3 +
17 files changed, 598 insertions(+), 21 deletions(-)
10.0% doc/src/sgml/
30.9% src/backend/utils/activity/
21.8% src/backend/utils/adt/
4.6% src/include/catalog/
7.1% src/include/
13.0% src/test/regress/expected/
11.6% src/test/regress/sql/
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e0c8325a39..8afca9b110 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8403,9 +8403,11 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
displayed in <link linkend="monitoring-pg-stat-database-view">
<structname>pg_stat_database</structname></link>,
<link linkend="monitoring-pg-stat-io-view">
- <structname>pg_stat_io</structname></link>, in the output of
- <xref linkend="sql-explain"/> when the <literal>BUFFERS</literal> option
- is used, in the output of <xref linkend="sql-vacuum"/> when
+ <structname>pg_stat_io</structname></link>, in the output of the
+ <link linkend="pg-stat-get-backend-io">
+ <function>pg_stat_get_backend_io()</function></link> function, in the
+ output of <xref linkend="sql-explain"/> when the <literal>BUFFERS</literal>
+ option is used, in the output of <xref linkend="sql-vacuum"/> when
the <literal>VERBOSE</literal> option is used, by autovacuum
for auto-vacuums and auto-analyzes, when <xref
linkend="guc-log-autovacuum-min-duration"/> is set and by
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 840d7f8161..fb13c462d1 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -4790,6 +4790,25 @@ description | Waiting for a newly initialized WAL file to reach durable storage
</para></entry>
</row>
+ <row>
+ <entry id="pg-stat-get-backend-io" role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>pg_stat_get_backend_io</primary>
+ </indexterm>
+ <function>pg_stat_get_backend_io</function> ( <type>integer</type> )
+ <returnvalue>setof record</returnvalue>
+ </para>
+ <para>
+ Returns I/O statistics about the backend with the specified
+ process ID. The output fields are exactly the same as the ones in the
+ <link linkend="monitoring-pg-stat-io-view"> <structname>pg_stat_io</structname></link>
+ view. The function does not return I/O statistics for the checkpointer,
+ the background writer, the startup process and the autovacuum launcher
+ as they are already visible in the <link linkend="monitoring-pg-stat-io-view"> <structname>pg_stat_io</structname></link>
+ view and there is only one of those.
+ </para></entry>
+ </row>
+
<row>
<entry role="func_table_entry"><para role="func_signature">
<indexterm>
@@ -4971,6 +4990,24 @@ description | Waiting for a newly initialized WAL file to reach durable storage
</para></entry>
</row>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>pg_stat_reset_single_backend_io_counters</primary>
+ </indexterm>
+ <function>pg_stat_reset_single_backend_io_counters</function> ( <type>integer</type> )
+ <returnvalue>void</returnvalue>
+ </para>
+ <para>
+ Resets I/O statistics for a single backend with the specified process ID
+ to zero.
+ </para>
+ <para>
+ This function is restricted to superusers by default, but other users
+ can be granted EXECUTE to run the function.
+ </para></entry>
+ </row>
+
<row>
<entry role="func_table_entry"><para role="func_signature">
<indexterm>
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index c51dfca802..1b21b242cd 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -711,6 +711,8 @@ REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_table_counters(oid) FROM public;
REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_function_counters(oid) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_backend_io_counters(integer) FROM public;
+
REVOKE EXECUTE ON FUNCTION pg_stat_reset_replication_slot(text) FROM public;
REVOKE EXECUTE ON FUNCTION pg_stat_have_stats(text, oid, int8) FROM public;
diff --git a/src/backend/utils/activity/Makefile b/src/backend/utils/activity/Makefile
index b9fd66ea17..24b64a2742 100644
--- a/src/backend/utils/activity/Makefile
+++ b/src/backend/utils/activity/Makefile
@@ -20,6 +20,7 @@ OBJS = \
backend_status.o \
pgstat.o \
pgstat_archiver.o \
+ pgstat_backend.o \
pgstat_bgwriter.o \
pgstat_checkpointer.o \
pgstat_database.o \
diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c
index 22c6dc378c..29779ff99a 100644
--- a/src/backend/utils/activity/backend_status.c
+++ b/src/backend/utils/activity/backend_status.c
@@ -249,6 +249,10 @@ pgstat_beinit(void)
Assert(MyProcNumber >= 0 && MyProcNumber < NumBackendStatSlots);
MyBEEntry = &BackendStatusArray[MyProcNumber];
+ /* Create the per-backend statistics entry */
+ if (pgstat_tracks_per_backend_bktype(MyBackendType))
+ pgstat_create_backend_stat(MyProcNumber);
+
/* Set up a process-exit hook to clean up */
on_shmem_exit(pgstat_beshutdown_hook, 0);
}
diff --git a/src/backend/utils/activity/meson.build b/src/backend/utils/activity/meson.build
index f73c22905c..380d3dd70c 100644
--- a/src/backend/utils/activity/meson.build
+++ b/src/backend/utils/activity/meson.build
@@ -5,6 +5,7 @@ backend_sources += files(
'backend_status.c',
'pgstat.c',
'pgstat_archiver.c',
+ 'pgstat_backend.c',
'pgstat_bgwriter.c',
'pgstat_checkpointer.c',
'pgstat_database.c',
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 18b7d9b47d..738ee778bb 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -77,6 +77,7 @@
*
* Each statistics kind is handled in a dedicated file:
* - pgstat_archiver.c
+ * - pgstat_backend.c
* - pgstat_bgwriter.c
* - pgstat_checkpointer.c
* - pgstat_database.c
@@ -358,6 +359,22 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
.reset_timestamp_cb = pgstat_subscription_reset_timestamp_cb,
},
+ [PGSTAT_KIND_PER_BACKEND] = {
+ .name = "per-backend",
+
+ .fixed_amount = false,
+ .write_to_file = false,
+
+ .accessed_across_databases = true,
+
+ .shared_size = sizeof(PgStatShared_Backend),
+ .shared_data_off = offsetof(PgStatShared_Backend, stats),
+ .shared_data_len = sizeof(((PgStatShared_Backend *) 0)->stats),
+ .pending_size = sizeof(PgStat_BackendPendingIO),
+
+ .flush_pending_cb = pgstat_per_backend_flush_cb,
+ .reset_timestamp_cb = pgstat_backend_reset_timestamp_cb,
+ },
/* stats for fixed-numbered (mostly 1) objects */
@@ -768,7 +785,7 @@ pgstat_report_stat(bool force)
partial_flush = false;
- /* flush database / relation / function / ... stats */
+ /* flush database / relation / function / backend / ... stats */
partial_flush |= pgstat_flush_pending_entries(nowait);
/* flush of fixed-numbered stats */
diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c
new file mode 100644
index 0000000000..dc7bf385c2
--- /dev/null
+++ b/src/backend/utils/activity/pgstat_backend.c
@@ -0,0 +1,182 @@
+/* -------------------------------------------------------------------------
+ *
+ * pgstat_backend.c
+ * Implementation of per-backend statistics.
+ *
+ * This file contains the implementation of per-backend statistics. It is kept
+ * separate from pgstat.c to enforce the line between the statistics access /
+ * storage implementation and the details about individual types of statistics.
+ *
+ * Copyright (c) 2024, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/backend/utils/activity/pgstat_backend.c
+ * -------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "utils/pgstat_internal.h"
+
+/*
+ * Returns backend's IO stats.
+ */
+PgStat_Backend *
+pgstat_fetch_proc_stat_io(ProcNumber procNumber)
+{
+ PgStat_Backend *backend_entry;
+
+ backend_entry = (PgStat_Backend *) pgstat_fetch_entry(PGSTAT_KIND_PER_BACKEND,
+ InvalidOid, procNumber);
+
+ return backend_entry;
+}
+
+/*
+ * Flush out locally pending backend statistics
+ *
+ * If no stats have been recorded, this function returns false.
+ */
+bool
+pgstat_per_backend_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
+{
+ PgStatShared_Backend *shbackendioent;
+ PgStat_BackendPendingIO *pendingent;
+ PgStat_BktypeIO *bktype_shstats;
+
+ if (!pgstat_lock_entry(entry_ref, nowait))
+ return false;
+
+ shbackendioent = (PgStatShared_Backend *) entry_ref->shared_stats;
+ bktype_shstats = &shbackendioent->stats.stats;
+ pendingent = (PgStat_BackendPendingIO *) entry_ref->pending;
+
+ for (int io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)
+ {
+ for (int io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++)
+ {
+ for (int io_op = 0; io_op < IOOP_NUM_TYPES; io_op++)
+ {
+ instr_time time;
+
+ bktype_shstats->counts[io_object][io_context][io_op] +=
+ pendingent->counts[io_object][io_context][io_op];
+
+ time = pendingent->pending_times[io_object][io_context][io_op];
+
+ bktype_shstats->times[io_object][io_context][io_op] +=
+ INSTR_TIME_GET_MICROSEC(time);
+ }
+ }
+ }
+
+ pgstat_unlock_entry(entry_ref);
+
+ return true;
+}
+
+/*
+ * Simpler wrapper of pgstat_per_backend_flush_cb()
+ */
+void
+pgstat_flush_per_backend(bool nowait)
+{
+ if (pgstat_tracks_per_backend_bktype(MyBackendType))
+ {
+ PgStat_EntryRef *entry_ref;
+
+ entry_ref = pgstat_get_entry_ref(PGSTAT_KIND_PER_BACKEND, InvalidOid,
+ MyProcNumber, false, NULL);
+ (void) pgstat_per_backend_flush_cb(entry_ref, nowait);
+ }
+}
+
+/*
+ * Create the per-backend statistics entry for procnum.
+ */
+void
+pgstat_create_backend_stat(ProcNumber procnum)
+{
+ PgStat_EntryRef *entry_ref;
+ PgStatShared_Backend *shstatent;
+
+ entry_ref = pgstat_prep_pending_entry(PGSTAT_KIND_PER_BACKEND, InvalidOid,
+ procnum, NULL);
+
+ shstatent = (PgStatShared_Backend *) entry_ref->shared_stats;
+
+ /*
+ * NB: need to accept that there might be stats from an older backend,
+ * e.g. if we previously used this proc number.
+ */
+ memset(&shstatent->stats, 0, sizeof(shstatent->stats));
+}
+
+/*
+ * Find or create a local PgStat_BackendPendingIO entry for procnum.
+ */
+PgStat_BackendPendingIO *
+pgstat_prep_per_backend_pending(ProcNumber procnum)
+{
+ PgStat_EntryRef *entry_ref;
+
+ entry_ref = pgstat_prep_pending_entry(PGSTAT_KIND_PER_BACKEND, InvalidOid,
+ procnum, NULL);
+
+ return entry_ref->pending;
+}
+
+/*
+ * per-backend statistics are not collected for all BackendTypes.
+ *
+ * The following BackendTypes do not participate in the per-backend stats
+ * subsystem:
+ * - The same and for the same reasons as in pgstat_tracks_io_bktype().
+ * - B_BG_WRITER, B_CHECKPOINTER, B_STARTUP and B_AUTOVAC_LAUNCHER because their
+ * I/O stats are already visible in pg_stat_io and there is only one of those.
+ *
+ * Function returns true if BackendType participates in the per-backend stats
+ * subsystem for IO and false if it does not.
+ *
+ * When adding a new BackendType, also consider adding relevant restrictions to
+ * pgstat_tracks_io_object() and pgstat_tracks_io_op().
+ */
+bool
+pgstat_tracks_per_backend_bktype(BackendType bktype)
+{
+ /*
+ * List every type so that new backend types trigger a warning about
+ * needing to adjust this switch.
+ */
+ switch (bktype)
+ {
+ case B_INVALID:
+ case B_AUTOVAC_LAUNCHER:
+ case B_DEAD_END_BACKEND:
+ case B_ARCHIVER:
+ case B_LOGGER:
+ case B_WAL_RECEIVER:
+ case B_WAL_WRITER:
+ case B_WAL_SUMMARIZER:
+ case B_BG_WRITER:
+ case B_CHECKPOINTER:
+ case B_STARTUP:
+ return false;
+
+ case B_AUTOVAC_WORKER:
+ case B_BACKEND:
+ case B_BG_WORKER:
+ case B_STANDALONE_BACKEND:
+ case B_SLOTSYNC_WORKER:
+ case B_WAL_SENDER:
+ return true;
+ }
+
+ return false;
+}
+
+void
+pgstat_backend_reset_timestamp_cb(PgStatShared_Common *header, TimestampTz ts)
+{
+ ((PgStatShared_Backend *) header)->stats.stat_reset_timestamp = ts;
+}
diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c
index f9883af2b3..93d34696b9 100644
--- a/src/backend/utils/activity/pgstat_io.c
+++ b/src/backend/utils/activity/pgstat_io.c
@@ -20,13 +20,7 @@
#include "storage/bufmgr.h"
#include "utils/pgstat_internal.h"
-
-typedef struct PgStat_PendingIO
-{
- PgStat_Counter counts[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
- instr_time pending_times[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
-} PgStat_PendingIO;
-
+typedef PgStat_BackendPendingIO PgStat_PendingIO;
static PgStat_PendingIO PendingIOStats;
static bool have_iostats = false;
@@ -87,6 +81,14 @@ pgstat_count_io_op_n(IOObject io_object, IOContext io_context, IOOp io_op, uint3
Assert((unsigned int) io_op < IOOP_NUM_TYPES);
Assert(pgstat_tracks_io_op(MyBackendType, io_object, io_context, io_op));
+ if (pgstat_tracks_per_backend_bktype(MyBackendType))
+ {
+ PgStat_PendingIO *entry_ref;
+
+ entry_ref = pgstat_prep_per_backend_pending(MyProcNumber);
+ entry_ref->counts[io_object][io_context][io_op] += cnt;
+ }
+
PendingIOStats.counts[io_object][io_context][io_op] += cnt;
have_iostats = true;
@@ -148,6 +150,15 @@ pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op,
INSTR_TIME_ADD(PendingIOStats.pending_times[io_object][io_context][io_op],
io_time);
+
+ if (pgstat_tracks_per_backend_bktype(MyBackendType))
+ {
+ PgStat_PendingIO *entry_ref;
+
+ entry_ref = pgstat_prep_per_backend_pending(MyProcNumber);
+ INSTR_TIME_ADD(entry_ref->pending_times[io_object][io_context][io_op],
+ io_time);
+ }
}
pgstat_count_io_op_n(io_object, io_context, io_op, cnt);
diff --git a/src/backend/utils/activity/pgstat_relation.c b/src/backend/utils/activity/pgstat_relation.c
index faba8b64d2..c083c5bf01 100644
--- a/src/backend/utils/activity/pgstat_relation.c
+++ b/src/backend/utils/activity/pgstat_relation.c
@@ -264,6 +264,7 @@ pgstat_report_vacuum(Oid tableoid, bool shared,
* VACUUM command has processed all tables and committed.
*/
pgstat_flush_io(false);
+ pgstat_flush_per_backend(false);
}
/*
@@ -350,6 +351,7 @@ pgstat_report_analyze(Relation rel,
/* see pgstat_report_vacuum() */
pgstat_flush_io(false);
+ pgstat_flush_per_backend(false);
}
/*
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index cdf37403e9..ce4d22e16c 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1474,6 +1474,151 @@ pg_stat_get_io(PG_FUNCTION_ARGS)
return (Datum) 0;
}
+Datum
+pg_stat_get_backend_io(PG_FUNCTION_ARGS)
+{
+ ReturnSetInfo *rsinfo;
+ PgStat_Backend *backend_stats;
+ Datum bktype_desc;
+ PgStat_BktypeIO *bktype_stats;
+ BackendType bktype;
+ Datum reset_time;
+ int num_backends = pgstat_fetch_stat_numbackends();
+ int curr_backend;
+ int pid;
+ PGPROC *proc;
+ ProcNumber procNumber;
+
+ InitMaterializedSRF(fcinfo, 0);
+ rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+
+ pid = PG_GETARG_INT32(0);
+ proc = BackendPidGetProc(pid);
+
+ /*
+ * Could be an auxiliary process but would not report any stats due to
+ * pgstat_tracks_per_backend_bktype() anyway. So don't need an extra call
+ * to AuxiliaryPidGetProc().
+ */
+ if (!proc)
+ return (Datum) 0;
+
+ procNumber = GetNumberFromPGProc(proc);
+ backend_stats = pgstat_fetch_proc_stat_io(procNumber);
+
+ if (!backend_stats)
+ return (Datum) 0;
+
+ /* just to keep compiler quiet */
+ bktype = B_INVALID;
+
+ /* Look for the backend type */
+ for (curr_backend = 1; curr_backend <= num_backends; curr_backend++)
+ {
+ LocalPgBackendStatus *local_beentry;
+ PgBackendStatus *beentry;
+
+ /* Get the next one in the list */
+ local_beentry = pgstat_get_local_beentry_by_index(curr_backend);
+ beentry = &local_beentry->backendStatus;
+
+ /* looking for specific PID, ignore all the others */
+ if (beentry->st_procpid != pid)
+ continue;
+
+ bktype = beentry->st_backendType;
+ break;
+ }
+
+ bktype_desc = CStringGetTextDatum(GetBackendTypeDesc(bktype));
+ bktype_stats = &backend_stats->stats;
+ reset_time = TimestampTzGetDatum(backend_stats->stat_reset_timestamp);
+
+ /*
+ * In Assert builds, we can afford an extra loop through all of the
+ * counters checking that only expected stats are non-zero, since it keeps
+ * the non-Assert code cleaner.
+ */
+ Assert(pgstat_bktype_io_stats_valid(bktype_stats, bktype));
+
+ for (int io_obj = 0; io_obj < IOOBJECT_NUM_TYPES; io_obj++)
+ {
+ const char *obj_name = pgstat_get_io_object_name(io_obj);
+
+ for (int io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++)
+ {
+ const char *context_name = pgstat_get_io_context_name(io_context);
+
+ Datum values[IO_NUM_COLUMNS] = {0};
+ bool nulls[IO_NUM_COLUMNS] = {0};
+
+ /*
+ * Some combinations of BackendType, IOObject, and IOContext are
+ * not valid for any type of IOOp. In such cases, omit the entire
+ * row from the view.
+ */
+ if (!pgstat_tracks_io_object(bktype, io_obj, io_context))
+ continue;
+
+ values[IO_COL_BACKEND_TYPE] = bktype_desc;
+ values[IO_COL_CONTEXT] = CStringGetTextDatum(context_name);
+ values[IO_COL_OBJECT] = CStringGetTextDatum(obj_name);
+ if (backend_stats->stat_reset_timestamp != 0)
+ values[IO_COL_RESET_TIME] = reset_time;
+ else
+ nulls[IO_COL_RESET_TIME] = true;
+
+ /*
+ * Hard-code this to the value of BLCKSZ for now. Future values
+ * could include XLOG_BLCKSZ, once WAL IO is tracked, and constant
+ * multipliers, once non-block-oriented IO (e.g. temporary file
+ * IO) is tracked.
+ */
+ values[IO_COL_CONVERSION] = Int64GetDatum(BLCKSZ);
+
+ for (int io_op = 0; io_op < IOOP_NUM_TYPES; io_op++)
+ {
+ int op_idx = pgstat_get_io_op_index(io_op);
+ int time_idx = pgstat_get_io_time_index(io_op);
+
+ /*
+ * Some combinations of BackendType and IOOp, of IOContext and
+ * IOOp, and of IOObject and IOOp are not tracked. Set these
+ * cells in the view NULL.
+ */
+ if (pgstat_tracks_io_op(bktype, io_obj, io_context, io_op))
+ {
+ PgStat_Counter count =
+ bktype_stats->counts[io_obj][io_context][io_op];
+
+ values[op_idx] = Int64GetDatum(count);
+ }
+ else
+ nulls[op_idx] = true;
+
+ /* not every operation is timed */
+ if (time_idx == IO_COL_INVALID)
+ continue;
+
+ if (!nulls[op_idx])
+ {
+ PgStat_Counter time =
+ bktype_stats->times[io_obj][io_context][io_op];
+
+ values[time_idx] = Float8GetDatum(pg_stat_us_to_ms(time));
+ }
+ else
+ nulls[time_idx] = true;
+ }
+
+ tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc,
+ values, nulls);
+ }
+ }
+
+ return (Datum) 0;
+}
+
/*
* Returns statistics of WAL activity
*/
@@ -1779,6 +1924,27 @@ pg_stat_reset_single_function_counters(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+Datum
+pg_stat_reset_single_backend_io_counters(PG_FUNCTION_ARGS)
+{
+ PGPROC *proc;
+ int backend_pid = PG_GETARG_INT32(0);
+
+ proc = BackendPidGetProc(backend_pid);
+
+ /*
+ * Could be an auxiliary process but would not report any stats due to
+ * pgstat_tracks_per_backend_bktype() anyway. So don't need an extra call
+ * to AuxiliaryPidGetProc().
+ */
+ if (!proc)
+ PG_RETURN_VOID();
+
+ pgstat_reset(PGSTAT_KIND_PER_BACKEND, InvalidOid, GetNumberFromPGProc(proc));
+
+ PG_RETURN_VOID();
+}
+
/* Reset SLRU counters (a specific one or all of them). */
Datum
pg_stat_reset_slru(PG_FUNCTION_ARGS)
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 0f22c21723..55dec76795 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5913,6 +5913,15 @@
proargnames => '{backend_type,object,context,reads,read_time,writes,write_time,writebacks,writeback_time,extends,extend_time,op_bytes,hits,evictions,reuses,fsyncs,fsync_time,stats_reset}',
prosrc => 'pg_stat_get_io' },
+{ oid => '8806', descr => 'statistics: per backend IO statistics',
+ proname => 'pg_stat_get_backend_io', prorows => '5', proretset => 't',
+ provolatile => 'v', proparallel => 'r', prorettype => 'record',
+ proargtypes => 'int4',
+ proallargtypes => '{int4,text,text,text,int8,float8,int8,float8,int8,float8,int8,float8,int8,int8,int8,int8,int8,float8,timestamptz}',
+ proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}',
+ proargnames => '{backend_pid,backend_type,object,context,reads,read_time,writes,write_time,writebacks,writeback_time,extends,extend_time,op_bytes,hits,evictions,reuses,fsyncs,fsync_time,stats_reset}',
+ prosrc => 'pg_stat_get_backend_io' },
+
{ oid => '1136', descr => 'statistics: information about WAL activity',
proname => 'pg_stat_get_wal', proisstrict => 'f', provolatile => 's',
proparallel => 'r', prorettype => 'record', proargtypes => '',
@@ -6052,6 +6061,11 @@
proname => 'pg_stat_reset_single_function_counters', provolatile => 'v',
prorettype => 'void', proargtypes => 'oid',
prosrc => 'pg_stat_reset_single_function_counters' },
+{ oid => '9987',
+ descr => 'statistics: reset collected IO statistics for a single backend',
+ proname => 'pg_stat_reset_single_backend_io_counters', provolatile => 'v',
+ prorettype => 'void', proargtypes => 'int4',
+ prosrc => 'pg_stat_reset_single_backend_io_counters' },
{ oid => '2307',
descr => 'statistics: reset collected statistics for a single SLRU',
proname => 'pg_stat_reset_slru', proisstrict => 'f', provolatile => 'v',
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 59c28b4aca..b7e8dcb1fd 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -49,14 +49,15 @@
#define PGSTAT_KIND_FUNCTION 3 /* per-function statistics */
#define PGSTAT_KIND_REPLSLOT 4 /* per-slot statistics */
#define PGSTAT_KIND_SUBSCRIPTION 5 /* per-subscription statistics */
+#define PGSTAT_KIND_PER_BACKEND 6 /* per-backend statistics */
/* stats for fixed-numbered objects */
-#define PGSTAT_KIND_ARCHIVER 6
-#define PGSTAT_KIND_BGWRITER 7
-#define PGSTAT_KIND_CHECKPOINTER 8
-#define PGSTAT_KIND_IO 9
-#define PGSTAT_KIND_SLRU 10
-#define PGSTAT_KIND_WAL 11
+#define PGSTAT_KIND_ARCHIVER 7
+#define PGSTAT_KIND_BGWRITER 8
+#define PGSTAT_KIND_CHECKPOINTER 9
+#define PGSTAT_KIND_IO 10
+#define PGSTAT_KIND_SLRU 11
+#define PGSTAT_KIND_WAL 12
#define PGSTAT_KIND_BUILTIN_MIN PGSTAT_KIND_DATABASE
#define PGSTAT_KIND_BUILTIN_MAX PGSTAT_KIND_WAL
@@ -347,12 +348,23 @@ typedef struct PgStat_BktypeIO
PgStat_Counter times[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
} PgStat_BktypeIO;
+typedef struct PgStat_BackendPendingIO
+{
+ PgStat_Counter counts[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
+ instr_time pending_times[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
+} PgStat_BackendPendingIO;
+
typedef struct PgStat_IO
{
TimestampTz stat_reset_timestamp;
PgStat_BktypeIO stats[BACKEND_NUM_TYPES];
} PgStat_IO;
+typedef struct PgStat_Backend
+{
+ TimestampTz stat_reset_timestamp;
+ PgStat_BktypeIO stats;
+} PgStat_Backend;
typedef struct PgStat_StatDBEntry
{
@@ -534,6 +546,13 @@ extern bool pgstat_have_entry(PgStat_Kind kind, Oid dboid, uint64 objid);
extern void pgstat_report_archiver(const char *xlog, bool failed);
extern PgStat_ArchiverStats *pgstat_fetch_stat_archiver(void);
+/*
+ * Functions in pgstat_backend.c
+ */
+
+extern PgStat_Backend *pgstat_fetch_proc_stat_io(ProcNumber procNumber);
+extern bool pgstat_tracks_per_backend_bktype(BackendType bktype);
+extern void pgstat_create_backend_stat(ProcNumber procnum);
/*
* Functions in pgstat_bgwriter.c
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index 7338bc1e28..141e16d1d3 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -450,6 +450,11 @@ typedef struct PgStatShared_ReplSlot
PgStat_StatReplSlotEntry stats;
} PgStatShared_ReplSlot;
+typedef struct PgStatShared_Backend
+{
+ PgStatShared_Common header;
+ PgStat_Backend stats;
+} PgStatShared_Backend;
/*
* Central shared memory entry for the cumulative stats system.
@@ -604,6 +609,15 @@ extern void pgstat_archiver_init_shmem_cb(void *stats);
extern void pgstat_archiver_reset_all_cb(TimestampTz ts);
extern void pgstat_archiver_snapshot_cb(void);
+/*
+ * Functions in pgstat_backend.c
+ */
+
+extern void pgstat_flush_per_backend(bool nowait);
+
+extern PgStat_BackendPendingIO *pgstat_prep_per_backend_pending(ProcNumber procnum);
+extern bool pgstat_per_backend_flush_cb(PgStat_EntryRef *entry_ref, bool nowait);
+extern void pgstat_backend_reset_timestamp_cb(PgStatShared_Common *header, TimestampTz ts);
/*
* Functions in pgstat_bgwriter.c
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 56771f83ed..ba8c3cfc88 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -1249,7 +1249,7 @@ SELECT pg_stat_get_subscription_stats(NULL);
(1 row)
--- Test that the following operations are tracked in pg_stat_io:
+-- Test that the following operations are tracked in pg_stat_io and in per-backend stats:
-- - reads of target blocks into shared buffers
-- - writes of shared buffers to permanent storage
-- - extends of relations using shared buffers
@@ -1259,11 +1259,19 @@ SELECT pg_stat_get_subscription_stats(NULL);
-- be sure of the state of shared buffers at the point the test is run.
-- Create a regular table and insert some data to generate IOCONTEXT_NORMAL
-- extends.
+SELECT pid AS checkpointer_pid FROM pg_stat_activity
+ WHERE backend_type = 'checkpointer' \gset
SELECT sum(extends) AS io_sum_shared_before_extends
FROM pg_stat_io WHERE context = 'normal' AND object = 'relation' \gset
+SELECT sum(extends) AS my_io_sum_shared_before_extends
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE context = 'normal' AND object = 'relation' \gset
SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
FROM pg_stat_io
WHERE object = 'relation' \gset io_sum_shared_before_
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE object = 'relation' \gset my_io_sum_shared_before_
CREATE TABLE test_io_shared(a int);
INSERT INTO test_io_shared SELECT i FROM generate_series(1,100)i;
SELECT pg_stat_force_next_flush();
@@ -1280,8 +1288,17 @@ SELECT :io_sum_shared_after_extends > :io_sum_shared_before_extends;
t
(1 row)
+SELECT sum(extends) AS my_io_sum_shared_after_extends
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE context = 'normal' AND object = 'relation' \gset
+SELECT :my_io_sum_shared_after_extends > :my_io_sum_shared_before_extends;
+ ?column?
+----------
+ t
+(1 row)
+
-- After a checkpoint, there should be some additional IOCONTEXT_NORMAL writes
--- and fsyncs.
+-- and fsyncs in the global stats (not for the backend).
-- See comment above for rationale for two explicit CHECKPOINTs.
CHECKPOINT;
CHECKPOINT;
@@ -1301,6 +1318,31 @@ SELECT current_setting('fsync') = 'off'
t
(1 row)
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE object = 'relation' \gset my_io_sum_shared_after_
+SELECT :my_io_sum_shared_after_writes >= :my_io_sum_shared_before_writes;
+ ?column?
+----------
+ t
+(1 row)
+
+SELECT current_setting('fsync') = 'off'
+ OR (:my_io_sum_shared_after_fsyncs = :my_io_sum_shared_before_fsyncs
+ AND :my_io_sum_shared_after_fsyncs= 0);
+ ?column?
+----------
+ t
+(1 row)
+
+-- Don't return any rows if querying other backend's stats that are excluded
+-- from the per-backend stats collection (like the checkpointer).
+SELECT count(1) = 0 FROM pg_stat_get_backend_io(:checkpointer_pid);
+ ?column?
+----------
+ t
+(1 row)
+
-- Change the tablespace so that the table is rewritten directly, then SELECT
-- from it to cause it to be read back into shared buffers.
SELECT sum(reads) AS io_sum_shared_before_reads
@@ -1521,6 +1563,8 @@ SELECT pg_stat_have_stats('io', 0, 0);
SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS io_stats_pre_reset
FROM pg_stat_io \gset
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_pre_reset
+ FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
SELECT pg_stat_reset_shared('io');
pg_stat_reset_shared
----------------------
@@ -1535,6 +1579,30 @@ SELECT :io_stats_post_reset < :io_stats_pre_reset;
t
(1 row)
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_post_reset
+ FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
+-- pg_stat_reset_shared() did not reset backend IO stats
+SELECT :my_io_stats_pre_reset <= :my_io_stats_post_reset;
+ ?column?
+----------
+ t
+(1 row)
+
+-- but pg_stat_reset_single_backend_io_counters() does
+SELECT pg_stat_reset_single_backend_io_counters(pg_backend_pid());
+ pg_stat_reset_single_backend_io_counters
+------------------------------------------
+
+(1 row)
+
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_post_backend_reset
+ FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
+SELECT :my_io_stats_pre_reset > :my_io_stats_post_backend_reset;
+ ?column?
+----------
+ t
+(1 row)
+
-- test BRIN index doesn't block HOT update
CREATE TABLE brin_hot (
id integer PRIMARY KEY,
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index 7147cc2f89..f17799402f 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -595,7 +595,7 @@ SELECT pg_stat_get_replication_slot(NULL);
SELECT pg_stat_get_subscription_stats(NULL);
--- Test that the following operations are tracked in pg_stat_io:
+-- Test that the following operations are tracked in pg_stat_io and in per-backend stats:
-- - reads of target blocks into shared buffers
-- - writes of shared buffers to permanent storage
-- - extends of relations using shared buffers
@@ -607,20 +607,32 @@ SELECT pg_stat_get_subscription_stats(NULL);
-- Create a regular table and insert some data to generate IOCONTEXT_NORMAL
-- extends.
+SELECT pid AS checkpointer_pid FROM pg_stat_activity
+ WHERE backend_type = 'checkpointer' \gset
SELECT sum(extends) AS io_sum_shared_before_extends
FROM pg_stat_io WHERE context = 'normal' AND object = 'relation' \gset
+SELECT sum(extends) AS my_io_sum_shared_before_extends
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE context = 'normal' AND object = 'relation' \gset
SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
FROM pg_stat_io
WHERE object = 'relation' \gset io_sum_shared_before_
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE object = 'relation' \gset my_io_sum_shared_before_
CREATE TABLE test_io_shared(a int);
INSERT INTO test_io_shared SELECT i FROM generate_series(1,100)i;
SELECT pg_stat_force_next_flush();
SELECT sum(extends) AS io_sum_shared_after_extends
FROM pg_stat_io WHERE context = 'normal' AND object = 'relation' \gset
SELECT :io_sum_shared_after_extends > :io_sum_shared_before_extends;
+SELECT sum(extends) AS my_io_sum_shared_after_extends
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE context = 'normal' AND object = 'relation' \gset
+SELECT :my_io_sum_shared_after_extends > :my_io_sum_shared_before_extends;
-- After a checkpoint, there should be some additional IOCONTEXT_NORMAL writes
--- and fsyncs.
+-- and fsyncs in the global stats (not for the backend).
-- See comment above for rationale for two explicit CHECKPOINTs.
CHECKPOINT;
CHECKPOINT;
@@ -630,6 +642,17 @@ SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
SELECT :io_sum_shared_after_writes > :io_sum_shared_before_writes;
SELECT current_setting('fsync') = 'off'
OR :io_sum_shared_after_fsyncs > :io_sum_shared_before_fsyncs;
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE object = 'relation' \gset my_io_sum_shared_after_
+SELECT :my_io_sum_shared_after_writes >= :my_io_sum_shared_before_writes;
+SELECT current_setting('fsync') = 'off'
+ OR (:my_io_sum_shared_after_fsyncs = :my_io_sum_shared_before_fsyncs
+ AND :my_io_sum_shared_after_fsyncs= 0);
+
+-- Don't return any rows if querying other backend's stats that are excluded
+-- from the per-backend stats collection (like the checkpointer).
+SELECT count(1) = 0 FROM pg_stat_get_backend_io(:checkpointer_pid);
-- Change the tablespace so that the table is rewritten directly, then SELECT
-- from it to cause it to be read back into shared buffers.
@@ -762,10 +785,21 @@ SELECT :io_sum_bulkwrite_strategy_extends_after > :io_sum_bulkwrite_strategy_ext
SELECT pg_stat_have_stats('io', 0, 0);
SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS io_stats_pre_reset
FROM pg_stat_io \gset
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_pre_reset
+ FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
SELECT pg_stat_reset_shared('io');
SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS io_stats_post_reset
FROM pg_stat_io \gset
SELECT :io_stats_post_reset < :io_stats_pre_reset;
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_post_reset
+ FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
+-- pg_stat_reset_shared() did not reset backend IO stats
+SELECT :my_io_stats_pre_reset <= :my_io_stats_post_reset;
+-- but pg_stat_reset_single_backend_io_counters() does
+SELECT pg_stat_reset_single_backend_io_counters(pg_backend_pid());
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_post_backend_reset
+ FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
+SELECT :my_io_stats_pre_reset > :my_io_stats_post_backend_reset;
-- test BRIN index doesn't block HOT update
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index ce33e55bf1..398dd92527 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2121,6 +2121,7 @@ PgFdwSamplingMethod
PgFdwScanState
PgIfAddrCallback
PgStatShared_Archiver
+PgStatShared_Backend
PgStatShared_BgWriter
PgStatShared_Checkpointer
PgStatShared_Common
@@ -2136,6 +2137,8 @@ PgStatShared_SLRU
PgStatShared_Subscription
PgStatShared_Wal
PgStat_ArchiverStats
+PgStat_Backend
+PgStat_BackendPendingIO
PgStat_BackendSubEntry
PgStat_BgWriterStats
PgStat_BktypeIO
--
2.34.1
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: per backend I/O statistics
2024-11-25 07:12 Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-11-25 07:18 ` Re: per backend I/O statistics[ Michael Paquier <[email protected]>
2024-11-25 15:47 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-12-12 04:52 ` Re: per backend I/O statistics Michael Paquier <[email protected]>
2024-12-12 14:02 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
@ 2024-12-13 02:02 ` Michael Paquier <[email protected]>
2024-12-13 09:20 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Michael Paquier @ 2024-12-13 02:02 UTC (permalink / raw)
To: Bertrand Drouvot <[email protected]>; +Cc: Nazir Bilal Yavuz <[email protected]>; Alvaro Herrera <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On Thu, Dec 12, 2024 at 02:02:38PM +0000, Bertrand Drouvot wrote:
> On Thu, Dec 12, 2024 at 01:52:03PM +0900, Michael Paquier wrote:
> Yeah, but it's needed in pg_stat_get_backend_io() for the stats filtering (to
> display only those linked to this backend type), later in the function here:
>
> + /*
> + * Some combinations of BackendType, IOObject, and IOContext are
> + * not valid for any type of IOOp. In such cases, omit the entire
> + * row from the view.
> + */
> + if (!pgstat_tracks_io_object(bktype, io_obj, io_context))
> + continue;
>
> OTOH, now that we get rid of the AuxiliaryPidGetProc() call in
> pg_stat_reset_single_backend_io_counters() we can also remove the need to
> look for the backend type in this function.
Ah, you need that for the pgstat_tracks_io_object() filtering. I'm
wondering if we should think about making that cheaper at some point.
As a O(N^2) when coupling a call of pg_stat_get_backend_io() with a
scan of pg_stat_activity, perhaps it does not matter much anyway..
Anyway, isn't it possible that this lookup loop finishes by finding
nothing depending on concurrent updates of other beentries? It sounds
to me that this warrants an early exit in the function.
> BTW, not related to this particular patch but I realized that pgstat_flush_io()
> is called for the walwriter. Indeed, it's coming from the pgstat_report_wal()
> call in WalWriterMain(). That can not report any I/O stats activity (as the
> walwriter is not part of the I/O stats tracking, see pgstat_tracks_io_bktype()).
>
> So it looks like, we could move pgstat_flush_io() outside of pgstat_report_wal()
> and add the pgstat_flush_io() calls only where they need to be made (and so, not
> in WalWriterMain()).
Perhaps, yes. pgstat_tracks_io_bktype() has always been discarded
walwriters since pgstat_io.c exists.
> But I'm not sure that's worth it until we don't have a need to add more
> pending stats per-backend, thoughts?
Okay with your argument here.
>> +{ oid => '8806', descr => 'statistics: per backend IO statistics',
>> + proname => 'pg_stat_get_backend_io', prorows => '5', proretset => 't',
>>
>> Similarly, s/pg_stat_get_backend_io/pg_stat_get_backend_stats/?
>
> I think that's fine to keep pg_stat_get_backend_io(). If we add more per-backend
> stats in the future then we could add "dedicated" get functions too and a generic
> one retrieving all of them.
Okay about this layer, discarding my remark. You have a point about
that: other stats associated to a single backend may be OK if not
returned as a SRF, and they'll most likely return a different set of
attributes.
>> + descr => 'statistics: reset collected IO statistics for a single backend',
>> + proname => 'pg_stat_reset_single_backend_io_counters', provolatile => 'v',
>>
>> And here, pg_stat_reset_backend_stats?
>
> Same as above, we could imagine that in the future the backend would get mutiple
> stats and that one would want to reset only the I/O ones for example.
Disagreed about this part. It is slightly simpler to do a full reset
of the stats in a single entry. If another subset of stats is added
to the backend-level entries, we could always introduce a new function
that has more control over what subset of a single backend entry is
reset. And I'm pretty sure that we are going to need the function
that does the full reset anyway.
>> I
>> would just add a note in the docs with a query showing how to use it
>> with pg_stat_activity. An example with LATERAL, doing the same work:
>> select a.pid, s.* from pg_stat_activity as a,
>> lateral pg_stat_get_backend_io(a.pid) as s
>> where pid = pg_backend_pid();
>
> I'm not sure it's worth it. I think that's clear that to get our own stats
> then we need to provide our own backend pid. For example pg_stat_get_activity()
> does not provide such an example using pg_stat_activity or using something like
> pg_stat_get_activity(pg_backend_pid()).
Okay.
As far as I can see, the patch relies entirely on write_to_file to
prevent any entries to be flushed out. It means that we leave in the
dshash entries that may sit idle for as long as the server is up once
a pgproc slot is used at least once. This scales depending on
max_connections. It also means that we skip the sanity check about
dropped entries at shutdown, which may be a good thing to do because
we don't need to loop through them when writing the stats file. Hmm.
Could it be better to be more aggressive with the handling of these
stats, marking them as dropped when their backend exists and cleanup
the dshash, without relying on the write flag to make sure that all
the entries are discarded at shutdown? The point is that we do
shutdown in a controlled manner, with all backends exiting before the
checkpointer writes the stats file after the shutdown checkpoint is
completed. The patch handles things so as entries are reset when a
procnum is reused, leaving past stats around until that happens. We
should perhaps aim for more consistency with the way beentry is
refreshed and be more proactive with the backend entry drop or reset
at backend shutdown (pgstat_beshutdown_hook?), so as what is in the
dshash reflects exactly what's in shared memory for each PGPROC and
beentry.
Not sure that the "_per_" added in the various references of the patch
are good to keep, like pgstat_tracks_per_backend_bktype. These could
be removed, I guess, doing also a PGSTAT_KIND_PER_BACKEND =>
PGSTAT_KIND_BACKEND?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: per backend I/O statistics
2024-11-25 07:12 Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-11-25 07:18 ` Re: per backend I/O statistics[ Michael Paquier <[email protected]>
2024-11-25 15:47 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-12-12 04:52 ` Re: per backend I/O statistics Michael Paquier <[email protected]>
2024-12-12 14:02 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-12-13 02:02 ` Re: per backend I/O statistics Michael Paquier <[email protected]>
@ 2024-12-13 09:20 ` Bertrand Drouvot <[email protected]>
2024-12-16 08:07 ` Re: per backend I/O statistics Michael Paquier <[email protected]>
0 siblings, 1 reply; 12+ messages in thread
From: Bertrand Drouvot @ 2024-12-13 09:20 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Nazir Bilal Yavuz <[email protected]>; Alvaro Herrera <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
Hi,
On Fri, Dec 13, 2024 at 11:02:53AM +0900, Michael Paquier wrote:
> On Thu, Dec 12, 2024 at 02:02:38PM +0000, Bertrand Drouvot wrote:
>
> Anyway, isn't it possible that this lookup loop finishes by finding
> nothing depending on concurrent updates of other beentries? It sounds
> to me that this warrants an early exit in the function.
Right, done that way in the attached.
> Perhaps, yes. pgstat_tracks_io_bktype() has always been discarded
> walwriters since pgstat_io.c exists.
Yeap. The comment on top of pgstat_tracks_io_bktype() says that it's not done
"for now". I think that we could update the code as proposed until it's done.
> >> + descr => 'statistics: reset collected IO statistics for a single backend',
> >> + proname => 'pg_stat_reset_single_backend_io_counters', provolatile => 'v',
> >>
> >> And here, pg_stat_reset_backend_stats?
> >
> > Same as above, we could imagine that in the future the backend would get mutiple
> > stats and that one would want to reset only the I/O ones for example.
>
> Disagreed about this part. It is slightly simpler to do a full reset
> of the stats in a single entry. If another subset of stats is added
> to the backend-level entries, we could always introduce a new function
> that has more control over what subset of a single backend entry is
> reset. And I'm pretty sure that we are going to need the function
> that does the full reset anyway.
Yeah, would have added it when a new stats subset would be added. It's fine
by me to have it now though, so done that way.
> As far as I can see, the patch relies entirely on write_to_file to
> prevent any entries to be flushed out.
Yes.
> It means that we leave in the
> dshash entries that may sit idle for as long as the server is up once
> a pgproc slot is used at least once. This scales depending on
> max_connections. It also means that we skip the sanity check about
> dropped entries at shutdown, which may be a good thing to do because
> we don't need to loop through them when writing the stats file.
Agree.
> Hmm.
> Could it be better to be more aggressive with the handling of these
> stats, marking them as dropped when their backend exists and cleanup
> the dshash, without relying on the write flag to make sure that all
> the entries are discarded at shutdown?
Yeah we can do it to be consistent with other stats kind, done.
> The point is that we do
> shutdown in a controlled manner, with all backends exiting before the
> checkpointer writes the stats file after the shutdown checkpoint is
> completed. The patch handles things so as entries are reset when a
> procnum is reused, leaving past stats around until that happens. We
> should perhaps aim for more consistency with the way beentry is
> refreshed and be more proactive with the backend entry drop or reset
> at backend shutdown (pgstat_beshutdown_hook?), so as what is in the
> dshash reflects exactly what's in shared memory for each PGPROC and
> beentry.
That can't be done in pgstat_beshutdown_hook because pgstat_shutdown_hook is called
before and so resets the pgStatLocal.shared_hash during pgstat_detach_shmem().
So, did it in pgstat_shutdown_hook instead.
> Not sure that the "_per_" added in the various references of the patch
> are good to keep, like pgstat_tracks_per_backend_bktype. These could
> be removed, I guess, doing also a PGSTAT_KIND_PER_BACKEND =>
> PGSTAT_KIND_BACKEND?
Yeah makes sense, that's consistent with other kinds: done.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[text/x-diff] v8-0001-per-backend-I-O-statistics.patch (36.9K, ../../[email protected]/2-v8-0001-per-backend-I-O-statistics.patch)
download | inline diff:
From 1b03df28ceea635a2687baee4f8d1b3a3c1ae728 Mon Sep 17 00:00:00 2001
From: Bertrand Drouvot <[email protected]>
Date: Mon, 28 Oct 2024 12:50:32 +0000
Subject: [PATCH v8] per backend I/O statistics
While pg_stat_io provides cluster-wide I/O statistics, this commit adds the
ability to track and display per backend I/O statistics.
It adds a new statistics kind and 2 new functions:
- pg_stat_reset_backend_stats() to be able to reset the stats for a given
backend pid.
- pg_stat_get_backend_io() to retrieve I/O statistics for a given backend pid.
The new KIND is named PGSTAT_KIND_BACKEND as it could be used in the future
to store other statistics (than the I/O ones) per backend. The new KIND is
a variable-numbered one and has an automatic cap on the maximum number of
entries (as its hash key contains the proc number).
There is no need to write the per backend I/O stats to disk (no point to
see stats for backends that do not exist anymore after a re-start), so using
"write_to_file = false".
Note that per backend I/O statistics are not collected for the checkpointer,
the background writer, the startup process and the autovacuum launcher as those
are already visible in pg_stat_io and there is only one of those.
XXX: Bump catalog version needs to be done.
---
doc/src/sgml/config.sgml | 8 +-
doc/src/sgml/monitoring.sgml | 37 ++++
src/backend/catalog/system_functions.sql | 2 +
src/backend/utils/activity/Makefile | 1 +
src/backend/utils/activity/backend_status.c | 4 +
src/backend/utils/activity/meson.build | 1 +
src/backend/utils/activity/pgstat.c | 22 ++-
src/backend/utils/activity/pgstat_backend.c | 182 +++++++++++++++++++
src/backend/utils/activity/pgstat_io.c | 25 ++-
src/backend/utils/activity/pgstat_relation.c | 2 +
src/backend/utils/adt/pgstatfuncs.c | 169 +++++++++++++++++
src/include/catalog/pg_proc.dat | 14 ++
src/include/pgstat.h | 31 +++-
src/include/utils/pgstat_internal.h | 14 ++
src/test/regress/expected/stats.out | 72 +++++++-
src/test/regress/sql/stats.sql | 38 +++-
src/tools/pgindent/typedefs.list | 3 +
17 files changed, 604 insertions(+), 21 deletions(-)
9.9% doc/src/sgml/
31.2% src/backend/utils/activity/
22.1% src/backend/utils/adt/
4.4% src/include/catalog/
7.1% src/include/
12.9% src/test/regress/expected/
11.5% src/test/regress/sql/
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e0c8325a39..8afca9b110 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -8403,9 +8403,11 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
displayed in <link linkend="monitoring-pg-stat-database-view">
<structname>pg_stat_database</structname></link>,
<link linkend="monitoring-pg-stat-io-view">
- <structname>pg_stat_io</structname></link>, in the output of
- <xref linkend="sql-explain"/> when the <literal>BUFFERS</literal> option
- is used, in the output of <xref linkend="sql-vacuum"/> when
+ <structname>pg_stat_io</structname></link>, in the output of the
+ <link linkend="pg-stat-get-backend-io">
+ <function>pg_stat_get_backend_io()</function></link> function, in the
+ output of <xref linkend="sql-explain"/> when the <literal>BUFFERS</literal>
+ option is used, in the output of <xref linkend="sql-vacuum"/> when
the <literal>VERBOSE</literal> option is used, by autovacuum
for auto-vacuums and auto-analyzes, when <xref
linkend="guc-log-autovacuum-min-duration"/> is set and by
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 840d7f8161..4a9464da61 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -4790,6 +4790,25 @@ description | Waiting for a newly initialized WAL file to reach durable storage
</para></entry>
</row>
+ <row>
+ <entry id="pg-stat-get-backend-io" role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>pg_stat_get_backend_io</primary>
+ </indexterm>
+ <function>pg_stat_get_backend_io</function> ( <type>integer</type> )
+ <returnvalue>setof record</returnvalue>
+ </para>
+ <para>
+ Returns I/O statistics about the backend with the specified
+ process ID. The output fields are exactly the same as the ones in the
+ <link linkend="monitoring-pg-stat-io-view"> <structname>pg_stat_io</structname></link>
+ view. The function does not return I/O statistics for the checkpointer,
+ the background writer, the startup process and the autovacuum launcher
+ as they are already visible in the <link linkend="monitoring-pg-stat-io-view"> <structname>pg_stat_io</structname></link>
+ view and there is only one of those.
+ </para></entry>
+ </row>
+
<row>
<entry role="func_table_entry"><para role="func_signature">
<indexterm>
@@ -4971,6 +4990,24 @@ description | Waiting for a newly initialized WAL file to reach durable storage
</para></entry>
</row>
+ <row>
+ <entry role="func_table_entry"><para role="func_signature">
+ <indexterm>
+ <primary>pg_stat_reset_backend_stats</primary>
+ </indexterm>
+ <function>pg_stat_reset_backend_stats</function> ( <type>integer</type> )
+ <returnvalue>void</returnvalue>
+ </para>
+ <para>
+ Resets statistics for a single backend with the specified process ID
+ to zero.
+ </para>
+ <para>
+ This function is restricted to superusers by default, but other users
+ can be granted EXECUTE to run the function.
+ </para></entry>
+ </row>
+
<row>
<entry role="func_table_entry"><para role="func_signature">
<indexterm>
diff --git a/src/backend/catalog/system_functions.sql b/src/backend/catalog/system_functions.sql
index c51dfca802..14eb99cd47 100644
--- a/src/backend/catalog/system_functions.sql
+++ b/src/backend/catalog/system_functions.sql
@@ -711,6 +711,8 @@ REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_table_counters(oid) FROM public;
REVOKE EXECUTE ON FUNCTION pg_stat_reset_single_function_counters(oid) FROM public;
+REVOKE EXECUTE ON FUNCTION pg_stat_reset_backend_stats(integer) FROM public;
+
REVOKE EXECUTE ON FUNCTION pg_stat_reset_replication_slot(text) FROM public;
REVOKE EXECUTE ON FUNCTION pg_stat_have_stats(text, oid, int8) FROM public;
diff --git a/src/backend/utils/activity/Makefile b/src/backend/utils/activity/Makefile
index b9fd66ea17..24b64a2742 100644
--- a/src/backend/utils/activity/Makefile
+++ b/src/backend/utils/activity/Makefile
@@ -20,6 +20,7 @@ OBJS = \
backend_status.o \
pgstat.o \
pgstat_archiver.o \
+ pgstat_backend.o \
pgstat_bgwriter.o \
pgstat_checkpointer.o \
pgstat_database.o \
diff --git a/src/backend/utils/activity/backend_status.c b/src/backend/utils/activity/backend_status.c
index 22c6dc378c..7f74011a00 100644
--- a/src/backend/utils/activity/backend_status.c
+++ b/src/backend/utils/activity/backend_status.c
@@ -249,6 +249,10 @@ pgstat_beinit(void)
Assert(MyProcNumber >= 0 && MyProcNumber < NumBackendStatSlots);
MyBEEntry = &BackendStatusArray[MyProcNumber];
+ /* Create the backend statistics entry */
+ if (pgstat_tracks_backend_bktype(MyBackendType))
+ pgstat_create_backend_stat(MyProcNumber);
+
/* Set up a process-exit hook to clean up */
on_shmem_exit(pgstat_beshutdown_hook, 0);
}
diff --git a/src/backend/utils/activity/meson.build b/src/backend/utils/activity/meson.build
index f73c22905c..380d3dd70c 100644
--- a/src/backend/utils/activity/meson.build
+++ b/src/backend/utils/activity/meson.build
@@ -5,6 +5,7 @@ backend_sources += files(
'backend_status.c',
'pgstat.c',
'pgstat_archiver.c',
+ 'pgstat_backend.c',
'pgstat_bgwriter.c',
'pgstat_checkpointer.c',
'pgstat_database.c',
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 18b7d9b47d..563f2443f8 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -77,6 +77,7 @@
*
* Each statistics kind is handled in a dedicated file:
* - pgstat_archiver.c
+ * - pgstat_backend.c
* - pgstat_bgwriter.c
* - pgstat_checkpointer.c
* - pgstat_database.c
@@ -358,6 +359,22 @@ static const PgStat_KindInfo pgstat_kind_builtin_infos[PGSTAT_KIND_BUILTIN_SIZE]
.reset_timestamp_cb = pgstat_subscription_reset_timestamp_cb,
},
+ [PGSTAT_KIND_BACKEND] = {
+ .name = "backend",
+
+ .fixed_amount = false,
+ .write_to_file = false,
+
+ .accessed_across_databases = true,
+
+ .shared_size = sizeof(PgStatShared_Backend),
+ .shared_data_off = offsetof(PgStatShared_Backend, stats),
+ .shared_data_len = sizeof(((PgStatShared_Backend *) 0)->stats),
+ .pending_size = sizeof(PgStat_BackendPendingIO),
+
+ .flush_pending_cb = pgstat_backend_flush_cb,
+ .reset_timestamp_cb = pgstat_backend_reset_timestamp_cb,
+ },
/* stats for fixed-numbered (mostly 1) objects */
@@ -602,6 +619,9 @@ pgstat_shutdown_hook(int code, Datum arg)
Assert(dlist_is_empty(&pgStatPending));
dlist_init(&pgStatPending);
+ /* drop the backend stats entry */
+ pgstat_drop_entry(PGSTAT_KIND_BACKEND, InvalidOid, MyProcNumber);
+
pgstat_detach_shmem();
#ifdef USE_ASSERT_CHECKING
@@ -768,7 +788,7 @@ pgstat_report_stat(bool force)
partial_flush = false;
- /* flush database / relation / function / ... stats */
+ /* flush database / relation / function / backend / ... stats */
partial_flush |= pgstat_flush_pending_entries(nowait);
/* flush of fixed-numbered stats */
diff --git a/src/backend/utils/activity/pgstat_backend.c b/src/backend/utils/activity/pgstat_backend.c
new file mode 100644
index 0000000000..e2d83024c2
--- /dev/null
+++ b/src/backend/utils/activity/pgstat_backend.c
@@ -0,0 +1,182 @@
+/* -------------------------------------------------------------------------
+ *
+ * pgstat_backend.c
+ * Implementation of backend statistics.
+ *
+ * This file contains the implementation of backend statistics. It is kept
+ * separate from pgstat.c to enforce the line between the statistics access /
+ * storage implementation and the details about individual types of statistics.
+ *
+ * Copyright (c) 2024, PostgreSQL Global Development Group
+ *
+ * IDENTIFICATION
+ * src/backend/utils/activity/pgstat_backend.c
+ * -------------------------------------------------------------------------
+ */
+
+#include "postgres.h"
+
+#include "utils/pgstat_internal.h"
+
+/*
+ * Returns backend's IO stats.
+ */
+PgStat_Backend *
+pgstat_fetch_proc_stat_io(ProcNumber procNumber)
+{
+ PgStat_Backend *backend_entry;
+
+ backend_entry = (PgStat_Backend *) pgstat_fetch_entry(PGSTAT_KIND_BACKEND,
+ InvalidOid, procNumber);
+
+ return backend_entry;
+}
+
+/*
+ * Flush out locally pending backend statistics
+ *
+ * If no stats have been recorded, this function returns false.
+ */
+bool
+pgstat_backend_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
+{
+ PgStatShared_Backend *shbackendioent;
+ PgStat_BackendPendingIO *pendingent;
+ PgStat_BktypeIO *bktype_shstats;
+
+ if (!pgstat_lock_entry(entry_ref, nowait))
+ return false;
+
+ shbackendioent = (PgStatShared_Backend *) entry_ref->shared_stats;
+ bktype_shstats = &shbackendioent->stats.stats;
+ pendingent = (PgStat_BackendPendingIO *) entry_ref->pending;
+
+ for (int io_object = 0; io_object < IOOBJECT_NUM_TYPES; io_object++)
+ {
+ for (int io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++)
+ {
+ for (int io_op = 0; io_op < IOOP_NUM_TYPES; io_op++)
+ {
+ instr_time time;
+
+ bktype_shstats->counts[io_object][io_context][io_op] +=
+ pendingent->counts[io_object][io_context][io_op];
+
+ time = pendingent->pending_times[io_object][io_context][io_op];
+
+ bktype_shstats->times[io_object][io_context][io_op] +=
+ INSTR_TIME_GET_MICROSEC(time);
+ }
+ }
+ }
+
+ pgstat_unlock_entry(entry_ref);
+
+ return true;
+}
+
+/*
+ * Simpler wrapper of pgstat_backend_flush_cb()
+ */
+void
+pgstat_flush_backend(bool nowait)
+{
+ if (pgstat_tracks_backend_bktype(MyBackendType))
+ {
+ PgStat_EntryRef *entry_ref;
+
+ entry_ref = pgstat_get_entry_ref(PGSTAT_KIND_BACKEND, InvalidOid,
+ MyProcNumber, false, NULL);
+ (void) pgstat_backend_flush_cb(entry_ref, nowait);
+ }
+}
+
+/*
+ * Create the backend statistics entry for procnum.
+ */
+void
+pgstat_create_backend_stat(ProcNumber procnum)
+{
+ PgStat_EntryRef *entry_ref;
+ PgStatShared_Backend *shstatent;
+
+ entry_ref = pgstat_prep_pending_entry(PGSTAT_KIND_BACKEND, InvalidOid,
+ procnum, NULL);
+
+ shstatent = (PgStatShared_Backend *) entry_ref->shared_stats;
+
+ /*
+ * NB: need to accept that there might be stats from an older backend,
+ * e.g. if we previously used this proc number.
+ */
+ memset(&shstatent->stats, 0, sizeof(shstatent->stats));
+}
+
+/*
+ * Find or create a local PgStat_BackendPendingIO entry for procnum.
+ */
+PgStat_BackendPendingIO *
+pgstat_prep_backend_pending(ProcNumber procnum)
+{
+ PgStat_EntryRef *entry_ref;
+
+ entry_ref = pgstat_prep_pending_entry(PGSTAT_KIND_BACKEND, InvalidOid,
+ procnum, NULL);
+
+ return entry_ref->pending;
+}
+
+/*
+ * Backend statistics are not collected for all BackendTypes.
+ *
+ * The following BackendTypes do not participate in the backend stats
+ * subsystem:
+ * - The same and for the same reasons as in pgstat_tracks_io_bktype().
+ * - B_BG_WRITER, B_CHECKPOINTER, B_STARTUP and B_AUTOVAC_LAUNCHER because their
+ * I/O stats are already visible in pg_stat_io and there is only one of those.
+ *
+ * Function returns true if BackendType participates in the backend stats
+ * subsystem for IO and false if it does not.
+ *
+ * When adding a new BackendType, also consider adding relevant restrictions to
+ * pgstat_tracks_io_object() and pgstat_tracks_io_op().
+ */
+bool
+pgstat_tracks_backend_bktype(BackendType bktype)
+{
+ /*
+ * List every type so that new backend types trigger a warning about
+ * needing to adjust this switch.
+ */
+ switch (bktype)
+ {
+ case B_INVALID:
+ case B_AUTOVAC_LAUNCHER:
+ case B_DEAD_END_BACKEND:
+ case B_ARCHIVER:
+ case B_LOGGER:
+ case B_WAL_RECEIVER:
+ case B_WAL_WRITER:
+ case B_WAL_SUMMARIZER:
+ case B_BG_WRITER:
+ case B_CHECKPOINTER:
+ case B_STARTUP:
+ return false;
+
+ case B_AUTOVAC_WORKER:
+ case B_BACKEND:
+ case B_BG_WORKER:
+ case B_STANDALONE_BACKEND:
+ case B_SLOTSYNC_WORKER:
+ case B_WAL_SENDER:
+ return true;
+ }
+
+ return false;
+}
+
+void
+pgstat_backend_reset_timestamp_cb(PgStatShared_Common *header, TimestampTz ts)
+{
+ ((PgStatShared_Backend *) header)->stats.stat_reset_timestamp = ts;
+}
diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c
index f9883af2b3..e0c206a453 100644
--- a/src/backend/utils/activity/pgstat_io.c
+++ b/src/backend/utils/activity/pgstat_io.c
@@ -20,13 +20,7 @@
#include "storage/bufmgr.h"
#include "utils/pgstat_internal.h"
-
-typedef struct PgStat_PendingIO
-{
- PgStat_Counter counts[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
- instr_time pending_times[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
-} PgStat_PendingIO;
-
+typedef PgStat_BackendPendingIO PgStat_PendingIO;
static PgStat_PendingIO PendingIOStats;
static bool have_iostats = false;
@@ -87,6 +81,14 @@ pgstat_count_io_op_n(IOObject io_object, IOContext io_context, IOOp io_op, uint3
Assert((unsigned int) io_op < IOOP_NUM_TYPES);
Assert(pgstat_tracks_io_op(MyBackendType, io_object, io_context, io_op));
+ if (pgstat_tracks_backend_bktype(MyBackendType))
+ {
+ PgStat_PendingIO *entry_ref;
+
+ entry_ref = pgstat_prep_backend_pending(MyProcNumber);
+ entry_ref->counts[io_object][io_context][io_op] += cnt;
+ }
+
PendingIOStats.counts[io_object][io_context][io_op] += cnt;
have_iostats = true;
@@ -148,6 +150,15 @@ pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op,
INSTR_TIME_ADD(PendingIOStats.pending_times[io_object][io_context][io_op],
io_time);
+
+ if (pgstat_tracks_backend_bktype(MyBackendType))
+ {
+ PgStat_PendingIO *entry_ref;
+
+ entry_ref = pgstat_prep_backend_pending(MyProcNumber);
+ INSTR_TIME_ADD(entry_ref->pending_times[io_object][io_context][io_op],
+ io_time);
+ }
}
pgstat_count_io_op_n(io_object, io_context, io_op, cnt);
diff --git a/src/backend/utils/activity/pgstat_relation.c b/src/backend/utils/activity/pgstat_relation.c
index faba8b64d2..85e65557bb 100644
--- a/src/backend/utils/activity/pgstat_relation.c
+++ b/src/backend/utils/activity/pgstat_relation.c
@@ -264,6 +264,7 @@ pgstat_report_vacuum(Oid tableoid, bool shared,
* VACUUM command has processed all tables and committed.
*/
pgstat_flush_io(false);
+ pgstat_flush_backend(false);
}
/*
@@ -350,6 +351,7 @@ pgstat_report_analyze(Relation rel,
/* see pgstat_report_vacuum() */
pgstat_flush_io(false);
+ pgstat_flush_backend(false);
}
/*
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index cdf37403e9..b939551d36 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1474,6 +1474,154 @@ pg_stat_get_io(PG_FUNCTION_ARGS)
return (Datum) 0;
}
+Datum
+pg_stat_get_backend_io(PG_FUNCTION_ARGS)
+{
+ ReturnSetInfo *rsinfo;
+ PgStat_Backend *backend_stats;
+ Datum bktype_desc;
+ PgStat_BktypeIO *bktype_stats;
+ BackendType bktype;
+ Datum reset_time;
+ int num_backends = pgstat_fetch_stat_numbackends();
+ int curr_backend;
+ int pid;
+ PGPROC *proc;
+ ProcNumber procNumber;
+
+ InitMaterializedSRF(fcinfo, 0);
+ rsinfo = (ReturnSetInfo *) fcinfo->resultinfo;
+
+ pid = PG_GETARG_INT32(0);
+ proc = BackendPidGetProc(pid);
+
+ /*
+ * Could be an auxiliary process but would not report any stats due to
+ * pgstat_tracks_backend_bktype() anyway. So don't need an extra call to
+ * AuxiliaryPidGetProc().
+ */
+ if (!proc)
+ return (Datum) 0;
+
+ procNumber = GetNumberFromPGProc(proc);
+ backend_stats = pgstat_fetch_proc_stat_io(procNumber);
+
+ if (!backend_stats)
+ return (Datum) 0;
+
+ bktype = B_INVALID;
+
+ /* Look for the backend type */
+ for (curr_backend = 1; curr_backend <= num_backends; curr_backend++)
+ {
+ LocalPgBackendStatus *local_beentry;
+ PgBackendStatus *beentry;
+
+ /* Get the next one in the list */
+ local_beentry = pgstat_get_local_beentry_by_index(curr_backend);
+ beentry = &local_beentry->backendStatus;
+
+ /* Looking for specific PID, ignore all the others */
+ if (beentry->st_procpid != pid)
+ continue;
+
+ bktype = beentry->st_backendType;
+ break;
+ }
+
+ /* Backend is gone */
+ if (bktype == B_INVALID)
+ return (Datum) 0;
+
+ bktype_desc = CStringGetTextDatum(GetBackendTypeDesc(bktype));
+ bktype_stats = &backend_stats->stats;
+ reset_time = TimestampTzGetDatum(backend_stats->stat_reset_timestamp);
+
+ /*
+ * In Assert builds, we can afford an extra loop through all of the
+ * counters checking that only expected stats are non-zero, since it keeps
+ * the non-Assert code cleaner.
+ */
+ Assert(pgstat_bktype_io_stats_valid(bktype_stats, bktype));
+
+ for (int io_obj = 0; io_obj < IOOBJECT_NUM_TYPES; io_obj++)
+ {
+ const char *obj_name = pgstat_get_io_object_name(io_obj);
+
+ for (int io_context = 0; io_context < IOCONTEXT_NUM_TYPES; io_context++)
+ {
+ const char *context_name = pgstat_get_io_context_name(io_context);
+
+ Datum values[IO_NUM_COLUMNS] = {0};
+ bool nulls[IO_NUM_COLUMNS] = {0};
+
+ /*
+ * Some combinations of BackendType, IOObject, and IOContext are
+ * not valid for any type of IOOp. In such cases, omit the entire
+ * row from the view.
+ */
+ if (!pgstat_tracks_io_object(bktype, io_obj, io_context))
+ continue;
+
+ values[IO_COL_BACKEND_TYPE] = bktype_desc;
+ values[IO_COL_CONTEXT] = CStringGetTextDatum(context_name);
+ values[IO_COL_OBJECT] = CStringGetTextDatum(obj_name);
+ if (backend_stats->stat_reset_timestamp != 0)
+ values[IO_COL_RESET_TIME] = reset_time;
+ else
+ nulls[IO_COL_RESET_TIME] = true;
+
+ /*
+ * Hard-code this to the value of BLCKSZ for now. Future values
+ * could include XLOG_BLCKSZ, once WAL IO is tracked, and constant
+ * multipliers, once non-block-oriented IO (e.g. temporary file
+ * IO) is tracked.
+ */
+ values[IO_COL_CONVERSION] = Int64GetDatum(BLCKSZ);
+
+ for (int io_op = 0; io_op < IOOP_NUM_TYPES; io_op++)
+ {
+ int op_idx = pgstat_get_io_op_index(io_op);
+ int time_idx = pgstat_get_io_time_index(io_op);
+
+ /*
+ * Some combinations of BackendType and IOOp, of IOContext and
+ * IOOp, and of IOObject and IOOp are not tracked. Set these
+ * cells in the view NULL.
+ */
+ if (pgstat_tracks_io_op(bktype, io_obj, io_context, io_op))
+ {
+ PgStat_Counter count =
+ bktype_stats->counts[io_obj][io_context][io_op];
+
+ values[op_idx] = Int64GetDatum(count);
+ }
+ else
+ nulls[op_idx] = true;
+
+ /* not every operation is timed */
+ if (time_idx == IO_COL_INVALID)
+ continue;
+
+ if (!nulls[op_idx])
+ {
+ PgStat_Counter time =
+ bktype_stats->times[io_obj][io_context][io_op];
+
+ values[time_idx] = Float8GetDatum(pg_stat_us_to_ms(time));
+ }
+ else
+ nulls[time_idx] = true;
+ }
+
+ tuplestore_putvalues(rsinfo->setResult, rsinfo->setDesc,
+ values, nulls);
+ }
+ }
+
+ return (Datum) 0;
+}
+
/*
* Returns statistics of WAL activity
*/
@@ -1779,6 +1927,27 @@ pg_stat_reset_single_function_counters(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+Datum
+pg_stat_reset_backend_stats(PG_FUNCTION_ARGS)
+{
+ PGPROC *proc;
+ int backend_pid = PG_GETARG_INT32(0);
+
+ proc = BackendPidGetProc(backend_pid);
+
+ /*
+ * Could be an auxiliary process but would not report any stats due to
+ * pgstat_tracks_backend_bktype() anyway. So don't need an extra call to
+ * AuxiliaryPidGetProc().
+ */
+ if (!proc)
+ PG_RETURN_VOID();
+
+ pgstat_reset(PGSTAT_KIND_BACKEND, InvalidOid, GetNumberFromPGProc(proc));
+
+ PG_RETURN_VOID();
+}
+
/* Reset SLRU counters (a specific one or all of them). */
Datum
pg_stat_reset_slru(PG_FUNCTION_ARGS)
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 0f22c21723..437157ffa3 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5913,6 +5913,15 @@
proargnames => '{backend_type,object,context,reads,read_time,writes,write_time,writebacks,writeback_time,extends,extend_time,op_bytes,hits,evictions,reuses,fsyncs,fsync_time,stats_reset}',
prosrc => 'pg_stat_get_io' },
+{ oid => '8806', descr => 'statistics: backend IO statistics',
+ proname => 'pg_stat_get_backend_io', prorows => '5', proretset => 't',
+ provolatile => 'v', proparallel => 'r', prorettype => 'record',
+ proargtypes => 'int4',
+ proallargtypes => '{int4,text,text,text,int8,float8,int8,float8,int8,float8,int8,float8,int8,int8,int8,int8,int8,float8,timestamptz}',
+ proargmodes => '{i,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o,o}',
+ proargnames => '{backend_pid,backend_type,object,context,reads,read_time,writes,write_time,writebacks,writeback_time,extends,extend_time,op_bytes,hits,evictions,reuses,fsyncs,fsync_time,stats_reset}',
+ prosrc => 'pg_stat_get_backend_io' },
+
{ oid => '1136', descr => 'statistics: information about WAL activity',
proname => 'pg_stat_get_wal', proisstrict => 'f', provolatile => 's',
proparallel => 'r', prorettype => 'record', proargtypes => '',
@@ -6052,6 +6061,11 @@
proname => 'pg_stat_reset_single_function_counters', provolatile => 'v',
prorettype => 'void', proargtypes => 'oid',
prosrc => 'pg_stat_reset_single_function_counters' },
+{ oid => '9987',
+ descr => 'statistics: reset statistics for a single backend',
+ proname => 'pg_stat_reset_backend_stats', provolatile => 'v',
+ prorettype => 'void', proargtypes => 'int4',
+ prosrc => 'pg_stat_reset_backend_stats' },
{ oid => '2307',
descr => 'statistics: reset collected statistics for a single SLRU',
proname => 'pg_stat_reset_slru', proisstrict => 'f', provolatile => 'v',
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index ebfeef2f46..479773cfd2 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -49,14 +49,15 @@
#define PGSTAT_KIND_FUNCTION 3 /* per-function statistics */
#define PGSTAT_KIND_REPLSLOT 4 /* per-slot statistics */
#define PGSTAT_KIND_SUBSCRIPTION 5 /* per-subscription statistics */
+#define PGSTAT_KIND_BACKEND 6 /* per-backend statistics */
/* stats for fixed-numbered objects */
-#define PGSTAT_KIND_ARCHIVER 6
-#define PGSTAT_KIND_BGWRITER 7
-#define PGSTAT_KIND_CHECKPOINTER 8
-#define PGSTAT_KIND_IO 9
-#define PGSTAT_KIND_SLRU 10
-#define PGSTAT_KIND_WAL 11
+#define PGSTAT_KIND_ARCHIVER 7
+#define PGSTAT_KIND_BGWRITER 8
+#define PGSTAT_KIND_CHECKPOINTER 9
+#define PGSTAT_KIND_IO 10
+#define PGSTAT_KIND_SLRU 11
+#define PGSTAT_KIND_WAL 12
#define PGSTAT_KIND_BUILTIN_MIN PGSTAT_KIND_DATABASE
#define PGSTAT_KIND_BUILTIN_MAX PGSTAT_KIND_WAL
@@ -362,12 +363,23 @@ typedef struct PgStat_BktypeIO
PgStat_Counter times[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
} PgStat_BktypeIO;
+typedef struct PgStat_BackendPendingIO
+{
+ PgStat_Counter counts[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
+ instr_time pending_times[IOOBJECT_NUM_TYPES][IOCONTEXT_NUM_TYPES][IOOP_NUM_TYPES];
+} PgStat_BackendPendingIO;
+
typedef struct PgStat_IO
{
TimestampTz stat_reset_timestamp;
PgStat_BktypeIO stats[BACKEND_NUM_TYPES];
} PgStat_IO;
+typedef struct PgStat_Backend
+{
+ TimestampTz stat_reset_timestamp;
+ PgStat_BktypeIO stats;
+} PgStat_Backend;
typedef struct PgStat_StatDBEntry
{
@@ -549,6 +561,13 @@ extern bool pgstat_have_entry(PgStat_Kind kind, Oid dboid, uint64 objid);
extern void pgstat_report_archiver(const char *xlog, bool failed);
extern PgStat_ArchiverStats *pgstat_fetch_stat_archiver(void);
+/*
+ * Functions in pgstat_backend.c
+ */
+
+extern PgStat_Backend *pgstat_fetch_proc_stat_io(ProcNumber procNumber);
+extern bool pgstat_tracks_backend_bktype(BackendType bktype);
+extern void pgstat_create_backend_stat(ProcNumber procnum);
/*
* Functions in pgstat_bgwriter.c
diff --git a/src/include/utils/pgstat_internal.h b/src/include/utils/pgstat_internal.h
index 7338bc1e28..811ed9b005 100644
--- a/src/include/utils/pgstat_internal.h
+++ b/src/include/utils/pgstat_internal.h
@@ -450,6 +450,11 @@ typedef struct PgStatShared_ReplSlot
PgStat_StatReplSlotEntry stats;
} PgStatShared_ReplSlot;
+typedef struct PgStatShared_Backend
+{
+ PgStatShared_Common header;
+ PgStat_Backend stats;
+} PgStatShared_Backend;
/*
* Central shared memory entry for the cumulative stats system.
@@ -604,6 +609,15 @@ extern void pgstat_archiver_init_shmem_cb(void *stats);
extern void pgstat_archiver_reset_all_cb(TimestampTz ts);
extern void pgstat_archiver_snapshot_cb(void);
+/*
+ * Functions in pgstat_backend.c
+ */
+
+extern void pgstat_flush_backend(bool nowait);
+
+extern PgStat_BackendPendingIO *pgstat_prep_backend_pending(ProcNumber procnum);
+extern bool pgstat_backend_flush_cb(PgStat_EntryRef *entry_ref, bool nowait);
+extern void pgstat_backend_reset_timestamp_cb(PgStatShared_Common *header, TimestampTz ts);
/*
* Functions in pgstat_bgwriter.c
diff --git a/src/test/regress/expected/stats.out b/src/test/regress/expected/stats.out
index 56771f83ed..3447e7b75d 100644
--- a/src/test/regress/expected/stats.out
+++ b/src/test/regress/expected/stats.out
@@ -1249,7 +1249,7 @@ SELECT pg_stat_get_subscription_stats(NULL);
(1 row)
--- Test that the following operations are tracked in pg_stat_io:
+-- Test that the following operations are tracked in pg_stat_io and in backend stats:
-- - reads of target blocks into shared buffers
-- - writes of shared buffers to permanent storage
-- - extends of relations using shared buffers
@@ -1259,11 +1259,19 @@ SELECT pg_stat_get_subscription_stats(NULL);
-- be sure of the state of shared buffers at the point the test is run.
-- Create a regular table and insert some data to generate IOCONTEXT_NORMAL
-- extends.
+SELECT pid AS checkpointer_pid FROM pg_stat_activity
+ WHERE backend_type = 'checkpointer' \gset
SELECT sum(extends) AS io_sum_shared_before_extends
FROM pg_stat_io WHERE context = 'normal' AND object = 'relation' \gset
+SELECT sum(extends) AS my_io_sum_shared_before_extends
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE context = 'normal' AND object = 'relation' \gset
SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
FROM pg_stat_io
WHERE object = 'relation' \gset io_sum_shared_before_
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE object = 'relation' \gset my_io_sum_shared_before_
CREATE TABLE test_io_shared(a int);
INSERT INTO test_io_shared SELECT i FROM generate_series(1,100)i;
SELECT pg_stat_force_next_flush();
@@ -1280,8 +1288,17 @@ SELECT :io_sum_shared_after_extends > :io_sum_shared_before_extends;
t
(1 row)
+SELECT sum(extends) AS my_io_sum_shared_after_extends
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE context = 'normal' AND object = 'relation' \gset
+SELECT :my_io_sum_shared_after_extends > :my_io_sum_shared_before_extends;
+ ?column?
+----------
+ t
+(1 row)
+
-- After a checkpoint, there should be some additional IOCONTEXT_NORMAL writes
--- and fsyncs.
+-- and fsyncs in the global stats (not for the backend).
-- See comment above for rationale for two explicit CHECKPOINTs.
CHECKPOINT;
CHECKPOINT;
@@ -1301,6 +1318,31 @@ SELECT current_setting('fsync') = 'off'
t
(1 row)
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE object = 'relation' \gset my_io_sum_shared_after_
+SELECT :my_io_sum_shared_after_writes >= :my_io_sum_shared_before_writes;
+ ?column?
+----------
+ t
+(1 row)
+
+SELECT current_setting('fsync') = 'off'
+ OR (:my_io_sum_shared_after_fsyncs = :my_io_sum_shared_before_fsyncs
+ AND :my_io_sum_shared_after_fsyncs= 0);
+ ?column?
+----------
+ t
+(1 row)
+
+-- Don't return any rows if querying other backend's stats that are excluded
+-- from the backend stats collection (like the checkpointer).
+SELECT count(1) = 0 FROM pg_stat_get_backend_io(:checkpointer_pid);
+ ?column?
+----------
+ t
+(1 row)
+
-- Change the tablespace so that the table is rewritten directly, then SELECT
-- from it to cause it to be read back into shared buffers.
SELECT sum(reads) AS io_sum_shared_before_reads
@@ -1521,6 +1563,8 @@ SELECT pg_stat_have_stats('io', 0, 0);
SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS io_stats_pre_reset
FROM pg_stat_io \gset
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_pre_reset
+ FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
SELECT pg_stat_reset_shared('io');
pg_stat_reset_shared
----------------------
@@ -1535,6 +1579,30 @@ SELECT :io_stats_post_reset < :io_stats_pre_reset;
t
(1 row)
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_post_reset
+ FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
+-- pg_stat_reset_shared() did not reset backend IO stats
+SELECT :my_io_stats_pre_reset <= :my_io_stats_post_reset;
+ ?column?
+----------
+ t
+(1 row)
+
+-- but pg_stat_reset_backend_stats() does
+SELECT pg_stat_reset_backend_stats(pg_backend_pid());
+ pg_stat_reset_backend_stats
+-----------------------------
+
+(1 row)
+
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_post_backend_reset
+ FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
+SELECT :my_io_stats_pre_reset > :my_io_stats_post_backend_reset;
+ ?column?
+----------
+ t
+(1 row)
+
-- test BRIN index doesn't block HOT update
CREATE TABLE brin_hot (
id integer PRIMARY KEY,
diff --git a/src/test/regress/sql/stats.sql b/src/test/regress/sql/stats.sql
index 7147cc2f89..9c925005be 100644
--- a/src/test/regress/sql/stats.sql
+++ b/src/test/regress/sql/stats.sql
@@ -595,7 +595,7 @@ SELECT pg_stat_get_replication_slot(NULL);
SELECT pg_stat_get_subscription_stats(NULL);
--- Test that the following operations are tracked in pg_stat_io:
+-- Test that the following operations are tracked in pg_stat_io and in backend stats:
-- - reads of target blocks into shared buffers
-- - writes of shared buffers to permanent storage
-- - extends of relations using shared buffers
@@ -607,20 +607,32 @@ SELECT pg_stat_get_subscription_stats(NULL);
-- Create a regular table and insert some data to generate IOCONTEXT_NORMAL
-- extends.
+SELECT pid AS checkpointer_pid FROM pg_stat_activity
+ WHERE backend_type = 'checkpointer' \gset
SELECT sum(extends) AS io_sum_shared_before_extends
FROM pg_stat_io WHERE context = 'normal' AND object = 'relation' \gset
+SELECT sum(extends) AS my_io_sum_shared_before_extends
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE context = 'normal' AND object = 'relation' \gset
SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
FROM pg_stat_io
WHERE object = 'relation' \gset io_sum_shared_before_
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE object = 'relation' \gset my_io_sum_shared_before_
CREATE TABLE test_io_shared(a int);
INSERT INTO test_io_shared SELECT i FROM generate_series(1,100)i;
SELECT pg_stat_force_next_flush();
SELECT sum(extends) AS io_sum_shared_after_extends
FROM pg_stat_io WHERE context = 'normal' AND object = 'relation' \gset
SELECT :io_sum_shared_after_extends > :io_sum_shared_before_extends;
+SELECT sum(extends) AS my_io_sum_shared_after_extends
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE context = 'normal' AND object = 'relation' \gset
+SELECT :my_io_sum_shared_after_extends > :my_io_sum_shared_before_extends;
-- After a checkpoint, there should be some additional IOCONTEXT_NORMAL writes
--- and fsyncs.
+-- and fsyncs in the global stats (not for the backend).
-- See comment above for rationale for two explicit CHECKPOINTs.
CHECKPOINT;
CHECKPOINT;
@@ -630,6 +642,17 @@ SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
SELECT :io_sum_shared_after_writes > :io_sum_shared_before_writes;
SELECT current_setting('fsync') = 'off'
OR :io_sum_shared_after_fsyncs > :io_sum_shared_before_fsyncs;
+SELECT sum(writes) AS writes, sum(fsyncs) AS fsyncs
+ FROM pg_stat_get_backend_io(pg_backend_pid())
+ WHERE object = 'relation' \gset my_io_sum_shared_after_
+SELECT :my_io_sum_shared_after_writes >= :my_io_sum_shared_before_writes;
+SELECT current_setting('fsync') = 'off'
+ OR (:my_io_sum_shared_after_fsyncs = :my_io_sum_shared_before_fsyncs
+ AND :my_io_sum_shared_after_fsyncs= 0);
+
+-- Don't return any rows if querying other backend's stats that are excluded
+-- from the backend stats collection (like the checkpointer).
+SELECT count(1) = 0 FROM pg_stat_get_backend_io(:checkpointer_pid);
-- Change the tablespace so that the table is rewritten directly, then SELECT
-- from it to cause it to be read back into shared buffers.
@@ -762,10 +785,21 @@ SELECT :io_sum_bulkwrite_strategy_extends_after > :io_sum_bulkwrite_strategy_ext
SELECT pg_stat_have_stats('io', 0, 0);
SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS io_stats_pre_reset
FROM pg_stat_io \gset
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_pre_reset
+ FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
SELECT pg_stat_reset_shared('io');
SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS io_stats_post_reset
FROM pg_stat_io \gset
SELECT :io_stats_post_reset < :io_stats_pre_reset;
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_post_reset
+ FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
+-- pg_stat_reset_shared() did not reset backend IO stats
+SELECT :my_io_stats_pre_reset <= :my_io_stats_post_reset;
+-- but pg_stat_reset_backend_stats() does
+SELECT pg_stat_reset_backend_stats(pg_backend_pid());
+SELECT sum(evictions) + sum(reuses) + sum(extends) + sum(fsyncs) + sum(reads) + sum(writes) + sum(writebacks) + sum(hits) AS my_io_stats_post_backend_reset
+ FROM pg_stat_get_backend_io(pg_backend_pid()) \gset
+SELECT :my_io_stats_pre_reset > :my_io_stats_post_backend_reset;
-- test BRIN index doesn't block HOT update
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index ce33e55bf1..398dd92527 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2121,6 +2121,7 @@ PgFdwSamplingMethod
PgFdwScanState
PgIfAddrCallback
PgStatShared_Archiver
+PgStatShared_Backend
PgStatShared_BgWriter
PgStatShared_Checkpointer
PgStatShared_Common
@@ -2136,6 +2137,8 @@ PgStatShared_SLRU
PgStatShared_Subscription
PgStatShared_Wal
PgStat_ArchiverStats
+PgStat_Backend
+PgStat_BackendPendingIO
PgStat_BackendSubEntry
PgStat_BgWriterStats
PgStat_BktypeIO
--
2.34.1
^ permalink raw reply [nested|flat] 12+ messages in thread
* Re: per backend I/O statistics
2024-11-25 07:12 Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-11-25 07:18 ` Re: per backend I/O statistics[ Michael Paquier <[email protected]>
2024-11-25 15:47 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-12-12 04:52 ` Re: per backend I/O statistics Michael Paquier <[email protected]>
2024-12-12 14:02 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-12-13 02:02 ` Re: per backend I/O statistics Michael Paquier <[email protected]>
2024-12-13 09:20 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
@ 2024-12-16 08:07 ` Michael Paquier <[email protected]>
0 siblings, 0 replies; 12+ messages in thread
From: Michael Paquier @ 2024-12-16 08:07 UTC (permalink / raw)
To: Bertrand Drouvot <[email protected]>; +Cc: Nazir Bilal Yavuz <[email protected]>; Alvaro Herrera <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On Fri, Dec 13, 2024 at 09:20:13AM +0000, Bertrand Drouvot wrote:
> Yeah makes sense, that's consistent with other kinds: done.
This looks to be taking shape. I don't have much more comments.
Not feeling so sure about the value brought by the backend_type
returned in pg_stat_get_backend_io(), but well..
+ /* drop the backend stats entry */
+ pgstat_drop_entry(PGSTAT_KIND_BACKEND, InvalidOid, MyProcNumber);
Oh, I've missed something. Shouldn't pgstat_request_entry_refs_gc()
be called when this returns false?
The creation of the dshash entry is a bit too early, I think.. How
about delaying it more so as we don't create entries that could be
useless if we fail the last steps of authentication? One spot would
be to delay the creation of the new entry at the end of
pgstat_bestart(), where we know that we are done with authentication
and that the backend is ready to report back to the client connected
to it. It is true that some subsystems could produce stats as of the
early transactions they generate, which is why pgstat_initialize() is
done that early in BaseInit(), but that's not really relevant for this
case?
I'm still feeling a bit uneasy about the drop done in
pgstat_shutdown_hook(); it would be nice to make sure that this
happens in a path that would run just after we're done with the
creation of the entry to limit windows where we have an entry but no
way to drop it, or vice-versa, so as the shutdown assertions would
never trigger. Perhaps there's an argument for an entirely separate
callback that would run before pgstat is plugged off, like a new
before_shmem_exit() callback registered after the entry is created?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 12+ messages in thread
end of thread, other threads:[~2024-12-16 08:07 UTC | newest]
Thread overview: 12+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-03-03 05:48 [PATCH v9 1/1] shared detoast datum yizhi.fzh <[email protected]>
2024-11-25 07:12 Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-11-25 07:18 ` Re: per backend I/O statistics[ Michael Paquier <[email protected]>
2024-11-25 15:47 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-11-27 06:33 ` Re: per backend I/O statistics Michael Paquier <[email protected]>
2024-11-27 16:00 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-12-12 04:11 ` Re: per backend I/O statistics Michael Paquier <[email protected]>
2024-12-12 04:52 ` Re: per backend I/O statistics Michael Paquier <[email protected]>
2024-12-12 14:02 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-12-13 02:02 ` Re: per backend I/O statistics Michael Paquier <[email protected]>
2024-12-13 09:20 ` Re: per backend I/O statistics Bertrand Drouvot <[email protected]>
2024-12-16 08:07 ` Re: per backend I/O statistics Michael Paquier <[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