agora inbox for [email protected]
help / color / mirror / Atom feedRe: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead
30+ messages / 7 participants
[nested] [flat]
* Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead
@ 2019-01-01 20:21 Nikolay Shaplov <[email protected]>
2019-01-02 03:05 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Nikolay Shaplov @ 2019-01-01 20:21 UTC (permalink / raw)
To: [email protected]; +Cc: Dmitry Dolgov <[email protected]>
В письме от пятница, 30 ноября 2018 г. 23:57:21 MSK пользователь Dmitry Dolgov
написал:
> Looks like there are some problems with this patch on windows:
> src/backend/access/common/reloptions.c(1469): error C2059: syntax error :
> '}'
>
> https://ci.appveyor.com/project/postgresql-cfbot/postgresql/build/1.0.22359
Phew... It took me a while to get proper windows building environment... But
finally I did it...
This was some MSVC specific error, that happens when you create empty array or
something like this. I've rewritten that function to remove this array at
all. Now MSVC successfully builds it.
I also did some codestyle improvements, and rebased the patch against the
current master.
The new patch is in attachment.
> > I get that, but I strongly suggest not creating 10 loosely related
> > threads, but keeping it as a patch series in one thread. It's really
> > hard to follow for people not continually paying otherwise.
>
> Totally agree. Probably this also makes it harder to see the big picture
> behind this patch - which is in turn probably preventing it from getting
> some more review. I hope it doesn't sounds ridiculous, taking into account
> your efforts by splitting the patch, but maybe it makes sense to gather
> these pieces together (as a separate commits, of course) in one thread?
The big picture is following. I started from the task: Add possibility to set
up opclass parameters. (Nikita Glukhov now doing it)
I found an reloptions code, that does almost same thing, but it is not flexible
at all, and I can't reuse it for opclass parameters as it is now.
So I came to decision to rewrite reloptions code, so it can be used for
reloptions opclass options and any other kind of options we may need in
future.
While rewriting the code, I found some places in the code that goes from what
seems to be a very long time, and also need refreshing.
This is one of the things.
It is not 100% necessary. Postgres will work with it as it is for ten years or
more. But since I've touched this part of the code, I want to make this code
more consistent, and more neat.
That's what I am doing. That is what all this patches about.
I'd be happy if we move this task at last.
Attachments:
[text/x-patch] get-rid-of-StrRdOptions_2.diff (36.9K, ../../1997857.dLiBphshJP@x200m/2-get-rid-of-StrRdOptions_2.diff)
download | inline diff:
diff --git a/.gitignore b/.gitignore
index 794e35b..37331c2 100644
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index eece89a..833d084 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -22,7 +22,7 @@
#include "access/htup_details.h"
#include "access/nbtree.h"
#include "access/reloptions.h"
-#include "access/spgist.h"
+#include "access/spgist_private.h"
#include "access/tuptoaster.h"
#include "catalog/pg_type.h"
#include "commands/defrem.h"
@@ -46,9 +46,8 @@
* upper and lower bounds (if applicable); for strings, consider a validation
* routine.
* (ii) add a record below (or use add_<type>_reloption).
- * (iii) add it to the appropriate options struct (perhaps StdRdOptions)
- * (iv) add it to the appropriate handling routine (perhaps
- * default_reloptions)
+ * (iii) add it to the appropriate options struct
+ * (iv) add it to the appropriate handling routine
* (v) make sure the lock level is set correctly for that operation
* (vi) don't forget to document the option
*
@@ -1019,7 +1018,7 @@ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
case RELKIND_TOASTVALUE:
case RELKIND_MATVIEW:
case RELKIND_PARTITIONED_TABLE:
- options = heap_reloptions(classForm->relkind, datum, false);
+ options = relation_reloptions(classForm->relkind, datum, false);
break;
case RELKIND_VIEW:
options = view_reloptions(datum, false);
@@ -1352,63 +1351,69 @@ fillRelOptions(void *rdopts, Size basesize,
/*
- * Option parser for anything that uses StdRdOptions.
+ * Option parsing definition for autovacuum. Used in toast and heap options.
+ */
+
+#define AUTOVACUUM_RELOPTIONS(OFFSET) \
+ {"autovacuum_enabled", RELOPT_TYPE_BOOL, \
+ OFFSET + offsetof(AutoVacOpts, enabled)}, \
+ {"autovacuum_vacuum_threshold", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, vacuum_threshold)}, \
+ {"autovacuum_analyze_threshold", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, analyze_threshold)}, \
+ {"autovacuum_vacuum_cost_delay", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, vacuum_cost_delay)}, \
+ {"autovacuum_vacuum_cost_limit", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, vacuum_cost_limit)}, \
+ {"autovacuum_freeze_min_age", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, freeze_min_age)}, \
+ {"autovacuum_freeze_max_age", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, freeze_max_age)}, \
+ {"autovacuum_freeze_table_age", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, freeze_table_age)}, \
+ {"autovacuum_multixact_freeze_min_age", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, multixact_freeze_min_age)}, \
+ {"autovacuum_multixact_freeze_max_age", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, multixact_freeze_max_age)}, \
+ {"autovacuum_multixact_freeze_table_age", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, multixact_freeze_table_age)}, \
+ {"log_autovacuum_min_duration", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, log_min_duration)}, \
+ {"autovacuum_vacuum_scale_factor", RELOPT_TYPE_REAL, \
+ OFFSET + offsetof(AutoVacOpts, vacuum_scale_factor)}, \
+ {"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL, \
+ OFFSET + offsetof(AutoVacOpts, analyze_scale_factor)}
+
+/*
+ * Option parser for heap
*/
bytea *
-default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
+heap_reloptions(Datum reloptions, bool validate)
{
relopt_value *options;
- StdRdOptions *rdopts;
+ HeapRelOptions *rdopts;
int numoptions;
static const relopt_parse_elt tab[] = {
- {"fillfactor", RELOPT_TYPE_INT, offsetof(StdRdOptions, fillfactor)},
- {"autovacuum_enabled", RELOPT_TYPE_BOOL,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, enabled)},
- {"autovacuum_vacuum_threshold", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_threshold)},
- {"autovacuum_analyze_threshold", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, analyze_threshold)},
- {"autovacuum_vacuum_cost_delay", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_delay)},
- {"autovacuum_vacuum_cost_limit", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_limit)},
- {"autovacuum_freeze_min_age", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_min_age)},
- {"autovacuum_freeze_max_age", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_max_age)},
- {"autovacuum_freeze_table_age", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_table_age)},
- {"autovacuum_multixact_freeze_min_age", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_min_age)},
- {"autovacuum_multixact_freeze_max_age", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_max_age)},
- {"autovacuum_multixact_freeze_table_age", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_table_age)},
- {"log_autovacuum_min_duration", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, log_min_duration)},
+ {"fillfactor", RELOPT_TYPE_INT, offsetof(HeapRelOptions, fillfactor)},
+ AUTOVACUUM_RELOPTIONS(offsetof(HeapRelOptions, autovacuum)),
{"toast_tuple_target", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, toast_tuple_target)},
- {"autovacuum_vacuum_scale_factor", RELOPT_TYPE_REAL,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_scale_factor)},
- {"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, analyze_scale_factor)},
+ offsetof(HeapRelOptions, toast_tuple_target)},
{"user_catalog_table", RELOPT_TYPE_BOOL,
- offsetof(StdRdOptions, user_catalog_table)},
+ offsetof(HeapRelOptions, user_catalog_table)},
{"parallel_workers", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, parallel_workers)},
- {"vacuum_cleanup_index_scale_factor", RELOPT_TYPE_REAL,
- offsetof(StdRdOptions, vacuum_cleanup_index_scale_factor)}
+ offsetof(HeapRelOptions, parallel_workers)}
};
- options = parseRelOptions(reloptions, validate, kind, &numoptions);
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_HEAP,
+ &numoptions);
/* if none set, we're done */
if (numoptions == 0)
return NULL;
- rdopts = allocateReloptStruct(sizeof(StdRdOptions), options, numoptions);
+ rdopts = allocateReloptStruct(sizeof(HeapRelOptions), options, numoptions);
- fillRelOptions((void *) rdopts, sizeof(StdRdOptions), options, numoptions,
+ fillRelOptions((void *) rdopts, sizeof(HeapRelOptions), options, numoptions,
validate, tab, lengthof(tab));
pfree(options);
@@ -1417,6 +1422,60 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
}
/*
+ * Option parser for toast
+ */
+bytea *
+toast_reloptions(Datum reloptions, bool validate)
+{
+ relopt_value *options;
+ ToastRelOptions *rdopts;
+ int numoptions;
+ static const relopt_parse_elt tab[] = {
+ AUTOVACUUM_RELOPTIONS(offsetof(ToastRelOptions, autovacuum)),
+ };
+
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_TOAST,
+ &numoptions);
+
+ /* if none set, we're done */
+ if (numoptions == 0)
+ return NULL;
+
+ rdopts = allocateReloptStruct(sizeof(ToastRelOptions), options, numoptions);
+
+ fillRelOptions((void *) rdopts, sizeof(ToastRelOptions), options,
+ numoptions, validate, tab, lengthof(tab));
+
+ /* adjust default-only parameters for TOAST relations */
+ rdopts->autovacuum.analyze_threshold = -1;
+ rdopts->autovacuum.analyze_scale_factor = -1;
+
+ pfree(options);
+
+ return (bytea *) rdopts;
+}
+
+/*
+ * Option parser for partitioned relations
+ */
+bytea *
+partitioned_reloptions(Datum reloptions, bool validate)
+{
+ int numoptions;
+
+ /*
+ * Since there is no options for patitioned table for now, we just do
+ * validation to report incorrect option error and leave.
+ */
+
+ if (validate)
+ parseRelOptions(reloptions, validate, RELOPT_KIND_PARTITIONED,
+ &numoptions);
+
+ return NULL;
+}
+
+/*
* Option parser for views
*/
bytea *
@@ -1449,39 +1508,26 @@ view_reloptions(Datum reloptions, bool validate)
}
/*
- * Parse options for heaps, views and toast tables.
+ * Parse options for heaps, toast, views and partitioned tables.
*/
bytea *
-heap_reloptions(char relkind, Datum reloptions, bool validate)
+relation_reloptions(char relkind, Datum reloptions, bool validate)
{
- StdRdOptions *rdopts;
-
switch (relkind)
{
case RELKIND_TOASTVALUE:
- rdopts = (StdRdOptions *)
- default_reloptions(reloptions, validate, RELOPT_KIND_TOAST);
- if (rdopts != NULL)
- {
- /* adjust default-only parameters for TOAST relations */
- rdopts->fillfactor = 100;
- rdopts->autovacuum.analyze_threshold = -1;
- rdopts->autovacuum.analyze_scale_factor = -1;
- }
- return (bytea *) rdopts;
+ return toast_reloptions(reloptions, validate);
case RELKIND_RELATION:
case RELKIND_MATVIEW:
- return default_reloptions(reloptions, validate, RELOPT_KIND_HEAP);
+ return heap_reloptions(reloptions, validate);
case RELKIND_PARTITIONED_TABLE:
- return default_reloptions(reloptions, validate,
- RELOPT_KIND_PARTITIONED);
+ return partitioned_reloptions(reloptions, validate);
default:
/* other relkinds are not supported */
return NULL;
}
}
-
/*
* Parse options for indexes.
*
diff --git a/src/backend/access/hash/hashpage.c b/src/backend/access/hash/hashpage.c
index 6825c14..e5a6dd8 100644
--- a/src/backend/access/hash/hashpage.c
+++ b/src/backend/access/hash/hashpage.c
@@ -369,7 +369,7 @@ _hash_init(Relation rel, double num_tuples, ForkNumber forkNum)
data_width = sizeof(uint32);
item_width = MAXALIGN(sizeof(IndexTupleData)) + MAXALIGN(data_width) +
sizeof(ItemIdData); /* include the line pointer */
- ffactor = RelationGetTargetPageUsage(rel, HASH_DEFAULT_FILLFACTOR) / item_width;
+ ffactor = (BLCKSZ * HashGetFillFactor(rel) / 100) / item_width;
/* keep to a sane range */
if (ffactor < 10)
ffactor = 10;
diff --git a/src/backend/access/hash/hashutil.c b/src/backend/access/hash/hashutil.c
index 7c9b2cf..8cc1bd1 100644
--- a/src/backend/access/hash/hashutil.c
+++ b/src/backend/access/hash/hashutil.c
@@ -289,7 +289,28 @@ _hash_checkpage(Relation rel, Buffer buf, int flags)
bytea *
hashoptions(Datum reloptions, bool validate)
{
- return default_reloptions(reloptions, validate, RELOPT_KIND_HASH);
+ relopt_value *options;
+ HashRelOptions *rdopts;
+ int numoptions;
+ static const relopt_parse_elt tab[] = {
+ {"fillfactor", RELOPT_TYPE_INT, offsetof(HashRelOptions, fillfactor)},
+ };
+
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_HASH,
+ &numoptions);
+
+ /* if none set, we're done */
+ if (numoptions == 0)
+ return NULL;
+
+ rdopts = allocateReloptStruct(sizeof(HashRelOptions), options, numoptions);
+
+ fillRelOptions((void *) rdopts, sizeof(HashRelOptions), options, numoptions,
+ validate, tab, lengthof(tab));
+
+ pfree(options);
+
+ return (bytea *) rdopts;
}
/*
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 9650145..f60321d 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2712,8 +2712,10 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
AssertArg(!(options & HEAP_INSERT_NO_LOGICAL));
needwal = !(options & HEAP_INSERT_SKIP_WAL) && RelationNeedsWAL(relation);
- saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
- HEAP_DEFAULT_FILLFACTOR);
+ if (IsToastRelation(relation))
+ saveFreeSpace = ToastGetTargetPageFreeSpace();
+ else
+ saveFreeSpace = HeapGetTargetPageFreeSpace(relation);
/* Toast and set header data in all the tuples */
heaptuples = palloc(ntuples * sizeof(HeapTuple));
diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index b8b5871..1574c0c 100644
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -19,6 +19,7 @@
#include "access/hio.h"
#include "access/htup_details.h"
#include "access/visibilitymap.h"
+#include "catalog/catalog.h"
#include "storage/bufmgr.h"
#include "storage/freespace.h"
#include "storage/lmgr.h"
@@ -339,8 +340,10 @@ RelationGetBufferForTuple(Relation relation, Size len,
len, MaxHeapTupleSize)));
/* Compute desired extra freespace due to fillfactor option */
- saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
- HEAP_DEFAULT_FILLFACTOR);
+ if (IsToastRelation(relation))
+ saveFreeSpace = ToastGetTargetPageFreeSpace();
+ else
+ saveFreeSpace = HeapGetTargetPageFreeSpace(relation);
if (otherBuffer != InvalidBuffer)
otherBlock = BufferGetBlockNumber(otherBuffer);
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index c2f5343..cc0b112 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -130,8 +130,11 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
* important than sometimes getting a wrong answer in what is after all
* just a heuristic estimate.
*/
- minfree = RelationGetTargetPageFreeSpace(relation,
- HEAP_DEFAULT_FILLFACTOR);
+
+ if (IsToastRelation(relation))
+ minfree = ToastGetTargetPageFreeSpace();
+ else
+ minfree = HeapGetTargetPageFreeSpace(relation);
minfree = Max(minfree, BLCKSZ / 10);
if (PageIsFull(page) || PageGetHeapFreeSpace(page) < minfree)
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 44caeca..3c5b033 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -683,8 +683,10 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
len, MaxHeapTupleSize)));
/* Compute desired extra freespace due to fillfactor option */
- saveFreeSpace = RelationGetTargetPageFreeSpace(state->rs_new_rel,
- HEAP_DEFAULT_FILLFACTOR);
+ if (IsToastRelation(state->rs_new_rel))
+ saveFreeSpace = ToastGetTargetPageFreeSpace();
+ else
+ saveFreeSpace = HeapGetTargetPageFreeSpace(state->rs_new_rel);
/* Now we can check to see if there's enough free space already. */
if (state->rs_buffer_valid)
diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c
index d26c081..1f83601 100644
--- a/src/backend/access/heap/tuptoaster.c
+++ b/src/backend/access/heap/tuptoaster.c
@@ -725,7 +725,7 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
hoff += BITMAPLEN(numAttrs);
hoff = MAXALIGN(hoff);
/* now convert to a limit on the tuple data size */
- maxDataLen = RelationGetToastTupleTarget(rel, TOAST_TUPLE_TARGET) - hoff;
+ maxDataLen = HeapGetToastTupleTarget(rel, TOAST_TUPLE_TARGET) - hoff;
/*
* Look for attributes with attstorage 'x' to compress. Also find large
diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c
index a5915a5..c8b0af3 100644
--- a/src/backend/access/nbtree/nbtinsert.c
+++ b/src/backend/access/nbtree/nbtinsert.c
@@ -1607,8 +1607,7 @@ _bt_findsplitloc(Relation rel,
state.is_rightmost = P_RIGHTMOST(opaque);
state.have_split = false;
if (state.is_leaf)
- state.fillfactor = RelationGetFillFactor(rel,
- BTREE_DEFAULT_FILLFACTOR);
+ state.fillfactor = BTGetFillFactor(rel);
else
state.fillfactor = BTREE_NONLEAF_FILLFACTOR;
state.newitemonleft = false; /* these just to keep compiler quiet */
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index e8725fb..958b801 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -814,7 +814,7 @@ _bt_vacuum_needs_cleanup(IndexVacuumInfo *info)
}
else
{
- StdRdOptions *relopts;
+ BTRelOptions *relopts;
float8 cleanup_scale_factor;
float8 prev_num_heap_tuples;
@@ -825,7 +825,7 @@ _bt_vacuum_needs_cleanup(IndexVacuumInfo *info)
* tuples exceeds vacuum_cleanup_index_scale_factor fraction of
* original tuples count.
*/
- relopts = (StdRdOptions *) info->index->rd_options;
+ relopts = (BTRelOptions *) info->index->rd_options;
cleanup_scale_factor = (relopts &&
relopts->vacuum_cleanup_index_scale_factor >= 0)
? relopts->vacuum_cleanup_index_scale_factor
diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index 16f5755..f8c4fb4 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -681,8 +681,9 @@ _bt_pagestate(BTWriteState *wstate, uint32 level)
if (level > 0)
state->btps_full = (BLCKSZ * (100 - BTREE_NONLEAF_FILLFACTOR) / 100);
else
- state->btps_full = RelationGetTargetPageFreeSpace(wstate->index,
- BTREE_DEFAULT_FILLFACTOR);
+ state->btps_full = (BLCKSZ * (100 - BTGetFillFactor(wstate->index))
+ / 100);
+
/* no parent level, yet */
state->btps_next = NULL;
diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c
index 205457e..11a1de3 100644
--- a/src/backend/access/nbtree/nbtutils.c
+++ b/src/backend/access/nbtree/nbtutils.c
@@ -2053,7 +2053,31 @@ BTreeShmemInit(void)
bytea *
btoptions(Datum reloptions, bool validate)
{
- return default_reloptions(reloptions, validate, RELOPT_KIND_BTREE);
+ relopt_value *options;
+ BTRelOptions *rdopts;
+ int numoptions;
+ static const relopt_parse_elt tab[] = {
+ {"fillfactor", RELOPT_TYPE_INT, offsetof(BTRelOptions, fillfactor)},
+ {"vacuum_cleanup_index_scale_factor", RELOPT_TYPE_REAL,
+ offsetof(BTRelOptions, vacuum_cleanup_index_scale_factor)}
+
+ };
+
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_BTREE,
+ &numoptions);
+
+ /* if none set, we're done */
+ if (numoptions == 0)
+ return NULL;
+
+ rdopts = allocateReloptStruct(sizeof(BTRelOptions), options, numoptions);
+
+ fillRelOptions((void *) rdopts, sizeof(BTRelOptions), options, numoptions,
+ validate, tab, lengthof(tab));
+
+ pfree(options);
+
+ return (bytea *) rdopts;
}
/*
diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c
index 9919e6f..44bdc5b 100644
--- a/src/backend/access/spgist/spgutils.c
+++ b/src/backend/access/spgist/spgutils.c
@@ -411,8 +411,7 @@ SpGistGetBuffer(Relation index, int flags, int needSpace, bool *isNew)
* related to the ones already on it. But fillfactor mustn't cause an
* error for requests that would otherwise be legal.
*/
- needSpace += RelationGetTargetPageFreeSpace(index,
- SPGIST_DEFAULT_FILLFACTOR);
+ needSpace += (BLCKSZ * (100 - SpGistGetFillFactor(index)) / 100);
needSpace = Min(needSpace, SPGIST_PAGE_CAPACITY);
/* Get the cache entry for this flags setting */
@@ -589,7 +588,29 @@ SpGistInitMetapage(Page page)
bytea *
spgoptions(Datum reloptions, bool validate)
{
- return default_reloptions(reloptions, validate, RELOPT_KIND_SPGIST);
+ relopt_value *options;
+ SpGistRelOptions *rdopts;
+ int numoptions;
+ static const relopt_parse_elt tab[] = {
+ {"fillfactor", RELOPT_TYPE_INT, offsetof(SpGistRelOptions, fillfactor)},
+ };
+
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_SPGIST,
+ &numoptions);
+
+ /* if none set, we're done */
+ if (numoptions == 0)
+ return NULL;
+
+ rdopts = allocateReloptStruct(sizeof(SpGistRelOptions), options,
+ numoptions);
+
+ fillRelOptions((void *) rdopts, sizeof(SpGistRelOptions), options,
+ numoptions, validate, tab, lengthof(tab));
+
+ pfree(options);
+
+ return (bytea *) rdopts;
}
/*
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index d01b258..d54f9e4 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -128,7 +128,7 @@ create_ctas_internal(List *attrList, IntoClause *into)
validnsps,
true, false);
- (void) heap_reloptions(RELKIND_TOASTVALUE, toast_options, true);
+ (void) relation_reloptions(RELKIND_TOASTVALUE, toast_options, true);
NewRelationCreateToastTable(intoRelationAddr.objectId, toast_options);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index c8c50e8..32c00ff 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -685,7 +685,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
if (relkind == RELKIND_VIEW)
(void) view_reloptions(reloptions, true);
else
- (void) heap_reloptions(relkind, reloptions, true);
+ (void) relation_reloptions(relkind, reloptions, true);
if (stmt->ofTypename)
{
@@ -4406,7 +4406,7 @@ ATRewriteTables(AlterTableStmt *parsetree, List **wqueue, LOCKMODE lockmode)
errmsg("cannot rewrite system relation \"%s\"",
RelationGetRelationName(OldHeap))));
- if (RelationIsUsedAsCatalogTable(OldHeap))
+ if (HeapIsUsedAsCatalogTable(OldHeap))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("cannot rewrite table \"%s\" used as a catalog table",
@@ -10674,7 +10674,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
case RELKIND_TOASTVALUE:
case RELKIND_MATVIEW:
case RELKIND_PARTITIONED_TABLE:
- (void) heap_reloptions(rel->rd_rel->relkind, newOptions, true);
+ (void) relation_reloptions(rel->rd_rel->relkind, newOptions, true);
break;
case RELKIND_VIEW:
(void) view_reloptions(newOptions, true);
@@ -10783,7 +10783,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
defList, "toast", validnsps, false,
operation == AT_ResetRelOptions);
- (void) heap_reloptions(RELKIND_TOASTVALUE, newOptions, true);
+ (void) relation_reloptions(RELKIND_TOASTVALUE, newOptions, true);
memset(repl_val, 0, sizeof(repl_val));
memset(repl_null, false, sizeof(repl_null));
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 5790b62..c8c0f79 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -146,7 +146,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
&rel->pages, &rel->tuples, &rel->allvisfrac);
/* Retrieve the parallel_workers reloption, or -1 if not set. */
- rel->rel_parallel_workers = RelationGetParallelWorkers(relation, -1);
+ rel->rel_parallel_workers = HeapGetParallelWorkers(relation, -1);
/*
* Make list of indexes. Ignore indexes on system catalogs if told to.
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 2099724..65ac3d6 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -3145,7 +3145,7 @@ transformOnConflictArbiter(ParseState *pstate,
exprLocation((Node *) onConflictClause))));
/* Same applies to table used by logical decoding as catalog table */
- if (RelationIsUsedAsCatalogTable(pstate->p_target_relation))
+ if (HeapIsUsedAsCatalogTable(pstate->p_target_relation))
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("ON CONFLICT is not supported on table \"%s\" used as a catalog table",
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 2d5086d..b605edd 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2720,6 +2720,7 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
{
bytea *relopts;
AutoVacOpts *av;
+ AutoVacOpts *src;
Assert(((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_RELATION ||
((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_MATVIEW ||
@@ -2729,8 +2730,13 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
if (relopts == NULL)
return NULL;
+ if (((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_TOASTVALUE)
+ src = &(((ToastRelOptions *) relopts)->autovacuum);
+ else
+ src = &(((HeapRelOptions *) relopts)->autovacuum);
+
av = palloc(sizeof(AutoVacOpts));
- memcpy(av, &(((StdRdOptions *) relopts)->autovacuum), sizeof(AutoVacOpts));
+ memcpy(av, src, sizeof(AutoVacOpts));
pfree(relopts);
return av;
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 43815d2..28dd02e 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -1591,7 +1591,7 @@ ApplyRetrieveRule(Query *parsetree,
rte->rtekind = RTE_SUBQUERY;
rte->subquery = rule_action;
- rte->security_barrier = RelationIsSecurityView(relation);
+ rte->security_barrier = ViewIsSecurityView(relation);
/* Clear fields that should not be set in a subquery RTE */
rte->relid = InvalidOid;
rte->relkind = 0;
@@ -3178,7 +3178,7 @@ rewriteTargetView(Query *parsetree, Relation view)
ChangeVarNodes(viewqual, base_rt_index, new_rt_index, 0);
- if (RelationIsSecurityView(view))
+ if (ViewIsSecurityView(view))
{
/*
* The view's quals go in front of existing barrier quals: those
@@ -3214,8 +3214,8 @@ rewriteTargetView(Query *parsetree, Relation view)
*/
if (parsetree->commandType != CMD_DELETE)
{
- bool has_wco = RelationHasCheckOption(view);
- bool cascaded = RelationHasCascadedCheckOption(view);
+ bool has_wco = ViewHasCheckOption(view);
+ bool cascaded = ViewHasCascadedCheckOption(view);
/*
* If the parent view has a cascaded check option, treat this view as
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 970c94e..1b28a6a 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1024,7 +1024,7 @@ ProcessUtilitySlow(ParseState *pstate,
validnsps,
true,
false);
- (void) heap_reloptions(RELKIND_TOASTVALUE,
+ (void) relation_reloptions(RELKIND_TOASTVALUE,
toast_options,
true);
diff --git a/src/include/access/hash.h b/src/include/access/hash.h
index 02ef67c..e6b9208 100644
--- a/src/include/access/hash.h
+++ b/src/include/access/hash.h
@@ -274,6 +274,17 @@ typedef struct HashMetaPageData
typedef HashMetaPageData *HashMetaPage;
+typedef struct HashRelOptions
+{
+ int32 varlena_header_; /* varlena header (do not touch directly!) */
+ int fillfactor; /* page fill factor in percent (0..100) */
+} HashRelOptions;
+
+#define HashGetFillFactor(relation) \
+ ((relation)->rd_options ? \
+ ((HashRelOptions *) (relation)->rd_options)->fillfactor : \
+ HASH_DEFAULT_FILLFACTOR)
+
/*
* Maximum size of a hash index item (it's okay to have only one per page)
*/
diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index ea495f1..d7066d0 100644
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -488,6 +488,19 @@ typedef BTScanOpaqueData *BTScanOpaque;
#define SK_BT_DESC (INDOPTION_DESC << SK_BT_INDOPTION_SHIFT)
#define SK_BT_NULLS_FIRST (INDOPTION_NULLS_FIRST << SK_BT_INDOPTION_SHIFT)
+typedef struct BTRelOptions
+{
+ int32 varlena_header_; /* varlena header (do not touch directly!) */
+ int fillfactor; /* page fill factor in percent (0..100) */
+ /* fraction of newly inserted tuples prior to trigger index cleanup */
+ float8 vacuum_cleanup_index_scale_factor;
+} BTRelOptions;
+
+#define BTGetFillFactor(relation) \
+ ((relation)->rd_options ? \
+ ((BTRelOptions *) (relation)->rd_options)->fillfactor : \
+ BTREE_DEFAULT_FILLFACTOR)
+
/*
* external entry points for btree, in nbtree.c
*/
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index 50690b9..6d309b3 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -271,9 +271,11 @@ extern void fillRelOptions(void *rdopts, Size basesize,
bool validate,
const relopt_parse_elt *elems, int nelems);
-extern bytea *default_reloptions(Datum reloptions, bool validate,
- relopt_kind kind);
-extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
+extern bytea *toast_reloptions(Datum reloptions, bool validate);
+extern bytea *partitioned_reloptions(Datum reloptions, bool validate);
+extern bytea *heap_reloptions(Datum reloptions, bool validate);
+extern bytea *relation_reloptions(char relkind, Datum reloptions,
+ bool validate);
extern bytea *view_reloptions(Datum reloptions, bool validate);
extern bytea *index_reloptions(amoptions_function amoptions, Datum reloptions,
bool validate);
diff --git a/src/include/access/spgist.h b/src/include/access/spgist.h
index 9c19e9e..4c57a0a 100644
--- a/src/include/access/spgist.h
+++ b/src/include/access/spgist.h
@@ -20,10 +20,6 @@
#include "lib/stringinfo.h"
-/* reloption parameters */
-#define SPGIST_MIN_FILLFACTOR 10
-#define SPGIST_DEFAULT_FILLFACTOR 80
-
/* SPGiST opclass support function numbers */
#define SPGIST_CONFIG_PROC 1
#define SPGIST_CHOOSE_PROC 2
diff --git a/src/include/access/spgist_private.h b/src/include/access/spgist_private.h
index d23862e..a567603 100644
--- a/src/include/access/spgist_private.h
+++ b/src/include/access/spgist_private.h
@@ -22,6 +22,17 @@
#include "utils/relcache.h"
+typedef struct SpGistRelOptions
+{
+ int32 varlena_header_; /* varlena header (do not touch directly!) */
+ int fillfactor; /* page fill factor in percent (0..100) */
+} SpGistRelOptions;
+
+#define SpGistGetFillFactor(relation) \
+ ((relation)->rd_options ? \
+ ((SpGistRelOptions *) (relation)->rd_options)->fillfactor : \
+ SPGIST_DEFAULT_FILLFACTOR)
+
/* Page numbers of fixed-location pages */
#define SPGIST_METAPAGE_BLKNO (0) /* metapage */
#define SPGIST_ROOT_BLKNO (1) /* root for normal entries */
@@ -417,6 +428,11 @@ typedef SpGistDeadTupleData *SpGistDeadTuple;
#define GBUF_REQ_NULLS(flags) ((flags) & GBUF_NULLS)
/* spgutils.c */
+
+/* reloption parameters */
+#define SPGIST_MIN_FILLFACTOR 10
+#define SPGIST_DEFAULT_FILLFACTOR 80
+
extern SpGistCache *spgGetCache(Relation index);
extern void initSpGistState(SpGistState *state, Relation index);
extern Buffer SpGistNewBuffer(Relation index);
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 2217081..3af3027 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -253,71 +253,93 @@ typedef struct AutoVacOpts
float8 analyze_scale_factor;
} AutoVacOpts;
-typedef struct StdRdOptions
+typedef struct HeapRelOptions
{
int32 vl_len_; /* varlena header (do not touch directly!) */
int fillfactor; /* page fill factor in percent (0..100) */
- /* fraction of newly inserted tuples prior to trigger index cleanup */
- float8 vacuum_cleanup_index_scale_factor;
int toast_tuple_target; /* target for tuple toasting */
AutoVacOpts autovacuum; /* autovacuum-related options */
bool user_catalog_table; /* use as an additional catalog relation */
int parallel_workers; /* max number of parallel workers */
-} StdRdOptions;
+} HeapRelOptions;
+
+typedef struct ToastRelOptions
+{
+ int32 vl_len_; /* varlena header (do not touch directly!) */
+ AutoVacOpts autovacuum; /* autovacuum-related options */
+} ToastRelOptions;
+
+typedef struct PartitionedRelOptions
+{
+ int32 vl_len_; /* varlena header (do not touch directly!) */
+ /* No options defined yet */
+} PartitionedRelOptions;
#define HEAP_MIN_FILLFACTOR 10
#define HEAP_DEFAULT_FILLFACTOR 100
+#define TOAST_DEFAULT_FILLFACTOR 100 /* Only default is actually used */
+
/*
- * RelationGetToastTupleTarget
- * Returns the relation's toast_tuple_target. Note multiple eval of argument!
+ * HeapGetToastTupleTarget
+ * Returns the heap's toast_tuple_target. Note multiple eval of argument!
*/
-#define RelationGetToastTupleTarget(relation, defaulttarg) \
+#define HeapGetToastTupleTarget(relation, defaulttarg) \
((relation)->rd_options ? \
- ((StdRdOptions *) (relation)->rd_options)->toast_tuple_target : (defaulttarg))
+ ((HeapRelOptions *) (relation)->rd_options)->toast_tuple_target : \
+ (defaulttarg))
/*
- * RelationGetFillFactor
- * Returns the relation's fillfactor. Note multiple eval of argument!
+ * HeapGetFillFactor
+ * Returns the heap's fillfactor. Note multiple eval of argument!
*/
-#define RelationGetFillFactor(relation, defaultff) \
+#define HeapGetFillFactor(relation) \
((relation)->rd_options ? \
- ((StdRdOptions *) (relation)->rd_options)->fillfactor : (defaultff))
+ ((HeapRelOptions *) (relation)->rd_options)->fillfactor : \
+ (HEAP_DEFAULT_FILLFACTOR))
/*
- * RelationGetTargetPageUsage
- * Returns the relation's desired space usage per page in bytes.
+ * HeapGetTargetPageUsage
+ * Returns the heap's desired space usage per page in bytes.
*/
-#define RelationGetTargetPageUsage(relation, defaultff) \
- (BLCKSZ * RelationGetFillFactor(relation, defaultff) / 100)
+#define HeapGetTargetPageUsage(relation) \
+ (BLCKSZ * HeapGetFillFactor(relation) / 100)
/*
- * RelationGetTargetPageFreeSpace
- * Returns the relation's desired freespace per page in bytes.
+ * HeapGetTargetPageFreeSpace
+ * Returns the heap's desired freespace per page in bytes.
*/
-#define RelationGetTargetPageFreeSpace(relation, defaultff) \
- (BLCKSZ * (100 - RelationGetFillFactor(relation, defaultff)) / 100)
+#define HeapGetTargetPageFreeSpace(relation) \
+ (BLCKSZ * (100 - HeapGetFillFactor(relation)) / 100)
/*
- * RelationIsUsedAsCatalogTable
- * Returns whether the relation should be treated as a catalog table
+ * HeapIsUsedAsCatalogTable
+ * Returns whether the heap should be treated as a catalog table
* from the pov of logical decoding. Note multiple eval of argument!
*/
-#define RelationIsUsedAsCatalogTable(relation) \
+#define HeapIsUsedAsCatalogTable(relation) \
((relation)->rd_options && \
((relation)->rd_rel->relkind == RELKIND_RELATION || \
(relation)->rd_rel->relkind == RELKIND_MATVIEW) ? \
- ((StdRdOptions *) (relation)->rd_options)->user_catalog_table : false)
+ ((HeapRelOptions *) (relation)->rd_options)->user_catalog_table : false)
/*
- * RelationGetParallelWorkers
- * Returns the relation's parallel_workers reloption setting.
+ * HeapGetParallelWorkers
+ * Returns the heap's parallel_workers reloption setting.
* Note multiple eval of argument!
*/
-#define RelationGetParallelWorkers(relation, defaultpw) \
+#define HeapGetParallelWorkers(relation, defaultpw) \
((relation)->rd_options ? \
- ((StdRdOptions *) (relation)->rd_options)->parallel_workers : (defaultpw))
+ ((HeapRelOptions *) (relation)->rd_options)->parallel_workers : \
+ (defaultpw))
+/*
+ * ToastGetTargetPageFreeSpace
+ * Returns the TOAST relation's desired freespace per page in bytes.
+ * Always calculated using default fillfactor value.
+ */
+#define ToastGetTargetPageFreeSpace() \
+ (BLCKSZ * (100 - TOAST_DEFAULT_FILLFACTOR) / 100)
/*
* ViewOptions
@@ -331,29 +353,29 @@ typedef struct ViewOptions
} ViewOptions;
/*
- * RelationIsSecurityView
- * Returns whether the relation is security view, or not. Note multiple
+ * ViewIsSecurityView
+ * Returns whether the view is security view, or not. Note multiple
* eval of argument!
*/
-#define RelationIsSecurityView(relation) \
+#define ViewIsSecurityView(relation) \
((relation)->rd_options ? \
((ViewOptions *) (relation)->rd_options)->security_barrier : false)
/*
- * RelationHasCheckOption
- * Returns true if the relation is a view defined with either the local
+ * ViewHasCheckOption
+ * Returns true if the view is defined with either the local
* or the cascaded check option. Note multiple eval of argument!
*/
-#define RelationHasCheckOption(relation) \
+#define ViewHasCheckOption(relation) \
((relation)->rd_options && \
((ViewOptions *) (relation)->rd_options)->check_option_offset != 0)
/*
- * RelationHasLocalCheckOption
- * Returns true if the relation is a view defined with the local check
+ * ViewHasLocalCheckOption
+ * Returns true if the view is defined with the local check
* option. Note multiple eval of argument!
*/
-#define RelationHasLocalCheckOption(relation) \
+#define ViewHasLocalCheckOption(relation) \
((relation)->rd_options && \
((ViewOptions *) (relation)->rd_options)->check_option_offset != 0 ? \
strcmp((char *) (relation)->rd_options + \
@@ -361,11 +383,11 @@ typedef struct ViewOptions
"local") == 0 : false)
/*
- * RelationHasCascadedCheckOption
- * Returns true if the relation is a view defined with the cascaded check
+ * ViewHasCascadedCheckOption
+ * Returns true if the view is defined with the cascaded check
* option. Note multiple eval of argument!
*/
-#define RelationHasCascadedCheckOption(relation) \
+#define ViewHasCascadedCheckOption(relation) \
((relation)->rd_options && \
((ViewOptions *) (relation)->rd_options)->check_option_offset != 0 ? \
strcmp((char *) (relation)->rd_options + \
@@ -564,7 +586,7 @@ typedef struct ViewOptions
#define RelationIsAccessibleInLogicalDecoding(relation) \
(XLogLogicalInfoActive() && \
RelationNeedsWAL(relation) && \
- (IsCatalogRelation(relation) || RelationIsUsedAsCatalogTable(relation)))
+ (IsCatalogRelation(relation) || HeapIsUsedAsCatalogTable(relation)))
/*
* RelationIsLogicallyLogged
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead
2019-01-01 20:21 Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
@ 2019-01-02 03:05 ` Alvaro Herrera <[email protected]>
2019-01-02 09:58 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Alvaro Herrera @ 2019-01-02 03:05 UTC (permalink / raw)
To: Nikolay Shaplov <[email protected]>; +Cc: [email protected]; Dmitry Dolgov <[email protected]>
One thing I would like to revise here is to avoid unnecessary API change
-- for example the RelationHasCascadedCheckOption macro does not really
need to be renamed because it only applies to views, so there's no
possible conflict with other relation types. We can keep the original
name and add a StaticAssert that the given relation is indeed a view.
This should keep churn somewhat low. Of course, this doesn't work for
some options where you need a different one for different relation
kinds, such as fillfactor, but that's unavoidable.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead
2019-01-01 20:21 Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
2019-01-02 03:05 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
@ 2019-01-02 09:58 ` Nikolay Shaplov <[email protected]>
2019-01-03 19:10 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Nikolay Shaplov @ 2019-01-02 09:58 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: [email protected]; Dmitry Dolgov <[email protected]>
В письме от среда, 2 января 2019 г. 0:05:10 MSK пользователь Alvaro Herrera
написал:
> One thing I would like to revise here is to avoid unnecessary API change
> -- for example the RelationHasCascadedCheckOption macro does not really
> need to be renamed because it only applies to views, so there's no
> possible conflict with other relation types. We can keep the original
> name and add a StaticAssert that the given relation is indeed a view.
> This should keep churn somewhat low. Of course, this doesn't work for
> some options where you need a different one for different relation
> kinds, such as fillfactor, but that's unavoidable.
My intention was to make API names more consistent. If you are addressing View
specific option, it is good to address it via View[Something] macros or
function. Relations[Something] seems to be a bad name, since we are dealing
not with any relation in general, but with a view relation.
This is internal API, right? If we change it everywhere, then it is changed
and nothing will be broken?
May be it may lead to problems I am unable to see, but I still unable to see
them so I can't make any judgment about it.
If you insist (you have much more experience than I) I would do as you advise,
but still I do not understand.
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead
2019-01-01 20:21 Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
2019-01-02 03:05 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
2019-01-02 09:58 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
@ 2019-01-03 19:10 ` Alvaro Herrera <[email protected]>
2019-01-03 19:22 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Alvaro Herrera @ 2019-01-03 19:10 UTC (permalink / raw)
To: Nikolay Shaplov <[email protected]>; +Cc: [email protected]; Dmitry Dolgov <[email protected]>
On 2019-Jan-02, Nikolay Shaplov wrote:
> This is internal API, right? If we change it everywhere, then it is changed
> and nothing will be broken?
No, it's exported for extensions to use. If we change it unnecessarily,
extension authors will hate me (not you) for breaking the compile and
requiring an #if VERSION patch.
These may not be in common usage, but I don't need any additional
reasons for people to hate me.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead
2019-01-01 20:21 Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
2019-01-02 03:05 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
2019-01-02 09:58 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
2019-01-03 19:10 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
@ 2019-01-03 19:22 ` Nikolay Shaplov <[email protected]>
2019-01-03 20:15 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Nikolay Shaplov @ 2019-01-03 19:22 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: [email protected]; Dmitry Dolgov <[email protected]>
В письме от четверг, 3 января 2019 г. 16:10:20 MSK пользователь Alvaro Herrera
написал:
> On 2019-Jan-02, Nikolay Shaplov wrote:
> > This is internal API, right? If we change it everywhere, then it is
> > changed and nothing will be broken?
>
> No, it's exported for extensions to use. If we change it unnecessarily,
> extension authors will hate me (not you) for breaking the compile and
> requiring an #if VERSION patch.
Ok, that's a good reason...
Can we think about backward compatibility aliases?
#define ViewHasCheckOption(relation) \
((relation)->rd_options && \
((ViewOptions *) (relation)->rd_options)->check_option_offset != 0)
/* Alias for backward compatibility */
#define RelationHasCheckOption(relation) ViewHasCheckOption(relation)
And keep them for as log as needed to avoid #if VERSION in thirdparty code?
Or that is not the case?
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead
2019-01-01 20:21 Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
2019-01-02 03:05 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
2019-01-02 09:58 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
2019-01-03 19:10 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
2019-01-03 19:22 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
@ 2019-01-03 20:15 ` Alvaro Herrera <[email protected]>
2019-01-07 16:26 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Alvaro Herrera @ 2019-01-03 20:15 UTC (permalink / raw)
To: Nikolay Shaplov <[email protected]>; +Cc: [email protected]; Dmitry Dolgov <[email protected]>
On 2019-Jan-03, Nikolay Shaplov wrote:
> Can we think about backward compatibility aliases?
>
> #define ViewHasCheckOption(relation) \
> ((relation)->rd_options && \
> ((ViewOptions *) (relation)->rd_options)->check_option_offset != 0)
>
> /* Alias for backward compatibility */
> #define RelationHasCheckOption(relation) ViewHasCheckOption(relation)
>
> And keep them for as log as needed to avoid #if VERSION in thirdparty code?
Well, if you do this, at some point you need to tell the extension
author to change the code. Or they can just keep the code unchanged
forever. I don't think it's worth the bother.
I would have liked to get a StaticAssert in the definition, but I don't
think it's possible. A standard Assert() should be possible, though.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead
2019-01-01 20:21 Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
2019-01-02 03:05 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
2019-01-02 09:58 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
2019-01-03 19:10 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
2019-01-03 19:22 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
2019-01-03 20:15 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
@ 2019-01-07 16:26 ` Nikolay Shaplov <[email protected]>
2019-01-17 23:33 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Nikolay Shaplov @ 2019-01-07 16:26 UTC (permalink / raw)
To: [email protected]; +Cc: Alvaro Herrera <[email protected]>; Dmitry Dolgov <[email protected]>
В письме от четверг, 3 января 2019 г. 17:15:08 MSK пользователь Alvaro Herrera
написал:
> > Can we think about backward compatibility aliases?
.....
> > And keep them for as log as needed to avoid #if VERSION in thirdparty
> > code?
> Well, if you do this, at some point you need to tell the extension
> author to change the code. Or they can just keep the code unchanged
> forever. I don't think it's worth the bother.
Ok, you are the Boss ;-)
I've reverted all the macros names back to Relation* except those related to
fillfactor.
> I would have liked to get a StaticAssert in the definition, but I don't
> think it's possible. A standard Assert() should be possible, though.
This is a really good idea. I felt uncomfortable with this (ViewOptions *)
type convertation without checking that it is really View. With Assert I fell
much more safe.
I've added AssertMacro to all options related macroses.
And so, adding asserts discovered a bug with parallel_workers options. That
are defined as Heap only option, but in get_relation_info in src/backend/
optimizer/util/plancat.c a RelationGetParallelWorkers macros is also called
for toasts and other kinds of relations.
I've started a new thread dedicated to this issue, since it needs to be fixed
regardless to this patch.
And for now I added here a hotfix for this issue that is good enough for now.
I also reworked some comments. BTW do you know what is this comments spoke
about:
* RelationGetFillFactor() and RelationGetTargetPageFreeSpace() can only
* be applied to relations that use this format or a superset for
* private options data.
It is speaking about some old times when there can be some other type of
options? 'cause I do not understand what it is speaking about.
I've removed it, I hope I did not remove anything important.
Attachments:
[text/x-patch] get-rid-of-StrRdOptions_3.diff (35.5K, ../../20909571.QVcMXUKARm@x200m/2-get-rid-of-StrRdOptions_3.diff)
download | inline diff:
diff --git a/.gitignore b/.gitignore
index 794e35b..2dfbbe1 100644
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index eece89a..833d084 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -22,7 +22,7 @@
#include "access/htup_details.h"
#include "access/nbtree.h"
#include "access/reloptions.h"
-#include "access/spgist.h"
+#include "access/spgist_private.h"
#include "access/tuptoaster.h"
#include "catalog/pg_type.h"
#include "commands/defrem.h"
@@ -46,9 +46,8 @@
* upper and lower bounds (if applicable); for strings, consider a validation
* routine.
* (ii) add a record below (or use add_<type>_reloption).
- * (iii) add it to the appropriate options struct (perhaps StdRdOptions)
- * (iv) add it to the appropriate handling routine (perhaps
- * default_reloptions)
+ * (iii) add it to the appropriate options struct
+ * (iv) add it to the appropriate handling routine
* (v) make sure the lock level is set correctly for that operation
* (vi) don't forget to document the option
*
@@ -1019,7 +1018,7 @@ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
case RELKIND_TOASTVALUE:
case RELKIND_MATVIEW:
case RELKIND_PARTITIONED_TABLE:
- options = heap_reloptions(classForm->relkind, datum, false);
+ options = relation_reloptions(classForm->relkind, datum, false);
break;
case RELKIND_VIEW:
options = view_reloptions(datum, false);
@@ -1352,63 +1351,69 @@ fillRelOptions(void *rdopts, Size basesize,
/*
- * Option parser for anything that uses StdRdOptions.
+ * Option parsing definition for autovacuum. Used in toast and heap options.
+ */
+
+#define AUTOVACUUM_RELOPTIONS(OFFSET) \
+ {"autovacuum_enabled", RELOPT_TYPE_BOOL, \
+ OFFSET + offsetof(AutoVacOpts, enabled)}, \
+ {"autovacuum_vacuum_threshold", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, vacuum_threshold)}, \
+ {"autovacuum_analyze_threshold", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, analyze_threshold)}, \
+ {"autovacuum_vacuum_cost_delay", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, vacuum_cost_delay)}, \
+ {"autovacuum_vacuum_cost_limit", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, vacuum_cost_limit)}, \
+ {"autovacuum_freeze_min_age", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, freeze_min_age)}, \
+ {"autovacuum_freeze_max_age", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, freeze_max_age)}, \
+ {"autovacuum_freeze_table_age", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, freeze_table_age)}, \
+ {"autovacuum_multixact_freeze_min_age", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, multixact_freeze_min_age)}, \
+ {"autovacuum_multixact_freeze_max_age", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, multixact_freeze_max_age)}, \
+ {"autovacuum_multixact_freeze_table_age", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, multixact_freeze_table_age)}, \
+ {"log_autovacuum_min_duration", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, log_min_duration)}, \
+ {"autovacuum_vacuum_scale_factor", RELOPT_TYPE_REAL, \
+ OFFSET + offsetof(AutoVacOpts, vacuum_scale_factor)}, \
+ {"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL, \
+ OFFSET + offsetof(AutoVacOpts, analyze_scale_factor)}
+
+/*
+ * Option parser for heap
*/
bytea *
-default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
+heap_reloptions(Datum reloptions, bool validate)
{
relopt_value *options;
- StdRdOptions *rdopts;
+ HeapRelOptions *rdopts;
int numoptions;
static const relopt_parse_elt tab[] = {
- {"fillfactor", RELOPT_TYPE_INT, offsetof(StdRdOptions, fillfactor)},
- {"autovacuum_enabled", RELOPT_TYPE_BOOL,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, enabled)},
- {"autovacuum_vacuum_threshold", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_threshold)},
- {"autovacuum_analyze_threshold", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, analyze_threshold)},
- {"autovacuum_vacuum_cost_delay", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_delay)},
- {"autovacuum_vacuum_cost_limit", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_limit)},
- {"autovacuum_freeze_min_age", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_min_age)},
- {"autovacuum_freeze_max_age", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_max_age)},
- {"autovacuum_freeze_table_age", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_table_age)},
- {"autovacuum_multixact_freeze_min_age", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_min_age)},
- {"autovacuum_multixact_freeze_max_age", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_max_age)},
- {"autovacuum_multixact_freeze_table_age", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_table_age)},
- {"log_autovacuum_min_duration", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, log_min_duration)},
+ {"fillfactor", RELOPT_TYPE_INT, offsetof(HeapRelOptions, fillfactor)},
+ AUTOVACUUM_RELOPTIONS(offsetof(HeapRelOptions, autovacuum)),
{"toast_tuple_target", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, toast_tuple_target)},
- {"autovacuum_vacuum_scale_factor", RELOPT_TYPE_REAL,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_scale_factor)},
- {"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, analyze_scale_factor)},
+ offsetof(HeapRelOptions, toast_tuple_target)},
{"user_catalog_table", RELOPT_TYPE_BOOL,
- offsetof(StdRdOptions, user_catalog_table)},
+ offsetof(HeapRelOptions, user_catalog_table)},
{"parallel_workers", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, parallel_workers)},
- {"vacuum_cleanup_index_scale_factor", RELOPT_TYPE_REAL,
- offsetof(StdRdOptions, vacuum_cleanup_index_scale_factor)}
+ offsetof(HeapRelOptions, parallel_workers)}
};
- options = parseRelOptions(reloptions, validate, kind, &numoptions);
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_HEAP,
+ &numoptions);
/* if none set, we're done */
if (numoptions == 0)
return NULL;
- rdopts = allocateReloptStruct(sizeof(StdRdOptions), options, numoptions);
+ rdopts = allocateReloptStruct(sizeof(HeapRelOptions), options, numoptions);
- fillRelOptions((void *) rdopts, sizeof(StdRdOptions), options, numoptions,
+ fillRelOptions((void *) rdopts, sizeof(HeapRelOptions), options, numoptions,
validate, tab, lengthof(tab));
pfree(options);
@@ -1417,6 +1422,60 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
}
/*
+ * Option parser for toast
+ */
+bytea *
+toast_reloptions(Datum reloptions, bool validate)
+{
+ relopt_value *options;
+ ToastRelOptions *rdopts;
+ int numoptions;
+ static const relopt_parse_elt tab[] = {
+ AUTOVACUUM_RELOPTIONS(offsetof(ToastRelOptions, autovacuum)),
+ };
+
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_TOAST,
+ &numoptions);
+
+ /* if none set, we're done */
+ if (numoptions == 0)
+ return NULL;
+
+ rdopts = allocateReloptStruct(sizeof(ToastRelOptions), options, numoptions);
+
+ fillRelOptions((void *) rdopts, sizeof(ToastRelOptions), options,
+ numoptions, validate, tab, lengthof(tab));
+
+ /* adjust default-only parameters for TOAST relations */
+ rdopts->autovacuum.analyze_threshold = -1;
+ rdopts->autovacuum.analyze_scale_factor = -1;
+
+ pfree(options);
+
+ return (bytea *) rdopts;
+}
+
+/*
+ * Option parser for partitioned relations
+ */
+bytea *
+partitioned_reloptions(Datum reloptions, bool validate)
+{
+ int numoptions;
+
+ /*
+ * Since there is no options for patitioned table for now, we just do
+ * validation to report incorrect option error and leave.
+ */
+
+ if (validate)
+ parseRelOptions(reloptions, validate, RELOPT_KIND_PARTITIONED,
+ &numoptions);
+
+ return NULL;
+}
+
+/*
* Option parser for views
*/
bytea *
@@ -1449,39 +1508,26 @@ view_reloptions(Datum reloptions, bool validate)
}
/*
- * Parse options for heaps, views and toast tables.
+ * Parse options for heaps, toast, views and partitioned tables.
*/
bytea *
-heap_reloptions(char relkind, Datum reloptions, bool validate)
+relation_reloptions(char relkind, Datum reloptions, bool validate)
{
- StdRdOptions *rdopts;
-
switch (relkind)
{
case RELKIND_TOASTVALUE:
- rdopts = (StdRdOptions *)
- default_reloptions(reloptions, validate, RELOPT_KIND_TOAST);
- if (rdopts != NULL)
- {
- /* adjust default-only parameters for TOAST relations */
- rdopts->fillfactor = 100;
- rdopts->autovacuum.analyze_threshold = -1;
- rdopts->autovacuum.analyze_scale_factor = -1;
- }
- return (bytea *) rdopts;
+ return toast_reloptions(reloptions, validate);
case RELKIND_RELATION:
case RELKIND_MATVIEW:
- return default_reloptions(reloptions, validate, RELOPT_KIND_HEAP);
+ return heap_reloptions(reloptions, validate);
case RELKIND_PARTITIONED_TABLE:
- return default_reloptions(reloptions, validate,
- RELOPT_KIND_PARTITIONED);
+ return partitioned_reloptions(reloptions, validate);
default:
/* other relkinds are not supported */
return NULL;
}
}
-
/*
* Parse options for indexes.
*
diff --git a/src/backend/access/hash/hashpage.c b/src/backend/access/hash/hashpage.c
index 6825c14..e5a6dd8 100644
--- a/src/backend/access/hash/hashpage.c
+++ b/src/backend/access/hash/hashpage.c
@@ -369,7 +369,7 @@ _hash_init(Relation rel, double num_tuples, ForkNumber forkNum)
data_width = sizeof(uint32);
item_width = MAXALIGN(sizeof(IndexTupleData)) + MAXALIGN(data_width) +
sizeof(ItemIdData); /* include the line pointer */
- ffactor = RelationGetTargetPageUsage(rel, HASH_DEFAULT_FILLFACTOR) / item_width;
+ ffactor = (BLCKSZ * HashGetFillFactor(rel) / 100) / item_width;
/* keep to a sane range */
if (ffactor < 10)
ffactor = 10;
diff --git a/src/backend/access/hash/hashutil.c b/src/backend/access/hash/hashutil.c
index 7c9b2cf..8cc1bd1 100644
--- a/src/backend/access/hash/hashutil.c
+++ b/src/backend/access/hash/hashutil.c
@@ -289,7 +289,28 @@ _hash_checkpage(Relation rel, Buffer buf, int flags)
bytea *
hashoptions(Datum reloptions, bool validate)
{
- return default_reloptions(reloptions, validate, RELOPT_KIND_HASH);
+ relopt_value *options;
+ HashRelOptions *rdopts;
+ int numoptions;
+ static const relopt_parse_elt tab[] = {
+ {"fillfactor", RELOPT_TYPE_INT, offsetof(HashRelOptions, fillfactor)},
+ };
+
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_HASH,
+ &numoptions);
+
+ /* if none set, we're done */
+ if (numoptions == 0)
+ return NULL;
+
+ rdopts = allocateReloptStruct(sizeof(HashRelOptions), options, numoptions);
+
+ fillRelOptions((void *) rdopts, sizeof(HashRelOptions), options, numoptions,
+ validate, tab, lengthof(tab));
+
+ pfree(options);
+
+ return (bytea *) rdopts;
}
/*
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 9650145..f60321d 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2712,8 +2712,10 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
AssertArg(!(options & HEAP_INSERT_NO_LOGICAL));
needwal = !(options & HEAP_INSERT_SKIP_WAL) && RelationNeedsWAL(relation);
- saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
- HEAP_DEFAULT_FILLFACTOR);
+ if (IsToastRelation(relation))
+ saveFreeSpace = ToastGetTargetPageFreeSpace();
+ else
+ saveFreeSpace = HeapGetTargetPageFreeSpace(relation);
/* Toast and set header data in all the tuples */
heaptuples = palloc(ntuples * sizeof(HeapTuple));
diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index b8b5871..1574c0c 100644
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -19,6 +19,7 @@
#include "access/hio.h"
#include "access/htup_details.h"
#include "access/visibilitymap.h"
+#include "catalog/catalog.h"
#include "storage/bufmgr.h"
#include "storage/freespace.h"
#include "storage/lmgr.h"
@@ -339,8 +340,10 @@ RelationGetBufferForTuple(Relation relation, Size len,
len, MaxHeapTupleSize)));
/* Compute desired extra freespace due to fillfactor option */
- saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
- HEAP_DEFAULT_FILLFACTOR);
+ if (IsToastRelation(relation))
+ saveFreeSpace = ToastGetTargetPageFreeSpace();
+ else
+ saveFreeSpace = HeapGetTargetPageFreeSpace(relation);
if (otherBuffer != InvalidBuffer)
otherBlock = BufferGetBlockNumber(otherBuffer);
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index c2f5343..cc0b112 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -130,8 +130,11 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
* important than sometimes getting a wrong answer in what is after all
* just a heuristic estimate.
*/
- minfree = RelationGetTargetPageFreeSpace(relation,
- HEAP_DEFAULT_FILLFACTOR);
+
+ if (IsToastRelation(relation))
+ minfree = ToastGetTargetPageFreeSpace();
+ else
+ minfree = HeapGetTargetPageFreeSpace(relation);
minfree = Max(minfree, BLCKSZ / 10);
if (PageIsFull(page) || PageGetHeapFreeSpace(page) < minfree)
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 44caeca..3c5b033 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -683,8 +683,10 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
len, MaxHeapTupleSize)));
/* Compute desired extra freespace due to fillfactor option */
- saveFreeSpace = RelationGetTargetPageFreeSpace(state->rs_new_rel,
- HEAP_DEFAULT_FILLFACTOR);
+ if (IsToastRelation(state->rs_new_rel))
+ saveFreeSpace = ToastGetTargetPageFreeSpace();
+ else
+ saveFreeSpace = HeapGetTargetPageFreeSpace(state->rs_new_rel);
/* Now we can check to see if there's enough free space already. */
if (state->rs_buffer_valid)
diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c
index a5915a5..c8b0af3 100644
--- a/src/backend/access/nbtree/nbtinsert.c
+++ b/src/backend/access/nbtree/nbtinsert.c
@@ -1607,8 +1607,7 @@ _bt_findsplitloc(Relation rel,
state.is_rightmost = P_RIGHTMOST(opaque);
state.have_split = false;
if (state.is_leaf)
- state.fillfactor = RelationGetFillFactor(rel,
- BTREE_DEFAULT_FILLFACTOR);
+ state.fillfactor = BTGetFillFactor(rel);
else
state.fillfactor = BTREE_NONLEAF_FILLFACTOR;
state.newitemonleft = false; /* these just to keep compiler quiet */
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index e8725fb..958b801 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -814,7 +814,7 @@ _bt_vacuum_needs_cleanup(IndexVacuumInfo *info)
}
else
{
- StdRdOptions *relopts;
+ BTRelOptions *relopts;
float8 cleanup_scale_factor;
float8 prev_num_heap_tuples;
@@ -825,7 +825,7 @@ _bt_vacuum_needs_cleanup(IndexVacuumInfo *info)
* tuples exceeds vacuum_cleanup_index_scale_factor fraction of
* original tuples count.
*/
- relopts = (StdRdOptions *) info->index->rd_options;
+ relopts = (BTRelOptions *) info->index->rd_options;
cleanup_scale_factor = (relopts &&
relopts->vacuum_cleanup_index_scale_factor >= 0)
? relopts->vacuum_cleanup_index_scale_factor
diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index 16f5755..f8c4fb4 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -681,8 +681,9 @@ _bt_pagestate(BTWriteState *wstate, uint32 level)
if (level > 0)
state->btps_full = (BLCKSZ * (100 - BTREE_NONLEAF_FILLFACTOR) / 100);
else
- state->btps_full = RelationGetTargetPageFreeSpace(wstate->index,
- BTREE_DEFAULT_FILLFACTOR);
+ state->btps_full = (BLCKSZ * (100 - BTGetFillFactor(wstate->index))
+ / 100);
+
/* no parent level, yet */
state->btps_next = NULL;
diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c
index 205457e..11a1de3 100644
--- a/src/backend/access/nbtree/nbtutils.c
+++ b/src/backend/access/nbtree/nbtutils.c
@@ -2053,7 +2053,31 @@ BTreeShmemInit(void)
bytea *
btoptions(Datum reloptions, bool validate)
{
- return default_reloptions(reloptions, validate, RELOPT_KIND_BTREE);
+ relopt_value *options;
+ BTRelOptions *rdopts;
+ int numoptions;
+ static const relopt_parse_elt tab[] = {
+ {"fillfactor", RELOPT_TYPE_INT, offsetof(BTRelOptions, fillfactor)},
+ {"vacuum_cleanup_index_scale_factor", RELOPT_TYPE_REAL,
+ offsetof(BTRelOptions, vacuum_cleanup_index_scale_factor)}
+
+ };
+
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_BTREE,
+ &numoptions);
+
+ /* if none set, we're done */
+ if (numoptions == 0)
+ return NULL;
+
+ rdopts = allocateReloptStruct(sizeof(BTRelOptions), options, numoptions);
+
+ fillRelOptions((void *) rdopts, sizeof(BTRelOptions), options, numoptions,
+ validate, tab, lengthof(tab));
+
+ pfree(options);
+
+ return (bytea *) rdopts;
}
/*
diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c
index 9919e6f..44bdc5b 100644
--- a/src/backend/access/spgist/spgutils.c
+++ b/src/backend/access/spgist/spgutils.c
@@ -411,8 +411,7 @@ SpGistGetBuffer(Relation index, int flags, int needSpace, bool *isNew)
* related to the ones already on it. But fillfactor mustn't cause an
* error for requests that would otherwise be legal.
*/
- needSpace += RelationGetTargetPageFreeSpace(index,
- SPGIST_DEFAULT_FILLFACTOR);
+ needSpace += (BLCKSZ * (100 - SpGistGetFillFactor(index)) / 100);
needSpace = Min(needSpace, SPGIST_PAGE_CAPACITY);
/* Get the cache entry for this flags setting */
@@ -589,7 +588,29 @@ SpGistInitMetapage(Page page)
bytea *
spgoptions(Datum reloptions, bool validate)
{
- return default_reloptions(reloptions, validate, RELOPT_KIND_SPGIST);
+ relopt_value *options;
+ SpGistRelOptions *rdopts;
+ int numoptions;
+ static const relopt_parse_elt tab[] = {
+ {"fillfactor", RELOPT_TYPE_INT, offsetof(SpGistRelOptions, fillfactor)},
+ };
+
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_SPGIST,
+ &numoptions);
+
+ /* if none set, we're done */
+ if (numoptions == 0)
+ return NULL;
+
+ rdopts = allocateReloptStruct(sizeof(SpGistRelOptions), options,
+ numoptions);
+
+ fillRelOptions((void *) rdopts, sizeof(SpGistRelOptions), options,
+ numoptions, validate, tab, lengthof(tab));
+
+ pfree(options);
+
+ return (bytea *) rdopts;
}
/*
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index d01b258..d54f9e4 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -128,7 +128,7 @@ create_ctas_internal(List *attrList, IntoClause *into)
validnsps,
true, false);
- (void) heap_reloptions(RELKIND_TOASTVALUE, toast_options, true);
+ (void) relation_reloptions(RELKIND_TOASTVALUE, toast_options, true);
NewRelationCreateToastTable(intoRelationAddr.objectId, toast_options);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index c8c50e8..93918cc 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -685,7 +685,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
if (relkind == RELKIND_VIEW)
(void) view_reloptions(reloptions, true);
else
- (void) heap_reloptions(relkind, reloptions, true);
+ (void) relation_reloptions(relkind, reloptions, true);
if (stmt->ofTypename)
{
@@ -10674,7 +10674,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
case RELKIND_TOASTVALUE:
case RELKIND_MATVIEW:
case RELKIND_PARTITIONED_TABLE:
- (void) heap_reloptions(rel->rd_rel->relkind, newOptions, true);
+ (void) relation_reloptions(rel->rd_rel->relkind, newOptions, true);
break;
case RELKIND_VIEW:
(void) view_reloptions(newOptions, true);
@@ -10783,7 +10783,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
defList, "toast", validnsps, false,
operation == AT_ResetRelOptions);
- (void) heap_reloptions(RELKIND_TOASTVALUE, newOptions, true);
+ (void) relation_reloptions(RELKIND_TOASTVALUE, newOptions, true);
memset(repl_val, 0, sizeof(repl_val));
memset(repl_null, false, sizeof(repl_null));
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 5790b62..a9e5e59 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -146,7 +146,24 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
&rel->pages, &rel->tuples, &rel->allvisfrac);
/* Retrieve the parallel_workers reloption, or -1 if not set. */
- rel->rel_parallel_workers = RelationGetParallelWorkers(relation, -1);
+
+ switch (relation->rd_rel->relkind)
+ {
+ case RELKIND_RELATION:
+ case RELKIND_MATVIEW:
+ rel->rel_parallel_workers =
+ RelationGetParallelWorkers(relation,-1);
+ break;
+ case RELKIND_TOASTVALUE:
+ case RELKIND_PARTITIONED_TABLE:
+ case RELKIND_SEQUENCE:
+ case REPLICA_IDENTITY_FULL: /* Foreign table */
+ rel->rel_parallel_workers = -1;
+ break;
+ default:
+ /* Other relkinds are not supported */
+ Assert(false);
+ }
/*
* Make list of indexes. Ignore indexes on system catalogs if told to.
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 2d5086d..b605edd 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2720,6 +2720,7 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
{
bytea *relopts;
AutoVacOpts *av;
+ AutoVacOpts *src;
Assert(((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_RELATION ||
((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_MATVIEW ||
@@ -2729,8 +2730,13 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
if (relopts == NULL)
return NULL;
+ if (((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_TOASTVALUE)
+ src = &(((ToastRelOptions *) relopts)->autovacuum);
+ else
+ src = &(((HeapRelOptions *) relopts)->autovacuum);
+
av = palloc(sizeof(AutoVacOpts));
- memcpy(av, &(((StdRdOptions *) relopts)->autovacuum), sizeof(AutoVacOpts));
+ memcpy(av, src, sizeof(AutoVacOpts));
pfree(relopts);
return av;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 970c94e..1b28a6a 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1024,7 +1024,7 @@ ProcessUtilitySlow(ParseState *pstate,
validnsps,
true,
false);
- (void) heap_reloptions(RELKIND_TOASTVALUE,
+ (void) relation_reloptions(RELKIND_TOASTVALUE,
toast_options,
true);
diff --git a/src/include/access/hash.h b/src/include/access/hash.h
index 02ef67c..e6b9208 100644
--- a/src/include/access/hash.h
+++ b/src/include/access/hash.h
@@ -274,6 +274,17 @@ typedef struct HashMetaPageData
typedef HashMetaPageData *HashMetaPage;
+typedef struct HashRelOptions
+{
+ int32 varlena_header_; /* varlena header (do not touch directly!) */
+ int fillfactor; /* page fill factor in percent (0..100) */
+} HashRelOptions;
+
+#define HashGetFillFactor(relation) \
+ ((relation)->rd_options ? \
+ ((HashRelOptions *) (relation)->rd_options)->fillfactor : \
+ HASH_DEFAULT_FILLFACTOR)
+
/*
* Maximum size of a hash index item (it's okay to have only one per page)
*/
diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index ea495f1..d7066d0 100644
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -488,6 +488,19 @@ typedef BTScanOpaqueData *BTScanOpaque;
#define SK_BT_DESC (INDOPTION_DESC << SK_BT_INDOPTION_SHIFT)
#define SK_BT_NULLS_FIRST (INDOPTION_NULLS_FIRST << SK_BT_INDOPTION_SHIFT)
+typedef struct BTRelOptions
+{
+ int32 varlena_header_; /* varlena header (do not touch directly!) */
+ int fillfactor; /* page fill factor in percent (0..100) */
+ /* fraction of newly inserted tuples prior to trigger index cleanup */
+ float8 vacuum_cleanup_index_scale_factor;
+} BTRelOptions;
+
+#define BTGetFillFactor(relation) \
+ ((relation)->rd_options ? \
+ ((BTRelOptions *) (relation)->rd_options)->fillfactor : \
+ BTREE_DEFAULT_FILLFACTOR)
+
/*
* external entry points for btree, in nbtree.c
*/
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index 50690b9..6d309b3 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -271,9 +271,11 @@ extern void fillRelOptions(void *rdopts, Size basesize,
bool validate,
const relopt_parse_elt *elems, int nelems);
-extern bytea *default_reloptions(Datum reloptions, bool validate,
- relopt_kind kind);
-extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
+extern bytea *toast_reloptions(Datum reloptions, bool validate);
+extern bytea *partitioned_reloptions(Datum reloptions, bool validate);
+extern bytea *heap_reloptions(Datum reloptions, bool validate);
+extern bytea *relation_reloptions(char relkind, Datum reloptions,
+ bool validate);
extern bytea *view_reloptions(Datum reloptions, bool validate);
extern bytea *index_reloptions(amoptions_function amoptions, Datum reloptions,
bool validate);
diff --git a/src/include/access/spgist.h b/src/include/access/spgist.h
index 9c19e9e..4c57a0a 100644
--- a/src/include/access/spgist.h
+++ b/src/include/access/spgist.h
@@ -20,10 +20,6 @@
#include "lib/stringinfo.h"
-/* reloption parameters */
-#define SPGIST_MIN_FILLFACTOR 10
-#define SPGIST_DEFAULT_FILLFACTOR 80
-
/* SPGiST opclass support function numbers */
#define SPGIST_CONFIG_PROC 1
#define SPGIST_CHOOSE_PROC 2
diff --git a/src/include/access/spgist_private.h b/src/include/access/spgist_private.h
index d23862e..a567603 100644
--- a/src/include/access/spgist_private.h
+++ b/src/include/access/spgist_private.h
@@ -22,6 +22,17 @@
#include "utils/relcache.h"
+typedef struct SpGistRelOptions
+{
+ int32 varlena_header_; /* varlena header (do not touch directly!) */
+ int fillfactor; /* page fill factor in percent (0..100) */
+} SpGistRelOptions;
+
+#define SpGistGetFillFactor(relation) \
+ ((relation)->rd_options ? \
+ ((SpGistRelOptions *) (relation)->rd_options)->fillfactor : \
+ SPGIST_DEFAULT_FILLFACTOR)
+
/* Page numbers of fixed-location pages */
#define SPGIST_METAPAGE_BLKNO (0) /* metapage */
#define SPGIST_ROOT_BLKNO (1) /* root for normal entries */
@@ -417,6 +428,11 @@ typedef SpGistDeadTupleData *SpGistDeadTuple;
#define GBUF_REQ_NULLS(flags) ((flags) & GBUF_NULLS)
/* spgutils.c */
+
+/* reloption parameters */
+#define SPGIST_MIN_FILLFACTOR 10
+#define SPGIST_DEFAULT_FILLFACTOR 80
+
extern SpGistCache *spgGetCache(Relation index);
extern void initSpGistState(SpGistState *state, Relation index);
extern Buffer SpGistNewBuffer(Relation index);
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 2217081..47f6dd9 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -226,15 +226,11 @@ typedef struct GenericIndexOpts
bool recheck_on_update;
} GenericIndexOpts;
+
/*
- * StdRdOptions
- * Standard contents of rd_options for heaps and generic indexes.
- *
- * RelationGetFillFactor() and RelationGetTargetPageFreeSpace() can only
- * be applied to relations that use this format or a superset for
- * private options data.
+ * AutoVacOpts
+ * Auto Vacuum options used both in Heap and Toast relations
*/
- /* autovacuum-related reloptions. */
typedef struct AutoVacOpts
{
bool enabled;
@@ -253,50 +249,88 @@ typedef struct AutoVacOpts
float8 analyze_scale_factor;
} AutoVacOpts;
-typedef struct StdRdOptions
+/*
+ * HeapRelOptions
+ * Binary representation of relation options for Heap relations.
+ */
+typedef struct HeapRelOptions
{
int32 vl_len_; /* varlena header (do not touch directly!) */
int fillfactor; /* page fill factor in percent (0..100) */
- /* fraction of newly inserted tuples prior to trigger index cleanup */
- float8 vacuum_cleanup_index_scale_factor;
int toast_tuple_target; /* target for tuple toasting */
- AutoVacOpts autovacuum; /* autovacuum-related options */
+ AutoVacOpts autovacuum; /* autovacuum-related options */
bool user_catalog_table; /* use as an additional catalog relation */
int parallel_workers; /* max number of parallel workers */
-} StdRdOptions;
+} HeapRelOptions;
+
+/*
+ * ToastRelOptions
+ * Binary representation of relation options for Toast relations.
+ */
+typedef struct ToastRelOptions
+{
+ int32 vl_len_; /* varlena header (do not touch directly!) */
+ AutoVacOpts autovacuum; /* autovacuum-related options */
+} ToastRelOptions;
+
+
+/*
+ * PartitionedRelOptions
+ * Binary representation of relation options for Partitioned relations.
+ */
+typedef struct PartitionedRelOptions
+{
+ int32 vl_len_; /* varlena header (do not touch directly!) */
+ /* No options defined yet */
+} PartitionedRelOptions;
#define HEAP_MIN_FILLFACTOR 10
#define HEAP_DEFAULT_FILLFACTOR 100
+#define TOAST_DEFAULT_FILLFACTOR 100 /* Only default is actually used */
+
+
+/*
+ * IsHeapRelation
+ * Returns true if relation is a Heap Relation or a Materialized View.
+ */
+#define IsHeapRelation(relation) \
+ (relation->rd_rel->relkind == RELKIND_RELATION || \
+ relation->rd_rel->relkind == RELKIND_MATVIEW)
+
/*
* RelationGetToastTupleTarget
- * Returns the relation's toast_tuple_target. Note multiple eval of argument!
+ * Returns the heap's toast_tuple_target. Note multiple eval of argument!
*/
-#define RelationGetToastTupleTarget(relation, defaulttarg) \
- ((relation)->rd_options ? \
- ((StdRdOptions *) (relation)->rd_options)->toast_tuple_target : (defaulttarg))
+#define RelationGetToastTupleTarget(relation, defaulttarg) \
+ (AssertMacro(IsHeapRelation(relation)), \
+ ((relation)->rd_options ? \
+ ((HeapRelOptions *) (relation)->rd_options)->toast_tuple_target : \
+ (defaulttarg)))
/*
- * RelationGetFillFactor
- * Returns the relation's fillfactor. Note multiple eval of argument!
+ * HeapGetFillFactor
+ * Returns the heap's fillfactor. Note multiple eval of argument!
*/
-#define RelationGetFillFactor(relation, defaultff) \
- ((relation)->rd_options ? \
- ((StdRdOptions *) (relation)->rd_options)->fillfactor : (defaultff))
+#define HeapGetFillFactor(relation) \
+ (AssertMacro(IsHeapRelation(relation)), \
+ ((relation)->rd_options ? \
+ ((HeapRelOptions *) (relation)->rd_options)->fillfactor : \
+ (HEAP_DEFAULT_FILLFACTOR)))
/*
- * RelationGetTargetPageUsage
- * Returns the relation's desired space usage per page in bytes.
+ * HeapGetTargetPageUsage
+ * Returns the heap's desired space usage per page in bytes.
*/
-#define RelationGetTargetPageUsage(relation, defaultff) \
- (BLCKSZ * RelationGetFillFactor(relation, defaultff) / 100)
+#define HeapGetTargetPageUsage(relation) \
+ (BLCKSZ * HeapGetFillFactor(relation) / 100)
/*
- * RelationGetTargetPageFreeSpace
- * Returns the relation's desired freespace per page in bytes.
+ * HeapGetTargetPageFreeSpace
+ * Returns the heap's desired freespace per page in bytes.
*/
-#define RelationGetTargetPageFreeSpace(relation, defaultff) \
- (BLCKSZ * (100 - RelationGetFillFactor(relation, defaultff)) / 100)
+#define HeapGetTargetPageFreeSpace(relation) \
+ (BLCKSZ * (100 - HeapGetFillFactor(relation)) / 100)
/*
* RelationIsUsedAsCatalogTable
@@ -307,17 +341,26 @@ typedef struct StdRdOptions
((relation)->rd_options && \
((relation)->rd_rel->relkind == RELKIND_RELATION || \
(relation)->rd_rel->relkind == RELKIND_MATVIEW) ? \
- ((StdRdOptions *) (relation)->rd_options)->user_catalog_table : false)
+ ((HeapRelOptions *) (relation)->rd_options)->user_catalog_table : false)
/*
* RelationGetParallelWorkers
- * Returns the relation's parallel_workers reloption setting.
+ * Returns the heap's parallel_workers reloption setting.
* Note multiple eval of argument!
*/
-#define RelationGetParallelWorkers(relation, defaultpw) \
- ((relation)->rd_options ? \
- ((StdRdOptions *) (relation)->rd_options)->parallel_workers : (defaultpw))
+#define RelationGetParallelWorkers(relation, defaultpw) \
+ (AssertMacro(IsHeapRelation(relation)), \
+ ((relation)->rd_options ? \
+ ((HeapRelOptions *) (relation)->rd_options)->parallel_workers : \
+ (defaultpw)))
+/*
+ * ToastGetTargetPageFreeSpace
+ * Returns the TOAST relation's desired freespace per page in bytes.
+ * Always calculated using default fillfactor value.
+ */
+#define ToastGetTargetPageFreeSpace() \
+ (BLCKSZ * (100 - TOAST_DEFAULT_FILLFACTOR) / 100)
/*
* ViewOptions
@@ -330,14 +373,23 @@ typedef struct ViewOptions
int check_option_offset;
} ViewOptions;
+
+/*
+ * IsViewRelation
+ * Returnt ture if relation is a View Relation
+ */
+#define IsViewRelation(relation) \
+ (relation->rd_rel->relkind == RELKIND_VIEW)
+
/*
* RelationIsSecurityView
* Returns whether the relation is security view, or not. Note multiple
* eval of argument!
*/
#define RelationIsSecurityView(relation) \
- ((relation)->rd_options ? \
- ((ViewOptions *) (relation)->rd_options)->security_barrier : false)
+ (AssertMacro(IsViewRelation(relation)), \
+ ((relation)->rd_options ? \
+ ((ViewOptions *) (relation)->rd_options)->security_barrier : false))
/*
* RelationHasCheckOption
@@ -345,8 +397,9 @@ typedef struct ViewOptions
* or the cascaded check option. Note multiple eval of argument!
*/
#define RelationHasCheckOption(relation) \
+ (AssertMacro(IsViewRelation(relation)), \
((relation)->rd_options && \
- ((ViewOptions *) (relation)->rd_options)->check_option_offset != 0)
+ ((ViewOptions *) (relation)->rd_options)->check_option_offset != 0))
/*
* RelationHasLocalCheckOption
@@ -354,11 +407,12 @@ typedef struct ViewOptions
* option. Note multiple eval of argument!
*/
#define RelationHasLocalCheckOption(relation) \
- ((relation)->rd_options && \
- ((ViewOptions *) (relation)->rd_options)->check_option_offset != 0 ? \
- strcmp((char *) (relation)->rd_options + \
- ((ViewOptions *) (relation)->rd_options)->check_option_offset, \
- "local") == 0 : false)
+ (AssertMacro(IsViewRelation(relation)), \
+ ((relation)->rd_options && \
+ ((ViewOptions *) (relation)->rd_options)->check_option_offset != 0 ? \
+ strcmp((char *) (relation)->rd_options + \
+ ((ViewOptions *) (relation)->rd_options)->check_option_offset, \
+ "local") == 0 : false))
/*
* RelationHasCascadedCheckOption
@@ -366,11 +420,12 @@ typedef struct ViewOptions
* option. Note multiple eval of argument!
*/
#define RelationHasCascadedCheckOption(relation) \
+ (AssertMacro(IsViewRelation(relation)), \
((relation)->rd_options && \
((ViewOptions *) (relation)->rd_options)->check_option_offset != 0 ? \
strcmp((char *) (relation)->rd_options + \
((ViewOptions *) (relation)->rd_options)->check_option_offset, \
- "cascaded") == 0 : false)
+ "cascaded") == 0 : false))
/*
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead
2019-01-01 20:21 Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
2019-01-02 03:05 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
2019-01-02 09:58 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
2019-01-03 19:10 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
2019-01-03 19:22 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
2019-01-03 20:15 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
2019-01-07 16:26 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
@ 2019-01-17 23:33 ` Alvaro Herrera <[email protected]>
2019-01-20 10:35 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Alvaro Herrera @ 2019-01-17 23:33 UTC (permalink / raw)
To: Nikolay Shaplov <[email protected]>; +Cc: [email protected]; Dmitry Dolgov <[email protected]>
You introduced new macros IsHeapRelation and IsViewRelation, but I don't
want to introduce such API. Such things have been heavily contested and
I don't to have one more thing to worry about for this patch, so please
just put the relkind directly in the code.
On 2019-Jan-07, Nikolay Shaplov wrote:
> I also reworked some comments. BTW do you know what is this comments spoke
> about:
>
> * RelationGetFillFactor() and RelationGetTargetPageFreeSpace() can only
> * be applied to relations that use this format or a superset for
> * private options data.
>
> It is speaking about some old times when there can be some other type of
> options? 'cause I do not understand what it is speaking about.
> I've removed it, I hope I did not remove anything important.
As I understand it's talking about the varlenas being StdRdOptions and
not anything else. I think extensibility could cause some relkinds to
use different options.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead
2019-01-01 20:21 Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
2019-01-02 03:05 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
2019-01-02 09:58 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
2019-01-03 19:10 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
2019-01-03 19:22 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
2019-01-03 20:15 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
2019-01-07 16:26 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
2019-01-17 23:33 ` Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Alvaro Herrera <[email protected]>
@ 2019-01-20 10:35 ` Nikolay Shaplov <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Nikolay Shaplov @ 2019-01-20 10:35 UTC (permalink / raw)
To: [email protected]; +Cc: Alvaro Herrera <[email protected]>; Dmitry Dolgov <[email protected]>
В письме от четверг, 17 января 2019 г. 20:33:06 MSK пользователь Alvaro
Herrera написал:
> You introduced new macros IsHeapRelation and IsViewRelation, but I don't
> want to introduce such API. Such things have been heavily contested and
> I don't to have one more thing to worry about for this patch, so please
> just put the relkind directly in the code.
Sorry.
I've been trying to avoid repeating
(AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION || \
relation->rd_rel->relkind == RELKIND_MATVIEW), \
all the time.
Fixed.
Also I make more relaxed parallel_workers behavior in get_relation_info as we
discussed in another thread.
Attachments:
[text/x-patch] get-rid-of-StrRdOptions_4.diff (35.2K, ../../1921952.xWjPjWaBPZ@x200m/2-get-rid-of-StrRdOptions_4.diff)
download | inline diff:
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index cdf1f4a..b1fd67c 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -22,7 +22,7 @@
#include "access/htup_details.h"
#include "access/nbtree.h"
#include "access/reloptions.h"
-#include "access/spgist.h"
+#include "access/spgist_private.h"
#include "access/tuptoaster.h"
#include "catalog/pg_type.h"
#include "commands/defrem.h"
@@ -46,9 +46,8 @@
* upper and lower bounds (if applicable); for strings, consider a validation
* routine.
* (ii) add a record below (or use add_<type>_reloption).
- * (iii) add it to the appropriate options struct (perhaps StdRdOptions)
- * (iv) add it to the appropriate handling routine (perhaps
- * default_reloptions)
+ * (iii) add it to the appropriate options struct
+ * (iv) add it to the appropriate handling routine
* (v) make sure the lock level is set correctly for that operation
* (vi) don't forget to document the option
*
@@ -1010,7 +1009,7 @@ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
case RELKIND_TOASTVALUE:
case RELKIND_MATVIEW:
case RELKIND_PARTITIONED_TABLE:
- options = heap_reloptions(classForm->relkind, datum, false);
+ options = relation_reloptions(classForm->relkind, datum, false);
break;
case RELKIND_VIEW:
options = view_reloptions(datum, false);
@@ -1343,63 +1342,69 @@ fillRelOptions(void *rdopts, Size basesize,
/*
- * Option parser for anything that uses StdRdOptions.
+ * Option parsing definition for autovacuum. Used in toast and heap options.
+ */
+
+#define AUTOVACUUM_RELOPTIONS(OFFSET) \
+ {"autovacuum_enabled", RELOPT_TYPE_BOOL, \
+ OFFSET + offsetof(AutoVacOpts, enabled)}, \
+ {"autovacuum_vacuum_threshold", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, vacuum_threshold)}, \
+ {"autovacuum_analyze_threshold", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, analyze_threshold)}, \
+ {"autovacuum_vacuum_cost_delay", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, vacuum_cost_delay)}, \
+ {"autovacuum_vacuum_cost_limit", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, vacuum_cost_limit)}, \
+ {"autovacuum_freeze_min_age", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, freeze_min_age)}, \
+ {"autovacuum_freeze_max_age", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, freeze_max_age)}, \
+ {"autovacuum_freeze_table_age", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, freeze_table_age)}, \
+ {"autovacuum_multixact_freeze_min_age", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, multixact_freeze_min_age)}, \
+ {"autovacuum_multixact_freeze_max_age", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, multixact_freeze_max_age)}, \
+ {"autovacuum_multixact_freeze_table_age", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, multixact_freeze_table_age)}, \
+ {"log_autovacuum_min_duration", RELOPT_TYPE_INT, \
+ OFFSET + offsetof(AutoVacOpts, log_min_duration)}, \
+ {"autovacuum_vacuum_scale_factor", RELOPT_TYPE_REAL, \
+ OFFSET + offsetof(AutoVacOpts, vacuum_scale_factor)}, \
+ {"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL, \
+ OFFSET + offsetof(AutoVacOpts, analyze_scale_factor)}
+
+/*
+ * Option parser for heap
*/
bytea *
-default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
+heap_reloptions(Datum reloptions, bool validate)
{
relopt_value *options;
- StdRdOptions *rdopts;
+ HeapRelOptions *rdopts;
int numoptions;
static const relopt_parse_elt tab[] = {
- {"fillfactor", RELOPT_TYPE_INT, offsetof(StdRdOptions, fillfactor)},
- {"autovacuum_enabled", RELOPT_TYPE_BOOL,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, enabled)},
- {"autovacuum_vacuum_threshold", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_threshold)},
- {"autovacuum_analyze_threshold", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, analyze_threshold)},
- {"autovacuum_vacuum_cost_delay", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_delay)},
- {"autovacuum_vacuum_cost_limit", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_limit)},
- {"autovacuum_freeze_min_age", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_min_age)},
- {"autovacuum_freeze_max_age", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_max_age)},
- {"autovacuum_freeze_table_age", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_table_age)},
- {"autovacuum_multixact_freeze_min_age", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_min_age)},
- {"autovacuum_multixact_freeze_max_age", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_max_age)},
- {"autovacuum_multixact_freeze_table_age", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_table_age)},
- {"log_autovacuum_min_duration", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, log_min_duration)},
+ {"fillfactor", RELOPT_TYPE_INT, offsetof(HeapRelOptions, fillfactor)},
+ AUTOVACUUM_RELOPTIONS(offsetof(HeapRelOptions, autovacuum)),
{"toast_tuple_target", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, toast_tuple_target)},
- {"autovacuum_vacuum_scale_factor", RELOPT_TYPE_REAL,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_scale_factor)},
- {"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL,
- offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, analyze_scale_factor)},
+ offsetof(HeapRelOptions, toast_tuple_target)},
{"user_catalog_table", RELOPT_TYPE_BOOL,
- offsetof(StdRdOptions, user_catalog_table)},
+ offsetof(HeapRelOptions, user_catalog_table)},
{"parallel_workers", RELOPT_TYPE_INT,
- offsetof(StdRdOptions, parallel_workers)},
- {"vacuum_cleanup_index_scale_factor", RELOPT_TYPE_REAL,
- offsetof(StdRdOptions, vacuum_cleanup_index_scale_factor)}
+ offsetof(HeapRelOptions, parallel_workers)}
};
- options = parseRelOptions(reloptions, validate, kind, &numoptions);
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_HEAP,
+ &numoptions);
/* if none set, we're done */
if (numoptions == 0)
return NULL;
- rdopts = allocateReloptStruct(sizeof(StdRdOptions), options, numoptions);
+ rdopts = allocateReloptStruct(sizeof(HeapRelOptions), options, numoptions);
- fillRelOptions((void *) rdopts, sizeof(StdRdOptions), options, numoptions,
+ fillRelOptions((void *) rdopts, sizeof(HeapRelOptions), options, numoptions,
validate, tab, lengthof(tab));
pfree(options);
@@ -1408,6 +1413,60 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
}
/*
+ * Option parser for toast
+ */
+bytea *
+toast_reloptions(Datum reloptions, bool validate)
+{
+ relopt_value *options;
+ ToastRelOptions *rdopts;
+ int numoptions;
+ static const relopt_parse_elt tab[] = {
+ AUTOVACUUM_RELOPTIONS(offsetof(ToastRelOptions, autovacuum)),
+ };
+
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_TOAST,
+ &numoptions);
+
+ /* if none set, we're done */
+ if (numoptions == 0)
+ return NULL;
+
+ rdopts = allocateReloptStruct(sizeof(ToastRelOptions), options, numoptions);
+
+ fillRelOptions((void *) rdopts, sizeof(ToastRelOptions), options,
+ numoptions, validate, tab, lengthof(tab));
+
+ /* adjust default-only parameters for TOAST relations */
+ rdopts->autovacuum.analyze_threshold = -1;
+ rdopts->autovacuum.analyze_scale_factor = -1;
+
+ pfree(options);
+
+ return (bytea *) rdopts;
+}
+
+/*
+ * Option parser for partitioned relations
+ */
+bytea *
+partitioned_reloptions(Datum reloptions, bool validate)
+{
+ int numoptions;
+
+ /*
+ * Since there is no options for patitioned table for now, we just do
+ * validation to report incorrect option error and leave.
+ */
+
+ if (validate)
+ parseRelOptions(reloptions, validate, RELOPT_KIND_PARTITIONED,
+ &numoptions);
+
+ return NULL;
+}
+
+/*
* Option parser for views
*/
bytea *
@@ -1440,39 +1499,26 @@ view_reloptions(Datum reloptions, bool validate)
}
/*
- * Parse options for heaps, views and toast tables.
+ * Parse options for heaps, toast, views and partitioned tables.
*/
bytea *
-heap_reloptions(char relkind, Datum reloptions, bool validate)
+relation_reloptions(char relkind, Datum reloptions, bool validate)
{
- StdRdOptions *rdopts;
-
switch (relkind)
{
case RELKIND_TOASTVALUE:
- rdopts = (StdRdOptions *)
- default_reloptions(reloptions, validate, RELOPT_KIND_TOAST);
- if (rdopts != NULL)
- {
- /* adjust default-only parameters for TOAST relations */
- rdopts->fillfactor = 100;
- rdopts->autovacuum.analyze_threshold = -1;
- rdopts->autovacuum.analyze_scale_factor = -1;
- }
- return (bytea *) rdopts;
+ return toast_reloptions(reloptions, validate);
case RELKIND_RELATION:
case RELKIND_MATVIEW:
- return default_reloptions(reloptions, validate, RELOPT_KIND_HEAP);
+ return heap_reloptions(reloptions, validate);
case RELKIND_PARTITIONED_TABLE:
- return default_reloptions(reloptions, validate,
- RELOPT_KIND_PARTITIONED);
+ return partitioned_reloptions(reloptions, validate);
default:
/* other relkinds are not supported */
return NULL;
}
}
-
/*
* Parse options for indexes.
*
diff --git a/src/backend/access/hash/hashpage.c b/src/backend/access/hash/hashpage.c
index 0f85256..47a9a5c 100644
--- a/src/backend/access/hash/hashpage.c
+++ b/src/backend/access/hash/hashpage.c
@@ -369,7 +369,7 @@ _hash_init(Relation rel, double num_tuples, ForkNumber forkNum)
data_width = sizeof(uint32);
item_width = MAXALIGN(sizeof(IndexTupleData)) + MAXALIGN(data_width) +
sizeof(ItemIdData); /* include the line pointer */
- ffactor = RelationGetTargetPageUsage(rel, HASH_DEFAULT_FILLFACTOR) / item_width;
+ ffactor = (BLCKSZ * HashGetFillFactor(rel) / 100) / item_width;
/* keep to a sane range */
if (ffactor < 10)
ffactor = 10;
diff --git a/src/backend/access/hash/hashutil.c b/src/backend/access/hash/hashutil.c
index 3fb92ea..5170634 100644
--- a/src/backend/access/hash/hashutil.c
+++ b/src/backend/access/hash/hashutil.c
@@ -289,7 +289,28 @@ _hash_checkpage(Relation rel, Buffer buf, int flags)
bytea *
hashoptions(Datum reloptions, bool validate)
{
- return default_reloptions(reloptions, validate, RELOPT_KIND_HASH);
+ relopt_value *options;
+ HashRelOptions *rdopts;
+ int numoptions;
+ static const relopt_parse_elt tab[] = {
+ {"fillfactor", RELOPT_TYPE_INT, offsetof(HashRelOptions, fillfactor)},
+ };
+
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_HASH,
+ &numoptions);
+
+ /* if none set, we're done */
+ if (numoptions == 0)
+ return NULL;
+
+ rdopts = allocateReloptStruct(sizeof(HashRelOptions), options, numoptions);
+
+ fillRelOptions((void *) rdopts, sizeof(HashRelOptions), options, numoptions,
+ validate, tab, lengthof(tab));
+
+ pfree(options);
+
+ return (bytea *) rdopts;
}
/*
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 9afbc82..6073491 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2715,8 +2715,10 @@ heap_multi_insert(Relation relation, HeapTuple *tuples, int ntuples,
AssertArg(!(options & HEAP_INSERT_NO_LOGICAL));
needwal = !(options & HEAP_INSERT_SKIP_WAL) && RelationNeedsWAL(relation);
- saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
- HEAP_DEFAULT_FILLFACTOR);
+ if (IsToastRelation(relation))
+ saveFreeSpace = ToastGetTargetPageFreeSpace();
+ else
+ saveFreeSpace = HeapGetTargetPageFreeSpace(relation);
/* Toast and set header data in all the tuples */
heaptuples = palloc(ntuples * sizeof(HeapTuple));
diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index 3da0b49..6458e25 100644
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -19,6 +19,7 @@
#include "access/hio.h"
#include "access/htup_details.h"
#include "access/visibilitymap.h"
+#include "catalog/catalog.h"
#include "storage/bufmgr.h"
#include "storage/freespace.h"
#include "storage/lmgr.h"
@@ -339,8 +340,10 @@ RelationGetBufferForTuple(Relation relation, Size len,
len, MaxHeapTupleSize)));
/* Compute desired extra freespace due to fillfactor option */
- saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
- HEAP_DEFAULT_FILLFACTOR);
+ if (IsToastRelation(relation))
+ saveFreeSpace = ToastGetTargetPageFreeSpace();
+ else
+ saveFreeSpace = HeapGetTargetPageFreeSpace(relation);
if (otherBuffer != InvalidBuffer)
otherBlock = BufferGetBlockNumber(otherBuffer);
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 2665f24..bd508c5 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -130,8 +130,11 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
* important than sometimes getting a wrong answer in what is after all
* just a heuristic estimate.
*/
- minfree = RelationGetTargetPageFreeSpace(relation,
- HEAP_DEFAULT_FILLFACTOR);
+
+ if (IsToastRelation(relation))
+ minfree = ToastGetTargetPageFreeSpace();
+ else
+ minfree = HeapGetTargetPageFreeSpace(relation);
minfree = Max(minfree, BLCKSZ / 10);
if (PageIsFull(page) || PageGetHeapFreeSpace(page) < minfree)
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index f6b0f1b..014fc3b1 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -683,8 +683,10 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
len, MaxHeapTupleSize)));
/* Compute desired extra freespace due to fillfactor option */
- saveFreeSpace = RelationGetTargetPageFreeSpace(state->rs_new_rel,
- HEAP_DEFAULT_FILLFACTOR);
+ if (IsToastRelation(state->rs_new_rel))
+ saveFreeSpace = ToastGetTargetPageFreeSpace();
+ else
+ saveFreeSpace = HeapGetTargetPageFreeSpace(state->rs_new_rel);
/* Now we can check to see if there's enough free space already. */
if (state->rs_buffer_valid)
diff --git a/src/backend/access/nbtree/nbtinsert.c b/src/backend/access/nbtree/nbtinsert.c
index 858df3b..59f585b 100644
--- a/src/backend/access/nbtree/nbtinsert.c
+++ b/src/backend/access/nbtree/nbtinsert.c
@@ -1607,8 +1607,7 @@ _bt_findsplitloc(Relation rel,
state.is_rightmost = P_RIGHTMOST(opaque);
state.have_split = false;
if (state.is_leaf)
- state.fillfactor = RelationGetFillFactor(rel,
- BTREE_DEFAULT_FILLFACTOR);
+ state.fillfactor = BTGetFillFactor(rel);
else
state.fillfactor = BTREE_NONLEAF_FILLFACTOR;
state.newitemonleft = false; /* these just to keep compiler quiet */
diff --git a/src/backend/access/nbtree/nbtree.c b/src/backend/access/nbtree/nbtree.c
index 98917de..56b3ad9 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -814,7 +814,7 @@ _bt_vacuum_needs_cleanup(IndexVacuumInfo *info)
}
else
{
- StdRdOptions *relopts;
+ BTRelOptions *relopts;
float8 cleanup_scale_factor;
float8 prev_num_heap_tuples;
@@ -825,7 +825,7 @@ _bt_vacuum_needs_cleanup(IndexVacuumInfo *info)
* tuples exceeds vacuum_cleanup_index_scale_factor fraction of
* original tuples count.
*/
- relopts = (StdRdOptions *) info->index->rd_options;
+ relopts = (BTRelOptions *) info->index->rd_options;
cleanup_scale_factor = (relopts &&
relopts->vacuum_cleanup_index_scale_factor >= 0)
? relopts->vacuum_cleanup_index_scale_factor
diff --git a/src/backend/access/nbtree/nbtsort.c b/src/backend/access/nbtree/nbtsort.c
index 5cc3cf5..4a8fa68 100644
--- a/src/backend/access/nbtree/nbtsort.c
+++ b/src/backend/access/nbtree/nbtsort.c
@@ -682,8 +682,9 @@ _bt_pagestate(BTWriteState *wstate, uint32 level)
if (level > 0)
state->btps_full = (BLCKSZ * (100 - BTREE_NONLEAF_FILLFACTOR) / 100);
else
- state->btps_full = RelationGetTargetPageFreeSpace(wstate->index,
- BTREE_DEFAULT_FILLFACTOR);
+ state->btps_full = (BLCKSZ * (100 - BTGetFillFactor(wstate->index))
+ / 100);
+
/* no parent level, yet */
state->btps_next = NULL;
diff --git a/src/backend/access/nbtree/nbtutils.c b/src/backend/access/nbtree/nbtutils.c
index 2c05fb5..e26c2b4 100644
--- a/src/backend/access/nbtree/nbtutils.c
+++ b/src/backend/access/nbtree/nbtutils.c
@@ -2053,7 +2053,31 @@ BTreeShmemInit(void)
bytea *
btoptions(Datum reloptions, bool validate)
{
- return default_reloptions(reloptions, validate, RELOPT_KIND_BTREE);
+ relopt_value *options;
+ BTRelOptions *rdopts;
+ int numoptions;
+ static const relopt_parse_elt tab[] = {
+ {"fillfactor", RELOPT_TYPE_INT, offsetof(BTRelOptions, fillfactor)},
+ {"vacuum_cleanup_index_scale_factor", RELOPT_TYPE_REAL,
+ offsetof(BTRelOptions, vacuum_cleanup_index_scale_factor)}
+
+ };
+
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_BTREE,
+ &numoptions);
+
+ /* if none set, we're done */
+ if (numoptions == 0)
+ return NULL;
+
+ rdopts = allocateReloptStruct(sizeof(BTRelOptions), options, numoptions);
+
+ fillRelOptions((void *) rdopts, sizeof(BTRelOptions), options, numoptions,
+ validate, tab, lengthof(tab));
+
+ pfree(options);
+
+ return (bytea *) rdopts;
}
/*
diff --git a/src/backend/access/spgist/spgutils.c b/src/backend/access/spgist/spgutils.c
index de147d7..ec9ea14 100644
--- a/src/backend/access/spgist/spgutils.c
+++ b/src/backend/access/spgist/spgutils.c
@@ -411,8 +411,7 @@ SpGistGetBuffer(Relation index, int flags, int needSpace, bool *isNew)
* related to the ones already on it. But fillfactor mustn't cause an
* error for requests that would otherwise be legal.
*/
- needSpace += RelationGetTargetPageFreeSpace(index,
- SPGIST_DEFAULT_FILLFACTOR);
+ needSpace += (BLCKSZ * (100 - SpGistGetFillFactor(index)) / 100);
needSpace = Min(needSpace, SPGIST_PAGE_CAPACITY);
/* Get the cache entry for this flags setting */
@@ -589,7 +588,29 @@ SpGistInitMetapage(Page page)
bytea *
spgoptions(Datum reloptions, bool validate)
{
- return default_reloptions(reloptions, validate, RELOPT_KIND_SPGIST);
+ relopt_value *options;
+ SpGistRelOptions *rdopts;
+ int numoptions;
+ static const relopt_parse_elt tab[] = {
+ {"fillfactor", RELOPT_TYPE_INT, offsetof(SpGistRelOptions, fillfactor)},
+ };
+
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_SPGIST,
+ &numoptions);
+
+ /* if none set, we're done */
+ if (numoptions == 0)
+ return NULL;
+
+ rdopts = allocateReloptStruct(sizeof(SpGistRelOptions), options,
+ numoptions);
+
+ fillRelOptions((void *) rdopts, sizeof(SpGistRelOptions), options,
+ numoptions, validate, tab, lengthof(tab));
+
+ pfree(options);
+
+ return (bytea *) rdopts;
}
/*
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 5947996..c7653a0 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -129,7 +129,7 @@ create_ctas_internal(List *attrList, IntoClause *into)
validnsps,
true, false);
- (void) heap_reloptions(RELKIND_TOASTVALUE, toast_options, true);
+ (void) relation_reloptions(RELKIND_TOASTVALUE, toast_options, true);
NewRelationCreateToastTable(intoRelationAddr.objectId, toast_options);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 1a1ac69..bb293b9 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -689,7 +689,7 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
if (relkind == RELKIND_VIEW)
(void) view_reloptions(reloptions, true);
else
- (void) heap_reloptions(relkind, reloptions, true);
+ (void) relation_reloptions(relkind, reloptions, true);
if (stmt->ofTypename)
{
@@ -11079,7 +11079,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
case RELKIND_TOASTVALUE:
case RELKIND_MATVIEW:
case RELKIND_PARTITIONED_TABLE:
- (void) heap_reloptions(rel->rd_rel->relkind, newOptions, true);
+ (void) relation_reloptions(rel->rd_rel->relkind, newOptions, true);
break;
case RELKIND_VIEW:
(void) view_reloptions(newOptions, true);
@@ -11188,7 +11188,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
defList, "toast", validnsps, false,
operation == AT_ResetRelOptions);
- (void) heap_reloptions(RELKIND_TOASTVALUE, newOptions, true);
+ (void) relation_reloptions(RELKIND_TOASTVALUE, newOptions, true);
memset(repl_val, 0, sizeof(repl_val));
memset(repl_null, false, sizeof(repl_null));
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 48ffc5f..d25664f 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -145,8 +145,15 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
estimate_rel_size(relation, rel->attr_widths - rel->min_attr,
&rel->pages, &rel->tuples, &rel->allvisfrac);
- /* Retrieve the parallel_workers reloption, or -1 if not set. */
- rel->rel_parallel_workers = RelationGetParallelWorkers(relation, -1);
+ /*
+ * Retrieve the parallel_workers for heap and mat.view relations.
+ * Use -1 if not set, or if we are dealing with other relation kinds
+ */
+ if (relation->rd_rel->relkind == RELKIND_RELATION ||
+ relation->rd_rel->relkind == RELKIND_MATVIEW)
+ rel->rel_parallel_workers = RelationGetParallelWorkers(relation, -1);
+ else
+ rel->rel_parallel_workers = -1;
/*
* Make list of indexes. Ignore indexes on system catalogs if told to.
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 4cf6787..889bbcc 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2720,6 +2720,7 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
{
bytea *relopts;
AutoVacOpts *av;
+ AutoVacOpts *src;
Assert(((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_RELATION ||
((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_MATVIEW ||
@@ -2729,8 +2730,13 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
if (relopts == NULL)
return NULL;
+ if (((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_TOASTVALUE)
+ src = &(((ToastRelOptions *) relopts)->autovacuum);
+ else
+ src = &(((HeapRelOptions *) relopts)->autovacuum);
+
av = palloc(sizeof(AutoVacOpts));
- memcpy(av, &(((StdRdOptions *) relopts)->autovacuum), sizeof(AutoVacOpts));
+ memcpy(av, src, sizeof(AutoVacOpts));
pfree(relopts);
return av;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 27ae6be..2f02b74 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1024,7 +1024,7 @@ ProcessUtilitySlow(ParseState *pstate,
validnsps,
true,
false);
- (void) heap_reloptions(RELKIND_TOASTVALUE,
+ (void) relation_reloptions(RELKIND_TOASTVALUE,
toast_options,
true);
diff --git a/src/include/access/hash.h b/src/include/access/hash.h
index 0b8eb64..dc93dbe 100644
--- a/src/include/access/hash.h
+++ b/src/include/access/hash.h
@@ -274,6 +274,17 @@ typedef struct HashMetaPageData
typedef HashMetaPageData *HashMetaPage;
+typedef struct HashRelOptions
+{
+ int32 varlena_header_; /* varlena header (do not touch directly!) */
+ int fillfactor; /* page fill factor in percent (0..100) */
+} HashRelOptions;
+
+#define HashGetFillFactor(relation) \
+ ((relation)->rd_options ? \
+ ((HashRelOptions *) (relation)->rd_options)->fillfactor : \
+ HASH_DEFAULT_FILLFACTOR)
+
/*
* Maximum size of a hash index item (it's okay to have only one per page)
*/
diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index 4fb92d6..c6744be 100644
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -488,6 +488,19 @@ typedef BTScanOpaqueData *BTScanOpaque;
#define SK_BT_DESC (INDOPTION_DESC << SK_BT_INDOPTION_SHIFT)
#define SK_BT_NULLS_FIRST (INDOPTION_NULLS_FIRST << SK_BT_INDOPTION_SHIFT)
+typedef struct BTRelOptions
+{
+ int32 varlena_header_; /* varlena header (do not touch directly!) */
+ int fillfactor; /* page fill factor in percent (0..100) */
+ /* fraction of newly inserted tuples prior to trigger index cleanup */
+ float8 vacuum_cleanup_index_scale_factor;
+} BTRelOptions;
+
+#define BTGetFillFactor(relation) \
+ ((relation)->rd_options ? \
+ ((BTRelOptions *) (relation)->rd_options)->fillfactor : \
+ BTREE_DEFAULT_FILLFACTOR)
+
/*
* external entry points for btree, in nbtree.c
*/
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index 7ade18e..9af06de 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -270,9 +270,11 @@ extern void fillRelOptions(void *rdopts, Size basesize,
bool validate,
const relopt_parse_elt *elems, int nelems);
-extern bytea *default_reloptions(Datum reloptions, bool validate,
- relopt_kind kind);
-extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
+extern bytea *toast_reloptions(Datum reloptions, bool validate);
+extern bytea *partitioned_reloptions(Datum reloptions, bool validate);
+extern bytea *heap_reloptions(Datum reloptions, bool validate);
+extern bytea *relation_reloptions(char relkind, Datum reloptions,
+ bool validate);
extern bytea *view_reloptions(Datum reloptions, bool validate);
extern bytea *index_reloptions(amoptions_function amoptions, Datum reloptions,
bool validate);
diff --git a/src/include/access/spgist.h b/src/include/access/spgist.h
index 2541d47..a176f51 100644
--- a/src/include/access/spgist.h
+++ b/src/include/access/spgist.h
@@ -20,10 +20,6 @@
#include "lib/stringinfo.h"
-/* reloption parameters */
-#define SPGIST_MIN_FILLFACTOR 10
-#define SPGIST_DEFAULT_FILLFACTOR 80
-
/* SPGiST opclass support function numbers */
#define SPGIST_CONFIG_PROC 1
#define SPGIST_CHOOSE_PROC 2
diff --git a/src/include/access/spgist_private.h b/src/include/access/spgist_private.h
index ce22bd3..23dde2f 100644
--- a/src/include/access/spgist_private.h
+++ b/src/include/access/spgist_private.h
@@ -22,6 +22,17 @@
#include "utils/relcache.h"
+typedef struct SpGistRelOptions
+{
+ int32 varlena_header_; /* varlena header (do not touch directly!) */
+ int fillfactor; /* page fill factor in percent (0..100) */
+} SpGistRelOptions;
+
+#define SpGistGetFillFactor(relation) \
+ ((relation)->rd_options ? \
+ ((SpGistRelOptions *) (relation)->rd_options)->fillfactor : \
+ SPGIST_DEFAULT_FILLFACTOR)
+
/* Page numbers of fixed-location pages */
#define SPGIST_METAPAGE_BLKNO (0) /* metapage */
#define SPGIST_ROOT_BLKNO (1) /* root for normal entries */
@@ -417,6 +428,11 @@ typedef SpGistDeadTupleData *SpGistDeadTuple;
#define GBUF_REQ_NULLS(flags) ((flags) & GBUF_NULLS)
/* spgutils.c */
+
+/* reloption parameters */
+#define SPGIST_MIN_FILLFACTOR 10
+#define SPGIST_DEFAULT_FILLFACTOR 80
+
extern SpGistCache *spgGetCache(Relation index);
extern void initSpGistState(SpGistState *state, Relation index);
extern Buffer SpGistNewBuffer(Relation index);
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index ff0a3ea..fefb1d1 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -216,15 +216,11 @@ typedef struct ForeignKeyCacheInfo
} ForeignKeyCacheInfo;
+
/*
- * StdRdOptions
- * Standard contents of rd_options for heaps and generic indexes.
- *
- * RelationGetFillFactor() and RelationGetTargetPageFreeSpace() can only
- * be applied to relations that use this format or a superset for
- * private options data.
+ * AutoVacOpts
+ * Auto Vacuum options used both in Heap and Toast relations
*/
- /* autovacuum-related reloptions. */
typedef struct AutoVacOpts
{
bool enabled;
@@ -243,50 +239,82 @@ typedef struct AutoVacOpts
float8 analyze_scale_factor;
} AutoVacOpts;
-typedef struct StdRdOptions
+/*
+ * HeapRelOptions
+ * Binary representation of relation options for Heap relations.
+ */
+typedef struct HeapRelOptions
{
int32 vl_len_; /* varlena header (do not touch directly!) */
int fillfactor; /* page fill factor in percent (0..100) */
- /* fraction of newly inserted tuples prior to trigger index cleanup */
- float8 vacuum_cleanup_index_scale_factor;
int toast_tuple_target; /* target for tuple toasting */
- AutoVacOpts autovacuum; /* autovacuum-related options */
+ AutoVacOpts autovacuum; /* autovacuum-related options */
bool user_catalog_table; /* use as an additional catalog relation */
int parallel_workers; /* max number of parallel workers */
-} StdRdOptions;
+} HeapRelOptions;
+
+/*
+ * ToastRelOptions
+ * Binary representation of relation options for Toast relations.
+ */
+typedef struct ToastRelOptions
+{
+ int32 vl_len_; /* varlena header (do not touch directly!) */
+ AutoVacOpts autovacuum; /* autovacuum-related options */
+} ToastRelOptions;
+
+
+/*
+ * PartitionedRelOptions
+ * Binary representation of relation options for Partitioned relations.
+ */
+typedef struct PartitionedRelOptions
+{
+ int32 vl_len_; /* varlena header (do not touch directly!) */
+ /* No options defined yet */
+} PartitionedRelOptions;
#define HEAP_MIN_FILLFACTOR 10
#define HEAP_DEFAULT_FILLFACTOR 100
+#define TOAST_DEFAULT_FILLFACTOR 100 /* Only default is actually used */
+
+
/*
* RelationGetToastTupleTarget
- * Returns the relation's toast_tuple_target. Note multiple eval of argument!
+ * Returns the heap's toast_tuple_target. Note multiple eval of argument!
*/
-#define RelationGetToastTupleTarget(relation, defaulttarg) \
- ((relation)->rd_options ? \
- ((StdRdOptions *) (relation)->rd_options)->toast_tuple_target : (defaulttarg))
+#define RelationGetToastTupleTarget(relation, defaulttarg) \
+ (AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION || \
+ relation->rd_rel->relkind == RELKIND_MATVIEW), \
+ ((relation)->rd_options ? \
+ ((HeapRelOptions *) (relation)->rd_options)->toast_tuple_target : \
+ (defaulttarg)))
/*
- * RelationGetFillFactor
- * Returns the relation's fillfactor. Note multiple eval of argument!
+ * HeapGetFillFactor
+ * Returns the heap's fillfactor. Note multiple eval of argument!
*/
-#define RelationGetFillFactor(relation, defaultff) \
- ((relation)->rd_options ? \
- ((StdRdOptions *) (relation)->rd_options)->fillfactor : (defaultff))
+#define HeapGetFillFactor(relation) \
+ (AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION || \
+ relation->rd_rel->relkind == RELKIND_MATVIEW), \
+ ((relation)->rd_options ? \
+ ((HeapRelOptions *) (relation)->rd_options)->fillfactor : \
+ (HEAP_DEFAULT_FILLFACTOR)))
/*
- * RelationGetTargetPageUsage
- * Returns the relation's desired space usage per page in bytes.
+ * HeapGetTargetPageUsage
+ * Returns the heap's desired space usage per page in bytes.
*/
-#define RelationGetTargetPageUsage(relation, defaultff) \
- (BLCKSZ * RelationGetFillFactor(relation, defaultff) / 100)
+#define HeapGetTargetPageUsage(relation) \
+ (BLCKSZ * HeapGetFillFactor(relation) / 100)
/*
- * RelationGetTargetPageFreeSpace
- * Returns the relation's desired freespace per page in bytes.
+ * HeapGetTargetPageFreeSpace
+ * Returns the heap's desired freespace per page in bytes.
*/
-#define RelationGetTargetPageFreeSpace(relation, defaultff) \
- (BLCKSZ * (100 - RelationGetFillFactor(relation, defaultff)) / 100)
+#define HeapGetTargetPageFreeSpace(relation) \
+ (BLCKSZ * (100 - HeapGetFillFactor(relation)) / 100)
/*
* RelationIsUsedAsCatalogTable
@@ -297,17 +325,27 @@ typedef struct StdRdOptions
((relation)->rd_options && \
((relation)->rd_rel->relkind == RELKIND_RELATION || \
(relation)->rd_rel->relkind == RELKIND_MATVIEW) ? \
- ((StdRdOptions *) (relation)->rd_options)->user_catalog_table : false)
+ ((HeapRelOptions *) (relation)->rd_options)->user_catalog_table : false)
/*
* RelationGetParallelWorkers
- * Returns the relation's parallel_workers reloption setting.
+ * Returns the heap's parallel_workers reloption setting.
* Note multiple eval of argument!
*/
-#define RelationGetParallelWorkers(relation, defaultpw) \
- ((relation)->rd_options ? \
- ((StdRdOptions *) (relation)->rd_options)->parallel_workers : (defaultpw))
+#define RelationGetParallelWorkers(relation, defaultpw) \
+ (AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION || \
+ relation->rd_rel->relkind == RELKIND_MATVIEW), \
+ ((relation)->rd_options ? \
+ ((HeapRelOptions *) (relation)->rd_options)->parallel_workers : \
+ (defaultpw)))
+/*
+ * ToastGetTargetPageFreeSpace
+ * Returns the TOAST relation's desired freespace per page in bytes.
+ * Always calculated using default fillfactor value.
+ */
+#define ToastGetTargetPageFreeSpace() \
+ (BLCKSZ * (100 - TOAST_DEFAULT_FILLFACTOR) / 100)
/*
* ViewOptions
@@ -325,9 +363,10 @@ typedef struct ViewOptions
* Returns whether the relation is security view, or not. Note multiple
* eval of argument!
*/
-#define RelationIsSecurityView(relation) \
- ((relation)->rd_options ? \
- ((ViewOptions *) (relation)->rd_options)->security_barrier : false)
+#define RelationIsSecurityView(relation) \
+ (AssertMacro(relation->rd_rel->relkind == RELKIND_VIEW), \
+ ((relation)->rd_options ? \
+ ((ViewOptions *) (relation)->rd_options)->security_barrier : false))
/*
* RelationHasCheckOption
@@ -335,8 +374,9 @@ typedef struct ViewOptions
* or the cascaded check option. Note multiple eval of argument!
*/
#define RelationHasCheckOption(relation) \
+ (AssertMacro(relation->rd_rel->relkind == RELKIND_VIEW), \
((relation)->rd_options && \
- ((ViewOptions *) (relation)->rd_options)->check_option_offset != 0)
+ ((ViewOptions *) (relation)->rd_options)->check_option_offset != 0))
/*
* RelationHasLocalCheckOption
@@ -344,11 +384,12 @@ typedef struct ViewOptions
* option. Note multiple eval of argument!
*/
#define RelationHasLocalCheckOption(relation) \
- ((relation)->rd_options && \
- ((ViewOptions *) (relation)->rd_options)->check_option_offset != 0 ? \
- strcmp((char *) (relation)->rd_options + \
- ((ViewOptions *) (relation)->rd_options)->check_option_offset, \
- "local") == 0 : false)
+ (AssertMacro(relation->rd_rel->relkind == RELKIND_VIEW), \
+ ((relation)->rd_options && \
+ ((ViewOptions *) (relation)->rd_options)->check_option_offset != 0 ? \
+ strcmp((char *) (relation)->rd_options + \
+ ((ViewOptions *) (relation)->rd_options)->check_option_offset, \
+ "local") == 0 : false))
/*
* RelationHasCascadedCheckOption
@@ -356,11 +397,12 @@ typedef struct ViewOptions
* option. Note multiple eval of argument!
*/
#define RelationHasCascadedCheckOption(relation) \
+ (AssertMacro(relation->rd_rel->relkind == RELKIND_VIEW), \
((relation)->rd_options && \
((ViewOptions *) (relation)->rd_options)->check_option_offset != 0 ? \
strcmp((char *) (relation)->rd_options + \
((ViewOptions *) (relation)->rd_options)->check_option_offset, \
- "cascaded") == 0 : false)
+ "cascaded") == 0 : false))
/*
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v7 3/4] Remove globals readOff, readLen and readSegNo
@ 2019-09-10 08:28 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Kyotaro Horiguchi @ 2019-09-10 08:28 UTC (permalink / raw)
The first two variables are functionally duplicate with them in
XLogReaderState. Remove the globals along with readSegNo, which
behaves in the similar way.
---
src/backend/access/transam/xlog.c | 79 ++++++++++++++-----------------
src/include/access/xlogreader.h | 1 +
2 files changed, 37 insertions(+), 43 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c138504f0d..a3eded30ad 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -796,18 +796,14 @@ static XLogSegNo openLogSegNo = 0;
* These variables are used similarly to the ones above, but for reading
* the XLOG. Note, however, that readOff generally represents the offset
* of the page just read, not the seek position of the FD itself, which
- * will be just past that page. readLen indicates how much of the current
- * page has been read into readBuf, and readSource indicates where we got
- * the currently open file from.
+ * will be just past that page. readSource indicates where we got the
+ * currently open file from.
* Note: we could use Reserve/ReleaseExternalFD to track consumption of
* this FD too; but it doesn't currently seem worthwhile, since the XLOG is
* not read by general-purpose sessions.
*/
static int readFile = -1;
-static XLogSegNo readSegNo = 0;
-static uint32 readOff = 0;
-static uint32 readLen = 0;
-static XLogSource readSource = XLOG_FROM_ANY;
+static XLogSource readSource = 0; /* XLOG_FROM_* code */
/*
* Keeps track of which source we're currently reading from. This is
@@ -893,10 +889,12 @@ static bool InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
static int XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli,
XLogSource source, bool notfoundOk);
static int XLogFileReadAnyTLI(XLogSegNo segno, int emode, XLogSource source);
-static bool XLogPageRead(XLogReaderState *xlogreader,
+static bool XLogPageRead(XLogReaderState *state,
bool fetching_ckpt, int emode, bool randAccess);
static bool WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
- bool fetching_ckpt, XLogRecPtr tliRecPtr);
+ bool fetching_ckpt,
+ XLogRecPtr tliRecPtr,
+ XLogSegNo readSegNo);
static int emode_for_corrupt_record(int emode, XLogRecPtr RecPtr);
static void XLogFileClose(void);
static void PreallocXlogFiles(XLogRecPtr endptr);
@@ -7586,7 +7584,8 @@ StartupXLOG(void)
XLogRecPtr pageBeginPtr;
pageBeginPtr = EndOfLog - (EndOfLog % XLOG_BLCKSZ);
- Assert(readOff == XLogSegmentOffset(pageBeginPtr, wal_segment_size));
+ Assert(XLogSegmentOffset(xlogreader->readPagePtr, wal_segment_size) ==
+ XLogSegmentOffset(pageBeginPtr, wal_segment_size));
firstIdx = XLogRecPtrToBufIdx(EndOfLog);
@@ -11652,13 +11651,14 @@ CancelBackup(void)
* sleep and retry.
*/
static bool
-XLogPageRead(XLogReaderState *xlogreader,
+XLogPageRead(XLogReaderState *state,
bool fetching_ckpt, int emode, bool randAccess)
{
- char *readBuf = xlogreader->readBuf;
- XLogRecPtr targetPagePtr = xlogreader->readPagePtr;
- int reqLen = xlogreader->readLen;
- XLogRecPtr targetRecPtr = xlogreader->ReadRecPtr;
+ char *readBuf = state->readBuf;
+ XLogRecPtr targetPagePtr = state->readPagePtr;
+ int reqLen = state->readLen;
+ int readLen = 0;
+ XLogRecPtr targetRecPtr = state->ReadRecPtr;
uint32 targetPageOff;
XLogSegNo targetSegNo PG_USED_FOR_ASSERTS_ONLY;
int r;
@@ -11671,7 +11671,7 @@ XLogPageRead(XLogReaderState *xlogreader,
* is not in the currently open one.
*/
if (readFile >= 0 &&
- !XLByteInSeg(targetPagePtr, readSegNo, wal_segment_size))
+ !XLByteInSeg(targetPagePtr, state->readSegNo, wal_segment_size))
{
/*
* Request a restartpoint if we've replayed too much xlog since the
@@ -11679,10 +11679,10 @@ XLogPageRead(XLogReaderState *xlogreader,
*/
if (bgwriterLaunched)
{
- if (XLogCheckpointNeeded(readSegNo))
+ if (XLogCheckpointNeeded(state->readSegNo))
{
(void) GetRedoRecPtr();
- if (XLogCheckpointNeeded(readSegNo))
+ if (XLogCheckpointNeeded(state->readSegNo))
RequestCheckpoint(CHECKPOINT_CAUSE_XLOG);
}
}
@@ -11692,7 +11692,7 @@ XLogPageRead(XLogReaderState *xlogreader,
readSource = XLOG_FROM_ANY;
}
- XLByteToSeg(targetPagePtr, readSegNo, wal_segment_size);
+ XLByteToSeg(targetPagePtr, state->readSegNo, wal_segment_size);
retry:
/* See if we need to retrieve more data */
@@ -11701,17 +11701,14 @@ retry:
receivedUpto < targetPagePtr + reqLen))
{
if (!WaitForWALToBecomeAvailable(targetPagePtr + reqLen,
- randAccess,
- fetching_ckpt,
- targetRecPtr))
+ randAccess, fetching_ckpt,
+ targetRecPtr, state->readSegNo))
{
if (readFile >= 0)
close(readFile);
readFile = -1;
- readLen = 0;
readSource = XLOG_FROM_ANY;
-
- xlogreader->readLen = -1;
+ state->readLen = -1;
return false;
}
}
@@ -11739,40 +11736,36 @@ retry:
else
readLen = XLOG_BLCKSZ;
- /* Read the requested page */
- readOff = targetPageOff;
-
pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
- r = pg_pread(readFile, readBuf, XLOG_BLCKSZ, (off_t) readOff);
+ r = pg_pread(readFile, readBuf, XLOG_BLCKSZ, (off_t) targetPageOff);
if (r != XLOG_BLCKSZ)
{
char fname[MAXFNAMELEN];
int save_errno = errno;
pgstat_report_wait_end();
- XLogFileName(fname, curFileTLI, readSegNo, wal_segment_size);
+ XLogFileName(fname, curFileTLI, state->readSegNo, wal_segment_size);
if (r < 0)
{
errno = save_errno;
ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen),
(errcode_for_file_access(),
errmsg("could not read from log segment %s, offset %u: %m",
- fname, readOff)));
+ fname, targetPageOff)));
}
else
ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen),
(errcode(ERRCODE_DATA_CORRUPTED),
errmsg("could not read from log segment %s, offset %u: read %d of %zu",
- fname, readOff, r, (Size) XLOG_BLCKSZ)));
+ fname, targetPageOff, r, (Size) XLOG_BLCKSZ)));
goto next_record_is_invalid;
}
pgstat_report_wait_end();
- Assert(targetSegNo == readSegNo);
- Assert(targetPageOff == readOff);
+ Assert(targetSegNo == state->readSegNo);
Assert(reqLen <= readLen);
- xlogreader->seg.ws_tli = curFileTLI;
+ state->seg.ws_tli = curFileTLI;
/*
* Check the page header immediately, so that we can retry immediately if
@@ -11800,15 +11793,15 @@ retry:
* Validating the page header is cheap enough that doing it twice
* shouldn't be a big deal from a performance point of view.
*/
- if (!XLogReaderValidatePageHeader(xlogreader, targetPagePtr, readBuf))
+ if (!XLogReaderValidatePageHeader(state, targetPagePtr, readBuf))
{
- /* reset any error XLogReaderValidatePageHeader() might have set */
- xlogreader->errormsg_buf[0] = '\0';
+ /* reset any error StateValidatePageHeader() might have set */
+ state->errormsg_buf[0] = '\0';
goto next_record_is_invalid;
}
- Assert(xlogreader->readPagePtr == targetPagePtr);
- xlogreader->readLen = readLen;
+ Assert(state->readPagePtr == targetPagePtr);
+ state->readLen = readLen;
return true;
next_record_is_invalid:
@@ -11817,14 +11810,13 @@ next_record_is_invalid:
if (readFile >= 0)
close(readFile);
readFile = -1;
- readLen = 0;
readSource = XLOG_FROM_ANY;
/* In standby-mode, keep trying */
if (StandbyMode)
goto retry;
- xlogreader->readLen = -1;
+ state->readLen = -1;
return false;
}
@@ -11856,7 +11848,8 @@ next_record_is_invalid:
*/
static bool
WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
- bool fetching_ckpt, XLogRecPtr tliRecPtr)
+ bool fetching_ckpt, XLogRecPtr tliRecPtr,
+ XLogSegNo readSegNo)
{
static TimestampTz last_fail_time = 0;
TimestampTz now;
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index e59e42bee3..a862db6d90 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -135,6 +135,7 @@ struct XLogReaderState
* read by reader, which must be larger than
* the request, or -1 on error */
TimeLineID readPageTLI; /* TLI for data currently in readBuf */
+ XLogSegNo readSegNo; /* Segment # for data currently in readBuf */
char *readBuf; /* buffer to store data */
bool page_verified; /* is the page header on the buffer verified? */
bool record_verified;/* is the current record header verified? */
--
2.18.2
----Next_Part(Tue_Mar_24_18_24_13_2020_275)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v7-0004-Change-policy-of-XLog-read-buffer-allocation.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v12 3/4] Remove globals readOff, readLen and readSegNo
@ 2019-09-10 08:28 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Kyotaro Horiguchi @ 2019-09-10 08:28 UTC (permalink / raw)
The first two variables are functionally duplicate with them in
XLogReaderState. Remove the globals along with readSegNo, which
behaves in the similar way.
---
src/backend/access/transam/xlog.c | 77 ++++++++++++++-----------------
src/include/access/xlogreader.h | 1 +
2 files changed, 36 insertions(+), 42 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index a4e606cebf..364b1948e4 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -783,14 +783,10 @@ static XLogSegNo openLogSegNo = 0;
* These variables are used similarly to the ones above, but for reading
* the XLOG. Note, however, that readOff generally represents the offset
* of the page just read, not the seek position of the FD itself, which
- * will be just past that page. readLen indicates how much of the current
- * page has been read into readBuf, and readSource indicates where we got
- * the currently open file from.
+ * will be just past that page. readSource indicates where we got the
+ * currently open file from.
*/
static int readFile = -1;
-static XLogSegNo readSegNo = 0;
-static uint32 readOff = 0;
-static uint32 readLen = 0;
static XLogSource readSource = 0; /* XLOG_FROM_* code */
/*
@@ -877,10 +873,12 @@ static bool InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
static int XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli,
int source, bool notfoundOk);
static int XLogFileReadAnyTLI(XLogSegNo segno, int emode, int source);
-static bool XLogPageRead(XLogReaderState *xlogreader,
+static bool XLogPageRead(XLogReaderState *state,
bool fetching_ckpt, int emode, bool randAccess);
static bool WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
- bool fetching_ckpt, XLogRecPtr tliRecPtr);
+ bool fetching_ckpt,
+ XLogRecPtr tliRecPtr,
+ XLogSegNo readSegNo);
static int emode_for_corrupt_record(int emode, XLogRecPtr RecPtr);
static void XLogFileClose(void);
static void PreallocXlogFiles(XLogRecPtr endptr);
@@ -7509,7 +7507,8 @@ StartupXLOG(void)
XLogRecPtr pageBeginPtr;
pageBeginPtr = EndOfLog - (EndOfLog % XLOG_BLCKSZ);
- Assert(readOff == XLogSegmentOffset(pageBeginPtr, wal_segment_size));
+ Assert(XLogSegmentOffset(xlogreader->readPagePtr, wal_segment_size) ==
+ XLogSegmentOffset(pageBeginPtr, wal_segment_size));
firstIdx = XLogRecPtrToBufIdx(EndOfLog);
@@ -11529,13 +11528,14 @@ CancelBackup(void)
* sleep and retry.
*/
static bool
-XLogPageRead(XLogReaderState *xlogreader,
+XLogPageRead(XLogReaderState *state,
bool fetching_ckpt, int emode, bool randAccess)
{
- char *readBuf = xlogreader->readBuf;
- XLogRecPtr targetPagePtr = xlogreader->readPagePtr;
- int reqLen = xlogreader->readLen;
- XLogRecPtr targetRecPtr = xlogreader->ReadRecPtr;
+ char *readBuf = state->readBuf;
+ XLogRecPtr targetPagePtr = state->readPagePtr;
+ int reqLen = state->readLen;
+ int readLen = 0;
+ XLogRecPtr targetRecPtr = state->ReadRecPtr;
uint32 targetPageOff;
XLogSegNo targetSegNo PG_USED_FOR_ASSERTS_ONLY;
int r;
@@ -11548,7 +11548,7 @@ XLogPageRead(XLogReaderState *xlogreader,
* is not in the currently open one.
*/
if (readFile >= 0 &&
- !XLByteInSeg(targetPagePtr, readSegNo, wal_segment_size))
+ !XLByteInSeg(targetPagePtr, state->readSegNo, wal_segment_size))
{
/*
* Request a restartpoint if we've replayed too much xlog since the
@@ -11556,10 +11556,10 @@ XLogPageRead(XLogReaderState *xlogreader,
*/
if (bgwriterLaunched)
{
- if (XLogCheckpointNeeded(readSegNo))
+ if (XLogCheckpointNeeded(state->readSegNo))
{
(void) GetRedoRecPtr();
- if (XLogCheckpointNeeded(readSegNo))
+ if (XLogCheckpointNeeded(state->readSegNo))
RequestCheckpoint(CHECKPOINT_CAUSE_XLOG);
}
}
@@ -11569,7 +11569,7 @@ XLogPageRead(XLogReaderState *xlogreader,
readSource = 0;
}
- XLByteToSeg(targetPagePtr, readSegNo, wal_segment_size);
+ XLByteToSeg(targetPagePtr, state->readSegNo, wal_segment_size);
retry:
/* See if we need to retrieve more data */
@@ -11578,17 +11578,14 @@ retry:
receivedUpto < targetPagePtr + reqLen))
{
if (!WaitForWALToBecomeAvailable(targetPagePtr + reqLen,
- randAccess,
- fetching_ckpt,
- targetRecPtr))
+ randAccess, fetching_ckpt,
+ targetRecPtr, state->readSegNo))
{
if (readFile >= 0)
close(readFile);
readFile = -1;
- readLen = 0;
readSource = 0;
-
- xlogreader->readLen = -1;
+ state->readLen = -1;
return false;
}
}
@@ -11616,40 +11613,36 @@ retry:
else
readLen = XLOG_BLCKSZ;
- /* Read the requested page */
- readOff = targetPageOff;
-
pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
- r = pg_pread(readFile, readBuf, XLOG_BLCKSZ, (off_t) readOff);
+ r = pg_pread(readFile, readBuf, XLOG_BLCKSZ, (off_t) targetPageOff);
if (r != XLOG_BLCKSZ)
{
char fname[MAXFNAMELEN];
int save_errno = errno;
pgstat_report_wait_end();
- XLogFileName(fname, curFileTLI, readSegNo, wal_segment_size);
+ XLogFileName(fname, curFileTLI, state->readSegNo, wal_segment_size);
if (r < 0)
{
errno = save_errno;
ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen),
(errcode_for_file_access(),
errmsg("could not read from log segment %s, offset %u: %m",
- fname, readOff)));
+ fname, targetPageOff)));
}
else
ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen),
(errcode(ERRCODE_DATA_CORRUPTED),
errmsg("could not read from log segment %s, offset %u: read %d of %zu",
- fname, readOff, r, (Size) XLOG_BLCKSZ)));
+ fname, targetPageOff, r, (Size) XLOG_BLCKSZ)));
goto next_record_is_invalid;
}
pgstat_report_wait_end();
- Assert(targetSegNo == readSegNo);
- Assert(targetPageOff == readOff);
+ Assert(targetSegNo == state->readSegNo);
Assert(reqLen <= readLen);
- xlogreader->seg.ws_tli = curFileTLI;
+ state->seg.ws_tli = curFileTLI;
/*
* Check the page header immediately, so that we can retry immediately if
@@ -11677,15 +11670,15 @@ retry:
* Validating the page header is cheap enough that doing it twice
* shouldn't be a big deal from a performance point of view.
*/
- if (!XLogReaderValidatePageHeader(xlogreader, targetPagePtr, readBuf))
+ if (!XLogReaderValidatePageHeader(state, targetPagePtr, readBuf))
{
- /* reset any error XLogReaderValidatePageHeader() might have set */
- xlogreader->errormsg_buf[0] = '\0';
+ /* reset any error StateValidatePageHeader() might have set */
+ state->errormsg_buf[0] = '\0';
goto next_record_is_invalid;
}
- Assert(xlogreader->readPagePtr == targetPagePtr);
- xlogreader->readLen = readLen;
+ Assert(state->readPagePtr == targetPagePtr);
+ state->readLen = readLen;
return true;
next_record_is_invalid:
@@ -11694,14 +11687,13 @@ next_record_is_invalid:
if (readFile >= 0)
close(readFile);
readFile = -1;
- readLen = 0;
readSource = 0;
/* In standby-mode, keep trying */
if (StandbyMode)
goto retry;
- xlogreader->readLen = -1;
+ state->readLen = -1;
return false;
}
@@ -11733,7 +11725,8 @@ next_record_is_invalid:
*/
static bool
WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
- bool fetching_ckpt, XLogRecPtr tliRecPtr)
+ bool fetching_ckpt, XLogRecPtr tliRecPtr,
+ XLogSegNo readSegNo)
{
static TimestampTz last_fail_time = 0;
TimestampTz now;
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index 2494b2416f..f6249a5844 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -132,6 +132,7 @@ struct XLogReaderState
* read by reader, which must be larger than
* the request, or -1 on error */
TimeLineID readPageTLI; /* TLI for data currently in readBuf */
+ XLogSegNo readSegNo; /* Segment # for data currently in readBuf */
char *readBuf; /* buffer to store data */
bool page_verified; /* is the page header on the buffer verified? */
bool record_verified;/* is the current record header verified? */
--
2.23.0
----Next_Part(Fri_Nov_29_17_14_21_2019_330)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v12-0004-Change-policy-of-XLog-read-buffer-allocation.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v11 3/4] Remove globals readOff, readLen and readSegNo
@ 2019-09-10 08:28 Kyotaro Horiguchi <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Kyotaro Horiguchi @ 2019-09-10 08:28 UTC (permalink / raw)
The first two variables are functionally duplicate with them in
XLogReaderState. Remove the globals along with readSegNo, which
behaves in the similar way.
---
src/backend/access/transam/xlog.c | 77 ++++++++++++++-----------------
src/include/access/xlogreader.h | 1 +
2 files changed, 36 insertions(+), 42 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index a4e606cebf..364b1948e4 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -783,14 +783,10 @@ static XLogSegNo openLogSegNo = 0;
* These variables are used similarly to the ones above, but for reading
* the XLOG. Note, however, that readOff generally represents the offset
* of the page just read, not the seek position of the FD itself, which
- * will be just past that page. readLen indicates how much of the current
- * page has been read into readBuf, and readSource indicates where we got
- * the currently open file from.
+ * will be just past that page. readSource indicates where we got the
+ * currently open file from.
*/
static int readFile = -1;
-static XLogSegNo readSegNo = 0;
-static uint32 readOff = 0;
-static uint32 readLen = 0;
static XLogSource readSource = 0; /* XLOG_FROM_* code */
/*
@@ -877,10 +873,12 @@ static bool InstallXLogFileSegment(XLogSegNo *segno, char *tmppath,
static int XLogFileRead(XLogSegNo segno, int emode, TimeLineID tli,
int source, bool notfoundOk);
static int XLogFileReadAnyTLI(XLogSegNo segno, int emode, int source);
-static bool XLogPageRead(XLogReaderState *xlogreader,
+static bool XLogPageRead(XLogReaderState *state,
bool fetching_ckpt, int emode, bool randAccess);
static bool WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
- bool fetching_ckpt, XLogRecPtr tliRecPtr);
+ bool fetching_ckpt,
+ XLogRecPtr tliRecPtr,
+ XLogSegNo readSegNo);
static int emode_for_corrupt_record(int emode, XLogRecPtr RecPtr);
static void XLogFileClose(void);
static void PreallocXlogFiles(XLogRecPtr endptr);
@@ -7509,7 +7507,8 @@ StartupXLOG(void)
XLogRecPtr pageBeginPtr;
pageBeginPtr = EndOfLog - (EndOfLog % XLOG_BLCKSZ);
- Assert(readOff == XLogSegmentOffset(pageBeginPtr, wal_segment_size));
+ Assert(XLogSegmentOffset(xlogreader->readPagePtr, wal_segment_size) ==
+ XLogSegmentOffset(pageBeginPtr, wal_segment_size));
firstIdx = XLogRecPtrToBufIdx(EndOfLog);
@@ -11529,13 +11528,14 @@ CancelBackup(void)
* sleep and retry.
*/
static bool
-XLogPageRead(XLogReaderState *xlogreader,
+XLogPageRead(XLogReaderState *state,
bool fetching_ckpt, int emode, bool randAccess)
{
- char *readBuf = xlogreader->readBuf;
- XLogRecPtr targetPagePtr = xlogreader->readPagePtr;
- int reqLen = xlogreader->readLen;
- XLogRecPtr targetRecPtr = xlogreader->ReadRecPtr;
+ char *readBuf = state->readBuf;
+ XLogRecPtr targetPagePtr = state->readPagePtr;
+ int reqLen = state->readLen;
+ int readLen = 0;
+ XLogRecPtr targetRecPtr = state->ReadRecPtr;
uint32 targetPageOff;
XLogSegNo targetSegNo PG_USED_FOR_ASSERTS_ONLY;
int r;
@@ -11548,7 +11548,7 @@ XLogPageRead(XLogReaderState *xlogreader,
* is not in the currently open one.
*/
if (readFile >= 0 &&
- !XLByteInSeg(targetPagePtr, readSegNo, wal_segment_size))
+ !XLByteInSeg(targetPagePtr, state->readSegNo, wal_segment_size))
{
/*
* Request a restartpoint if we've replayed too much xlog since the
@@ -11556,10 +11556,10 @@ XLogPageRead(XLogReaderState *xlogreader,
*/
if (bgwriterLaunched)
{
- if (XLogCheckpointNeeded(readSegNo))
+ if (XLogCheckpointNeeded(state->readSegNo))
{
(void) GetRedoRecPtr();
- if (XLogCheckpointNeeded(readSegNo))
+ if (XLogCheckpointNeeded(state->readSegNo))
RequestCheckpoint(CHECKPOINT_CAUSE_XLOG);
}
}
@@ -11569,7 +11569,7 @@ XLogPageRead(XLogReaderState *xlogreader,
readSource = 0;
}
- XLByteToSeg(targetPagePtr, readSegNo, wal_segment_size);
+ XLByteToSeg(targetPagePtr, state->readSegNo, wal_segment_size);
retry:
/* See if we need to retrieve more data */
@@ -11578,17 +11578,14 @@ retry:
receivedUpto < targetPagePtr + reqLen))
{
if (!WaitForWALToBecomeAvailable(targetPagePtr + reqLen,
- randAccess,
- fetching_ckpt,
- targetRecPtr))
+ randAccess, fetching_ckpt,
+ targetRecPtr, state->readSegNo))
{
if (readFile >= 0)
close(readFile);
readFile = -1;
- readLen = 0;
readSource = 0;
-
- xlogreader->readLen = -1;
+ state->readLen = -1;
return false;
}
}
@@ -11616,40 +11613,36 @@ retry:
else
readLen = XLOG_BLCKSZ;
- /* Read the requested page */
- readOff = targetPageOff;
-
pgstat_report_wait_start(WAIT_EVENT_WAL_READ);
- r = pg_pread(readFile, readBuf, XLOG_BLCKSZ, (off_t) readOff);
+ r = pg_pread(readFile, readBuf, XLOG_BLCKSZ, (off_t) targetPageOff);
if (r != XLOG_BLCKSZ)
{
char fname[MAXFNAMELEN];
int save_errno = errno;
pgstat_report_wait_end();
- XLogFileName(fname, curFileTLI, readSegNo, wal_segment_size);
+ XLogFileName(fname, curFileTLI, state->readSegNo, wal_segment_size);
if (r < 0)
{
errno = save_errno;
ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen),
(errcode_for_file_access(),
errmsg("could not read from log segment %s, offset %u: %m",
- fname, readOff)));
+ fname, targetPageOff)));
}
else
ereport(emode_for_corrupt_record(emode, targetPagePtr + reqLen),
(errcode(ERRCODE_DATA_CORRUPTED),
errmsg("could not read from log segment %s, offset %u: read %d of %zu",
- fname, readOff, r, (Size) XLOG_BLCKSZ)));
+ fname, targetPageOff, r, (Size) XLOG_BLCKSZ)));
goto next_record_is_invalid;
}
pgstat_report_wait_end();
- Assert(targetSegNo == readSegNo);
- Assert(targetPageOff == readOff);
+ Assert(targetSegNo == state->readSegNo);
Assert(reqLen <= readLen);
- xlogreader->seg.ws_tli = curFileTLI;
+ state->seg.ws_tli = curFileTLI;
/*
* Check the page header immediately, so that we can retry immediately if
@@ -11677,15 +11670,15 @@ retry:
* Validating the page header is cheap enough that doing it twice
* shouldn't be a big deal from a performance point of view.
*/
- if (!XLogReaderValidatePageHeader(xlogreader, targetPagePtr, readBuf))
+ if (!XLogReaderValidatePageHeader(state, targetPagePtr, readBuf))
{
- /* reset any error XLogReaderValidatePageHeader() might have set */
- xlogreader->errormsg_buf[0] = '\0';
+ /* reset any error StateValidatePageHeader() might have set */
+ state->errormsg_buf[0] = '\0';
goto next_record_is_invalid;
}
- Assert(xlogreader->readPagePtr == targetPagePtr);
- xlogreader->readLen = readLen;
+ Assert(state->readPagePtr == targetPagePtr);
+ state->readLen = readLen;
return true;
next_record_is_invalid:
@@ -11694,14 +11687,13 @@ next_record_is_invalid:
if (readFile >= 0)
close(readFile);
readFile = -1;
- readLen = 0;
readSource = 0;
/* In standby-mode, keep trying */
if (StandbyMode)
goto retry;
- xlogreader->readLen = -1;
+ state->readLen = -1;
return false;
}
@@ -11733,7 +11725,8 @@ next_record_is_invalid:
*/
static bool
WaitForWALToBecomeAvailable(XLogRecPtr RecPtr, bool randAccess,
- bool fetching_ckpt, XLogRecPtr tliRecPtr)
+ bool fetching_ckpt, XLogRecPtr tliRecPtr,
+ XLogSegNo readSegNo)
{
static TimestampTz last_fail_time = 0;
TimestampTz now;
diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h
index 2494b2416f..f6249a5844 100644
--- a/src/include/access/xlogreader.h
+++ b/src/include/access/xlogreader.h
@@ -132,6 +132,7 @@ struct XLogReaderState
* read by reader, which must be larger than
* the request, or -1 on error */
TimeLineID readPageTLI; /* TLI for data currently in readBuf */
+ XLogSegNo readSegNo; /* Segment # for data currently in readBuf */
char *readBuf; /* buffer to store data */
bool page_verified; /* is the page header on the buffer verified? */
bool record_verified;/* is the current record header verified? */
--
2.23.0
----Next_Part(Wed_Nov_27_12_09_23_2019_240)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline;
filename="v11-0004-Change-policy-of-XLog-read-buffer-allocation.patch"
^ permalink raw reply [nested|flat] 30+ messages in thread
* [PATCH v1 1/1] remove db_user_namespace
@ 2023-06-30 19:46 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Nathan Bossart @ 2023-06-30 19:46 UTC (permalink / raw)
---
doc/src/sgml/config.sgml | 52 -------------------
src/backend/libpq/auth.c | 5 --
src/backend/libpq/hba.c | 12 -----
src/backend/postmaster/postmaster.c | 19 -------
src/backend/utils/misc/guc_tables.c | 9 ----
src/backend/utils/misc/postgresql.conf.sample | 1 -
src/include/libpq/pqcomm.h | 2 -
7 files changed, 100 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 6262cb7bb2..e6cea8ddfc 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1188,58 +1188,6 @@ include_dir 'conf.d'
</para>
</listitem>
</varlistentry>
-
- <varlistentry id="guc-db-user-namespace" xreflabel="db_user_namespace">
- <term><varname>db_user_namespace</varname> (<type>boolean</type>)
- <indexterm>
- <primary><varname>db_user_namespace</varname> configuration parameter</primary>
- </indexterm>
- </term>
- <listitem>
- <para>
- This parameter enables per-database user names. It is off by default.
- This parameter can only be set in the <filename>postgresql.conf</filename>
- file or on the server command line.
- </para>
-
- <para>
- If this is on, you should create users as <replaceable>username@dbname</replaceable>.
- When <replaceable>username</replaceable> is passed by a connecting client,
- <literal>@</literal> and the database name are appended to the user
- name and that database-specific user name is looked up by the
- server. Note that when you create users with names containing
- <literal>@</literal> within the SQL environment, you will need to
- quote the user name.
- </para>
-
- <para>
- With this parameter enabled, you can still create ordinary global
- users. Simply append <literal>@</literal> when specifying the user
- name in the client, e.g., <literal>joe@</literal>. The <literal>@</literal>
- will be stripped off before the user name is looked up by the
- server.
- </para>
-
- <para>
- <varname>db_user_namespace</varname> causes the client's and
- server's user name representation to differ.
- Authentication checks are always done with the server's user name
- so authentication methods must be configured for the
- server's user name, not the client's. Because
- <literal>md5</literal> uses the user name as salt on both the
- client and server, <literal>md5</literal> cannot be used with
- <varname>db_user_namespace</varname>.
- </para>
-
- <note>
- <para>
- This feature is intended as a temporary measure until a
- complete solution is found. At that time, this option will
- be removed.
- </para>
- </note>
- </listitem>
- </varlistentry>
</variablelist>
</sect2>
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index a98b934a8e..65d452f099 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -873,11 +873,6 @@ CheckMD5Auth(Port *port, char *shadow_pass, const char **logdetail)
char *passwd;
int result;
- if (Db_user_namespace)
- ereport(FATAL,
- (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
- errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled")));
-
/* include the salt to use for computing the response */
if (!pg_strong_random(md5Salt, 4))
{
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index f89f138f3c..5d4ddbb04d 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -1741,19 +1741,7 @@ parse_hba_line(TokenizedAuthLine *tok_line, int elevel)
else if (strcmp(token->string, "reject") == 0)
parsedline->auth_method = uaReject;
else if (strcmp(token->string, "md5") == 0)
- {
- if (Db_user_namespace)
- {
- ereport(elevel,
- (errcode(ERRCODE_CONFIG_FILE_ERROR),
- errmsg("MD5 authentication is not supported when \"db_user_namespace\" is enabled"),
- errcontext("line %d of configuration file \"%s\"",
- line_num, file_name)));
- *err_msg = "MD5 authentication is not supported when \"db_user_namespace\" is enabled";
- return NULL;
- }
parsedline->auth_method = uaMD5;
- }
else if (strcmp(token->string, "scram-sha-256") == 0)
parsedline->auth_method = uaSCRAM;
else if (strcmp(token->string, "pam") == 0)
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 4c49393fc5..33a13fdf32 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -236,7 +236,6 @@ int AuthenticationTimeout = 60;
bool log_hostname; /* for ps display and logging */
bool Log_connections = false;
-bool Db_user_namespace = false;
bool enable_bonjour = false;
char *bonjour_name;
@@ -2272,24 +2271,6 @@ retry1:
if (port->database_name == NULL || port->database_name[0] == '\0')
port->database_name = pstrdup(port->user_name);
- if (Db_user_namespace)
- {
- /*
- * If user@, it is a global user, remove '@'. We only want to do this
- * if there is an '@' at the end and no earlier in the user string or
- * they may fake as a local user of another database attaching to this
- * database.
- */
- if (strchr(port->user_name, '@') ==
- port->user_name + strlen(port->user_name) - 1)
- *strchr(port->user_name, '@') = '\0';
- else
- {
- /* Append '@' and dbname */
- port->user_name = psprintf("%s@%s", port->user_name, port->database_name);
- }
- }
-
/*
* Truncate given database and user names to length of a Postgres name.
* This avoids lookup failures when overlength names are given.
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index 71e27f8eb0..25d9008bb6 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -1534,15 +1534,6 @@ struct config_bool ConfigureNamesBool[] =
false,
NULL, NULL, NULL
},
- {
- {"db_user_namespace", PGC_SIGHUP, CONN_AUTH_AUTH,
- gettext_noop("Enables per-database user names."),
- NULL
- },
- &Db_user_namespace,
- false,
- NULL, NULL, NULL
- },
{
{"default_transaction_read_only", PGC_USERSET, CLIENT_CONN_STATEMENT,
gettext_noop("Sets the default read-only status of new transactions."),
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index e4c0269fa3..c768af9a73 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -96,7 +96,6 @@
#authentication_timeout = 1min # 1s-600s
#password_encryption = scram-sha-256 # scram-sha-256 or md5
#scram_iterations = 4096
-#db_user_namespace = off
# GSSAPI using Kerberos
#krb_server_keyfile = 'FILE:${sysconfdir}/krb5.keytab'
diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h
index c85090259d..3da00f7983 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -103,8 +103,6 @@ typedef ProtocolVersion MsgType;
typedef uint32 PacketLen;
-extern PGDLLIMPORT bool Db_user_namespace;
-
/*
* In protocol 3.0 and later, the startup packet length is not fixed, but
* we set an arbitrary limit on it anyway. This is just to prevent simple
--
2.25.1
--Kj7319i9nmIyA2yE--
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
@ 2024-09-16 14:30 Anton A. Melnikov <[email protected]>
2024-09-17 02:47 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Anton A. Melnikov @ 2024-09-16 14:30 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Anton A. Melnikov <[email protected]>; Andres Freund <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
Hi!
On 13.09.2024 18:20, Fujii Masao wrote:
>
> If I understand correctly, restartpoints_timed and restartpoints_done were
> separated because a restartpoint can be skipped. restartpoints_timed counts
> when a restartpoint is triggered by a timeout, whether it runs or not,
> while restartpoints_done only tracks completed restartpoints.
>
> Similarly, I believe checkpoints should be handled the same way.
> Checkpoints can also be skipped when the system is idle, but currently,
> num_timed counts even the skipped ones, despite its documentation stating
> it's the "Number of scheduled checkpoints that have been performed."
>
> Why not separate num_timed into something like checkpoints_timed and
> checkpoints_done to reflect these different counters?
+1
This idea seems quite tenable to me.
There is a small clarification. Now if there were no skipped restartpoints then
restartpoints_done will be equal to restartpoints_timed + restartpoints_req.
Similar for checkpoints.
So i tried to introduce num_done counter for checkpoints in the patch attached.
I'm not sure should we include testing for the case when num_done is less than
num_timed + num_requested to the regress tests. I haven't been able to get it in a short time yet.
E.g. such a case may be obtained when an a error "checkpoints are
occurring too frequently" as follows:
-set checkpoint_timeout = 30 and checkpoint_warning = 40 in the postgresql.conf
-start server
-do periodically bulk insertions in the 1st client (e.g. insert into test values (generate_series(1,1E7));)
-watch for pg_stat_checkpointer in the 2nd one:
# SELECT CURRENT_TIME; select * from pg_stat_checkpointer;
# \watch
After some time, in the log will appear:
2024-09-16 16:38:47.888 MSK [193733] LOG: checkpoints are occurring too frequently (13 seconds apart)
2024-09-16 16:38:47.888 MSK [193733] HINT: Consider increasing the configuration parameter "max_wal_size".
And num_timed + num_requested will become greater than num_done.
Would be nice to find some simpler and faster way.
With the best regards,
--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachments:
[text/x-patch] v1-0001-Introduce-num_done-counter-in-the-pg_stat_checkpointer.patch (10.7K, ../../[email protected]/2-v1-0001-Introduce-num_done-counter-in-the-pg_stat_checkpointer.patch)
download | inline diff:
From fcd0b61d1f1718dbf664cb3509aad16543d65375 Mon Sep 17 00:00:00 2001
From: "Anton A. Melnikov" <[email protected]>
Date: Mon, 16 Sep 2024 16:12:07 +0300
Subject: [PATCH] Introduce num_done counter in the pg_stat_checkpointer view
that reflects number of really performed checkpoints.
---
doc/src/sgml/monitoring.sgml | 13 +++++-
src/backend/catalog/system_views.sql | 1 +
src/backend/postmaster/checkpointer.c | 2 +
.../utils/activity/pgstat_checkpointer.c | 2 +
src/backend/utils/adt/pgstatfuncs.c | 6 +++
src/include/catalog/pg_proc.dat | 40 +++++++++++--------
src/include/pgstat.h | 1 +
src/test/regress/expected/rules.out | 1 +
8 files changed, 47 insertions(+), 19 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 933de6fe07f..dad7e236a43 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3051,7 +3051,7 @@ description | Waiting for a newly initialized WAL file to reach durable storage
<structfield>num_timed</structfield> <type>bigint</type>
</para>
<para>
- Number of scheduled checkpoints that have been performed
+ Number of scheduled checkpoints
</para></entry>
</row>
@@ -3060,7 +3060,16 @@ description | Waiting for a newly initialized WAL file to reach durable storage
<structfield>num_requested</structfield> <type>bigint</type>
</para>
<para>
- Number of requested checkpoints that have been performed
+ Number of backend requested checkpoints
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>num_done</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Number of checkpoints that have been performed
</para></entry>
</row>
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 7fd5d256a18..49109dbdc86 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1138,6 +1138,7 @@ CREATE VIEW pg_stat_checkpointer AS
SELECT
pg_stat_get_checkpointer_num_timed() AS num_timed,
pg_stat_get_checkpointer_num_requested() AS num_requested,
+ pg_stat_get_checkpointer_num_performed() AS num_done,
pg_stat_get_checkpointer_restartpoints_timed() AS restartpoints_timed,
pg_stat_get_checkpointer_restartpoints_requested() AS restartpoints_req,
pg_stat_get_checkpointer_restartpoints_performed() AS restartpoints_done,
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index eeb73c85726..06ad2f52f27 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -495,6 +495,8 @@ CheckpointerMain(char *startup_data, size_t startup_data_len)
if (do_restartpoint)
PendingCheckpointerStats.restartpoints_performed++;
+ else
+ PendingCheckpointerStats.num_performed++;
}
else
{
diff --git a/src/backend/utils/activity/pgstat_checkpointer.c b/src/backend/utils/activity/pgstat_checkpointer.c
index bbfc9c7e183..4a0a2d1493a 100644
--- a/src/backend/utils/activity/pgstat_checkpointer.c
+++ b/src/backend/utils/activity/pgstat_checkpointer.c
@@ -49,6 +49,7 @@ pgstat_report_checkpointer(void)
#define CHECKPOINTER_ACC(fld) stats_shmem->stats.fld += PendingCheckpointerStats.fld
CHECKPOINTER_ACC(num_timed);
CHECKPOINTER_ACC(num_requested);
+ CHECKPOINTER_ACC(num_performed);
CHECKPOINTER_ACC(restartpoints_timed);
CHECKPOINTER_ACC(restartpoints_requested);
CHECKPOINTER_ACC(restartpoints_performed);
@@ -127,6 +128,7 @@ pgstat_checkpointer_snapshot_cb(void)
#define CHECKPOINTER_COMP(fld) pgStatLocal.snapshot.checkpointer.fld -= reset.fld;
CHECKPOINTER_COMP(num_timed);
CHECKPOINTER_COMP(num_requested);
+ CHECKPOINTER_COMP(num_performed);
CHECKPOINTER_COMP(restartpoints_timed);
CHECKPOINTER_COMP(restartpoints_requested);
CHECKPOINTER_COMP(restartpoints_performed);
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 33c7b25560b..5624bef18e7 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1191,6 +1191,12 @@ pg_stat_get_checkpointer_num_requested(PG_FUNCTION_ARGS)
PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->num_requested);
}
+Datum
+pg_stat_get_checkpointer_num_performed(PG_FUNCTION_ARGS)
+{
+ PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->num_performed);
+}
+
Datum
pg_stat_get_checkpointer_restartpoints_timed(PG_FUNCTION_ARGS)
{
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 53a081ed886..c8c1b79ff63 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5807,42 +5807,58 @@
proargmodes => '{o,o,o,o,o,o,o}',
proargnames => '{archived_count,last_archived_wal,last_archived_time,failed_count,last_failed_wal,last_failed_time,stats_reset}',
prosrc => 'pg_stat_get_archiver' },
-{ oid => '2769',
+{ oid => '6347',
descr => 'statistics: number of timed checkpoints started by the checkpointer',
proname => 'pg_stat_get_checkpointer_num_timed', provolatile => 's',
proparallel => 'r', prorettype => 'int8', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_num_timed' },
-{ oid => '2770',
+{ oid => '6348',
descr => 'statistics: number of backend requested checkpoints started by the checkpointer',
proname => 'pg_stat_get_checkpointer_num_requested', provolatile => 's',
proparallel => 'r', prorettype => 'int8', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_num_requested' },
-{ oid => '6327',
+{ oid => '6349',
+ descr => 'statistics: number of checkpoints performed by the checkpointer',
+ proname => 'pg_stat_get_checkpointer_num_performed',
+ provolatile => 's', proparallel => 'r', prorettype => 'int8',
+ proargtypes => '',
+ prosrc => 'pg_stat_get_checkpointer_num_performed' },
+{ oid => '6350',
descr => 'statistics: number of timed restartpoints started by the checkpointer',
proname => 'pg_stat_get_checkpointer_restartpoints_timed', provolatile => 's',
proparallel => 'r', prorettype => 'int8', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_restartpoints_timed' },
-{ oid => '6328',
+{ oid => '6351',
descr => 'statistics: number of backend requested restartpoints started by the checkpointer',
proname => 'pg_stat_get_checkpointer_restartpoints_requested',
provolatile => 's', proparallel => 'r', prorettype => 'int8',
proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_restartpoints_requested' },
-{ oid => '6329',
+{ oid => '6352',
descr => 'statistics: number of backend performed restartpoints',
proname => 'pg_stat_get_checkpointer_restartpoints_performed',
provolatile => 's', proparallel => 'r', prorettype => 'int8',
proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_restartpoints_performed' },
-{ oid => '2771',
+{ oid => '6353',
descr => 'statistics: number of buffers written during checkpoints and restartpoints',
proname => 'pg_stat_get_checkpointer_buffers_written', provolatile => 's',
proparallel => 'r', prorettype => 'int8', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_buffers_written' },
-{ oid => '6314', descr => 'statistics: last reset for the checkpointer',
+{ oid => '6354', descr => 'statistics: last reset for the checkpointer',
proname => 'pg_stat_get_checkpointer_stat_reset_time', provolatile => 's',
proparallel => 'r', prorettype => 'timestamptz', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_stat_reset_time' },
+{ oid => '6355',
+ descr => 'statistics: checkpoint/restartpoint time spent writing buffers to disk, in milliseconds',
+ proname => 'pg_stat_get_checkpointer_write_time', provolatile => 's',
+ proparallel => 'r', prorettype => 'float8', proargtypes => '',
+ prosrc => 'pg_stat_get_checkpointer_write_time' },
+{ oid => '6356',
+ descr => 'statistics: checkpoint/restartpoint time spent synchronizing buffers to disk, in milliseconds',
+ proname => 'pg_stat_get_checkpointer_sync_time', provolatile => 's',
+ proparallel => 'r', prorettype => 'float8', proargtypes => '',
+ prosrc => 'pg_stat_get_checkpointer_sync_time' },
{ oid => '2772',
descr => 'statistics: number of buffers written by the bgwriter for cleaning dirty buffers',
proname => 'pg_stat_get_bgwriter_buf_written_clean', provolatile => 's',
@@ -5857,16 +5873,6 @@
proname => 'pg_stat_get_bgwriter_stat_reset_time', provolatile => 's',
proparallel => 'r', prorettype => 'timestamptz', proargtypes => '',
prosrc => 'pg_stat_get_bgwriter_stat_reset_time' },
-{ oid => '3160',
- descr => 'statistics: checkpoint/restartpoint time spent writing buffers to disk, in milliseconds',
- proname => 'pg_stat_get_checkpointer_write_time', provolatile => 's',
- proparallel => 'r', prorettype => 'float8', proargtypes => '',
- prosrc => 'pg_stat_get_checkpointer_write_time' },
-{ oid => '3161',
- descr => 'statistics: checkpoint/restartpoint time spent synchronizing buffers to disk, in milliseconds',
- proname => 'pg_stat_get_checkpointer_sync_time', provolatile => 's',
- proparallel => 'r', prorettype => 'float8', proargtypes => '',
- prosrc => 'pg_stat_get_checkpointer_sync_time' },
{ oid => '2859', descr => 'statistics: number of buffer allocations',
proname => 'pg_stat_get_buf_alloc', provolatile => 's', proparallel => 'r',
prorettype => 'int8', proargtypes => '', prosrc => 'pg_stat_get_buf_alloc' },
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index be2c91168a1..39850d8f14f 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -294,6 +294,7 @@ typedef struct PgStat_CheckpointerStats
{
PgStat_Counter num_timed;
PgStat_Counter num_requested;
+ PgStat_Counter num_performed;
PgStat_Counter restartpoints_timed;
PgStat_Counter restartpoints_requested;
PgStat_Counter restartpoints_performed;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index a1626f3fae9..f5434d8365c 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1824,6 +1824,7 @@ pg_stat_bgwriter| SELECT pg_stat_get_bgwriter_buf_written_clean() AS buffers_cle
pg_stat_get_bgwriter_stat_reset_time() AS stats_reset;
pg_stat_checkpointer| SELECT pg_stat_get_checkpointer_num_timed() AS num_timed,
pg_stat_get_checkpointer_num_requested() AS num_requested,
+ pg_stat_get_checkpointer_num_performed() AS num_done,
pg_stat_get_checkpointer_restartpoints_timed() AS restartpoints_timed,
pg_stat_get_checkpointer_restartpoints_requested() AS restartpoints_req,
pg_stat_get_checkpointer_restartpoints_performed() AS restartpoints_done,
--
2.46.0
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
2024-09-16 14:30 Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
@ 2024-09-17 02:47 ` Fujii Masao <[email protected]>
2024-09-18 10:21 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-18 14:35 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
0 siblings, 2 replies; 30+ messages in thread
From: Fujii Masao @ 2024-09-17 02:47 UTC (permalink / raw)
To: Anton A. Melnikov <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Anton A. Melnikov <[email protected]>; Andres Freund <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On 2024/09/16 23:30, Anton A. Melnikov wrote:
> +1
> This idea seems quite tenable to me.
>
> There is a small clarification. Now if there were no skipped restartpoints then
> restartpoints_done will be equal to restartpoints_timed + restartpoints_req.
> Similar for checkpoints.
> So i tried to introduce num_done counter for checkpoints in the patch attached.
Thanks for the patch! I believe this change is targeted for v18. For v17, however,
we should update the description of num_timed in the documentation. Thought?
Here's a suggestion:
"Number of scheduled checkpoints due to timeout. Note that checkpoints may be
skipped if the server has been idle since the last one, and this value counts
both completed and skipped checkpoints."
Regarding the patch:
if (do_restartpoint)
PendingCheckpointerStats.restartpoints_performed++;
+ else
+ PendingCheckpointerStats.num_performed++;
I expected the counter not to be incremented when a checkpoint is skipped,
but in this code, when a checkpoint is skipped, ckpt_performed is set to true,
triggering the counter increment. This seems wrong.
> I'm not sure should we include testing for the case when num_done is less than
> num_timed + num_requested to the regress tests. I haven't been able to get it in a short time yet.
I'm not sure if that test is really necessary...
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
2024-09-16 14:30 Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-17 02:47 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
@ 2024-09-18 10:21 ` Fujii Masao <[email protected]>
2024-09-18 12:22 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Alexander Korotkov <[email protected]>
1 sibling, 1 reply; 30+ messages in thread
From: Fujii Masao @ 2024-09-18 10:21 UTC (permalink / raw)
To: Anton A. Melnikov <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Anton A. Melnikov <[email protected]>; Andres Freund <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On 2024/09/17 11:47, Fujii Masao wrote:
>
>
> On 2024/09/16 23:30, Anton A. Melnikov wrote:
>> +1
>> This idea seems quite tenable to me.
>>
>> There is a small clarification. Now if there were no skipped restartpoints then
>> restartpoints_done will be equal to restartpoints_timed + restartpoints_req.
>> Similar for checkpoints.
>> So i tried to introduce num_done counter for checkpoints in the patch attached.
>
> Thanks for the patch! I believe this change is targeted for v18. For v17, however,
> we should update the description of num_timed in the documentation. Thought?
> Here's a suggestion:
>
> "Number of scheduled checkpoints due to timeout. Note that checkpoints may be
> skipped if the server has been idle since the last one, and this value counts
> both completed and skipped checkpoints."
Patch attached.
Unless there are any objections, I plan to commit this and back-patch it to v17.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From 96dd9e0f923aef44ee1d73ea2bafea8d9db805bf Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Wed, 18 Sep 2024 19:03:53 +0900
Subject: [PATCH v1] docs: Improve the description of num_timed column in
pg_stat_checkpointer.
The previous documentation stated that num_timed reflects the number of
scheduled checkpoints performed. However, checkpoints may be skipped
if the server has been idle, and num_timed counts both skipped and completed
checkpoints. This commit clarifies the description to make it clear that
the counter includes both skipped and completed checkpoints.
Back-patch to v17 where pg_stat_checkpointer was added.
Discussion: https://postgr.es/m/[email protected]
---
doc/src/sgml/monitoring.sgml | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 933de6fe07..a2fda4677d 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3051,7 +3051,10 @@ description | Waiting for a newly initialized WAL file to reach durable storage
<structfield>num_timed</structfield> <type>bigint</type>
</para>
<para>
- Number of scheduled checkpoints that have been performed
+ Number of scheduled checkpoints due to timeout.
+ Note that checkpoints may be skipped if the server has been idle
+ since the last one, and this value counts both completed and
+ skipped checkpoints
</para></entry>
</row>
--
2.45.2
Attachments:
[text/plain] v1-0001-docs-Improve-the-description-of-num_timed-column-.patch (1.5K, ../../[email protected]/2-v1-0001-docs-Improve-the-description-of-num_timed-column-.patch)
download | inline diff:
From 96dd9e0f923aef44ee1d73ea2bafea8d9db805bf Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Wed, 18 Sep 2024 19:03:53 +0900
Subject: [PATCH v1] docs: Improve the description of num_timed column in
pg_stat_checkpointer.
The previous documentation stated that num_timed reflects the number of
scheduled checkpoints performed. However, checkpoints may be skipped
if the server has been idle, and num_timed counts both skipped and completed
checkpoints. This commit clarifies the description to make it clear that
the counter includes both skipped and completed checkpoints.
Back-patch to v17 where pg_stat_checkpointer was added.
Discussion: https://postgr.es/m/[email protected]
---
doc/src/sgml/monitoring.sgml | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 933de6fe07..a2fda4677d 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3051,7 +3051,10 @@ description | Waiting for a newly initialized WAL file to reach durable storage
<structfield>num_timed</structfield> <type>bigint</type>
</para>
<para>
- Number of scheduled checkpoints that have been performed
+ Number of scheduled checkpoints due to timeout.
+ Note that checkpoints may be skipped if the server has been idle
+ since the last one, and this value counts both completed and
+ skipped checkpoints
</para></entry>
</row>
--
2.45.2
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
2024-09-16 14:30 Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-17 02:47 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-18 10:21 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
@ 2024-09-18 12:22 ` Alexander Korotkov <[email protected]>
2024-09-18 17:21 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Alexander Korotkov @ 2024-09-18 12:22 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: Anton A. Melnikov <[email protected]>; Magnus Hagander <[email protected]>; Anton A. Melnikov <[email protected]>; Andres Freund <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On Wed, Sep 18, 2024 at 1:21 PM Fujii Masao <[email protected]> wrote:
> On 2024/09/17 11:47, Fujii Masao wrote:
> >
> >
> > On 2024/09/16 23:30, Anton A. Melnikov wrote:
> >> +1
> >> This idea seems quite tenable to me.
> >>
> >> There is a small clarification. Now if there were no skipped restartpoints then
> >> restartpoints_done will be equal to restartpoints_timed + restartpoints_req.
> >> Similar for checkpoints.
> >> So i tried to introduce num_done counter for checkpoints in the patch attached.
> >
> > Thanks for the patch! I believe this change is targeted for v18. For v17, however,
> > we should update the description of num_timed in the documentation. Thought?
> > Here's a suggestion:
> >
> > "Number of scheduled checkpoints due to timeout. Note that checkpoints may be
> > skipped if the server has been idle since the last one, and this value counts
> > both completed and skipped checkpoints."
>
> Patch attached.
> Unless there are any objections, I plan to commit this and back-patch it to v17.
I've checked this patch, it looks good to me.
Generally, it looks like I should be in charge for this, given I've
committed previous patch by Anton. Thank you for reacting here faster
than me. Please, go ahead with the patch.
------
Regards,
Alexander Korotkov
Supabase
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
2024-09-16 14:30 Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-17 02:47 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-18 10:21 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-18 12:22 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Alexander Korotkov <[email protected]>
@ 2024-09-18 17:21 ` Fujii Masao <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Fujii Masao @ 2024-09-18 17:21 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Anton A. Melnikov <[email protected]>; Magnus Hagander <[email protected]>; Anton A. Melnikov <[email protected]>; Andres Freund <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On 2024/09/18 21:22, Alexander Korotkov wrote:
>> Patch attached.
>> Unless there are any objections, I plan to commit this and back-patch it to v17.
>
> I've checked this patch, it looks good to me.
>
> Generally, it looks like I should be in charge for this, given I've
> committed previous patch by Anton. Thank you for reacting here faster
> than me. Please, go ahead with the patch.
Thanks for the review! Pushed!
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
2024-09-16 14:30 Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-17 02:47 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
@ 2024-09-18 14:35 ` Anton A. Melnikov <[email protected]>
2024-09-18 18:04 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
1 sibling, 1 reply; 30+ messages in thread
From: Anton A. Melnikov @ 2024-09-18 14:35 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Anton A. Melnikov <[email protected]>; Andres Freund <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
Fujii, Alexander thanks a lot!
On 17.09.2024 05:47, Fujii Masao wrote:
>
> Regarding the patch:
> if (do_restartpoint)
> PendingCheckpointerStats.restartpoints_performed++;
> + else
> + PendingCheckpointerStats.num_performed++;
>
> I expected the counter not to be incremented when a checkpoint is skipped,
> but in this code, when a checkpoint is skipped, ckpt_performed is set to true,
> triggering the counter increment. This seems wrong.
Tried to fix it via returning bool value from the CreateCheckPoint()
similarly to the CreateRestartPoint().
And slightly adjusted the patch so that it could be applied after yours.
With the best wishes,
--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachments:
[text/x-patch] v2-0002-Introduce-num_done-counter-in-the-pg_stat_checkpoint.patch (12.2K, ../../[email protected]/2-v2-0002-Introduce-num_done-counter-in-the-pg_stat_checkpoint.patch)
download | inline diff:
From 97595f65cb12eb2243e1b7391e1bc77bd161f41c Mon Sep 17 00:00:00 2001
From: "Anton A. Melnikov" <[email protected]>
Date: Mon, 16 Sep 2024 16:12:07 +0300
Subject: [PATCH] Introduce num_done counter in the pg_stat_checkpointer view
that reflects number of really performed checkpoints.
---
doc/src/sgml/monitoring.sgml | 11 ++++-
src/backend/access/transam/xlog.c | 6 ++-
src/backend/catalog/system_views.sql | 1 +
src/backend/postmaster/checkpointer.c | 5 ++-
.../utils/activity/pgstat_checkpointer.c | 2 +
src/backend/utils/adt/pgstatfuncs.c | 6 +++
src/include/access/xlog.h | 2 +-
src/include/catalog/pg_proc.dat | 40 +++++++++++--------
src/include/pgstat.h | 1 +
src/test/regress/expected/rules.out | 1 +
10 files changed, 52 insertions(+), 23 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index a2fda4677d7..19bf0164f1c 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3063,7 +3063,16 @@ description | Waiting for a newly initialized WAL file to reach durable storage
<structfield>num_requested</structfield> <type>bigint</type>
</para>
<para>
- Number of requested checkpoints that have been performed
+ Number of backend requested checkpoints
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>num_done</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Number of checkpoints that have been performed
</para></entry>
</row>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 853ab06812b..ca1155567dc 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6879,7 +6879,7 @@ update_checkpoint_display(int flags, bool restartpoint, bool reset)
* both the record marking the completion of the checkpoint and the location
* from which WAL replay would begin if needed.
*/
-void
+bool
CreateCheckPoint(int flags)
{
bool shutdown;
@@ -6971,7 +6971,7 @@ CreateCheckPoint(int flags)
END_CRIT_SECTION();
ereport(DEBUG1,
(errmsg_internal("checkpoint skipped because system is idle")));
- return;
+ return false;
}
}
@@ -7353,6 +7353,8 @@ CreateCheckPoint(int flags)
CheckpointStats.ckpt_segs_added,
CheckpointStats.ckpt_segs_removed,
CheckpointStats.ckpt_segs_recycled);
+
+ return true;
}
/*
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 7fd5d256a18..49109dbdc86 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1138,6 +1138,7 @@ CREATE VIEW pg_stat_checkpointer AS
SELECT
pg_stat_get_checkpointer_num_timed() AS num_timed,
pg_stat_get_checkpointer_num_requested() AS num_requested,
+ pg_stat_get_checkpointer_num_performed() AS num_done,
pg_stat_get_checkpointer_restartpoints_timed() AS restartpoints_timed,
pg_stat_get_checkpointer_restartpoints_requested() AS restartpoints_req,
pg_stat_get_checkpointer_restartpoints_performed() AS restartpoints_done,
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index eeb73c85726..ef29cb439b2 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -461,8 +461,7 @@ CheckpointerMain(char *startup_data, size_t startup_data_len)
*/
if (!do_restartpoint)
{
- CreateCheckPoint(flags);
- ckpt_performed = true;
+ ckpt_performed = CreateCheckPoint(flags);
}
else
ckpt_performed = CreateRestartPoint(flags);
@@ -495,6 +494,8 @@ CheckpointerMain(char *startup_data, size_t startup_data_len)
if (do_restartpoint)
PendingCheckpointerStats.restartpoints_performed++;
+ else
+ PendingCheckpointerStats.num_performed++;
}
else
{
diff --git a/src/backend/utils/activity/pgstat_checkpointer.c b/src/backend/utils/activity/pgstat_checkpointer.c
index bbfc9c7e183..4a0a2d1493a 100644
--- a/src/backend/utils/activity/pgstat_checkpointer.c
+++ b/src/backend/utils/activity/pgstat_checkpointer.c
@@ -49,6 +49,7 @@ pgstat_report_checkpointer(void)
#define CHECKPOINTER_ACC(fld) stats_shmem->stats.fld += PendingCheckpointerStats.fld
CHECKPOINTER_ACC(num_timed);
CHECKPOINTER_ACC(num_requested);
+ CHECKPOINTER_ACC(num_performed);
CHECKPOINTER_ACC(restartpoints_timed);
CHECKPOINTER_ACC(restartpoints_requested);
CHECKPOINTER_ACC(restartpoints_performed);
@@ -127,6 +128,7 @@ pgstat_checkpointer_snapshot_cb(void)
#define CHECKPOINTER_COMP(fld) pgStatLocal.snapshot.checkpointer.fld -= reset.fld;
CHECKPOINTER_COMP(num_timed);
CHECKPOINTER_COMP(num_requested);
+ CHECKPOINTER_COMP(num_performed);
CHECKPOINTER_COMP(restartpoints_timed);
CHECKPOINTER_COMP(restartpoints_requested);
CHECKPOINTER_COMP(restartpoints_performed);
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 9c23ac7c8c8..17b0fc02ef0 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1191,6 +1191,12 @@ pg_stat_get_checkpointer_num_requested(PG_FUNCTION_ARGS)
PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->num_requested);
}
+Datum
+pg_stat_get_checkpointer_num_performed(PG_FUNCTION_ARGS)
+{
+ PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->num_performed);
+}
+
Datum
pg_stat_get_checkpointer_restartpoints_timed(PG_FUNCTION_ARGS)
{
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 083810f5b4c..36f6e4e4b4e 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -239,7 +239,7 @@ extern void LocalProcessControlFile(bool reset);
extern WalLevel GetActiveWalLevelOnStandby(void);
extern void StartupXLOG(void);
extern void ShutdownXLOG(int code, Datum arg);
-extern void CreateCheckPoint(int flags);
+extern bool CreateCheckPoint(int flags);
extern bool CreateRestartPoint(int flags);
extern WALAvailability GetWALAvailability(XLogRecPtr targetLSN);
extern void XLogPutNextOid(Oid nextOid);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 43f608d7a0a..83c96140b01 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5807,42 +5807,58 @@
proargmodes => '{o,o,o,o,o,o,o}',
proargnames => '{archived_count,last_archived_wal,last_archived_time,failed_count,last_failed_wal,last_failed_time,stats_reset}',
prosrc => 'pg_stat_get_archiver' },
-{ oid => '2769',
+{ oid => '6347',
descr => 'statistics: number of timed checkpoints started by the checkpointer',
proname => 'pg_stat_get_checkpointer_num_timed', provolatile => 's',
proparallel => 'r', prorettype => 'int8', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_num_timed' },
-{ oid => '2770',
+{ oid => '6348',
descr => 'statistics: number of backend requested checkpoints started by the checkpointer',
proname => 'pg_stat_get_checkpointer_num_requested', provolatile => 's',
proparallel => 'r', prorettype => 'int8', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_num_requested' },
-{ oid => '6327',
+{ oid => '6349',
+ descr => 'statistics: number of checkpoints performed by the checkpointer',
+ proname => 'pg_stat_get_checkpointer_num_performed',
+ provolatile => 's', proparallel => 'r', prorettype => 'int8',
+ proargtypes => '',
+ prosrc => 'pg_stat_get_checkpointer_num_performed' },
+{ oid => '6350',
descr => 'statistics: number of timed restartpoints started by the checkpointer',
proname => 'pg_stat_get_checkpointer_restartpoints_timed', provolatile => 's',
proparallel => 'r', prorettype => 'int8', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_restartpoints_timed' },
-{ oid => '6328',
+{ oid => '6351',
descr => 'statistics: number of backend requested restartpoints started by the checkpointer',
proname => 'pg_stat_get_checkpointer_restartpoints_requested',
provolatile => 's', proparallel => 'r', prorettype => 'int8',
proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_restartpoints_requested' },
-{ oid => '6329',
+{ oid => '6352',
descr => 'statistics: number of backend performed restartpoints',
proname => 'pg_stat_get_checkpointer_restartpoints_performed',
provolatile => 's', proparallel => 'r', prorettype => 'int8',
proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_restartpoints_performed' },
-{ oid => '2771',
+{ oid => '6353',
descr => 'statistics: number of buffers written during checkpoints and restartpoints',
proname => 'pg_stat_get_checkpointer_buffers_written', provolatile => 's',
proparallel => 'r', prorettype => 'int8', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_buffers_written' },
-{ oid => '6314', descr => 'statistics: last reset for the checkpointer',
+{ oid => '6354', descr => 'statistics: last reset for the checkpointer',
proname => 'pg_stat_get_checkpointer_stat_reset_time', provolatile => 's',
proparallel => 'r', prorettype => 'timestamptz', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_stat_reset_time' },
+{ oid => '6355',
+ descr => 'statistics: checkpoint/restartpoint time spent writing buffers to disk, in milliseconds',
+ proname => 'pg_stat_get_checkpointer_write_time', provolatile => 's',
+ proparallel => 'r', prorettype => 'float8', proargtypes => '',
+ prosrc => 'pg_stat_get_checkpointer_write_time' },
+{ oid => '6356',
+ descr => 'statistics: checkpoint/restartpoint time spent synchronizing buffers to disk, in milliseconds',
+ proname => 'pg_stat_get_checkpointer_sync_time', provolatile => 's',
+ proparallel => 'r', prorettype => 'float8', proargtypes => '',
+ prosrc => 'pg_stat_get_checkpointer_sync_time' },
{ oid => '2772',
descr => 'statistics: number of buffers written by the bgwriter for cleaning dirty buffers',
proname => 'pg_stat_get_bgwriter_buf_written_clean', provolatile => 's',
@@ -5857,16 +5873,6 @@
proname => 'pg_stat_get_bgwriter_stat_reset_time', provolatile => 's',
proparallel => 'r', prorettype => 'timestamptz', proargtypes => '',
prosrc => 'pg_stat_get_bgwriter_stat_reset_time' },
-{ oid => '3160',
- descr => 'statistics: checkpoint/restartpoint time spent writing buffers to disk, in milliseconds',
- proname => 'pg_stat_get_checkpointer_write_time', provolatile => 's',
- proparallel => 'r', prorettype => 'float8', proargtypes => '',
- prosrc => 'pg_stat_get_checkpointer_write_time' },
-{ oid => '3161',
- descr => 'statistics: checkpoint/restartpoint time spent synchronizing buffers to disk, in milliseconds',
- proname => 'pg_stat_get_checkpointer_sync_time', provolatile => 's',
- proparallel => 'r', prorettype => 'float8', proargtypes => '',
- prosrc => 'pg_stat_get_checkpointer_sync_time' },
{ oid => '2859', descr => 'statistics: number of buffer allocations',
proname => 'pg_stat_get_buf_alloc', provolatile => 's', proparallel => 'r',
prorettype => 'int8', proargtypes => '', prosrc => 'pg_stat_get_buf_alloc' },
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 4752dfe7197..476acd680c0 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -294,6 +294,7 @@ typedef struct PgStat_CheckpointerStats
{
PgStat_Counter num_timed;
PgStat_Counter num_requested;
+ PgStat_Counter num_performed;
PgStat_Counter restartpoints_timed;
PgStat_Counter restartpoints_requested;
PgStat_Counter restartpoints_performed;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index a1626f3fae9..f5434d8365c 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1824,6 +1824,7 @@ pg_stat_bgwriter| SELECT pg_stat_get_bgwriter_buf_written_clean() AS buffers_cle
pg_stat_get_bgwriter_stat_reset_time() AS stats_reset;
pg_stat_checkpointer| SELECT pg_stat_get_checkpointer_num_timed() AS num_timed,
pg_stat_get_checkpointer_num_requested() AS num_requested,
+ pg_stat_get_checkpointer_num_performed() AS num_done,
pg_stat_get_checkpointer_restartpoints_timed() AS restartpoints_timed,
pg_stat_get_checkpointer_restartpoints_requested() AS restartpoints_req,
pg_stat_get_checkpointer_restartpoints_performed() AS restartpoints_done,
--
2.46.0
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
2024-09-16 14:30 Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-17 02:47 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-18 14:35 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
@ 2024-09-18 18:04 ` Fujii Masao <[email protected]>
2024-09-19 10:16 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Fujii Masao @ 2024-09-18 18:04 UTC (permalink / raw)
To: Anton A. Melnikov <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Anton A. Melnikov <[email protected]>; Andres Freund <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On 2024/09/18 23:35, Anton A. Melnikov wrote:
> Fujii, Alexander thanks a lot!
>
> On 17.09.2024 05:47, Fujii Masao wrote:
>>
>> Regarding the patch:
>> if (do_restartpoint)
>> PendingCheckpointerStats.restartpoints_performed++;
>> + else
>> + PendingCheckpointerStats.num_performed++;
>>
>> I expected the counter not to be incremented when a checkpoint is skipped,
>> but in this code, when a checkpoint is skipped, ckpt_performed is set to true,
>> triggering the counter increment. This seems wrong.
>
> Tried to fix it via returning bool value from the CreateCheckPoint()
> similarly to the CreateRestartPoint().
>
> And slightly adjusted the patch so that it could be applied after yours.
Thanks for updating the patch!
-void
+bool
CreateCheckPoint(int flags)
It would be helpful to explain the new return value in the comment
at the top of this function.
- CreateCheckPoint(flags);
- ckpt_performed = true;
+ ckpt_performed = CreateCheckPoint(flags);
This change could result in the next scheduled checkpoint being
triggered in 15 seconds if a checkpoint is skipped, which isn’t
the intended behavior.
-{ oid => '2769',
+{ oid => '6347',
I don't think that the existing functions need to be reassigned new OIDs.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
2024-09-16 14:30 Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-17 02:47 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-18 14:35 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-18 18:04 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
@ 2024-09-19 10:16 ` Anton A. Melnikov <[email protected]>
2024-09-20 16:19 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Anton A. Melnikov @ 2024-09-19 10:16 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Anton A. Melnikov <[email protected]>; Andres Freund <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On 18.09.2024 21:04, Fujii Masao wrote:
>
> - CreateCheckPoint(flags);
> - ckpt_performed = true;
> + ckpt_performed = CreateCheckPoint(flags);
>
> This change could result in the next scheduled checkpoint being
> triggered in 15 seconds if a checkpoint is skipped, which isn’t
> the intended behavior.
Thanks for pointing this out! This is really bug.
Rearranged the logic a bit to save the previous behavior
in the v3 attached.
> -void
> +bool
> CreateCheckPoint(int flags)
>
> It would be helpful to explain the new return value in the comment
> at the top of this function.
Sure. Added an info about return value to the comment.
> -{ oid => '2769',
> +{ oid => '6347',
>
> I don't think that the existing functions need to be reassigned new OIDs.
Ok. Left oids as is in the v3. Just added a new one for
pg_stat_get_checkpointer_num_performed().
With the best regards!
--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Attachments:
[text/x-patch] v3-0001-Introduce-num_done-counter-in-the-pg_stat_checkpoint.patch (12.0K, ../../[email protected]/2-v3-0001-Introduce-num_done-counter-in-the-pg_stat_checkpoint.patch)
download | inline diff:
From 5832b1beb453b96a6ccbe72c404f2ad5373d0497 Mon Sep 17 00:00:00 2001
From: "Anton A. Melnikov" <[email protected]>
Date: Thu, 19 Sep 2024 12:51:17 +0300
Subject: [PATCH] Introduce num_done counter in the pg_stat_checkpointer view
that reflects number of really performed checkpoints.
---
doc/src/sgml/monitoring.sgml | 11 ++++-
src/backend/access/transam/xlog.c | 9 +++-
src/backend/catalog/system_views.sql | 1 +
src/backend/postmaster/checkpointer.c | 45 ++++++++++++-------
.../utils/activity/pgstat_checkpointer.c | 2 +
src/backend/utils/adt/pgstatfuncs.c | 6 +++
src/include/access/xlog.h | 2 +-
src/include/catalog/pg_proc.dat | 26 ++++++-----
src/include/pgstat.h | 1 +
src/test/regress/expected/rules.out | 1 +
10 files changed, 74 insertions(+), 30 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index a2fda4677d7..19bf0164f1c 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3063,7 +3063,16 @@ description | Waiting for a newly initialized WAL file to reach durable storage
<structfield>num_requested</structfield> <type>bigint</type>
</para>
<para>
- Number of requested checkpoints that have been performed
+ Number of backend requested checkpoints
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>num_done</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Number of checkpoints that have been performed
</para></entry>
</row>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 853ab06812b..c9d37cba453 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6878,8 +6878,11 @@ update_checkpoint_display(int flags, bool restartpoint, bool reset)
* In this case, we only insert an XLOG_CHECKPOINT_SHUTDOWN record, and it's
* both the record marking the completion of the checkpoint and the location
* from which WAL replay would begin if needed.
+ *
+ * Returns true if a new checkpoint was performed or false if checkpoint
+ * was skipped because system is idle.
*/
-void
+bool
CreateCheckPoint(int flags)
{
bool shutdown;
@@ -6971,7 +6974,7 @@ CreateCheckPoint(int flags)
END_CRIT_SECTION();
ereport(DEBUG1,
(errmsg_internal("checkpoint skipped because system is idle")));
- return;
+ return false;
}
}
@@ -7353,6 +7356,8 @@ CreateCheckPoint(int flags)
CheckpointStats.ckpt_segs_added,
CheckpointStats.ckpt_segs_removed,
CheckpointStats.ckpt_segs_recycled);
+
+ return true;
}
/*
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 7fd5d256a18..49109dbdc86 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1138,6 +1138,7 @@ CREATE VIEW pg_stat_checkpointer AS
SELECT
pg_stat_get_checkpointer_num_timed() AS num_timed,
pg_stat_get_checkpointer_num_requested() AS num_requested,
+ pg_stat_get_checkpointer_num_performed() AS num_done,
pg_stat_get_checkpointer_restartpoints_timed() AS restartpoints_timed,
pg_stat_get_checkpointer_restartpoints_requested() AS restartpoints_req,
pg_stat_get_checkpointer_restartpoints_performed() AS restartpoints_done,
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index eeb73c85726..8d610a3f1a7 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -461,8 +461,7 @@ CheckpointerMain(char *startup_data, size_t startup_data_len)
*/
if (!do_restartpoint)
{
- CreateCheckPoint(flags);
- ckpt_performed = true;
+ ckpt_performed = CreateCheckPoint(flags);
}
else
ckpt_performed = CreateRestartPoint(flags);
@@ -484,27 +483,41 @@ CheckpointerMain(char *startup_data, size_t startup_data_len)
ConditionVariableBroadcast(&CheckpointerShmem->done_cv);
- if (ckpt_performed)
+ if (!do_restartpoint)
{
/*
- * Note we record the checkpoint start time not end time as
- * last_checkpoint_time. This is so that time-driven
- * checkpoints happen at a predictable spacing.
- */
+ * Note we record the checkpoint start time not end time as
+ * last_checkpoint_time. This is so that time-driven
+ * checkpoints happen at a predictable spacing.
+ */
last_checkpoint_time = now;
- if (do_restartpoint)
- PendingCheckpointerStats.restartpoints_performed++;
+ if(ckpt_performed)
+ PendingCheckpointerStats.num_performed++;
+
}
else
{
- /*
- * We were not able to perform the restartpoint (checkpoints
- * throw an ERROR in case of error). Most likely because we
- * have not received any new checkpoint WAL records since the
- * last restartpoint. Try again in 15 s.
- */
- last_checkpoint_time = now - CheckPointTimeout + 15;
+ if (ckpt_performed)
+ {
+ /*
+ * The same as for checkpoint.
+ * Please see the corresponding comment.
+ */
+ last_checkpoint_time = now;
+
+ PendingCheckpointerStats.restartpoints_performed++;
+ }
+ else
+ {
+ /*
+ * We were not able to perform the restartpoint (checkpoints
+ * throw an ERROR in case of error). Most likely because we
+ * have not received any new checkpoint WAL records since the
+ * last restartpoint. Try again in 15 s.
+ */
+ last_checkpoint_time = now - CheckPointTimeout + 15;
+ }
}
ckpt_active = false;
diff --git a/src/backend/utils/activity/pgstat_checkpointer.c b/src/backend/utils/activity/pgstat_checkpointer.c
index bbfc9c7e183..4a0a2d1493a 100644
--- a/src/backend/utils/activity/pgstat_checkpointer.c
+++ b/src/backend/utils/activity/pgstat_checkpointer.c
@@ -49,6 +49,7 @@ pgstat_report_checkpointer(void)
#define CHECKPOINTER_ACC(fld) stats_shmem->stats.fld += PendingCheckpointerStats.fld
CHECKPOINTER_ACC(num_timed);
CHECKPOINTER_ACC(num_requested);
+ CHECKPOINTER_ACC(num_performed);
CHECKPOINTER_ACC(restartpoints_timed);
CHECKPOINTER_ACC(restartpoints_requested);
CHECKPOINTER_ACC(restartpoints_performed);
@@ -127,6 +128,7 @@ pgstat_checkpointer_snapshot_cb(void)
#define CHECKPOINTER_COMP(fld) pgStatLocal.snapshot.checkpointer.fld -= reset.fld;
CHECKPOINTER_COMP(num_timed);
CHECKPOINTER_COMP(num_requested);
+ CHECKPOINTER_COMP(num_performed);
CHECKPOINTER_COMP(restartpoints_timed);
CHECKPOINTER_COMP(restartpoints_requested);
CHECKPOINTER_COMP(restartpoints_performed);
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 9c23ac7c8c8..17b0fc02ef0 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1191,6 +1191,12 @@ pg_stat_get_checkpointer_num_requested(PG_FUNCTION_ARGS)
PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->num_requested);
}
+Datum
+pg_stat_get_checkpointer_num_performed(PG_FUNCTION_ARGS)
+{
+ PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->num_performed);
+}
+
Datum
pg_stat_get_checkpointer_restartpoints_timed(PG_FUNCTION_ARGS)
{
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 083810f5b4c..36f6e4e4b4e 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -239,7 +239,7 @@ extern void LocalProcessControlFile(bool reset);
extern WalLevel GetActiveWalLevelOnStandby(void);
extern void StartupXLOG(void);
extern void ShutdownXLOG(int code, Datum arg);
-extern void CreateCheckPoint(int flags);
+extern bool CreateCheckPoint(int flags);
extern bool CreateRestartPoint(int flags);
extern WALAvailability GetWALAvailability(XLogRecPtr targetLSN);
extern void XLogPutNextOid(Oid nextOid);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 43f608d7a0a..baa6120e664 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5817,6 +5817,12 @@
proname => 'pg_stat_get_checkpointer_num_requested', provolatile => 's',
proparallel => 'r', prorettype => 'int8', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_num_requested' },
+{ oid => '2775',
+ descr => 'statistics: number of checkpoints performed by the checkpointer',
+ proname => 'pg_stat_get_checkpointer_num_performed',
+ provolatile => 's', proparallel => 'r', prorettype => 'int8',
+ proargtypes => '',
+ prosrc => 'pg_stat_get_checkpointer_num_performed' },
{ oid => '6327',
descr => 'statistics: number of timed restartpoints started by the checkpointer',
proname => 'pg_stat_get_checkpointer_restartpoints_timed', provolatile => 's',
@@ -5843,6 +5849,16 @@
proname => 'pg_stat_get_checkpointer_stat_reset_time', provolatile => 's',
proparallel => 'r', prorettype => 'timestamptz', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_stat_reset_time' },
+ { oid => '3160',
+ descr => 'statistics: checkpoint/restartpoint time spent writing buffers to disk, in milliseconds',
+ proname => 'pg_stat_get_checkpointer_write_time', provolatile => 's',
+ proparallel => 'r', prorettype => 'float8', proargtypes => '',
+ prosrc => 'pg_stat_get_checkpointer_write_time' },
+{ oid => '3161',
+ descr => 'statistics: checkpoint/restartpoint time spent synchronizing buffers to disk, in milliseconds',
+ proname => 'pg_stat_get_checkpointer_sync_time', provolatile => 's',
+ proparallel => 'r', prorettype => 'float8', proargtypes => '',
+ prosrc => 'pg_stat_get_checkpointer_sync_time' },
{ oid => '2772',
descr => 'statistics: number of buffers written by the bgwriter for cleaning dirty buffers',
proname => 'pg_stat_get_bgwriter_buf_written_clean', provolatile => 's',
@@ -5857,16 +5873,6 @@
proname => 'pg_stat_get_bgwriter_stat_reset_time', provolatile => 's',
proparallel => 'r', prorettype => 'timestamptz', proargtypes => '',
prosrc => 'pg_stat_get_bgwriter_stat_reset_time' },
-{ oid => '3160',
- descr => 'statistics: checkpoint/restartpoint time spent writing buffers to disk, in milliseconds',
- proname => 'pg_stat_get_checkpointer_write_time', provolatile => 's',
- proparallel => 'r', prorettype => 'float8', proargtypes => '',
- prosrc => 'pg_stat_get_checkpointer_write_time' },
-{ oid => '3161',
- descr => 'statistics: checkpoint/restartpoint time spent synchronizing buffers to disk, in milliseconds',
- proname => 'pg_stat_get_checkpointer_sync_time', provolatile => 's',
- proparallel => 'r', prorettype => 'float8', proargtypes => '',
- prosrc => 'pg_stat_get_checkpointer_sync_time' },
{ oid => '2859', descr => 'statistics: number of buffer allocations',
proname => 'pg_stat_get_buf_alloc', provolatile => 's', proparallel => 'r',
prorettype => 'int8', proargtypes => '', prosrc => 'pg_stat_get_buf_alloc' },
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 4752dfe7197..476acd680c0 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -294,6 +294,7 @@ typedef struct PgStat_CheckpointerStats
{
PgStat_Counter num_timed;
PgStat_Counter num_requested;
+ PgStat_Counter num_performed;
PgStat_Counter restartpoints_timed;
PgStat_Counter restartpoints_requested;
PgStat_Counter restartpoints_performed;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index a1626f3fae9..f5434d8365c 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1824,6 +1824,7 @@ pg_stat_bgwriter| SELECT pg_stat_get_bgwriter_buf_written_clean() AS buffers_cle
pg_stat_get_bgwriter_stat_reset_time() AS stats_reset;
pg_stat_checkpointer| SELECT pg_stat_get_checkpointer_num_timed() AS num_timed,
pg_stat_get_checkpointer_num_requested() AS num_requested,
+ pg_stat_get_checkpointer_num_performed() AS num_done,
pg_stat_get_checkpointer_restartpoints_timed() AS restartpoints_timed,
pg_stat_get_checkpointer_restartpoints_requested() AS restartpoints_req,
pg_stat_get_checkpointer_restartpoints_performed() AS restartpoints_done,
--
2.46.0
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
2024-09-16 14:30 Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-17 02:47 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-18 14:35 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-18 18:04 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-19 10:16 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
@ 2024-09-20 16:19 ` Fujii Masao <[email protected]>
2024-09-22 04:55 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Fujii Masao @ 2024-09-20 16:19 UTC (permalink / raw)
To: Anton A. Melnikov <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Anton A. Melnikov <[email protected]>; Andres Freund <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On 2024/09/19 19:16, Anton A. Melnikov wrote:
>
> On 18.09.2024 21:04, Fujii Masao wrote:
>>
>> - CreateCheckPoint(flags);
>> - ckpt_performed = true;
>> + ckpt_performed = CreateCheckPoint(flags);
>>
>> This change could result in the next scheduled checkpoint being
>> triggered in 15 seconds if a checkpoint is skipped, which isn’t
>> the intended behavior.
>
> Thanks for pointing this out! This is really bug.
> Rearranged the logic a bit to save the previous behavior
> in the v3 attached.
Thanks for updating the patch!
I've attached the updated version (0001.patch). I made some cosmetic changes,
including reverting the switch in the entries for pg_stat_get_checkpointer_write_time
and pg_stat_get_checkpointer_sync_time in pg_proc.dat, as I didn’t think
that change was necessary. Could you please review the latest version?
After we commit 0001.patch, how about applying 0002.patch, which updates
the documentation for the pg_stat_checkpointer view to clarify what types
of checkpoints and restartpoints each counter tracks?
In 0002.patch, I also modified the description of num_requested from
"Number of backend requested checkpoints" to remove "backend," as it can
be confusing since num_requested includes requests from sources other than
the backend. Thought?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From 859f3fecb4fb4900b6ce12f6346c5d9565fbc072 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Fri, 20 Sep 2024 11:33:07 +0900
Subject: [PATCH v4 1/2] Add num_done counter to the pg_stat_checkpointer view.
Checkpoints can be skipped when the server is idle. The existing num_timed and
num_requested counters in pg_stat_checkpointer track both completed and
skipped checkpoints, but there was no way to count only the completed ones.
This commit introduces the num_done counter, which tracks only completed
checkpoints, making it easier to see how many were actually performed.
Bump catalog version.
Author: Anton A. Melnikov
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/[email protected]
---
doc/src/sgml/monitoring.sgml | 11 +++++-
src/backend/access/transam/xlog.c | 9 ++++-
src/backend/catalog/system_views.sql | 1 +
src/backend/postmaster/checkpointer.c | 39 ++++++++++++-------
.../utils/activity/pgstat_checkpointer.c | 2 +
src/backend/utils/adt/pgstatfuncs.c | 6 +++
src/include/access/xlog.h | 2 +-
src/include/catalog/pg_proc.dat | 6 +++
src/include/pgstat.h | 1 +
src/test/regress/expected/rules.out | 1 +
10 files changed, 60 insertions(+), 18 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index a2fda4677d..19bf0164f1 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3063,7 +3063,16 @@ description | Waiting for a newly initialized WAL file to reach durable storage
<structfield>num_requested</structfield> <type>bigint</type>
</para>
<para>
- Number of requested checkpoints that have been performed
+ Number of backend requested checkpoints
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>num_done</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Number of checkpoints that have been performed
</para></entry>
</row>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 853ab06812..64304d77d3 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6878,8 +6878,11 @@ update_checkpoint_display(int flags, bool restartpoint, bool reset)
* In this case, we only insert an XLOG_CHECKPOINT_SHUTDOWN record, and it's
* both the record marking the completion of the checkpoint and the location
* from which WAL replay would begin if needed.
+ *
+ * Returns true if a new checkpoint was performed, or false if it was skipped
+ * because the system was idle.
*/
-void
+bool
CreateCheckPoint(int flags)
{
bool shutdown;
@@ -6971,7 +6974,7 @@ CreateCheckPoint(int flags)
END_CRIT_SECTION();
ereport(DEBUG1,
(errmsg_internal("checkpoint skipped because system is idle")));
- return;
+ return false;
}
}
@@ -7353,6 +7356,8 @@ CreateCheckPoint(int flags)
CheckpointStats.ckpt_segs_added,
CheckpointStats.ckpt_segs_removed,
CheckpointStats.ckpt_segs_recycled);
+
+ return true;
}
/*
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 7fd5d256a1..49109dbdc8 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1138,6 +1138,7 @@ CREATE VIEW pg_stat_checkpointer AS
SELECT
pg_stat_get_checkpointer_num_timed() AS num_timed,
pg_stat_get_checkpointer_num_requested() AS num_requested,
+ pg_stat_get_checkpointer_num_performed() AS num_done,
pg_stat_get_checkpointer_restartpoints_timed() AS restartpoints_timed,
pg_stat_get_checkpointer_restartpoints_requested() AS restartpoints_req,
pg_stat_get_checkpointer_restartpoints_performed() AS restartpoints_done,
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index eeb73c8572..9087e3f8db 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -460,10 +460,7 @@ CheckpointerMain(char *startup_data, size_t startup_data_len)
* Do the checkpoint.
*/
if (!do_restartpoint)
- {
- CreateCheckPoint(flags);
- ckpt_performed = true;
- }
+ ckpt_performed = CreateCheckPoint(flags);
else
ckpt_performed = CreateRestartPoint(flags);
@@ -484,7 +481,7 @@ CheckpointerMain(char *startup_data, size_t startup_data_len)
ConditionVariableBroadcast(&CheckpointerShmem->done_cv);
- if (ckpt_performed)
+ if (!do_restartpoint)
{
/*
* Note we record the checkpoint start time not end time as
@@ -493,18 +490,32 @@ CheckpointerMain(char *startup_data, size_t startup_data_len)
*/
last_checkpoint_time = now;
- if (do_restartpoint)
- PendingCheckpointerStats.restartpoints_performed++;
+ if (ckpt_performed)
+ PendingCheckpointerStats.num_performed++;
}
else
{
- /*
- * We were not able to perform the restartpoint (checkpoints
- * throw an ERROR in case of error). Most likely because we
- * have not received any new checkpoint WAL records since the
- * last restartpoint. Try again in 15 s.
- */
- last_checkpoint_time = now - CheckPointTimeout + 15;
+ if (ckpt_performed)
+ {
+ /*
+ * The same as for checkpoint. Please see the
+ * corresponding comment.
+ */
+ last_checkpoint_time = now;
+
+ PendingCheckpointerStats.restartpoints_performed++;
+ }
+ else
+ {
+ /*
+ * We were not able to perform the restartpoint
+ * (checkpoints throw an ERROR in case of error). Most
+ * likely because we have not received any new checkpoint
+ * WAL records since the last restartpoint. Try again in
+ * 15 s.
+ */
+ last_checkpoint_time = now - CheckPointTimeout + 15;
+ }
}
ckpt_active = false;
diff --git a/src/backend/utils/activity/pgstat_checkpointer.c b/src/backend/utils/activity/pgstat_checkpointer.c
index bbfc9c7e18..4a0a2d1493 100644
--- a/src/backend/utils/activity/pgstat_checkpointer.c
+++ b/src/backend/utils/activity/pgstat_checkpointer.c
@@ -49,6 +49,7 @@ pgstat_report_checkpointer(void)
#define CHECKPOINTER_ACC(fld) stats_shmem->stats.fld += PendingCheckpointerStats.fld
CHECKPOINTER_ACC(num_timed);
CHECKPOINTER_ACC(num_requested);
+ CHECKPOINTER_ACC(num_performed);
CHECKPOINTER_ACC(restartpoints_timed);
CHECKPOINTER_ACC(restartpoints_requested);
CHECKPOINTER_ACC(restartpoints_performed);
@@ -127,6 +128,7 @@ pgstat_checkpointer_snapshot_cb(void)
#define CHECKPOINTER_COMP(fld) pgStatLocal.snapshot.checkpointer.fld -= reset.fld;
CHECKPOINTER_COMP(num_timed);
CHECKPOINTER_COMP(num_requested);
+ CHECKPOINTER_COMP(num_performed);
CHECKPOINTER_COMP(restartpoints_timed);
CHECKPOINTER_COMP(restartpoints_requested);
CHECKPOINTER_COMP(restartpoints_performed);
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 9c23ac7c8c..17b0fc02ef 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1191,6 +1191,12 @@ pg_stat_get_checkpointer_num_requested(PG_FUNCTION_ARGS)
PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->num_requested);
}
+Datum
+pg_stat_get_checkpointer_num_performed(PG_FUNCTION_ARGS)
+{
+ PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->num_performed);
+}
+
Datum
pg_stat_get_checkpointer_restartpoints_timed(PG_FUNCTION_ARGS)
{
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 083810f5b4..36f6e4e4b4 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -239,7 +239,7 @@ extern void LocalProcessControlFile(bool reset);
extern WalLevel GetActiveWalLevelOnStandby(void);
extern void StartupXLOG(void);
extern void ShutdownXLOG(int code, Datum arg);
-extern void CreateCheckPoint(int flags);
+extern bool CreateCheckPoint(int flags);
extern bool CreateRestartPoint(int flags);
extern WALAvailability GetWALAvailability(XLogRecPtr targetLSN);
extern void XLogPutNextOid(Oid nextOid);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 43f608d7a0..52fca54f3a 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5817,6 +5817,12 @@
proname => 'pg_stat_get_checkpointer_num_requested', provolatile => 's',
proparallel => 'r', prorettype => 'int8', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_num_requested' },
+{ oid => '2775',
+ descr => 'statistics: number of checkpoints performed by the checkpointer',
+ proname => 'pg_stat_get_checkpointer_num_performed',
+ provolatile => 's', proparallel => 'r', prorettype => 'int8',
+ proargtypes => '',
+ prosrc => 'pg_stat_get_checkpointer_num_performed' },
{ oid => '6327',
descr => 'statistics: number of timed restartpoints started by the checkpointer',
proname => 'pg_stat_get_checkpointer_restartpoints_timed', provolatile => 's',
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 4752dfe719..476acd680c 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -294,6 +294,7 @@ typedef struct PgStat_CheckpointerStats
{
PgStat_Counter num_timed;
PgStat_Counter num_requested;
+ PgStat_Counter num_performed;
PgStat_Counter restartpoints_timed;
PgStat_Counter restartpoints_requested;
PgStat_Counter restartpoints_performed;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index a1626f3fae..f5434d8365 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1824,6 +1824,7 @@ pg_stat_bgwriter| SELECT pg_stat_get_bgwriter_buf_written_clean() AS buffers_cle
pg_stat_get_bgwriter_stat_reset_time() AS stats_reset;
pg_stat_checkpointer| SELECT pg_stat_get_checkpointer_num_timed() AS num_timed,
pg_stat_get_checkpointer_num_requested() AS num_requested,
+ pg_stat_get_checkpointer_num_performed() AS num_done,
pg_stat_get_checkpointer_restartpoints_timed() AS restartpoints_timed,
pg_stat_get_checkpointer_restartpoints_requested() AS restartpoints_req,
pg_stat_get_checkpointer_restartpoints_performed() AS restartpoints_done,
--
2.45.2
From d9311aee5a0e7665eb0ea32928f728a8cd01fab5 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Sat, 21 Sep 2024 00:47:12 +0900
Subject: [PATCH v4 2/2] docs: Enhance the pg_stat_checkpointer view
documentation.
This commit updates the documentation for the pg_stat_checkpointer view
to clarify what kind of checkpoints or restartpoints each counter tracks.
This makes it easier to understand the meaning of each counter.
---
doc/src/sgml/monitoring.sgml | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 19bf0164f1..3484a0490a 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3051,10 +3051,7 @@ description | Waiting for a newly initialized WAL file to reach durable storage
<structfield>num_timed</structfield> <type>bigint</type>
</para>
<para>
- Number of scheduled checkpoints due to timeout.
- Note that checkpoints may be skipped if the server has been idle
- since the last one, and this value counts both completed and
- skipped checkpoints
+ Number of scheduled checkpoints due to timeout
</para></entry>
</row>
@@ -3063,7 +3060,7 @@ description | Waiting for a newly initialized WAL file to reach durable storage
<structfield>num_requested</structfield> <type>bigint</type>
</para>
<para>
- Number of backend requested checkpoints
+ Number of requested checkpoints
</para></entry>
</row>
@@ -3146,6 +3143,18 @@ description | Waiting for a newly initialized WAL file to reach durable storage
</tgroup>
</table>
+ <para>
+ Checkpoints may be skipped if the server has been idle since the last one.
+ <structfield>num_timed</structfield> and
+ <structfield>num_requested</structfield> count both completed and skipped
+ checkpoints, while <structfield>num_done</structfield> tracks only
+ the completed ones. Similarly, restartpoints may be skipped
+ if the last replayed checkpoint record is already the last restartpoint.
+ <structfield>restartpoints_timed</structfield> and
+ <structfield>restartpoints_req</structfield> count both completed and
+ skipped restartpoints, while <structfield>restartpoints_done</structfield>
+ tracks only the completed ones.
+ </para>
</sect2>
<sect2 id="monitoring-pg-stat-wal-view">
--
2.45.2
Attachments:
[text/plain] v4-0001-Add-num_done-counter-to-the-pg_stat_checkpointer-.patch (10.2K, ../../[email protected]/2-v4-0001-Add-num_done-counter-to-the-pg_stat_checkpointer-.patch)
download | inline diff:
From 859f3fecb4fb4900b6ce12f6346c5d9565fbc072 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Fri, 20 Sep 2024 11:33:07 +0900
Subject: [PATCH v4 1/2] Add num_done counter to the pg_stat_checkpointer view.
Checkpoints can be skipped when the server is idle. The existing num_timed and
num_requested counters in pg_stat_checkpointer track both completed and
skipped checkpoints, but there was no way to count only the completed ones.
This commit introduces the num_done counter, which tracks only completed
checkpoints, making it easier to see how many were actually performed.
Bump catalog version.
Author: Anton A. Melnikov
Reviewed-by: Fujii Masao
Discussion: https://postgr.es/m/[email protected]
---
doc/src/sgml/monitoring.sgml | 11 +++++-
src/backend/access/transam/xlog.c | 9 ++++-
src/backend/catalog/system_views.sql | 1 +
src/backend/postmaster/checkpointer.c | 39 ++++++++++++-------
.../utils/activity/pgstat_checkpointer.c | 2 +
src/backend/utils/adt/pgstatfuncs.c | 6 +++
src/include/access/xlog.h | 2 +-
src/include/catalog/pg_proc.dat | 6 +++
src/include/pgstat.h | 1 +
src/test/regress/expected/rules.out | 1 +
10 files changed, 60 insertions(+), 18 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index a2fda4677d..19bf0164f1 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3063,7 +3063,16 @@ description | Waiting for a newly initialized WAL file to reach durable storage
<structfield>num_requested</structfield> <type>bigint</type>
</para>
<para>
- Number of requested checkpoints that have been performed
+ Number of backend requested checkpoints
+ </para></entry>
+ </row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>num_done</structfield> <type>bigint</type>
+ </para>
+ <para>
+ Number of checkpoints that have been performed
</para></entry>
</row>
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 853ab06812..64304d77d3 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6878,8 +6878,11 @@ update_checkpoint_display(int flags, bool restartpoint, bool reset)
* In this case, we only insert an XLOG_CHECKPOINT_SHUTDOWN record, and it's
* both the record marking the completion of the checkpoint and the location
* from which WAL replay would begin if needed.
+ *
+ * Returns true if a new checkpoint was performed, or false if it was skipped
+ * because the system was idle.
*/
-void
+bool
CreateCheckPoint(int flags)
{
bool shutdown;
@@ -6971,7 +6974,7 @@ CreateCheckPoint(int flags)
END_CRIT_SECTION();
ereport(DEBUG1,
(errmsg_internal("checkpoint skipped because system is idle")));
- return;
+ return false;
}
}
@@ -7353,6 +7356,8 @@ CreateCheckPoint(int flags)
CheckpointStats.ckpt_segs_added,
CheckpointStats.ckpt_segs_removed,
CheckpointStats.ckpt_segs_recycled);
+
+ return true;
}
/*
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 7fd5d256a1..49109dbdc8 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -1138,6 +1138,7 @@ CREATE VIEW pg_stat_checkpointer AS
SELECT
pg_stat_get_checkpointer_num_timed() AS num_timed,
pg_stat_get_checkpointer_num_requested() AS num_requested,
+ pg_stat_get_checkpointer_num_performed() AS num_done,
pg_stat_get_checkpointer_restartpoints_timed() AS restartpoints_timed,
pg_stat_get_checkpointer_restartpoints_requested() AS restartpoints_req,
pg_stat_get_checkpointer_restartpoints_performed() AS restartpoints_done,
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index eeb73c8572..9087e3f8db 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -460,10 +460,7 @@ CheckpointerMain(char *startup_data, size_t startup_data_len)
* Do the checkpoint.
*/
if (!do_restartpoint)
- {
- CreateCheckPoint(flags);
- ckpt_performed = true;
- }
+ ckpt_performed = CreateCheckPoint(flags);
else
ckpt_performed = CreateRestartPoint(flags);
@@ -484,7 +481,7 @@ CheckpointerMain(char *startup_data, size_t startup_data_len)
ConditionVariableBroadcast(&CheckpointerShmem->done_cv);
- if (ckpt_performed)
+ if (!do_restartpoint)
{
/*
* Note we record the checkpoint start time not end time as
@@ -493,18 +490,32 @@ CheckpointerMain(char *startup_data, size_t startup_data_len)
*/
last_checkpoint_time = now;
- if (do_restartpoint)
- PendingCheckpointerStats.restartpoints_performed++;
+ if (ckpt_performed)
+ PendingCheckpointerStats.num_performed++;
}
else
{
- /*
- * We were not able to perform the restartpoint (checkpoints
- * throw an ERROR in case of error). Most likely because we
- * have not received any new checkpoint WAL records since the
- * last restartpoint. Try again in 15 s.
- */
- last_checkpoint_time = now - CheckPointTimeout + 15;
+ if (ckpt_performed)
+ {
+ /*
+ * The same as for checkpoint. Please see the
+ * corresponding comment.
+ */
+ last_checkpoint_time = now;
+
+ PendingCheckpointerStats.restartpoints_performed++;
+ }
+ else
+ {
+ /*
+ * We were not able to perform the restartpoint
+ * (checkpoints throw an ERROR in case of error). Most
+ * likely because we have not received any new checkpoint
+ * WAL records since the last restartpoint. Try again in
+ * 15 s.
+ */
+ last_checkpoint_time = now - CheckPointTimeout + 15;
+ }
}
ckpt_active = false;
diff --git a/src/backend/utils/activity/pgstat_checkpointer.c b/src/backend/utils/activity/pgstat_checkpointer.c
index bbfc9c7e18..4a0a2d1493 100644
--- a/src/backend/utils/activity/pgstat_checkpointer.c
+++ b/src/backend/utils/activity/pgstat_checkpointer.c
@@ -49,6 +49,7 @@ pgstat_report_checkpointer(void)
#define CHECKPOINTER_ACC(fld) stats_shmem->stats.fld += PendingCheckpointerStats.fld
CHECKPOINTER_ACC(num_timed);
CHECKPOINTER_ACC(num_requested);
+ CHECKPOINTER_ACC(num_performed);
CHECKPOINTER_ACC(restartpoints_timed);
CHECKPOINTER_ACC(restartpoints_requested);
CHECKPOINTER_ACC(restartpoints_performed);
@@ -127,6 +128,7 @@ pgstat_checkpointer_snapshot_cb(void)
#define CHECKPOINTER_COMP(fld) pgStatLocal.snapshot.checkpointer.fld -= reset.fld;
CHECKPOINTER_COMP(num_timed);
CHECKPOINTER_COMP(num_requested);
+ CHECKPOINTER_COMP(num_performed);
CHECKPOINTER_COMP(restartpoints_timed);
CHECKPOINTER_COMP(restartpoints_requested);
CHECKPOINTER_COMP(restartpoints_performed);
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 9c23ac7c8c..17b0fc02ef 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1191,6 +1191,12 @@ pg_stat_get_checkpointer_num_requested(PG_FUNCTION_ARGS)
PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->num_requested);
}
+Datum
+pg_stat_get_checkpointer_num_performed(PG_FUNCTION_ARGS)
+{
+ PG_RETURN_INT64(pgstat_fetch_stat_checkpointer()->num_performed);
+}
+
Datum
pg_stat_get_checkpointer_restartpoints_timed(PG_FUNCTION_ARGS)
{
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 083810f5b4..36f6e4e4b4 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -239,7 +239,7 @@ extern void LocalProcessControlFile(bool reset);
extern WalLevel GetActiveWalLevelOnStandby(void);
extern void StartupXLOG(void);
extern void ShutdownXLOG(int code, Datum arg);
-extern void CreateCheckPoint(int flags);
+extern bool CreateCheckPoint(int flags);
extern bool CreateRestartPoint(int flags);
extern WALAvailability GetWALAvailability(XLogRecPtr targetLSN);
extern void XLogPutNextOid(Oid nextOid);
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 43f608d7a0..52fca54f3a 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5817,6 +5817,12 @@
proname => 'pg_stat_get_checkpointer_num_requested', provolatile => 's',
proparallel => 'r', prorettype => 'int8', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_num_requested' },
+{ oid => '2775',
+ descr => 'statistics: number of checkpoints performed by the checkpointer',
+ proname => 'pg_stat_get_checkpointer_num_performed',
+ provolatile => 's', proparallel => 'r', prorettype => 'int8',
+ proargtypes => '',
+ prosrc => 'pg_stat_get_checkpointer_num_performed' },
{ oid => '6327',
descr => 'statistics: number of timed restartpoints started by the checkpointer',
proname => 'pg_stat_get_checkpointer_restartpoints_timed', provolatile => 's',
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 4752dfe719..476acd680c 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -294,6 +294,7 @@ typedef struct PgStat_CheckpointerStats
{
PgStat_Counter num_timed;
PgStat_Counter num_requested;
+ PgStat_Counter num_performed;
PgStat_Counter restartpoints_timed;
PgStat_Counter restartpoints_requested;
PgStat_Counter restartpoints_performed;
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index a1626f3fae..f5434d8365 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1824,6 +1824,7 @@ pg_stat_bgwriter| SELECT pg_stat_get_bgwriter_buf_written_clean() AS buffers_cle
pg_stat_get_bgwriter_stat_reset_time() AS stats_reset;
pg_stat_checkpointer| SELECT pg_stat_get_checkpointer_num_timed() AS num_timed,
pg_stat_get_checkpointer_num_requested() AS num_requested,
+ pg_stat_get_checkpointer_num_performed() AS num_done,
pg_stat_get_checkpointer_restartpoints_timed() AS restartpoints_timed,
pg_stat_get_checkpointer_restartpoints_requested() AS restartpoints_req,
pg_stat_get_checkpointer_restartpoints_performed() AS restartpoints_done,
--
2.45.2
[text/plain] v4-0002-docs-Enhance-the-pg_stat_checkpointer-view-docume.patch (2.4K, ../../[email protected]/3-v4-0002-docs-Enhance-the-pg_stat_checkpointer-view-docume.patch)
download | inline diff:
From d9311aee5a0e7665eb0ea32928f728a8cd01fab5 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Sat, 21 Sep 2024 00:47:12 +0900
Subject: [PATCH v4 2/2] docs: Enhance the pg_stat_checkpointer view
documentation.
This commit updates the documentation for the pg_stat_checkpointer view
to clarify what kind of checkpoints or restartpoints each counter tracks.
This makes it easier to understand the meaning of each counter.
---
doc/src/sgml/monitoring.sgml | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index 19bf0164f1..3484a0490a 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -3051,10 +3051,7 @@ description | Waiting for a newly initialized WAL file to reach durable storage
<structfield>num_timed</structfield> <type>bigint</type>
</para>
<para>
- Number of scheduled checkpoints due to timeout.
- Note that checkpoints may be skipped if the server has been idle
- since the last one, and this value counts both completed and
- skipped checkpoints
+ Number of scheduled checkpoints due to timeout
</para></entry>
</row>
@@ -3063,7 +3060,7 @@ description | Waiting for a newly initialized WAL file to reach durable storage
<structfield>num_requested</structfield> <type>bigint</type>
</para>
<para>
- Number of backend requested checkpoints
+ Number of requested checkpoints
</para></entry>
</row>
@@ -3146,6 +3143,18 @@ description | Waiting for a newly initialized WAL file to reach durable storage
</tgroup>
</table>
+ <para>
+ Checkpoints may be skipped if the server has been idle since the last one.
+ <structfield>num_timed</structfield> and
+ <structfield>num_requested</structfield> count both completed and skipped
+ checkpoints, while <structfield>num_done</structfield> tracks only
+ the completed ones. Similarly, restartpoints may be skipped
+ if the last replayed checkpoint record is already the last restartpoint.
+ <structfield>restartpoints_timed</structfield> and
+ <structfield>restartpoints_req</structfield> count both completed and
+ skipped restartpoints, while <structfield>restartpoints_done</structfield>
+ tracks only the completed ones.
+ </para>
</sect2>
<sect2 id="monitoring-pg-stat-wal-view">
--
2.45.2
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
2024-09-16 14:30 Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-17 02:47 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-18 14:35 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-18 18:04 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-19 10:16 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-20 16:19 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
@ 2024-09-22 04:55 ` Anton A. Melnikov <[email protected]>
2024-09-30 03:26 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Anton A. Melnikov @ 2024-09-22 04:55 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Anton A. Melnikov <[email protected]>; Andres Freund <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On 20.09.2024 19:19, Fujii Masao wrote:
> I've attached the updated version (0001.patch). I made some cosmetic changes,
> including reverting the switch in the entries for pg_stat_get_checkpointer_write_time
> and pg_stat_get_checkpointer_sync_time in pg_proc.dat, as I didn’t think
> that change was necessary. Could you please review the latest version?
Thanks for corrections!
All looks good for me.
As for switching in the pg_proc.dat entries the idea was to put them in order
so that the pg_stat_get_checkpointer* functions were grouped together.
I don't know if this is the common and accepted practice. Simply i like it better this way.
Sure, if you think it's unnecessary, let it stay as is with minimal diff.
> After we commit 0001.patch, how about applying 0002.patch, which updates
> the documentation for the pg_stat_checkpointer view to clarify what types
> of checkpoints and restartpoints each counter tracks?
I liked that the short definitions of the counters are now separated from
the description of its work features which are combined into one paragraph.
It seems to me that is much more logical and easier to understand.
In addition, checkpoints may be skipped due to "checkpoints are occurring
too frequently" error. Not sure, but maybe add this information to
the new description?
> In 0002.patch, I also modified the description of num_requested from
> "Number of backend requested checkpoints" to remove "backend," as it can
> be confusing since num_requested includes requests from sources other than
> the backend. Thought?
Agreed. E.g. from xlog. Then maybe changed it also in the function
descriptions in the pg_proc.dat? For pg_stat_get_checkpointer_num_requested()
and pg_stat_get_checkpointer_restartpoints_requested().
Also checked v4 with the travis patch-tester. All is ok.
With the best wishes!
--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
2024-09-16 14:30 Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-17 02:47 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-18 14:35 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-18 18:04 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-19 10:16 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-20 16:19 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-22 04:55 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
@ 2024-09-30 03:26 ` Fujii Masao <[email protected]>
2024-09-30 07:00 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-10-08 12:42 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
0 siblings, 2 replies; 30+ messages in thread
From: Fujii Masao @ 2024-09-30 03:26 UTC (permalink / raw)
To: Anton A. Melnikov <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Anton A. Melnikov <[email protected]>; Andres Freund <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On 2024/09/22 13:55, Anton A. Melnikov wrote:
> On 20.09.2024 19:19, Fujii Masao wrote:
>> I've attached the updated version (0001.patch). I made some cosmetic changes,
>> including reverting the switch in the entries for pg_stat_get_checkpointer_write_time
>> and pg_stat_get_checkpointer_sync_time in pg_proc.dat, as I didn’t think
>> that change was necessary. Could you please review the latest version?
>
> Thanks for corrections!
> All looks good for me.
Thanks for the review! I've pushed the 0001 patch.
> As for switching in the pg_proc.dat entries the idea was to put them in order
> so that the pg_stat_get_checkpointer* functions were grouped together.
> I don't know if this is the common and accepted practice. Simply i like it better this way.
> Sure, if you think it's unnecessary, let it stay as is with minimal diff.
I understand your point, but I didn't made that change to keep the diff minimal,
which should make future back-patching easier.
>> After we commit 0001.patch, how about applying 0002.patch, which updates
>> the documentation for the pg_stat_checkpointer view to clarify what types
>> of checkpoints and restartpoints each counter tracks?
>
> I liked that the short definitions of the counters are now separated from
> the description of its work features which are combined into one paragraph.
> It seems to me that is much more logical and easier to understand.
Thanks for the review!
> In addition, checkpoints may be skipped due to "checkpoints are occurring
> too frequently" error. Not sure, but maybe add this information to
> the new description?
From what I can see in the code, that error message doesn’t seem to indicate
the checkpoint is being skipped. In fact, checkpoints are still happening
actually when that message appears. Am I misunderstanding something?
>> In 0002.patch, I also modified the description of num_requested from
>> "Number of backend requested checkpoints" to remove "backend," as it can
>> be confusing since num_requested includes requests from sources other than
>> the backend. Thought?
>
> Agreed. E.g. from xlog. Then maybe changed it also in the function
> descriptions in the pg_proc.dat? For pg_stat_get_checkpointer_num_requested()
> and pg_stat_get_checkpointer_restartpoints_requested().
Yes, good catch!
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
2024-09-16 14:30 Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-17 02:47 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-18 14:35 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-18 18:04 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-19 10:16 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-20 16:19 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-22 04:55 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-30 03:26 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
@ 2024-09-30 07:00 ` Anton A. Melnikov <[email protected]>
2024-09-30 17:09 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
1 sibling, 1 reply; 30+ messages in thread
From: Anton A. Melnikov @ 2024-09-30 07:00 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Anton A. Melnikov <[email protected]>; Andres Freund <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On 30.09.2024 06:26, Fujii Masao wrote:
> Thanks for the review! I've pushed the 0001 patch.
Thanks a lot!
>> As for switching in the pg_proc.dat entries the idea was to put them in order
>> so that the pg_stat_get_checkpointer* functions were grouped together.
>> I don't know if this is the common and accepted practice. Simply i like it better this way.
>> Sure, if you think it's unnecessary, let it stay as is with minimal diff.
>
> I understand your point, but I didn't made that change to keep the diff minimal,
> which should make future back-patching easier.
Agreed. Its quite reasonable. I've not take into account the backporting
possibility at all. This is of course wrong.
>> In addition, checkpoints may be skipped due to "checkpoints are occurring
>> too frequently" error. Not sure, but maybe add this information to
>> the new description?
>
> From what I can see in the code, that error message doesn’t seem to indicate
> the checkpoint is being skipped. In fact, checkpoints are still happening
> actually when that message appears. Am I misunderstanding something?
No, you are right! This is my oversight. I didn't notice that elevel is just a log
not a error. Thanks!
With the best wishes,
--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
2024-09-16 14:30 Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-17 02:47 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-18 14:35 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-18 18:04 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-19 10:16 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-20 16:19 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-22 04:55 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-30 03:26 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-30 07:00 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
@ 2024-09-30 17:09 ` Fujii Masao <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Fujii Masao @ 2024-09-30 17:09 UTC (permalink / raw)
To: Anton A. Melnikov <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Anton A. Melnikov <[email protected]>; Andres Freund <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On 2024/09/30 16:00, Anton A. Melnikov wrote:
>
> On 30.09.2024 06:26, Fujii Masao wrote:
>> Thanks for the review! I've pushed the 0001 patch.
>
> Thanks a lot!
>
>>> As for switching in the pg_proc.dat entries the idea was to put them in order
>>> so that the pg_stat_get_checkpointer* functions were grouped together.
>>> I don't know if this is the common and accepted practice. Simply i like it better this way.
>>> Sure, if you think it's unnecessary, let it stay as is with minimal diff.
>>
>> I understand your point, but I didn't made that change to keep the diff minimal,
>> which should make future back-patching easier.
>
> Agreed. Its quite reasonable. I've not take into account the backporting
> possibility at all. This is of course wrong.
>
>>> In addition, checkpoints may be skipped due to "checkpoints are occurring
>>> too frequently" error. Not sure, but maybe add this information to
>>> the new description?
>>
>> From what I can see in the code, that error message doesn’t seem to indicate
>> the checkpoint is being skipped. In fact, checkpoints are still happening
>> actually when that message appears. Am I misunderstanding something?
>
> No, you are right! This is my oversight. I didn't notice that elevel is just a log
> not a error. Thanks!
Ok, so I pushed 0002.patch. Thanks for the review!
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
2024-09-16 14:30 Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-17 02:47 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-18 14:35 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-18 18:04 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-19 10:16 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-20 16:19 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-22 04:55 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-30 03:26 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
@ 2024-10-08 12:42 ` Fujii Masao <[email protected]>
2024-10-08 14:16 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
1 sibling, 1 reply; 30+ messages in thread
From: Fujii Masao @ 2024-10-08 12:42 UTC (permalink / raw)
To: Anton A. Melnikov <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Anton A. Melnikov <[email protected]>; Andres Freund <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On 2024/09/30 12:26, Fujii Masao wrote:
>>> In 0002.patch, I also modified the description of num_requested from
>>> "Number of backend requested checkpoints" to remove "backend," as it can
>>> be confusing since num_requested includes requests from sources other than
>>> the backend. Thought?
>>
>> Agreed. E.g. from xlog. Then maybe changed it also in the function
>> descriptions in the pg_proc.dat? For pg_stat_get_checkpointer_num_requested()
>> and pg_stat_get_checkpointer_restartpoints_requested().
>
> Yes, good catch!
Patch attached.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From 5c935f5263fc4d516ceaf8d46c2a06daf035f1f7 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Tue, 8 Oct 2024 21:12:48 +0900
Subject: [PATCH v1] Improve descriptions of some pg_stat_checkpoints functions
in pg_proc.dat.
Previously, the descriptions of pg_stat_get_checkpointer_num_requested(),
pg_stat_get_checkpointer_restartpoints_requested(),
and pg_stat_get_checkpointer_restartpoints_performed() in pg_proc.dat
referred to "backend". This was misleading because these functions report
the number of checkpoints or restartpoints requested or performed
by other than backends as well.
This commit removes "backend" from these descriptions to avoid confusion.
---
src/include/catalog/pg_proc.dat | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 77f54a79e6..2ba144a7aa 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5816,7 +5816,7 @@
proparallel => 'r', prorettype => 'int8', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_num_timed' },
{ oid => '2770',
- descr => 'statistics: number of backend requested checkpoints started by the checkpointer',
+ descr => 'statistics: number of requested checkpoints started by the checkpointer',
proname => 'pg_stat_get_checkpointer_num_requested', provolatile => 's',
proparallel => 'r', prorettype => 'int8', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_num_requested' },
@@ -5831,13 +5831,13 @@
proparallel => 'r', prorettype => 'int8', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_restartpoints_timed' },
{ oid => '6328',
- descr => 'statistics: number of backend requested restartpoints started by the checkpointer',
+ descr => 'statistics: number of requested restartpoints started by the checkpointer',
proname => 'pg_stat_get_checkpointer_restartpoints_requested',
provolatile => 's', proparallel => 'r', prorettype => 'int8',
proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_restartpoints_requested' },
{ oid => '6329',
- descr => 'statistics: number of backend performed restartpoints',
+ descr => 'statistics: number of restartpoints performed by the checkpointer',
proname => 'pg_stat_get_checkpointer_restartpoints_performed',
provolatile => 's', proparallel => 'r', prorettype => 'int8',
proargtypes => '',
--
2.46.2
Attachments:
[text/plain] v1-0001-Improve-descriptions-of-some-pg_stat_checkpoints-.patch (2.4K, ../../[email protected]/2-v1-0001-Improve-descriptions-of-some-pg_stat_checkpoints-.patch)
download | inline diff:
From 5c935f5263fc4d516ceaf8d46c2a06daf035f1f7 Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Tue, 8 Oct 2024 21:12:48 +0900
Subject: [PATCH v1] Improve descriptions of some pg_stat_checkpoints functions
in pg_proc.dat.
Previously, the descriptions of pg_stat_get_checkpointer_num_requested(),
pg_stat_get_checkpointer_restartpoints_requested(),
and pg_stat_get_checkpointer_restartpoints_performed() in pg_proc.dat
referred to "backend". This was misleading because these functions report
the number of checkpoints or restartpoints requested or performed
by other than backends as well.
This commit removes "backend" from these descriptions to avoid confusion.
---
src/include/catalog/pg_proc.dat | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 77f54a79e6..2ba144a7aa 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -5816,7 +5816,7 @@
proparallel => 'r', prorettype => 'int8', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_num_timed' },
{ oid => '2770',
- descr => 'statistics: number of backend requested checkpoints started by the checkpointer',
+ descr => 'statistics: number of requested checkpoints started by the checkpointer',
proname => 'pg_stat_get_checkpointer_num_requested', provolatile => 's',
proparallel => 'r', prorettype => 'int8', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_num_requested' },
@@ -5831,13 +5831,13 @@
proparallel => 'r', prorettype => 'int8', proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_restartpoints_timed' },
{ oid => '6328',
- descr => 'statistics: number of backend requested restartpoints started by the checkpointer',
+ descr => 'statistics: number of requested restartpoints started by the checkpointer',
proname => 'pg_stat_get_checkpointer_restartpoints_requested',
provolatile => 's', proparallel => 'r', prorettype => 'int8',
proargtypes => '',
prosrc => 'pg_stat_get_checkpointer_restartpoints_requested' },
{ oid => '6329',
- descr => 'statistics: number of backend performed restartpoints',
+ descr => 'statistics: number of restartpoints performed by the checkpointer',
proname => 'pg_stat_get_checkpointer_restartpoints_performed',
provolatile => 's', proparallel => 'r', prorettype => 'int8',
proargtypes => '',
--
2.46.2
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
2024-09-16 14:30 Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-17 02:47 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-18 14:35 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-18 18:04 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-19 10:16 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-20 16:19 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-22 04:55 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-30 03:26 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-10-08 12:42 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
@ 2024-10-08 14:16 ` Anton A. Melnikov <[email protected]>
2024-10-10 15:14 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Anton A. Melnikov @ 2024-10-08 14:16 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Anton A. Melnikov <[email protected]>; Andres Freund <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On 08.10.2024 15:42, Fujii Masao wrote:
> On 2024/09/30 12:26, Fujii Masao wrote:
>>>> In 0002.patch, I also modified the description of num_requested from
>>>> "Number of backend requested checkpoints" to remove "backend," as it can
>>>> be confusing since num_requested includes requests from sources other than
>>>> the backend. Thought?
>>>
>>> Agreed. E.g. from xlog. Then maybe changed it also in the function
>>> descriptions in the pg_proc.dat? For pg_stat_get_checkpointer_num_requested()
>>> and pg_stat_get_checkpointer_restartpoints_requested().
>>
>> Yes, good catch!
>
> Patch attached.
Looked at the patch. Just in case, checked that neither
“backend completed” nor “backend requested” were found anywhere else.
All is ok for me.
Thanks a lot!
With the best wishes,
--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
2024-09-16 14:30 Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-17 02:47 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-18 14:35 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-18 18:04 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-19 10:16 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-20 16:19 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-22 04:55 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-30 03:26 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-10-08 12:42 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-10-08 14:16 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
@ 2024-10-10 15:14 ` Fujii Masao <[email protected]>
2024-10-10 15:37 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
0 siblings, 1 reply; 30+ messages in thread
From: Fujii Masao @ 2024-10-10 15:14 UTC (permalink / raw)
To: Anton A. Melnikov <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Anton A. Melnikov <[email protected]>; Andres Freund <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On 2024/10/08 23:16, Anton A. Melnikov wrote:
>
> On 08.10.2024 15:42, Fujii Masao wrote:
>> On 2024/09/30 12:26, Fujii Masao wrote:
>>>>> In 0002.patch, I also modified the description of num_requested from
>>>>> "Number of backend requested checkpoints" to remove "backend," as it can
>>>>> be confusing since num_requested includes requests from sources other than
>>>>> the backend. Thought?
>>>>
>>>> Agreed. E.g. from xlog. Then maybe changed it also in the function
>>>> descriptions in the pg_proc.dat? For pg_stat_get_checkpointer_num_requested()
>>>> and pg_stat_get_checkpointer_restartpoints_requested().
>>>
>>> Yes, good catch!
>>
>> Patch attached.
>
> Looked at the patch. Just in case, checked that neither
> “backend completed” nor “backend requested” were found anywhere else.
> All is ok for me.
Thanks for the review! Pushed.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 30+ messages in thread
* Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica.
2024-09-16 14:30 Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-17 02:47 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-18 14:35 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-18 18:04 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-19 10:16 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-20 16:19 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-22 04:55 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-30 03:26 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-10-08 12:42 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-10-08 14:16 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-10-10 15:14 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
@ 2024-10-10 15:37 ` Anton A. Melnikov <[email protected]>
0 siblings, 0 replies; 30+ messages in thread
From: Anton A. Melnikov @ 2024-10-10 15:37 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; Alexander Korotkov <[email protected]>; +Cc: Magnus Hagander <[email protected]>; Anton A. Melnikov <[email protected]>; Andres Freund <[email protected]>; Kyotaro Horiguchi <[email protected]>; [email protected]
On 10.10.2024 18:14, Fujii Masao wrote:
> Thanks for the review! Pushed.
Thanks a lot!
With the best regards,
--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
^ permalink raw reply [nested|flat] 30+ messages in thread
end of thread, other threads:[~2024-10-10 15:37 UTC | newest]
Thread overview: 30+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2019-01-01 20:21 Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead Nikolay Shaplov <[email protected]>
2019-01-02 03:05 ` Alvaro Herrera <[email protected]>
2019-01-02 09:58 ` Nikolay Shaplov <[email protected]>
2019-01-03 19:10 ` Alvaro Herrera <[email protected]>
2019-01-03 19:22 ` Nikolay Shaplov <[email protected]>
2019-01-03 20:15 ` Alvaro Herrera <[email protected]>
2019-01-07 16:26 ` Nikolay Shaplov <[email protected]>
2019-01-17 23:33 ` Alvaro Herrera <[email protected]>
2019-01-20 10:35 ` Nikolay Shaplov <[email protected]>
2019-09-10 08:28 [PATCH v7 3/4] Remove globals readOff, readLen and readSegNo Kyotaro Horiguchi <[email protected]>
2019-09-10 08:28 [PATCH v12 3/4] Remove globals readOff, readLen and readSegNo Kyotaro Horiguchi <[email protected]>
2019-09-10 08:28 [PATCH v11 3/4] Remove globals readOff, readLen and readSegNo Kyotaro Horiguchi <[email protected]>
2023-06-30 19:46 [PATCH v1 1/1] remove db_user_namespace Nathan Bossart <[email protected]>
2024-09-16 14:30 Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-17 02:47 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-18 10:21 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-18 12:22 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Alexander Korotkov <[email protected]>
2024-09-18 17:21 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-18 14:35 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-18 18:04 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-19 10:16 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-20 16:19 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-22 04:55 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-30 03:26 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-09-30 07:00 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-09-30 17:09 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-10-08 12:42 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-10-08 14:16 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[email protected]>
2024-10-10 15:14 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Fujii Masao <[email protected]>
2024-10-10 15:37 ` Re: May be BUG. Periodic burst growth of the checkpoint_req counter on replica. Anton A. Melnikov <[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