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,
&backupFromStandby))
{
wasShutdown = ProcessBackupLabel(xlogreader, &checkPoint,
checkPointLoc,
&haveTblspcMap);
/* set flag to delete it later */
haveBackupLabel = true;
}
else
{
/* Clean up any orphaned tablespace map files with no backup
label. */
CleanUpTablespaceMap();
...
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:
http://www.postgresql.org/mailpref/pgsql-hackers
