[PATCH] Change "checkpoint starting" message to use "wal"

Started by Christoph Bergabout 7 years ago12 messages
#1Christoph Berg
christoph.berg@credativ.de
1 attachment(s)

A customer was complaining that the "checkpoint starting: xlog"
message was not included in the grand PG10 rename of user-visible
bits. The attached patch fixes that.

Christoph
--
Senior Berater, Tel.: +49 2166 9901 187
credativ GmbH, HRB M�nchengladbach 12080, USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 M�nchengladbach
Gesch�ftsf�hrung: Dr. Michael Meskes, J�rg Folz, Sascha Heuer
Unser Umgang mit personenbezogenen Daten unterliegt
folgenden Bestimmungen: https://www.credativ.de/datenschutz

Attachments:

0001-Change-checkpoint-starting-message-to-use-wal.patchtext/x-diff; charset=us-asciiDownload
From 231dab5c98b295756e119db837d5f19e5c8f30eb Mon Sep 17 00:00:00 2001
From: Christoph Berg <christoph.berg@credativ.de>
Date: Thu, 29 Nov 2018 09:44:16 +0100
Subject: [PATCH] Change "checkpoint starting" message to use "wal"

---
 src/backend/access/transam/xlog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index c80b14ed97..aace586ff9 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8369,21 +8369,21 @@ ShutdownXLOG(int code, Datum arg)
 static void
 LogCheckpointStart(int flags, bool restartpoint)
 {
 	elog(LOG, "%s starting:%s%s%s%s%s%s%s%s",
 		 restartpoint ? "restartpoint" : "checkpoint",
 		 (flags & CHECKPOINT_IS_SHUTDOWN) ? " shutdown" : "",
 		 (flags & CHECKPOINT_END_OF_RECOVERY) ? " end-of-recovery" : "",
 		 (flags & CHECKPOINT_IMMEDIATE) ? " immediate" : "",
 		 (flags & CHECKPOINT_FORCE) ? " force" : "",
 		 (flags & CHECKPOINT_WAIT) ? " wait" : "",
-		 (flags & CHECKPOINT_CAUSE_XLOG) ? " xlog" : "",
+		 (flags & CHECKPOINT_CAUSE_XLOG) ? " wal" : "",
 		 (flags & CHECKPOINT_CAUSE_TIME) ? " time" : "",
 		 (flags & CHECKPOINT_FLUSH_ALL) ? " flush-all" : "");
 }
 
 /*
  * Log end of a checkpoint.
  */
 static void
 LogCheckpointEnd(bool restartpoint)
 {
-- 
2.19.1

#2Michael Paquier
michael@paquier.xyz
In reply to: Christoph Berg (#1)
Re: [PATCH] Change "checkpoint starting" message to use "wal"

On Thu, Nov 29, 2018 at 09:47:09AM +0100, Christoph Berg wrote:

A customer was complaining that the "checkpoint starting: xlog"
message was not included in the grand PG10 rename of user-visible
bits. The attached patch fixes that.

At the same time it would make sense to rename CHECKPOINT_CAUSE_XLOG? I
would not risk back-patching such a change as that would be annoying for
tools parsing logs like pgbadger.
--
Michael

#3Christoph Berg
christoph.berg@credativ.de
In reply to: Michael Paquier (#2)
Re: [PATCH] Change "checkpoint starting" message to use "wal"

Re: Michael Paquier 2018-11-29 <20181129085902.GD9004@paquier.xyz>

On Thu, Nov 29, 2018 at 09:47:09AM +0100, Christoph Berg wrote:

A customer was complaining that the "checkpoint starting: xlog"
message was not included in the grand PG10 rename of user-visible
bits. The attached patch fixes that.

At the same time it would make sense to rename CHECKPOINT_CAUSE_XLOG? I
would not risk back-patching such a change as that would be annoying for
tools parsing logs like pgbadger.

There's hundreds of other internal uses of xlog that were not touched
either, only the user-facing parts were changed.

If this gets accepted, I'll submit a patch for pgbadger so it can be
updated before PG12 hits the field.

Christoph

#4Michael Paquier
michael@paquier.xyz
In reply to: Christoph Berg (#3)
Re: [PATCH] Change "checkpoint starting" message to use "wal"

On Thu, Nov 29, 2018 at 10:04:12AM +0100, Christoph Berg wrote:

There's hundreds of other internal uses of xlog that were not touched
either, only the user-facing parts were changed.

I have heard of them ;)
Just wondering if this one is worth renaming as the variable is
isolated. It is not a big deal to do nothing though.

If this gets accepted, I'll submit a patch for pgbadger so it can be
updated before PG12 hits the field.

Your change looks acceptable to me FWIW.
--
Michael

#5Stephen Frost
sfrost@snowman.net
In reply to: Christoph Berg (#1)
Re: [PATCH] Change "checkpoint starting" message to use "wal"

Greetings,

* Christoph Berg (christoph.berg@credativ.de) wrote:

A customer was complaining that the "checkpoint starting: xlog"
message was not included in the grand PG10 rename of user-visible
bits. The attached patch fixes that.

+1

Thanks!

Stephen

#6Robert Haas
robertmhaas@gmail.com
In reply to: Michael Paquier (#4)
Re: [PATCH] Change "checkpoint starting" message to use "wal"

On Thu, Nov 29, 2018 at 4:10 AM Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Nov 29, 2018 at 10:04:12AM +0100, Christoph Berg wrote:

There's hundreds of other internal uses of xlog that were not touched
either, only the user-facing parts were changed.

I have heard of them ;)
Just wondering if this one is worth renaming as the variable is
isolated. It is not a big deal to do nothing though.

Well, if we rename the user-visible part but not the internal part,
then they don't match, which is odd.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#7Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Robert Haas (#6)
Re: [PATCH] Change "checkpoint starting" message to use "wal"

On 2018-Nov-29, Robert Haas wrote:

On Thu, Nov 29, 2018 at 4:10 AM Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Nov 29, 2018 at 10:04:12AM +0100, Christoph Berg wrote:

There's hundreds of other internal uses of xlog that were not touched
either, only the user-facing parts were changed.

I have heard of them ;)
Just wondering if this one is worth renaming as the variable is
isolated. It is not a big deal to do nothing though.

Well, if we rename the user-visible part but not the internal part,
then they don't match, which is odd.

But we already did that when we renamed all the xlog to WAL terminology
... why do we care about it now particularly?

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#8Stephen Frost
sfrost@snowman.net
In reply to: Alvaro Herrera (#7)
Re: [PATCH] Change "checkpoint starting" message to use "wal"

Greetings,

* Alvaro Herrera (alvherre@2ndquadrant.com) wrote:

On 2018-Nov-29, Robert Haas wrote:

On Thu, Nov 29, 2018 at 4:10 AM Michael Paquier <michael@paquier.xyz> wrote:

On Thu, Nov 29, 2018 at 10:04:12AM +0100, Christoph Berg wrote:

There's hundreds of other internal uses of xlog that were not touched
either, only the user-facing parts were changed.

I have heard of them ;)
Just wondering if this one is worth renaming as the variable is
isolated. It is not a big deal to do nothing though.

Well, if we rename the user-visible part but not the internal part,
then they don't match, which is odd.

But we already did that when we renamed all the xlog to WAL terminology
... why do we care about it now particularly?

I thought the idea was that we'd adjust things in the actual code as
that code was refactored or adjusted for other reasons, to minimize the
back-patching pain. That said, in this particular case that would mean
just changing one variable when the other related ones aren't changed
and I suspect that might just be more confusing than having this
difference between the code and the user-messages.

So, at least in this instance, my feeling is that we keep the variable
as-is and just adjust the user message. When, down the road, there's a
larger refactoring or change in this part of the code, that would be the
time to change the code to refer to WAL instead of XLOG.

Thanks!

Stephen

#9Tsunakawa, Takayuki
tsunakawa.takay@jp.fujitsu.com
In reply to: Christoph Berg (#1)
RE: [PATCH] Change "checkpoint starting" message to use "wal"

From: Christoph Berg [mailto:christoph.berg@credativ.de]

A customer was complaining that the "checkpoint starting: xlog"
message was not included in the grand PG10 rename of user-visible bits.
The attached patch fixes that.

Can we make use of this chance to change elog() to ereport(), so that the two messages are translated?

Regards
Takayuki Tsunakawa

#10Stephen Frost
sfrost@snowman.net
In reply to: Tsunakawa, Takayuki (#9)
Re: [PATCH] Change "checkpoint starting" message to use "wal"

Greetings,

* Tsunakawa, Takayuki (tsunakawa.takay@jp.fujitsu.com) wrote:

From: Christoph Berg [mailto:christoph.berg@credativ.de]

A customer was complaining that the "checkpoint starting: xlog"
message was not included in the grand PG10 rename of user-visible bits.
The attached patch fixes that.

Can we make use of this chance to change elog() to ereport(), so that the two messages are translated?

+1, and we should probably look for nearby cases to fix also..

Thanks!

Stephen

#11Michael Paquier
michael@paquier.xyz
In reply to: Stephen Frost (#10)
Re: [PATCH] Change "checkpoint starting" message to use "wal"

On Thu, Nov 29, 2018 at 07:31:02PM -0500, Stephen Frost wrote:

* Tsunakawa, Takayuki (tsunakawa.takay@jp.fujitsu.com) wrote:

From: Christoph Berg [mailto:christoph.berg@credativ.de]

A customer was complaining that the "checkpoint starting: xlog"
message was not included in the grand PG10 rename of user-visible bits.
The attached patch fixes that.

Can we make use of this chance to change elog() to ereport(), so that the two messages are translated?

+1, and we should probably look for nearby cases to fix also..

Good point. +1.
--
Michael

#12Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Christoph Berg (#1)
Re: [PATCH] Change "checkpoint starting" message to use "wal"

On 29/11/2018 09:47, Christoph Berg wrote:

A customer was complaining that the "checkpoint starting: xlog"
message was not included in the grand PG10 rename of user-visible
bits. The attached patch fixes that.

committed

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services