pgsql: xlog.c: Remove global variables ReadRecPtr and EndRecPtr.
xlog.c: Remove global variables ReadRecPtr and EndRecPtr.
In most places, the variables necessarily store the same value as the
eponymous members of the XLogReaderState that we use during WAL
replay, because ReadRecord() assigns the values from the structure
members to the global variables just after XLogReadRecord() returns.
However, XLogBeginRead() adjusts the structure members but not the
global variables, so after XLogBeginRead() and before the completion
of XLogReadRecord() the values can differ. Otherwise, they must be
identical. According to my analysis, the only place where either
variable is referenced at a point where it might not have the same
value as the structure member is the refrence to EndRecPtr within
XLogPageRead.
Therefore, at every other place where we are using the global
variable, we can just switch to using the structure member instead,
and remove the global variable. However, we can, and in fact should,
do this in XLogPageRead() as well, because at that point in the code,
the global variable will actually store the start of the record we
want to read - either because it's where the last WAL record ended, or
because the read position has been changed using XLogBeginRead since
the last record was read. The structure member, on the other hand,
will already have been updated to point to the end of the record we
just read. Elsewhere, the latter is what we use as an argument to
emode_for_corrupt_record(), so we should do the same here.
This part of the patch is perhaps a bug fix, but I don't think it has
any important consequences, so no back-patch. The point here is just
to continue to whittle down the entirely excessive use of global
variables in xlog.c.
Discussion: /messages/by-id/CA+Tgmoao96EuNeSPd+hspRKcsCddu=b1h-QNRuKfY8VmfNQdfg@mail.gmail.com
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/d2ddfa681db27a138acb63c8defa8cc6fa588922
Modified Files
--------------
src/backend/access/transam/xlog.c | 53 ++++++++++++++++++---------------------
1 file changed, 24 insertions(+), 29 deletions(-)
On Wed, Nov 24, 2021 at 11:28 AM Robert Haas <rhaas@postgresql.org> wrote:
xlog.c: Remove global variables ReadRecPtr and EndRecPtr.
https://git.postgresql.org/pg/commitdiff/d2ddfa681db27a138acb63c8defa8cc6fa588922
There is a buildfarm failure on lapwing which looks likely to be
attributable to this commit, but I don't understand what has gone
wrong exactly. The error message that is reported is:
[17:07:19] t/025_stuck_on_old_timeline.pl ....... ok 6048 ms
# poll_query_until timed out executing this query:
# SELECT '0/201E8E0'::pg_lsn <= pg_last_wal_replay_lsn()
# expecting this output:
# t
# last actual query output:
#
# with stderr:
# psql: error: connection to server on socket
"/tmp/SrZekgRh7K/.s.PGSQL.57959" failed: No such file or directory
# Is the server running locally and accepting connections on that socket?
This output confused me for a while because I thought the failure was
in test 025, but I should have realized that the problem must be in
026, since 025 finished with "ok". Anyway, in
026_overwrite_contrecord_standby.log there's this:
2021-11-24 17:07:41.388 UTC [1473:1] LOG: started streaming WAL from
primary at 0/2000000 on timeline 1
2021-11-24 17:07:41.428 UTC [1449:6] FATAL: mismatching overwritten
LSN 0/1FFE014 -> 0/1FFE000
2021-11-24 17:07:41.428 UTC [1449:7] CONTEXT: WAL redo at 0/2000024
for XLOG/OVERWRITE_CONTRECORD: lsn 0/1FFE014; time 2021-11-24
17:07:41.127049+00
Beyond the fact that something bad has happened, I'm not sure what
that is trying to tell me.
--
Robert Haas
EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
2021-11-24 17:07:41.428 UTC [1449:6] FATAL: mismatching overwritten
LSN 0/1FFE014 -> 0/1FFE000
2021-11-24 17:07:41.428 UTC [1449:7] CONTEXT: WAL redo at 0/2000024
for XLOG/OVERWRITE_CONTRECORD: lsn 0/1FFE014; time 2021-11-24
17:07:41.127049+00
Beyond the fact that something bad has happened, I'm not sure what
that is trying to tell me.
Pre-existing issue:
/messages/by-id/45597.1637694259@sss.pgh.pa.us
regards, tom lane
On 2021-Nov-24, Robert Haas wrote:
On Wed, Nov 24, 2021 at 11:28 AM Robert Haas <rhaas@postgresql.org> wrote:
xlog.c: Remove global variables ReadRecPtr and EndRecPtr.
https://git.postgresql.org/pg/commitdiff/d2ddfa681db27a138acb63c8defa8cc6fa588922There is a buildfarm failure on lapwing which looks likely to be
attributable to this commit, but I don't understand what has gone
wrong exactly. The error message that is reported is:
No, this is an unrelated problem; Tom reported this yesterday also:
/messages/by-id/45597.1637694259@sss.pgh.pa.us
--
Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/
"La espina, desde que nace, ya pincha" (Proverbio africano)
On Wed, Nov 24, 2021 at 1:51 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
There is a buildfarm failure on lapwing which looks likely to be
attributable to this commit, but I don't understand what has gone
wrong exactly. The error message that is reported is:No, this is an unrelated problem; Tom reported this yesterday also:
/messages/by-id/45597.1637694259@sss.pgh.pa.us
Oh, OK. I couldn't see exactly what that could have to do with my
commit, but it happened right afterwards so I was hesitant to call it
a coincidence.
Thanks,
--
Robert Haas
EDB: http://www.enterprisedb.com
Hi,
On 2021-11-24 16:27:58 +0000, Robert Haas wrote:
xlog.c: Remove global variables ReadRecPtr and EndRecPtr.
This fails when building with WAL_DEBUG. There's remaining Read/EndRecPtr
references in #ifdef WAL_DEBUG sections...
Greetings,
Andres Freund
Hi,
On 2021-11-24 15:12:06 -0800, Andres Freund wrote:
This fails when building with WAL_DEBUG. There's remaining Read/EndRecPtr
references in #ifdef WAL_DEBUG sections...
Pushed the obvious fix for that. Somehow thought I'd seen more compile failure
than the one WAL_DEBUG...
On Wed, Nov 24, 2021 at 8:01 PM Andres Freund <andres@anarazel.de> wrote:
Pushed the obvious fix for that. Somehow thought I'd seen more compile failure
than the one WAL_DEBUG...
Hmm, thanks. I guess i put too much trust in the compiler.
--
Robert Haas
EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Nov 24, 2021 at 8:01 PM Andres Freund <andres@anarazel.de> wrote:
Pushed the obvious fix for that. Somehow thought I'd seen more compile failure
than the one WAL_DEBUG...
Hmm, thanks. I guess i put too much trust in the compiler.
My approach to such patches is always "in grep we trust, all others
pay cash". Even without #ifdef issues, you are highly likely to
miss comments that need to be updated.
regards, tom lane
On Thu, Nov 25, 2021 at 11:02 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Wed, Nov 24, 2021 at 8:01 PM Andres Freund <andres@anarazel.de> wrote:
Pushed the obvious fix for that. Somehow thought I'd seen more compile failure
than the one WAL_DEBUG...Hmm, thanks. I guess i put too much trust in the compiler.
My approach to such patches is always "in grep we trust, all others
pay cash". Even without #ifdef issues, you are highly likely to
miss comments that need to be updated.
Right. I didn't rely completely on grep and did spend a lot of time
looking through the code. But sometimes my eyes glaze over and I get
sloppy after too much time working on one thing, and this seems to
have been one of those times.
Fortunately, it seems to have been only a minor oversight.
--
Robert Haas
EDB: http://www.enterprisedb.com
Hi,
On November 26, 2021 5:53:31 PM PST, Andres Freund <andres@anarazel.de> wrote:
Hi,
On November 26, 2021 7:24:15 AM PST, Robert Haas <robertmhaas@gmail.com> wrote:
Fortunately, it seems to have been only a minor oversight.
Agreed.
I wonder if we should turn some of these ifdefs into something boiling down to if (0) { ifdef body}... No need to make it impossible for the compiler to help us in most of these cases...
Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.