Update stale code comment in CheckpointerMain()

Started by Amul Sulabout 4 years ago5 messages
#1Amul Sul
sulamul@gmail.com
1 attachment(s)

Hi,

The attached patch updates the code comment which is no longer true
after commit # 4a92a1c3d1c361ffb031ed05bf65b801241d7cdd

--
Regards,
Amul Sul
EDB: http://www.enterprisedb.com

Attachments:

update_code_comment.patchapplication/x-patch; name=update_code_comment.patchDownload
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index be7366379d0..25a18b7a14b 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -384,11 +384,7 @@ CheckpointerMain(void)
 			bool		ckpt_performed = false;
 			bool		do_restartpoint;
 
-			/*
-			 * Check if we should perform a checkpoint or a restartpoint. As a
-			 * side-effect, RecoveryInProgress() initializes TimeLineID if
-			 * it's not set yet.
-			 */
+			/* Check if we should perform a checkpoint or a restartpoint. */
 			do_restartpoint = RecoveryInProgress();
 
 			/*
#2Daniel Gustafsson
daniel@yesql.se
In reply to: Amul Sul (#1)
1 attachment(s)
Re: Update stale code comment in CheckpointerMain()

On 30 Nov 2021, at 08:00, Amul Sul <sulamul@gmail.com> wrote:

The attached patch updates the code comment which is no longer true
after commit # 4a92a1c3d1c361ffb031ed05bf65b801241d7cdd

Agreed, but looking at this shouldn't we also tweak the comment on
RecoveryInProgress() as per the attached v2 diff?

--
Daniel Gustafsson https://vmware.com/

Attachments:

update_code_comment_v2.patchapplication/octet-stream; name=update_code_comment_v2.patch; x-unix-mode=0644Download
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index b980c6ac21..d894af310a 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8391,8 +8391,8 @@ PerformRecoveryXLogAction(void)
  * Unlike testing InRecovery, this works in any process that's connected to
  * shared memory.
  *
- * As a side-effect, we initialize the local TimeLineID and RedoRecPtr
- * variables the first time we see that recovery is finished.
+ * As a side-effect, we initialize the local RedoRecPtr variable the first
+ * time we see that recovery is finished.
  */
 bool
 RecoveryInProgress(void)
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index be7366379d..25a18b7a14 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -384,11 +384,7 @@ CheckpointerMain(void)
 			bool		ckpt_performed = false;
 			bool		do_restartpoint;
 
-			/*
-			 * Check if we should perform a checkpoint or a restartpoint. As a
-			 * side-effect, RecoveryInProgress() initializes TimeLineID if
-			 * it's not set yet.
-			 */
+			/* Check if we should perform a checkpoint or a restartpoint. */
 			do_restartpoint = RecoveryInProgress();
 
 			/*
#3Amul Sul
sulamul@gmail.com
In reply to: Daniel Gustafsson (#2)
Re: Update stale code comment in CheckpointerMain()

On Tue, Nov 30, 2021 at 3:09 PM Daniel Gustafsson <daniel@yesql.se> wrote:

On 30 Nov 2021, at 08:00, Amul Sul <sulamul@gmail.com> wrote:

The attached patch updates the code comment which is no longer true
after commit # 4a92a1c3d1c361ffb031ed05bf65b801241d7cdd

Agreed, but looking at this shouldn't we also tweak the comment on
RecoveryInProgress() as per the attached v2 diff?

Yes, we should -- diff looks good to me, thanks.

Regards,
Amul

#4Daniel Gustafsson
daniel@yesql.se
In reply to: Amul Sul (#3)
Re: Update stale code comment in CheckpointerMain()

On 1 Dec 2021, at 07:19, Amul Sul <sulamul@gmail.com> wrote:

On Tue, Nov 30, 2021 at 3:09 PM Daniel Gustafsson <daniel@yesql.se> wrote:

On 30 Nov 2021, at 08:00, Amul Sul <sulamul@gmail.com> wrote:

The attached patch updates the code comment which is no longer true
after commit # 4a92a1c3d1c361ffb031ed05bf65b801241d7cdd

Agreed, but looking at this shouldn't we also tweak the comment on
RecoveryInProgress() as per the attached v2 diff?

Yes, we should -- diff looks good to me, thanks.

Thanks for confirming, I've applied this to master.

--
Daniel Gustafsson https://vmware.com/

#5Robert Haas
robertmhaas@gmail.com
In reply to: Daniel Gustafsson (#4)
Re: Update stale code comment in CheckpointerMain()

On Wed, Dec 1, 2021 at 8:24 AM Daniel Gustafsson <daniel@yesql.se> wrote:

The attached patch updates the code comment which is no longer true
after commit # 4a92a1c3d1c361ffb031ed05bf65b801241d7cdd

Agreed, but looking at this shouldn't we also tweak the comment on
RecoveryInProgress() as per the attached v2 diff?

Yes, we should -- diff looks good to me, thanks.

Thanks for confirming, I've applied this to master.

Thanks both of you.

--
Robert Haas
EDB: http://www.enterprisedb.com