Re: Split xlog.c

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: vignesh C <vignesh21(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Split xlog.c
Date: 2021-07-31 07:54:12
Message-ID: 6e2358c7-c309-05c1-0934-c5478e05c8e4@iki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 31/07/2021 02:11, Andres Freund wrote:
> Hi,
>
> I think it'd make sense to apply the first few patches now, they seem
> uncontroversial and simple enough.

Pushed those, thanks!

>> From da11050ca890ce0311d9e97d2832a6a61bc43e10 Mon Sep 17 00:00:00 2001
>> From: Heikki Linnakangas <heikki(dot)linnakangas(at)iki(dot)fi>
>> Date: Fri, 18 Jun 2021 12:15:04 +0300
>> Subject: [PATCH 5/7] Move code around in StartupXLOG().
>>
>> This is the order that things will happen with the next commit, this
>> makes it more explicit. To aid review, I added "BEGIN/END function"
>> comments to mark which blocks of code are moved to separate functions in
>> in the next commit.
>
>> ---
>> src/backend/access/transam/xlog.c | 605 ++++++++++++++++--------------
>> 1 file changed, 315 insertions(+), 290 deletions(-)
>>
>> diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
>> index efb3ca273ed..b9d96d6de26 100644
>> --- a/src/backend/access/transam/xlog.c
>> +++ b/src/backend/access/transam/xlog.c
>> @@ -882,7 +882,6 @@ static MemoryContext walDebugCxt = NULL;
>>
>> static void readRecoverySignalFile(void);
>> static void validateRecoveryParameters(void);
>> -static void exitArchiveRecovery(TimeLineID endTLI, XLogRecPtr endOfLog);
>> static bool recoveryStopsBefore(XLogReaderState *record);
>> static bool recoveryStopsAfter(XLogReaderState *record);
>> static char *getRecoveryStopReason(void);
>> @@ -5592,111 +5591,6 @@ validateRecoveryParameters(void)
>> }
>> }
>>
>> -/*
>> - * Exit archive-recovery state
>> - */
>> -static void
>> -exitArchiveRecovery(TimeLineID endTLI, XLogRecPtr endOfLog)
>> -{
>
> I don't really understand the motivation for this part of the change? This
> kind of seems to run counter to the stated goals of the patch series? Seems
> like it'd need a different commit message at last?

Hmm. Some parts of exitArchiveRecovery are being moved into
xlogrecovery.c, so it becomes smaller than before. Maybe there's still
enough code left there that a separate function makes sense. I'll try
that differently.

>> + /*---- BEGIN FreeWalRecovery ----*/
>> +
>> /* Shut down xlogreader */
>> if (readFile >= 0)
>> {
>
> FWIW, FreeWalRecovery() for something that closes and unlinks files among
> other things doesn't seem like a great name.

Rename to CloseWalRecovery(), maybe? I'll try that.

- Heikki

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrey Borodin 2021-07-31 08:21:58 Re: Replace l337sp34k in comments.
Previous Message Ajin Cherian 2021-07-31 05:42:00 Re: [HACKERS] logical decoding of two-phase transactions