public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 05/17] Fix partially-updated comment
7+ messages / 4 participants
[nested] [flat]
* [PATCH 05/17] Fix partially-updated comment
@ 2020-12-02 23:22 Justin Pryzby <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Justin Pryzby @ 2020-12-02 23:22 UTC (permalink / raw)
f0e44751d: implements partitioning and changed query to exclude partitioned tables
44c528810: Change the way pg_dump retrieves partitioning info (but failed to update comment)
0896ae561: Partially updated comment
---
src/bin/pg_dump/pg_dump.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 8b1e5cc2b5..1ab98a2286 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -7018,10 +7018,7 @@ getInherits(Archive *fout, int *numInherits)
int i_inhrelid;
int i_inhparent;
- /*
- * Find all the inheritance information, excluding implicit inheritance
- * via partitioning.
- */
+ /* find all the inheritance information */
appendPQExpBufferStr(query, "SELECT inhrelid, inhparent FROM pg_inherits");
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
--
2.17.0
--PVZB3C997XEKXh6G
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0006-review-docs-for-pg12dev-broken-commas.patch"
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Add checkpoint and redo LSN to LogCheckpointEnd log message
@ 2022-03-16 00:19 Kyotaro Horiguchi <[email protected]>
2022-03-16 01:29 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Kyotaro Horiguchi <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Kyotaro Horiguchi @ 2022-03-16 00:19 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
At Tue, 15 Mar 2022 18:26:26 +0900, Michael Paquier <[email protected]> wrote in
> On Tue, Mar 15, 2022 at 05:23:40PM +0900, Kyotaro Horiguchi wrote:
> > At Tue, 15 Mar 2022 12:19:47 +0530, Bharath Rupireddy <[email protected]> wrote in
> >> On Fri, Mar 4, 2022 at 10:40 AM Kyotaro Horiguchi
> >> <[email protected]> wrote:
> >> 0001 - I don't think you need to do this as UpdateControlFile
> >> (update_controlfile) will anyway update it, no?
> >> + /* Update control file using current time */
> >> + ControlFile->time = (pg_time_t) time(NULL);
> >
> > Ugh.. Yes. It is a copy-pasto from older versions. They may have the
> > same copy-pasto..
>
> This thread has shifted to an entirely different discussion,
> presenting patches that touch code paths unrelated to what was first
> stated. Shouldn't you create a new thread with a proper $subject to
> attract a more correct audience?
I felt the same since some messages ago. I thought Fujii-san thought
that he wants to fix the CreateRestartPoint issue before the
checkpoint log patch but he looks like busy these days.
Since the CreateRestartPoint issue is orthogonal to the main patch,
I'll separate that part into anther thread.
This thread is discussing one other topic, wordings in user-facing
texts. This is also orthogonal (3-dimentionally?) to the two topics.
In short, I split out the two topics other than checkpoint log to
other threads.
Thanks for cueing me to do that!
regareds.
--
Kyotaro Horiguchi
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Add checkpoint and redo LSN to LogCheckpointEnd log message
2022-03-16 00:19 Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Kyotaro Horiguchi <[email protected]>
@ 2022-03-16 01:29 ` Kyotaro Horiguchi <[email protected]>
2022-03-28 19:31 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Greg Stark <[email protected]>
2022-07-06 16:11 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Fujii Masao <[email protected]>
0 siblings, 2 replies; 7+ messages in thread
From: Kyotaro Horiguchi @ 2022-03-16 01:29 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
At Wed, 16 Mar 2022 09:19:13 +0900 (JST), Kyotaro Horiguchi <[email protected]> wrote in
> In short, I split out the two topics other than checkpoint log to
> other threads.
So, this is about the main topic of this thread, adding LSNs to
checkpint log. Other topics have moved to other treads [1], [2] ,
[3].
I think this is no longer controversial alone. So this patch is now
really Read-for-Commiter and is waiting to be picked up.
regards.
[1] https://www.postgresql.org/message-id/20220316.102444.2193181487576617583.horikyota.ntt%40gmail.com
[2] https://www.postgresql.org/message-id/20220316.102900.2003692961119672246.horikyota.ntt%40gmail.com
[3] https://www.postgresql.org/message-id/20220316.102509.785466054344164656.horikyota.ntt%40gmail.com
--
Kyotaro Horiguchi
NTT Open Source Software Center
Attachments:
[text/x-patch] v13-0001-Add-checkpoint-and-redo-LSN-to-LogCheckpointEnd-.patch (3.1K, ../../[email protected]/2-v13-0001-Add-checkpoint-and-redo-LSN-to-LogCheckpointEnd-.patch)
download | inline diff:
From 4c3d40e7aaf29cb905f3561a291d35981d762456 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <[email protected]>
Date: Fri, 4 Mar 2022 13:22:41 +0900
Subject: [PATCH v13] Add checkpoint and redo LSN to LogCheckpointEnd log
message
It is useful (for debugging purposes) if the checkpoint end message
has the checkpoint LSN(end) and REDO LSN(start). It gives more
context while analyzing checkpoint-related issues. The pg_controldata
gives the last checkpoint LSN and REDO LSN, but having this info
alongside the log message helps analyze issues that happened
previously, connect the dots and identify the root cause.
---
src/backend/access/transam/xlog.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index ed16f279b1..b85c76d8f8 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6121,7 +6121,8 @@ LogCheckpointEnd(bool restartpoint)
"%d WAL file(s) added, %d removed, %d recycled; "
"write=%ld.%03d s, sync=%ld.%03d s, total=%ld.%03d s; "
"sync files=%d, longest=%ld.%03d s, average=%ld.%03d s; "
- "distance=%d kB, estimate=%d kB",
+ "distance=%d kB, estimate=%d kB; "
+ "lsn=%X/%X, redo lsn=%X/%X",
CheckpointStats.ckpt_bufs_written,
(double) CheckpointStats.ckpt_bufs_written * 100 / NBuffers,
CheckpointStats.ckpt_segs_added,
@@ -6134,14 +6135,21 @@ LogCheckpointEnd(bool restartpoint)
longest_msecs / 1000, (int) (longest_msecs % 1000),
average_msecs / 1000, (int) (average_msecs % 1000),
(int) (PrevCheckPointDistance / 1024.0),
- (int) (CheckPointDistanceEstimate / 1024.0))));
+ (int) (CheckPointDistanceEstimate / 1024.0),
+ /*
+ * ControlFileLock is not required as we are the only
+ * updator of these variables.
+ */
+ LSN_FORMAT_ARGS(ControlFile->checkPoint),
+ LSN_FORMAT_ARGS(ControlFile->checkPointCopy.redo))));
else
ereport(LOG,
(errmsg("checkpoint complete: wrote %d buffers (%.1f%%); "
"%d WAL file(s) added, %d removed, %d recycled; "
"write=%ld.%03d s, sync=%ld.%03d s, total=%ld.%03d s; "
"sync files=%d, longest=%ld.%03d s, average=%ld.%03d s; "
- "distance=%d kB, estimate=%d kB",
+ "distance=%d kB, estimate=%d kB; "
+ "lsn=%X/%X, redo lsn=%X/%X",
CheckpointStats.ckpt_bufs_written,
(double) CheckpointStats.ckpt_bufs_written * 100 / NBuffers,
CheckpointStats.ckpt_segs_added,
@@ -6154,7 +6162,13 @@ LogCheckpointEnd(bool restartpoint)
longest_msecs / 1000, (int) (longest_msecs % 1000),
average_msecs / 1000, (int) (average_msecs % 1000),
(int) (PrevCheckPointDistance / 1024.0),
- (int) (CheckPointDistanceEstimate / 1024.0))));
+ (int) (CheckPointDistanceEstimate / 1024.0),
+ /*
+ * ControlFileLock is not required as we are the only
+ * updator of these variables.
+ */
+ LSN_FORMAT_ARGS(ControlFile->checkPoint),
+ LSN_FORMAT_ARGS(ControlFile->checkPointCopy.redo))));
}
/*
--
2.27.0
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Add checkpoint and redo LSN to LogCheckpointEnd log message
2022-03-16 00:19 Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Kyotaro Horiguchi <[email protected]>
2022-03-16 01:29 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Kyotaro Horiguchi <[email protected]>
@ 2022-03-28 19:31 ` Greg Stark <[email protected]>
1 sibling, 0 replies; 7+ messages in thread
From: Greg Stark @ 2022-03-28 19:31 UTC (permalink / raw)
To: Kyotaro Horiguchi <[email protected]>; +Cc: Michael Paquier <[email protected]>; Bharath Rupireddy <[email protected]>; [email protected]; [email protected]; Stephen Frost <[email protected]>; Bossart, Nathan <[email protected]>; Julien Rouhaud <[email protected]>; PostgreSQL Hackers <[email protected]>
This patch is currently showing up with a test failure in the CFBot
however I do *not* believe this is a bug in the patch. I think it's a
bug in that test which is being discussed elsewhere.
It's also a very short and straightforward patch that a committer
could probably make a decision about whether it's a good idea or not
and then apply it quickly if so.
Just to give people a leg up and an idea how short the patch is...
Here's the entire patch:
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index ed16f279b1..b85c76d8f8 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -6121,7 +6121,8 @@ LogCheckpointEnd(bool restartpoint)
"%d WAL file(s) added, %d removed, %d recycled; "
"write=%ld.%03d s, sync=%ld.%03d s, total=%ld.%03d s; "
"sync files=%d, longest=%ld.%03d s, average=%ld.%03d s; "
- "distance=%d kB, estimate=%d kB",
+ "distance=%d kB, estimate=%d kB; "
+ "lsn=%X/%X, redo lsn=%X/%X",
CheckpointStats.ckpt_bufs_written,
(double) CheckpointStats.ckpt_bufs_written * 100 / NBuffers,
CheckpointStats.ckpt_segs_added,
@@ -6134,14 +6135,21 @@ LogCheckpointEnd(bool restartpoint)
longest_msecs / 1000, (int) (longest_msecs % 1000),
average_msecs / 1000, (int) (average_msecs % 1000),
(int) (PrevCheckPointDistance / 1024.0),
- (int) (CheckPointDistanceEstimate / 1024.0))));
+ (int) (CheckPointDistanceEstimate / 1024.0),
+ /*
+ * ControlFileLock is not required as we are the only
+ * updator of these variables.
+ */
+ LSN_FORMAT_ARGS(ControlFile->checkPoint),
+ LSN_FORMAT_ARGS(ControlFile->checkPointCopy.redo))));
else
ereport(LOG,
(errmsg("checkpoint complete: wrote %d buffers (%.1f%%); "
"%d WAL file(s) added, %d removed, %d recycled; "
"write=%ld.%03d s, sync=%ld.%03d s, total=%ld.%03d s; "
"sync files=%d, longest=%ld.%03d s, average=%ld.%03d s; "
- "distance=%d kB, estimate=%d kB",
+ "distance=%d kB, estimate=%d kB; "
+ "lsn=%X/%X, redo lsn=%X/%X",
CheckpointStats.ckpt_bufs_written,
(double) CheckpointStats.ckpt_bufs_written * 100 / NBuffers,
CheckpointStats.ckpt_segs_added,
@@ -6154,7 +6162,13 @@ LogCheckpointEnd(bool restartpoint)
longest_msecs / 1000, (int) (longest_msecs % 1000),
average_msecs / 1000, (int) (average_msecs % 1000),
(int) (PrevCheckPointDistance / 1024.0),
- (int) (CheckPointDistanceEstimate / 1024.0))));
+ (int) (CheckPointDistanceEstimate / 1024.0),
+ /*
+ * ControlFileLock is not required as we are the only
+ * updator of these variables.
+ */
+ LSN_FORMAT_ARGS(ControlFile->checkPoint),
+ LSN_FORMAT_ARGS(ControlFile->checkPointCopy.redo))));
}
/*
--
2.27.0
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Add checkpoint and redo LSN to LogCheckpointEnd log message
2022-03-16 00:19 Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Kyotaro Horiguchi <[email protected]>
2022-03-16 01:29 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Kyotaro Horiguchi <[email protected]>
@ 2022-07-06 16:11 ` Fujii Masao <[email protected]>
2022-07-07 07:26 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Kyotaro Horiguchi <[email protected]>
1 sibling, 1 reply; 7+ messages in thread
From: Fujii Masao @ 2022-07-06 16:11 UTC (permalink / raw)
To: Kyotaro Horiguchi <[email protected]>; [email protected]; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
On 2022/03/16 10:29, Kyotaro Horiguchi wrote:
> At Wed, 16 Mar 2022 09:19:13 +0900 (JST), Kyotaro Horiguchi <[email protected]> wrote in
>> In short, I split out the two topics other than checkpoint log to
>> other threads.
>
> So, this is about the main topic of this thread, adding LSNs to
> checkpint log. Other topics have moved to other treads [1], [2] ,
> [3].
>
> I think this is no longer controversial alone. So this patch is now
> really Read-for-Commiter and is waiting to be picked up.
+1
+ * ControlFileLock is not required as we are the only
+ * updator of these variables.
Isn't it better to add "at this time" or something at the end of the comment because only we're not always updator of them? No?
Barring any objection, I'm thinking to apply the above small change and commit the patch.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Add checkpoint and redo LSN to LogCheckpointEnd log message
2022-03-16 00:19 Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Kyotaro Horiguchi <[email protected]>
2022-03-16 01:29 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Kyotaro Horiguchi <[email protected]>
2022-07-06 16:11 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Fujii Masao <[email protected]>
@ 2022-07-07 07:26 ` Kyotaro Horiguchi <[email protected]>
2022-07-07 13:45 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Fujii Masao <[email protected]>
0 siblings, 1 reply; 7+ messages in thread
From: Kyotaro Horiguchi @ 2022-07-07 07:26 UTC (permalink / raw)
To: [email protected]; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
At Thu, 7 Jul 2022 01:11:33 +0900, Fujii Masao <[email protected]> wrote in
>
>
> On 2022/03/16 10:29, Kyotaro Horiguchi wrote:
> > At Wed, 16 Mar 2022 09:19:13 +0900 (JST), Kyotaro Horiguchi
> > <[email protected]> wrote in
> >> In short, I split out the two topics other than checkpoint log to
> >> other threads.
> > So, this is about the main topic of this thread, adding LSNs to
> > checkpint log. Other topics have moved to other treads [1], [2] ,
> > [3].
> > I think this is no longer controversial alone. So this patch is now
> > really Read-for-Commiter and is waiting to be picked up.
>
> +1
>
> + * ControlFileLock is not required as we are the only
> + * updator of these variables.
>
> Isn't it better to add "at this time" or something at the end of the
> comment because only we're not always updator of them? No?
Excluding initialization, (I believe) checkpointer is really the only
updator of the variables/members. But I'm fine with the addition.
> Barring any objection, I'm thinking to apply the above small change
> and commit the patch.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 7+ messages in thread
* Re: Add checkpoint and redo LSN to LogCheckpointEnd log message
2022-03-16 00:19 Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Kyotaro Horiguchi <[email protected]>
2022-03-16 01:29 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Kyotaro Horiguchi <[email protected]>
2022-07-06 16:11 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Fujii Masao <[email protected]>
2022-07-07 07:26 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Kyotaro Horiguchi <[email protected]>
@ 2022-07-07 13:45 ` Fujii Masao <[email protected]>
0 siblings, 0 replies; 7+ messages in thread
From: Fujii Masao @ 2022-07-07 13:45 UTC (permalink / raw)
To: Kyotaro Horiguchi <[email protected]>; +Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
On 2022/07/07 16:26, Kyotaro Horiguchi wrote:
>> + * ControlFileLock is not required as we are the only
>> + * updator of these variables.
>>
>> Isn't it better to add "at this time" or something at the end of the
>> comment because only we're not always updator of them? No?
>
> Excluding initialization, (I believe) checkpointer is really the only
> updator of the variables/members. But I'm fine with the addition.
Ok, so I modified the patch slightly and pushed it. Thanks!
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 7+ messages in thread
end of thread, other threads:[~2022-07-07 13:45 UTC | newest]
Thread overview: 7+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-12-02 23:22 [PATCH 05/17] Fix partially-updated comment Justin Pryzby <[email protected]>
2022-03-16 00:19 Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Kyotaro Horiguchi <[email protected]>
2022-03-16 01:29 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Kyotaro Horiguchi <[email protected]>
2022-03-28 19:31 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Greg Stark <[email protected]>
2022-07-06 16:11 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Fujii Masao <[email protected]>
2022-07-07 07:26 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Kyotaro Horiguchi <[email protected]>
2022-07-07 13:45 ` Re: Add checkpoint and redo LSN to LogCheckpointEnd log message Fujii Masao <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox