On Tue, Sep 27, 2022 at 2:20 PM Kyotaro Horiguchi <horikyota....@gmail.com> wrote: > > > -1 from me. We have the function context and the structure name there > > to represent that the parameter name 'state' is actually 'backup > > state'. I don't think we gain anything here. Whenever the BackupState > > is used away from any function, the variable name backup_state is > > used, static variable in xlogfuncs.c > > There's no shadowing caused by the change. If we mind the same > variable names between files, we could rename backup_state in > xlogfuncs.c to process_backup_state or session_backup_state.
-1. > If this is still unacceptable, I propose to change the comment. (I > found that the previous patch forgets about do_pg_backup_stop()) > > - * It fills in backup_state with the information required for the backup, > + * It fills in the parameter "state" with the information required for the > backup, +1. There's another place that uses backup_state in the comments. I modified that as well. Please see the attached patch. -- Bharath Rupireddy PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
From b66c67712f3499259427c3ea32d102ac802941ac Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com> Date: Tue, 27 Sep 2022 09:36:32 +0000 Subject: [PATCH v1] Adjust BackupState comments to not use backup_state --- src/backend/access/transam/xlog.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 1dd6df0fe1..675f851daa 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -8256,9 +8256,10 @@ issue_xlog_fsync(int fd, XLogSegNo segno, TimeLineID tli) * symlinks while extracting files from tar. However for consistency and * platform-independence, we do it the same way everywhere. * - * It fills in backup_state with the information required for the backup, - * such as the minimum WAL location that must be present to restore from - * this backup (starttli) and the corresponding timeline ID (starttli). + * It fills in the parameter "state" with the information required for the + * backup, such as the minimum WAL location that must be present to restore + * from this backup (starttli) and the corresponding timeline ID (starttli), + * etc. * * Every successfully started backup must be stopped by calling * do_pg_backup_stop() or do_pg_abort_backup(). There can be many @@ -8574,8 +8575,9 @@ get_backup_status(void) * file (if required), resets sessionBackupState and so on. It can optionally * wait for WAL segments to be archived. * - * backup_state is filled with the information necessary to restore from this - * backup with its stop LSN (stoppoint), its timeline ID (stoptli), etc. + * It fills in the parameter "state" with the information necessary to restore + * from this backup with its stop LSN (stoppoint), its timeline ID (stoptli), + * etc. * * It is the responsibility of the caller of this function to verify the * permissions of the calling user! -- 2.34.1
