On Mon, Mar 13, 2023 at 12:26 PM Michael Paquier <mich...@paquier.xyz> wrote:
>
> On Fri, Mar 10, 2023 at 04:45:06PM +0530, Bharath Rupireddy wrote:
>
> After that comes the rest of the patch, and I have found a couple of
> mistakes.
>
> - pg_get_wal_records_info(start_lsn pg_lsn, end_lsn pg_lsn)
> + pg_get_wal_records_info(start_lsn pg_lsn, end_lsn pg_lsn DEFAULT NULL)
> returns setof record
> [...]
> - pg_get_wal_stats(start_lsn pg_lsn, end_lsn pg_lsn, per_record boolean
> DEFAULT false)
> + pg_get_wal_stats(start_lsn pg_lsn, end_lsn pg_lsn DEFAULT NULL,
> per_record boolean DEFAULT false)
>
> This part of the documentation is now incorrect.
Oh, yeah. Thanks for fixing it.
> +-- Make sure checkpoints don't interfere with the test.
> +SELECT 'init' FROM
> pg_create_physical_replication_slot('regress_pg_walinspect_slot', true,
> false);
>
> Adding a physical slot is better for stability of course, but the test
> also has to drop it or installcheck would cause an existing cluster to
> have that still around. The third argument could be true, as well, so
> as we'd use a temporary slot.
# Disabled because these tests require "wal_level=replica", which
# some installcheck users do not have (e.g. buildfarm clients).
NO_INSTALLCHECK = 1
pg_walinspect can't be run under installcheck. I don't think dropping
the slot at the end is needed, it's unnecessary. I saw
oldextversions.sql using the same replication slot name, well no
problem, but I changed it to a unique name.
> Hmm. I would simplify that, and just mention that an error is raised
> when the start LSN is available, without caring about the rest (valid
> end LSN being past the current insert LSN, and error if start > end,
> the second being obvious).
Okay.
> + <note>
> + <para>
> + Note that <function>pg_get_wal_records_info_till_end_of_wal</function> and
> + <function>pg_get_wal_stats_till_end_of_wal</function> functions have been
> + removed in the <filename>pg_walinspect</filename> version
> + <literal>1.1</literal>. The same functionality can be achieved with
> + <function>pg_get_wal_records_info</function> and
> + <function>pg_get_wal_stats</function> functions by specifying a future
> + <replaceable>end_lsn</replaceable>. However,
> <function>till_end_of_wal</function>
> + functions will still work if the extension is installed explicitly with
> + version <literal>1.0</literal>.
> + </para>
> + </note>
>
> Not convinced that this is necessary.
As hackers we know that these functions have been removed and how to
achieve till_end_of_wal with the other functions. I noticed that
you've removed my changes (see below) from the docs that were saying
how to get info/stats till_end_of_wal. That leaves end users confused
as to how they can achieve till_end_of_wal functionality. All users
can't look for commit history/message but they can easily read the
docs. I prefer to have the following (did so in the attached v7) and
get rid of the above note if you don't feel strongly about it.
+ If a future <replaceable>end_lsn</replaceable>
+ (i.e. the LSN server doesn't know about) is specified, it returns
+ informaton till end of WAL.
> + GetInputLSNs(fcinfo, &start_lsn, &end_lsn, till_end_of_wal);
> +
> + stats_per_record = PG_GETARG_BOOL(2);
>
> This code in GetWalStats() is incorrect.
> pg_get_wal_stats_till_end_of_wal() has a stats_per_record, but as
> *second* argument, so this would be broken.
Oh, yeah. Thanks for fixing it.
> + GetInputLSNs(fcinfo, &start_lsn, &end_lsn, till_end_of_wal);
>
> Coming from the last point, I think that this interface is confusing,
> and actually incorrect. From what I can see, we should be doing what
> ~15 has by grepping the argument values within the main function
> calls, and just pass them down to the internal routines GetWalStats()
> and GetWALRecordsInfo().
Hm, what you have in v6 works for me.
> At the end, I am finishing with the attached. ValidateInputLSNs()
> ought to be called, IMO, when the caller of the SQL functions can
> directly specify an end_lsn. This means that there is no point to do
> this check in the two till_end_* functions. This has as cost two
> extra checks to make sure that the start_lsn is not higher than the
> current LSN, but I am fine to live with that. It seemed rather
> natural to me to let ValidateInputLSNs() do a refresh of the end_lsn
> if it sees that it is higher than the current LSN. And if you look
> closely, you will see that we only call *once* GetCurrentLSN() for
> each function call, so the maths are more precise.
>
> I have cleaned up the comments of the modules, while on it, as there
> was not much value in copy-pasting how a function fails while there is
> a centralized validation code path. The tests for the till_end()
> functions have been moved to the test path where we install 1.0.
I have some comments and fixed them in the attached v7 patch:
1.
+ * pg_get_wal_records_info
*
+ * pg_get_wal_stats
*
I think you wanted to be consistent with function comments with
function names atop, but missed adding for all functions. Actually, I
don't have a strong opinion on these changes as they unnecessarily
bloat the changes, so I removed them.
2.
+ if (start_lsn > curr_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("cannot accept future start LSN"),
- errdetail("Last known WAL LSN on the database system
is at %X/%X.",
- LSN_FORMAT_ARGS(curr_lsn))));
- }
+ errmsg("WAL start LSN must be less than current LSN")));
I don't like this inconsistency much, especially when
pg_get_wal_record_info emits "cannot accept future input LSN" with the
current LSN details (this current LSN will give a bit more information
to the user). Also, let's be consistent across returning NULLs if
input LSN/start LSN equal to the current LSN. I've done these changes
in the attached v7 patch.
3. I wanted COUNT(*) >= 0 for successful function execution to be
COUNT(*) >= 1 so that we will check for at least the functions
returning 1 record. And failures to be SELECT * FROM. This was my
intention but I don't see that in this patch or in the previous
test-refactoring commit. I added that in the attached v7 patch again.
Also, made test comments conssitent.
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
From 188a199d1c1ed6dd12021030c592751d2cadc2fb Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: Mon, 13 Mar 2023 10:06:16 +0000
Subject: [PATCH v7] Rework pg_walinspect functions
---
.../pg_walinspect/expected/oldextversions.out | 41 +++-
.../pg_walinspect/expected/pg_walinspect.out | 66 +++---
.../pg_walinspect/pg_walinspect--1.0--1.1.sql | 4 +
contrib/pg_walinspect/pg_walinspect.c | 215 +++++++-----------
contrib/pg_walinspect/sql/oldextversions.sql | 19 ++
contrib/pg_walinspect/sql/pg_walinspect.sql | 43 ++--
doc/src/sgml/pgwalinspect.sgml | 58 ++---
7 files changed, 205 insertions(+), 241 deletions(-)
diff --git a/contrib/pg_walinspect/expected/oldextversions.out b/contrib/pg_walinspect/expected/oldextversions.out
index 0db3538693..fb453feec5 100644
--- a/contrib/pg_walinspect/expected/oldextversions.out
+++ b/contrib/pg_walinspect/expected/oldextversions.out
@@ -1,5 +1,7 @@
-- test old extension version entry points
CREATE EXTENSION pg_walinspect WITH VERSION '1.0';
+-- Mask DETAIL messages as these could refer to current LSN positions
+\set VERBOSITY terse
-- List what version 1.0 contains
\dx+ pg_walinspect
Objects in extension "pg_walinspect"
@@ -12,19 +14,46 @@ CREATE EXTENSION pg_walinspect WITH VERSION '1.0';
function pg_get_wal_stats_till_end_of_wal(pg_lsn,boolean)
(5 rows)
+-- Make sure checkpoints don't interfere with the test
+SELECT 'init' FROM pg_create_physical_replication_slot('regress_pg_walinspect_slot_old_ext_ver', true, false);
+ ?column?
+----------
+ init
+(1 row)
+
+CREATE TABLE sample_tbl(col1 int, col2 int);
+SELECT pg_current_wal_lsn() AS wal_lsn1 \gset
+INSERT INTO sample_tbl SELECT * FROM generate_series(1, 2);
+-- Tests for the past functions
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info_till_end_of_wal(:'wal_lsn1');
+ ok
+----
+ t
+(1 row)
+
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_stats_till_end_of_wal(:'wal_lsn1');
+ ok
+----
+ t
+(1 row)
+
+-- Failures with start LSNs
+SELECT * FROM pg_get_wal_records_info_till_end_of_wal('FFFFFFFF/FFFFFFFF');
+ERROR: WAL start LSN must be less than current LSN
+SELECT * FROM pg_get_wal_stats_till_end_of_wal('FFFFFFFF/FFFFFFFF');
+ERROR: WAL start LSN must be less than current LSN
-- Move to new version 1.1
ALTER EXTENSION pg_walinspect UPDATE TO '1.1';
-- List what version 1.1 contains
\dx+ pg_walinspect
- Objects in extension "pg_walinspect"
- Object description
------------------------------------------------------------
+ Objects in extension "pg_walinspect"
+ Object description
+--------------------------------------------------
function pg_get_wal_block_info(pg_lsn,pg_lsn)
function pg_get_wal_record_info(pg_lsn)
function pg_get_wal_records_info(pg_lsn,pg_lsn)
- function pg_get_wal_records_info_till_end_of_wal(pg_lsn)
function pg_get_wal_stats(pg_lsn,pg_lsn,boolean)
- function pg_get_wal_stats_till_end_of_wal(pg_lsn,boolean)
-(6 rows)
+(4 rows)
+-- Clean up
DROP EXTENSION pg_walinspect;
diff --git a/contrib/pg_walinspect/expected/pg_walinspect.out b/contrib/pg_walinspect/expected/pg_walinspect.out
index c9fb781055..2042ac365e 100644
--- a/contrib/pg_walinspect/expected/pg_walinspect.out
+++ b/contrib/pg_walinspect/expected/pg_walinspect.out
@@ -9,7 +9,7 @@ SELECT 'init' FROM pg_create_physical_replication_slot('regress_pg_walinspect_sl
(1 row)
CREATE TABLE sample_tbl(col1 int, col2 int);
--- Save some LSNs for comparisons
+-- Save some LSNs for comparisons.
SELECT pg_current_wal_lsn() AS wal_lsn1 \gset
INSERT INTO sample_tbl SELECT * FROM generate_series(1, 2);
SELECT pg_current_wal_lsn() AS wal_lsn2 \gset
@@ -35,60 +35,56 @@ ERROR: WAL start LSN must be less than end LSN
SELECT * FROM pg_get_wal_block_info(:'wal_lsn2', :'wal_lsn1');
ERROR: WAL start LSN must be less than end LSN
-- LSNs with the highest value possible.
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_record_info('FFFFFFFF/FFFFFFFF');
-ERROR: cannot accept future input LSN
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info_till_end_of_wal('FFFFFFFF/FFFFFFFF');
-ERROR: cannot accept future start LSN
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats_till_end_of_wal('FFFFFFFF/FFFFFFFF');
-ERROR: cannot accept future start LSN
--- failures with end LSNs
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
-ERROR: cannot accept future end LSN
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
-ERROR: cannot accept future end LSN
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_block_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
-ERROR: cannot accept future end LSN
--- failures with start LSNs
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: cannot accept future start LSN
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: cannot accept future start LSN
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_block_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-ERROR: cannot accept future start LSN
--- ===================================================================
--- Tests for all function executions
--- ===================================================================
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_record_info(:'wal_lsn1');
+SELECT * FROM pg_get_wal_record_info('FFFFFFFF/FFFFFFFF');
+ERROR: WAL input LSN must be less than current LSN
+-- Failures with start LSNs.
+SELECT * FROM pg_get_wal_records_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
+ERROR: WAL start LSN must be less than current LSN
+SELECT * FROM pg_get_wal_stats('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
+ERROR: WAL start LSN must be less than current LSN
+SELECT * FROM pg_get_wal_block_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
+ERROR: WAL start LSN must be less than current LSN
+-- Success with end LSNs.
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
ok
----
t
(1 row)
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info_till_end_of_wal(:'wal_lsn1');
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_stats(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
ok
----
t
(1 row)
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats_till_end_of_wal(:'wal_lsn1');
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_block_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
ok
----
t
(1 row)
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', :'wal_lsn2');
+-- ===================================================================
+-- Tests for all function executions
+-- ===================================================================
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_record_info(:'wal_lsn1');
ok
----
t
(1 row)
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(:'wal_lsn1', :'wal_lsn2');
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', :'wal_lsn2');
ok
----
t
(1 row)
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_block_info(:'wal_lsn1', :'wal_lsn2');
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_stats(:'wal_lsn1', :'wal_lsn2');
+ ok
+----
+ t
+(1 row)
+
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_block_info(:'wal_lsn1', :'wal_lsn2');
ok
----
t
@@ -119,7 +115,7 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', :'wal_lsn2'
-- ===================================================================
-- Tests to get block information from WAL record
-- ===================================================================
--- Update table to generate some block data
+-- Update table to generate some block data.
SELECT pg_current_wal_lsn() AS wal_lsn3 \gset
UPDATE sample_tbl SET col1 = col1 + 1 WHERE col1 = 1;
SELECT pg_current_wal_lsn() AS wal_lsn4 \gset
@@ -176,7 +172,7 @@ SELECT has_function_privilege('regress_pg_walinspect',
f
(1 row)
--- Functions accessible by users with role pg_read_server_files
+-- Functions accessible by users with role pg_read_server_files.
GRANT pg_read_server_files TO regress_pg_walinspect;
SELECT has_function_privilege('regress_pg_walinspect',
'pg_get_wal_record_info(pg_lsn)', 'EXECUTE'); -- yes
@@ -256,11 +252,5 @@ REVOKE EXECUTE ON FUNCTION pg_get_wal_block_info(pg_lsn, pg_lsn)
-- Clean up
-- ===================================================================
DROP ROLE regress_pg_walinspect;
-SELECT pg_drop_replication_slot('regress_pg_walinspect_slot');
- pg_drop_replication_slot
---------------------------
-
-(1 row)
-
DROP TABLE sample_tbl;
DROP EXTENSION pg_walinspect;
diff --git a/contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql b/contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql
index e674ef25aa..586c3b4467 100644
--- a/contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql
+++ b/contrib/pg_walinspect/pg_walinspect--1.0--1.1.sql
@@ -3,6 +3,10 @@
-- complain if script is sourced in psql, rather than via ALTER EXTENSION
\echo Use "ALTER EXTENSION pg_walinspect UPDATE TO '1.1'" to load this file. \quit
+-- Unsupported functions after 1.1.
+DROP FUNCTION pg_get_wal_records_info_till_end_of_wal(pg_lsn);
+DROP FUNCTION pg_get_wal_stats_till_end_of_wal(pg_lsn, boolean);
+
--
-- pg_get_wal_block_info()
--
diff --git a/contrib/pg_walinspect/pg_walinspect.c b/contrib/pg_walinspect/pg_walinspect.c
index ee88dc4992..3b3215daf5 100644
--- a/contrib/pg_walinspect/pg_walinspect.c
+++ b/contrib/pg_walinspect/pg_walinspect.c
@@ -38,14 +38,14 @@ PG_FUNCTION_INFO_V1(pg_get_wal_records_info_till_end_of_wal);
PG_FUNCTION_INFO_V1(pg_get_wal_stats);
PG_FUNCTION_INFO_V1(pg_get_wal_stats_till_end_of_wal);
-static bool IsFutureLSN(XLogRecPtr lsn, XLogRecPtr *curr_lsn);
+static void ValidateInputLSNs(XLogRecPtr start_lsn, XLogRecPtr *end_lsn);
+static XLogRecPtr GetCurrentLSN(void);
static XLogReaderState *InitXLogReaderState(XLogRecPtr lsn);
static XLogRecord *ReadNextXLogRecord(XLogReaderState *xlogreader);
static void GetWALRecordInfo(XLogReaderState *record, Datum *values,
bool *nulls, uint32 ncols);
-static XLogRecPtr ValidateInputLSNs(bool till_end_of_wal,
- XLogRecPtr start_lsn, XLogRecPtr end_lsn);
-static void GetWALRecordsInfo(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
+static void GetWALRecordsInfo(FunctionCallInfo fcinfo,
+ XLogRecPtr start_lsn,
XLogRecPtr end_lsn);
static void GetXLogSummaryStats(XLogStats *stats, ReturnSetInfo *rsinfo,
Datum *values, bool *nulls, uint32 ncols,
@@ -55,32 +55,32 @@ static void FillXLogStatsRow(const char *name, uint64 n, uint64 total_count,
uint64 fpi_len, uint64 total_fpi_len,
uint64 tot_len, uint64 total_len,
Datum *values, bool *nulls, uint32 ncols);
-static void GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
- XLogRecPtr end_lsn, bool stats_per_record);
+static void GetWalStats(FunctionCallInfo fcinfo,
+ XLogRecPtr start_lsn,
+ XLogRecPtr end_lsn,
+ bool stats_per_record);
static void GetWALBlockInfo(FunctionCallInfo fcinfo, XLogReaderState *record);
/*
- * Check if the given LSN is in future. Also, return the LSN up to which the
- * server has WAL.
+ * Return the LSN up to which the server has WAL.
*/
-static bool
-IsFutureLSN(XLogRecPtr lsn, XLogRecPtr *curr_lsn)
+static XLogRecPtr
+GetCurrentLSN(void)
{
+ XLogRecPtr curr_lsn;
+
/*
* We determine the current LSN of the server similar to how page_read
* callback read_local_xlog_page_no_wait does.
*/
if (!RecoveryInProgress())
- *curr_lsn = GetFlushRecPtr(NULL);
+ curr_lsn = GetFlushRecPtr(NULL);
else
- *curr_lsn = GetXLogReplayRecPtr(NULL);
-
- Assert(!XLogRecPtrIsInvalid(*curr_lsn));
+ curr_lsn = GetXLogReplayRecPtr(NULL);
- if (lsn >= *curr_lsn)
- return true;
+ Assert(!XLogRecPtrIsInvalid(curr_lsn));
- return false;
+ return curr_lsn;
}
/*
@@ -354,23 +354,17 @@ GetWALBlockInfo(FunctionCallInfo fcinfo, XLogReaderState *record)
* their relation information, and the data saved in each block associated
* to a record. Decompression is applied to the full page images, if
* necessary.
- *
- * This function emits an error if a future start or end WAL LSN i.e. WAL LSN
- * the database system doesn't know about is specified.
*/
Datum
pg_get_wal_block_info(PG_FUNCTION_ARGS)
{
- XLogRecPtr start_lsn;
- XLogRecPtr end_lsn;
+ XLogRecPtr start_lsn = PG_GETARG_LSN(0);
+ XLogRecPtr end_lsn = PG_GETARG_LSN(1);
XLogReaderState *xlogreader;
MemoryContext old_cxt;
MemoryContext tmp_cxt;
- start_lsn = PG_GETARG_LSN(0);
- end_lsn = PG_GETARG_LSN(1);
-
- end_lsn = ValidateInputLSNs(false, start_lsn, end_lsn);
+ ValidateInputLSNs(start_lsn, &end_lsn);
InitMaterializedSRF(fcinfo, 0);
@@ -404,9 +398,6 @@ pg_get_wal_block_info(PG_FUNCTION_ARGS)
/*
* Get WAL record info.
- *
- * This function emits an error if a future WAL LSN i.e. WAL LSN the database
- * system doesn't know about is specified.
*/
Datum
pg_get_wal_record_info(PG_FUNCTION_ARGS)
@@ -422,20 +413,14 @@ pg_get_wal_record_info(PG_FUNCTION_ARGS)
HeapTuple tuple;
lsn = PG_GETARG_LSN(0);
+ curr_lsn = GetCurrentLSN();
- if (IsFutureLSN(lsn, &curr_lsn))
- {
- /*
- * GetFlushRecPtr or GetXLogReplayRecPtr gives "end+1" LSN of the last
- * record flushed or replayed respectively. But let's use the LSN up
- * to "end" in user facing message.
- */
+ if (lsn > curr_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("cannot accept future input LSN"),
- errdetail("Last known WAL LSN on the database system is at %X/%X.",
+ errmsg("WAL input LSN must be less than current LSN"),
+ errdetail("Current WAL LSN on the database system is at %X/%X.",
LSN_FORMAT_ARGS(curr_lsn))));
- }
/* Build a tuple descriptor for our result type. */
if (get_call_result_type(fcinfo, NULL, &tupdesc) != TYPEFUNC_COMPOSITE)
@@ -462,44 +447,30 @@ pg_get_wal_record_info(PG_FUNCTION_ARGS)
}
/*
- * Validate the input LSNs and compute end LSN for till_end_of_wal versions.
+ * Validate start and end LSNs coming from the function inputs.
+ *
+ * If end_lsn is found to be higher than the current LSN reported by the
+ * cluster, use the current LSN as the upper bound.
*/
-static XLogRecPtr
-ValidateInputLSNs(bool till_end_of_wal, XLogRecPtr start_lsn,
- XLogRecPtr end_lsn)
+static void
+ValidateInputLSNs(XLogRecPtr start_lsn, XLogRecPtr *end_lsn)
{
- XLogRecPtr curr_lsn;
+ XLogRecPtr curr_lsn = GetCurrentLSN();
- if (IsFutureLSN(start_lsn, &curr_lsn))
- {
- /*
- * GetFlushRecPtr or GetXLogReplayRecPtr gives "end+1" LSN of the last
- * record flushed or replayed respectively. But let's use the LSN up
- * to "end" in user facing message.
- */
+ if (start_lsn > curr_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("cannot accept future start LSN"),
- errdetail("Last known WAL LSN on the database system is at %X/%X.",
+ errmsg("WAL start LSN must be less than current LSN"),
+ errdetail("Current WAL LSN on the database system is at %X/%X.",
LSN_FORMAT_ARGS(curr_lsn))));
- }
- if (till_end_of_wal)
- end_lsn = curr_lsn;
-
- if (end_lsn > curr_lsn)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("cannot accept future end LSN"),
- errdetail("Last known WAL LSN on the database system is at %X/%X.",
- LSN_FORMAT_ARGS(curr_lsn))));
-
- if (start_lsn >= end_lsn)
+ if (start_lsn > *end_lsn)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("WAL start LSN must be less than end LSN")));
- return end_lsn;
+ if (*end_lsn > curr_lsn)
+ *end_lsn = curr_lsn;
}
/*
@@ -517,6 +488,8 @@ GetWALRecordsInfo(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
MemoryContext old_cxt;
MemoryContext tmp_cxt;
+ Assert(start_lsn <= end_lsn);
+
InitMaterializedSRF(fcinfo, 0);
xlogreader = InitXLogReaderState(start_lsn);
@@ -553,42 +526,14 @@ GetWALRecordsInfo(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
/*
* Get info and data of all WAL records between start LSN and end LSN.
- *
- * This function emits an error if a future start or end WAL LSN i.e. WAL LSN
- * the database system doesn't know about is specified.
*/
Datum
pg_get_wal_records_info(PG_FUNCTION_ARGS)
{
- XLogRecPtr start_lsn;
- XLogRecPtr end_lsn;
-
- start_lsn = PG_GETARG_LSN(0);
- end_lsn = PG_GETARG_LSN(1);
-
- end_lsn = ValidateInputLSNs(false, start_lsn, end_lsn);
-
- GetWALRecordsInfo(fcinfo, start_lsn, end_lsn);
-
- PG_RETURN_VOID();
-}
-
-/*
- * Get info and data of all WAL records from start LSN till end of WAL.
- *
- * This function emits an error if a future start i.e. WAL LSN the database
- * system doesn't know about is specified.
- */
-Datum
-pg_get_wal_records_info_till_end_of_wal(PG_FUNCTION_ARGS)
-{
- XLogRecPtr start_lsn;
- XLogRecPtr end_lsn = InvalidXLogRecPtr;
-
- start_lsn = PG_GETARG_LSN(0);
-
- end_lsn = ValidateInputLSNs(true, start_lsn, end_lsn);
+ XLogRecPtr start_lsn = PG_GETARG_LSN(0);
+ XLogRecPtr end_lsn = PG_GETARG_LSN(1);
+ ValidateInputLSNs(start_lsn, &end_lsn);
GetWALRecordsInfo(fcinfo, start_lsn, end_lsn);
PG_RETURN_VOID();
@@ -648,13 +593,13 @@ GetXLogSummaryStats(XLogStats *stats, ReturnSetInfo *rsinfo,
Datum *values, bool *nulls, uint32 ncols,
bool stats_per_record)
{
- MemoryContext old_cxt;
- MemoryContext tmp_cxt;
- uint64 total_count = 0;
- uint64 total_rec_len = 0;
- uint64 total_fpi_len = 0;
- uint64 total_len = 0;
- int ri;
+ MemoryContext old_cxt;
+ MemoryContext tmp_cxt;
+ uint64 total_count = 0;
+ uint64 total_rec_len = 0;
+ uint64 total_fpi_len = 0;
+ uint64 total_len = 0;
+ int ri;
/*
* Each row shows its percentages of the total, so make a first pass to
@@ -757,8 +702,8 @@ GetXLogSummaryStats(XLogStats *stats, ReturnSetInfo *rsinfo,
* Get WAL stats between start LSN and end LSN.
*/
static void
-GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
- XLogRecPtr end_lsn, bool stats_per_record)
+GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn, XLogRecPtr end_lsn,
+ bool stats_per_record)
{
#define PG_GET_WAL_STATS_COLS 9
XLogReaderState *xlogreader;
@@ -767,6 +712,8 @@ GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
Datum values[PG_GET_WAL_STATS_COLS] = {0};
bool nulls[PG_GET_WAL_STATS_COLS] = {0};
+ Assert(start_lsn <= end_lsn);
+
InitMaterializedSRF(fcinfo, 0);
xlogreader = InitXLogReaderState(start_lsn);
@@ -791,45 +738,55 @@ GetWalStats(FunctionCallInfo fcinfo, XLogRecPtr start_lsn,
/*
* Get stats of all WAL records between start LSN and end LSN.
- *
- * This function emits an error if a future start or end WAL LSN i.e. WAL LSN
- * the database system doesn't know about is specified.
*/
Datum
pg_get_wal_stats(PG_FUNCTION_ARGS)
{
- XLogRecPtr start_lsn;
- XLogRecPtr end_lsn;
- bool stats_per_record;
-
- start_lsn = PG_GETARG_LSN(0);
- end_lsn = PG_GETARG_LSN(1);
- stats_per_record = PG_GETARG_BOOL(2);
-
- end_lsn = ValidateInputLSNs(false, start_lsn, end_lsn);
+ XLogRecPtr start_lsn = PG_GETARG_LSN(0);
+ XLogRecPtr end_lsn = PG_GETARG_LSN(1);
+ bool stats_per_record = PG_GETARG_BOOL(2);
+ ValidateInputLSNs(start_lsn, &end_lsn);
GetWalStats(fcinfo, start_lsn, end_lsn, stats_per_record);
PG_RETURN_VOID();
}
/*
- * Get stats of all WAL records from start LSN till end of WAL.
- *
- * This function emits an error if a future start i.e. WAL LSN the database
- * system doesn't know about is specified.
+ * The following functions have been removed in newer versions in 1.1, but
+ * they are kept around for compatibility.
*/
Datum
-pg_get_wal_stats_till_end_of_wal(PG_FUNCTION_ARGS)
+pg_get_wal_records_info_till_end_of_wal(PG_FUNCTION_ARGS)
{
- XLogRecPtr start_lsn;
- XLogRecPtr end_lsn = InvalidXLogRecPtr;
- bool stats_per_record;
+ XLogRecPtr start_lsn = PG_GETARG_LSN(0);
+ XLogRecPtr end_lsn = GetCurrentLSN();
+
+ if (start_lsn > end_lsn)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("WAL start LSN must be less than current LSN"),
+ errdetail("Current WAL LSN on the database system is at %X/%X.",
+ LSN_FORMAT_ARGS(end_lsn))));
- start_lsn = PG_GETARG_LSN(0);
- stats_per_record = PG_GETARG_BOOL(1);
+ GetWALRecordsInfo(fcinfo, start_lsn, end_lsn);
- end_lsn = ValidateInputLSNs(true, start_lsn, end_lsn);
+ PG_RETURN_VOID();
+}
+
+Datum
+pg_get_wal_stats_till_end_of_wal(PG_FUNCTION_ARGS)
+{
+ XLogRecPtr start_lsn = PG_GETARG_LSN(0);
+ XLogRecPtr end_lsn = GetCurrentLSN();
+ bool stats_per_record = PG_GETARG_BOOL(1);
+
+ if (start_lsn > end_lsn)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("WAL start LSN must be less than current LSN"),
+ errdetail("Current WAL LSN on the database system is at %X/%X.",
+ LSN_FORMAT_ARGS(end_lsn))));
GetWalStats(fcinfo, start_lsn, end_lsn, stats_per_record);
diff --git a/contrib/pg_walinspect/sql/oldextversions.sql b/contrib/pg_walinspect/sql/oldextversions.sql
index f448094add..bd4523d0e6 100644
--- a/contrib/pg_walinspect/sql/oldextversions.sql
+++ b/contrib/pg_walinspect/sql/oldextversions.sql
@@ -2,13 +2,32 @@
CREATE EXTENSION pg_walinspect WITH VERSION '1.0';
+-- Mask DETAIL messages as these could refer to current LSN positions
+\set VERBOSITY terse
+
-- List what version 1.0 contains
\dx+ pg_walinspect
+-- Make sure checkpoints don't interfere with the test
+SELECT 'init' FROM pg_create_physical_replication_slot('regress_pg_walinspect_slot_old_ext_ver', true, false);
+
+CREATE TABLE sample_tbl(col1 int, col2 int);
+SELECT pg_current_wal_lsn() AS wal_lsn1 \gset
+INSERT INTO sample_tbl SELECT * FROM generate_series(1, 2);
+
+-- Tests for the past functions
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info_till_end_of_wal(:'wal_lsn1');
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_stats_till_end_of_wal(:'wal_lsn1');
+
+-- Failures with start LSNs
+SELECT * FROM pg_get_wal_records_info_till_end_of_wal('FFFFFFFF/FFFFFFFF');
+SELECT * FROM pg_get_wal_stats_till_end_of_wal('FFFFFFFF/FFFFFFFF');
+
-- Move to new version 1.1
ALTER EXTENSION pg_walinspect UPDATE TO '1.1';
-- List what version 1.1 contains
\dx+ pg_walinspect
+-- Clean up
DROP EXTENSION pg_walinspect;
diff --git a/contrib/pg_walinspect/sql/pg_walinspect.sql b/contrib/pg_walinspect/sql/pg_walinspect.sql
index 766ed6b87d..e9c2a4d1ee 100644
--- a/contrib/pg_walinspect/sql/pg_walinspect.sql
+++ b/contrib/pg_walinspect/sql/pg_walinspect.sql
@@ -8,7 +8,7 @@ SELECT 'init' FROM pg_create_physical_replication_slot('regress_pg_walinspect_sl
CREATE TABLE sample_tbl(col1 int, col2 int);
--- Save some LSNs for comparisons
+-- Save some LSNs for comparisons.
SELECT pg_current_wal_lsn() AS wal_lsn1 \gset
INSERT INTO sample_tbl SELECT * FROM generate_series(1, 2);
SELECT pg_current_wal_lsn() AS wal_lsn2 \gset
@@ -32,28 +32,26 @@ SELECT * FROM pg_get_wal_stats(:'wal_lsn2', :'wal_lsn1');
SELECT * FROM pg_get_wal_block_info(:'wal_lsn2', :'wal_lsn1');
-- LSNs with the highest value possible.
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_record_info('FFFFFFFF/FFFFFFFF');
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info_till_end_of_wal('FFFFFFFF/FFFFFFFF');
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats_till_end_of_wal('FFFFFFFF/FFFFFFFF');
--- failures with end LSNs
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_block_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
--- failures with start LSNs
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_block_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
+SELECT * FROM pg_get_wal_record_info('FFFFFFFF/FFFFFFFF');
+
+-- Failures with start LSNs.
+SELECT * FROM pg_get_wal_records_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
+SELECT * FROM pg_get_wal_stats('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
+SELECT * FROM pg_get_wal_block_info('FFFFFFFF/FFFFFFFE', 'FFFFFFFF/FFFFFFFF');
+
+-- Success with end LSNs.
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_stats(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_block_info(:'wal_lsn1', 'FFFFFFFF/FFFFFFFF');
-- ===================================================================
-- Tests for all function executions
-- ===================================================================
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_record_info(:'wal_lsn1');
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info_till_end_of_wal(:'wal_lsn1');
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats_till_end_of_wal(:'wal_lsn1');
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', :'wal_lsn2');
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_stats(:'wal_lsn1', :'wal_lsn2');
-SELECT COUNT(*) >= 0 AS ok FROM pg_get_wal_block_info(:'wal_lsn1', :'wal_lsn2');
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_record_info(:'wal_lsn1');
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', :'wal_lsn2');
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_stats(:'wal_lsn1', :'wal_lsn2');
+SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_block_info(:'wal_lsn1', :'wal_lsn2');
-- ===================================================================
-- Test for filtering out WAL records of a particular table
@@ -76,7 +74,7 @@ SELECT COUNT(*) >= 1 AS ok FROM pg_get_wal_records_info(:'wal_lsn1', :'wal_lsn2'
-- Tests to get block information from WAL record
-- ===================================================================
--- Update table to generate some block data
+-- Update table to generate some block data.
SELECT pg_current_wal_lsn() AS wal_lsn3 \gset
UPDATE sample_tbl SET col1 = col1 + 1 WHERE col1 = 1;
SELECT pg_current_wal_lsn() AS wal_lsn4 \gset
@@ -107,9 +105,9 @@ SELECT has_function_privilege('regress_pg_walinspect',
SELECT has_function_privilege('regress_pg_walinspect',
'pg_get_wal_block_info(pg_lsn, pg_lsn) ', 'EXECUTE'); -- no
--- Functions accessible by users with role pg_read_server_files
-
+-- Functions accessible by users with role pg_read_server_files.
GRANT pg_read_server_files TO regress_pg_walinspect;
+
SELECT has_function_privilege('regress_pg_walinspect',
'pg_get_wal_record_info(pg_lsn)', 'EXECUTE'); -- yes
SELECT has_function_privilege('regress_pg_walinspect',
@@ -154,8 +152,5 @@ REVOKE EXECUTE ON FUNCTION pg_get_wal_block_info(pg_lsn, pg_lsn)
-- ===================================================================
DROP ROLE regress_pg_walinspect;
-
-SELECT pg_drop_replication_slot('regress_pg_walinspect_slot');
-
DROP TABLE sample_tbl;
DROP EXTENSION pg_walinspect;
diff --git a/doc/src/sgml/pgwalinspect.sgml b/doc/src/sgml/pgwalinspect.sgml
index 3b19863dce..15e1f6d5ef 100644
--- a/doc/src/sgml/pgwalinspect.sgml
+++ b/doc/src/sgml/pgwalinspect.sgml
@@ -94,9 +94,11 @@ block_ref | blkref #0: rel 1663/5/60221 fork main blk 2
<para>
Gets information of all the valid WAL records between
<replaceable>start_lsn</replaceable> and <replaceable>end_lsn</replaceable>.
- Returns one row per WAL record. If <replaceable>start_lsn</replaceable>
- or <replaceable>end_lsn</replaceable> are not yet available, the
- function will raise an error. For example:
+ Returns one row per WAL record. If a future <replaceable>end_lsn</replaceable>
+ (i.e. the LSN server doesn't know about) is specified, it returns
+ informaton till end of WAL. The function raises an error if
+ <replaceable>start_lsn</replaceable> is not available. For example, usage
+ of the function is as follows:
<screen>
postgres=# SELECT * FROM pg_get_wal_records_info('0/1E913618', '0/1E913740') LIMIT 1;
-[ RECORD 1 ]----+--------------------------------------------------------------
@@ -116,23 +118,6 @@ block_ref |
</listitem>
</varlistentry>
- <varlistentry id="pgwalinspect-funcs-pg-get-wal-records-info-till-end-of-wal">
- <term>
- <function>
- pg_get_wal_records_info_till_end_of_wal(start_lsn pg_lsn)
- returns setof record
- </function>
- </term>
-
- <listitem>
- <para>
- This function is the same as <function>pg_get_wal_records_info()</function>,
- except that it gets information of all the valid WAL records from
- <replaceable>start_lsn</replaceable> till the end of WAL.
- </para>
- </listitem>
- </varlistentry>
-
<varlistentry id="pgwalinspect-funcs-pg-get-wal-stats">
<term>
<function>
@@ -148,10 +133,11 @@ block_ref |
<replaceable>end_lsn</replaceable>. By default, it returns one row per
<replaceable>resource_manager</replaceable> type. When
<replaceable>per_record</replaceable> is set to <literal>true</literal>,
- it returns one row per <replaceable>record_type</replaceable>.
- If <replaceable>start_lsn</replaceable>
- or <replaceable>end_lsn</replaceable> are not yet available, the
- function will raise an error. For example:
+ it returns one row per <replaceable>record_type</replaceable>. If a
+ future <replaceable>end_lsn</replaceable> (i.e. the LSN server doesn't
+ know about) is specified, it returns statistics till end of WAL. An error
+ is raised if <replaceable>start_lsn</replaceable> is not available. For
+ example, usage of the function is as follows:
<screen>
postgres=# SELECT * FROM pg_get_wal_stats('0/1E847D00', '0/1E84F500')
WHERE count > 0 LIMIT 1 AND
@@ -171,23 +157,6 @@ combined_size_percentage | 2.8634072910530795
</listitem>
</varlistentry>
- <varlistentry id="pgwalinspect-funcs-pg-get-wal-stats-till-end-of-wal">
- <term>
- <function>
- pg_get_wal_stats_till_end_of_wal(start_lsn pg_lsn, per_record boolean DEFAULT false)
- returns setof record
- </function>
- </term>
-
- <listitem>
- <para>
- This function is the same as <function>pg_get_wal_stats()</function>,
- except that it gets statistics of all the valid WAL records from
- <replaceable>start_lsn</replaceable> till end of WAL.
- </para>
- </listitem>
- </varlistentry>
-
<varlistentry>
<term>
<function>pg_get_wal_block_info(start_lsn pg_lsn, end_lsn pg_lsn) returns setof record</function>
@@ -202,9 +171,10 @@ combined_size_percentage | 2.8634072910530795
and their information associated with all the valid WAL records between
<replaceable>start_lsn</replaceable> and
<replaceable>end_lsn</replaceable>. Returns one row per block registered
- in a WAL record. If <replaceable>start_lsn</replaceable> or
- <replaceable>end_lsn</replaceable> are not yet available, the function
- will raise an error. For example:
+ in a WAL record. If a future <replaceable>end_lsn</replaceable> (i.e. the
+ LSN server doesn't know about) is specified, it returns statistics till
+ end of WAL. An error is raised if <replaceable>start_lsn</replaceable> is
+ not available. For example, usage of the function is as follows:
<screen>
postgres=# SELECT lsn, blockid, reltablespace, reldatabase, relfilenode,
relblocknumber, forkname,
--
2.34.1
