XLOG_PARAMETER_CHANGE handling of wal_log_hints

Started by Petr Jelinekabout 11 years ago4 messages
#1Petr Jelinek
petr@2ndquadrant.com
1 attachment(s)

Hi,

when I was fixing how commit_ts handles the XLOG_PARAMETER_CHANGE I
noticed that for wal_log_hints we assign the value in ControFile to
current value instead of value that comes from WAL.

ISTM it has just information value so it should not have any practical
impact, but it looks like a bug anyway so here is simple patch to change
that.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

wal_log_hints_controlfile-fix.patchtext/x-diff; name=wal_log_hints_controlfile-fix.patchDownload
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 5cc7e47..be67da4 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8967,7 +8967,7 @@ xlog_redo(XLogReaderState *record)
 		ControlFile->max_prepared_xacts = xlrec.max_prepared_xacts;
 		ControlFile->max_locks_per_xact = xlrec.max_locks_per_xact;
 		ControlFile->wal_level = xlrec.wal_level;
-		ControlFile->wal_log_hints = wal_log_hints;
+		ControlFile->wal_log_hints = xlrec.wal_log_hints;
 		ControlFile->track_commit_timestamp = track_commit_timestamp;
 
 		/*
#2Michael Paquier
michael.paquier@gmail.com
In reply to: Petr Jelinek (#1)
Re: XLOG_PARAMETER_CHANGE handling of wal_log_hints

On Wed, Jan 7, 2015 at 4:24 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:

Hi,

when I was fixing how commit_ts handles the XLOG_PARAMETER_CHANGE I noticed
that for wal_log_hints we assign the value in ControFile to current value
instead of value that comes from WAL.

ISTM it has just information value so it should not have any practical
impact, but it looks like a bug anyway so here is simple patch to change
that.

Right. That's something that should get fixed in 9.4 as well..

ControlFile->track_commit_timestamp = track_commit_timestamp;
The new value of track_commit_timestamp should be taken as well from
xlrec on master btw.
--
Michael

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Petr Jelinek
petr@2ndquadrant.com
In reply to: Michael Paquier (#2)
Re: XLOG_PARAMETER_CHANGE handling of wal_log_hints

On 07/01/15 00:59, Michael Paquier wrote:

On Wed, Jan 7, 2015 at 4:24 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:

Hi,

when I was fixing how commit_ts handles the XLOG_PARAMETER_CHANGE I noticed
that for wal_log_hints we assign the value in ControFile to current value
instead of value that comes from WAL.

ISTM it has just information value so it should not have any practical
impact, but it looks like a bug anyway so here is simple patch to change
that.

Right. That's something that should get fixed in 9.4 as well..

ControlFile->track_commit_timestamp = track_commit_timestamp;
The new value of track_commit_timestamp should be taken as well from
xlrec on master btw.

That's part of the larger fix for CommitTs that I sent separately in
response to the bug report from Fujii - there is much more to be done
there for CommitTs than just this.

--
Petr Jelinek http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Heikki Linnakangas
hlinnakangas@vmware.com
In reply to: Petr Jelinek (#3)
Re: XLOG_PARAMETER_CHANGE handling of wal_log_hints

On 01/07/2015 11:53 AM, Petr Jelinek wrote:

On 07/01/15 00:59, Michael Paquier wrote:

On Wed, Jan 7, 2015 at 4:24 AM, Petr Jelinek <petr@2ndquadrant.com> wrote:

Hi,

when I was fixing how commit_ts handles the XLOG_PARAMETER_CHANGE I noticed
that for wal_log_hints we assign the value in ControFile to current value
instead of value that comes from WAL.

ISTM it has just information value so it should not have any practical
impact, but it looks like a bug anyway so here is simple patch to change
that.

Right. That's something that should get fixed in 9.4 as well..

ControlFile->track_commit_timestamp = track_commit_timestamp;
The new value of track_commit_timestamp should be taken as well from
xlrec on master btw.

That's part of the larger fix for CommitTs that I sent separately in
response to the bug report from Fujii - there is much more to be done
there for CommitTs than just this.

Pushed this part now. I'll let you and Fujii handle that larger fix. Thanks!

- Heikki

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers