pg_walinspect: fix LSN validation messages and empty range handling - Mailing list pgsql-hackers

From Chao Li
Subject pg_walinspect: fix LSN validation messages and empty range handling
Date
Msg-id 7B8F5F12-98A5-4618-867A-904EA1334FD4@gmail.com
Whole thread
Responses Re: pg_walinspect: fix LSN validation messages and empty range handling
Re: pg_walinspect: fix LSN validation messages and empty range handling
List pgsql-hackers
Hi,

While working on patch [1], I noticed two small issues with pg_walinspect.

1. In pg_get_wal_record_info() as well as a few other functions, there are checks like:
```
    if (lsn > curr_lsn)
        ereport(ERROR,
                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                 errmsg("WAL input LSN must be less than current LSN"),
                 errdetail("Current WAL LSN on the database system is at %X/%08X.",
                           LSN_FORMAT_ARGS(curr_lsn))));
```

The check itself uses a > comparison, so equality is accepted by this validation check. However, the error message says
"mustbe less than", which implies that equality is not accepted. Thus, the check and the error message are
inconsistent.

Commit 5c1b6628075a changed the check from >= to > and changed the error message from "cannot accept future input LSN"
to"WAL input LSN must be less than current LSN". This seems to have been an oversight. 

The error message can be changed to say "must be less than or equal to", matching the actual validation condition.

2. pg_get_wal_records_info() accepts an end_lsn equal to start_lsn, but the same fixed range can produce different
results.For example: 
```
evantest=# select pg_current_wal_flush_lsn();
 pg_current_wal_flush_lsn
--------------------------
 0/01D61428
(1 row)
evantest=# SELECT * FROM pg_get_wal_records_info('0/01D61428', '0/01D61428');
ERROR:  could not find a valid record after 0/01D61428

evantest=# checkpoint;
CHECKPOINT
evantest=# SELECT * FROM pg_get_wal_records_info('0/01D61428', '0/01D61428');
 start_lsn | end_lsn | prev_lsn | xid | resource_manager | record_type | record_length | main_data_length | fpi_length
|description | block_ref 

-----------+---------+----------+-----+------------------+-------------+---------------+------------------+------------+-------------+-----------
(0 rows)
```

When I passed the current flushed LSN to pg_get_wal_records_info() as both start_lsn and end_lsn, it raised an error
becauseno record was available at or after that LSN. After I ran CHECKPOINT to generate more WAL records, the same
queryreturned zero rows. 

Thus, the same fixed range can either raise an error or return zero rows depending on whether WAL exists after end_lsn,
eventhough WAL after end_lsn cannot belong to the requested range. This may confuse users. 

To fix, I think an empty LSN range cannot contain a complete WAL record, so it can be handled without initializing a
WALreader. So that, the record and block information functions can return zero rows, while pg_get_wal_stats() can
preserveits zero-valued aggregate output. 

[1] https://www.postgresql.org/message-id/80E9F0AD-CFC5-4BE5-81DE-D8FE35E10A1C%40gmail.com

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/





Attachment

pgsql-hackers by date:

Previous
From: "Wei Sun"
Date:
Subject: Re: Severe performance degradation with concurrent updates due to excessive EvalPlanQual (EPQ) re‑evaluation
Next
From: Bharath Rupireddy
Date:
Subject: Re: Add a hook for handling logical decoding messages on subscribers.