On 09/24/2016 05:01 AM, Thomas Munro wrote:
What would the appetite be for that kind of refactoring work,
considering the increased burden on committers who have to backpatch
bug fixes?  Is it a project goal to reduce the size of large
complicated functions like StartupXLOG and heap_update?  It seems like
a good way for new players to learn how they work.

+1. Yes, it does increase the burden of backpatching, but I think it'd still be very much worth it.

A couple of little details that caught my eye at a quick read:

        /* Try to find a backup label. */
        if (read_backup_label(&checkPointLoc, &backupEndRequired,
                wasShutdown = ProcessBackupLabel(xlogreader, &checkPoint, 

                /* set flag to delete it later */
                haveBackupLabel = true;
                /* Clean up any orphaned tablespace map files with no backup 
label. */

This is a bit asymmetric: In the true-branch, ProcessBackupLabel() reads the tablespace map, and sets InArchiveRecovery and StandbyMode, but in the false-branch, StartupXLog() calls CleanupTablespaceMap() and sets those variables directly.

For functions like BeginRedo, BeginHotStandby, ReplayRedo, etc., I think it'd be better to have the "if (InRecovery)" checks in the caller, rather than in the functions.

- Heikki

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to