8000 Remove InitXLOGAccess(). · f0rk/postgres@fa0e03c · GitHub
[go: up one dir, main page]

Skip to content

Commit fa0e03c

Browse files
committed
Remove InitXLOGAccess().
It's not great that RecoveryInProgress() calls InitXLOGAccess(), because a status inquiry function typically shouldn't have the side effect of performing initializations. We could fix that by calling InitXLOGAccess() from some other place, but instead, let's remove it altogether. One thing InitXLogAccess() did is initialize wal_segment_size, but it doesn't need to do that. In the postmaster, PostmasterMain() calls LocalProcessControlFile(), and all child processes will inherit that value -- except in EXEC_BACKEND bulds, but then each backend runs SubPostmasterMain() which also calls LocalProcessControlFile(). The other thing InitXLOGAccess() did is update RedoRecPtr and doPageWrites, but that's not critical, because all code that uses them will just retry if it turns out that they've changed. The only difference is that most code will now see an initial value that is definitely invalid instead of one that might have just been way out of date, but that will only happen once per backend lifetime, so it shouldn't be a big deal. Patch by me, reviewed by Nathan Bossart, Michael Paquier, Andres Freund, Heikki Linnakangas, and Álvaro Herrera. Discussion: http://postgr.es/m/CA+TgmoY7b65qRjzHN_tWUk8B4sJqk1vj1d31uepVzmgPnZKeLg@mail.gmail.com
1 parent 64da07c commit fa0e03c

File tree

4 files changed

+22
-64
lines changed
  • include/access
  • 4 files changed

    +22
    -64
    lines changed

    src/backend/access/transam/xlog.c

    Lines changed: 17 additions & 46 deletions
    Original file line numberDiff line numberDiff line change
    @@ -350,15 +350,27 @@ XLogRecPtr XactLastCommitEnd = InvalidXLogRecPtr;
    350350
    * XLogCtl->Insert.RedoRecPtr, whenever we can safely do so (ie, when we
    351351
    * hold an insertion lock). See XLogInsertRecord for details. We are also
    352352
    * allowed to update from XLogCtl->RedoRecPtr if we hold the info_lck;
    353-
    * see GetRedoRecPtr. A freshly spawned backend obtains the value during
    354-
    * InitXLOGAccess.
    353+
    * see GetRedoRecPtr.
    354+
    *
    355+
    * NB: Code that uses this variable must be prepared not only for the
    356+
    * possibility that it may be arbitrarily out of date, but also for the
    357+
    * possibility that it might be set to InvalidXLogRecPtr. We used to
    358+
    * initialize it as a side effect of the first call to RecoveryInProgress(),
    359+
    * which meant that most code that might use it could assume that it had a
    360+
    * real if perhaps stale value. That's no longer the case.
    355361
    */
    356362
    static XLogRecPtr RedoRecPtr;
    357363

    358364
    /*
    359365
    * doPageWrites is this backend's local copy of (forcePageWrites ||
    360366
    * fullPageWrites). It is used together with RedoRecPtr to decide whether
    361367
    * a full-page image of a page need to be taken.
    368+
    *
    369+
    * NB: Initially this is false, and there's no guarantee that it will be
    370+
    * initialized to any other value before it is first used. Any code that
    371+
    * makes use of it must recheck the value after obtaining a WALInsertLock,
    372+
    * and respond appropriately if it turns out that the previous value wasn't
    373+
    * accurate.
    362374
    */
    363375
    static bool doPageWrites;
    364376

    @@ -8390,9 +8402,6 @@ PerformRecoveryXLogAction(void)
    83908402
    *
    83918403
    * Unlike testing InRecovery, this works in any process that's connected to
    83928404
    * shared memory.
    8393-
    *
    8394-
    * As a side-effect, we initialize the local RedoRecPtr variable the first
    8395-
    * time we see that recovery is finished.
    83968405
    */
    83978406
    bool
    83988407
    RecoveryInProgress(void)
    @@ -8414,23 +8423,6 @@ RecoveryInProgress(void)
    84148423

    84158424
    LocalRecoveryInProgress = (xlogctl->SharedRecoveryState != RECOVERY_STATE_DONE);
    84168425

    8417-
    /*
    8418-
    * Initialize TimeLineID and RedoRecPtr when we discover that recovery
    8419-
    * is finished. InitPostgres() relies upon this behaviour to ensure
    8420-
    * that InitXLOGAccess() is called at backend startup. (If you change
    8421-
    * this, see also LocalSetXLogInsertAllowed.)
    8422-
    */
    8423-
    if (!LocalRecoveryInProgress)
    8424-
    {
    8425-
    /*
    8426-
    * If we just exited recovery, make sure we read TimeLineID and
    8427-
    * RedoRecPtr after SharedRecoveryState (for machines with weak
    8428-
    * memory ordering).
    8429-
    */
    8430-
    pg_memory_barrier();
    8431-
    InitXLOGAccess();
    8432-
    }
    8433-
    84348426
    /*
    84358427
    * Note: We don't need a memory barrier when we're still in recovery.
    84368428
    * We might exit recovery immediately after return, so the caller
    @@ -8547,9 +8539,6 @@ LocalSetXLogInsertAllowed(void)
    85478539

    85488540
    LocalXLogInsertAllowed = 1;
    85498541

    8550-
    /* Initialize as RecoveryInProgress() would do when switching state */
    8551-
    InitXLOGAccess();
    8552-
    85538542
    return oldXLogAllowed;
    85548543
    }
    85558544

    @@ -8656,25 +8645,6 @@ ReadCheckpointRecord(XLogReaderState *xlogreader, XLogRecPtr RecPtr,
    86568645
    return record;
    86578646
    }
    86588647

    8659-
    /*
    8660-
    * This must be called in a backend process before creating WAL records
    8661-
    * (except in a standalone backend, which does StartupXLOG instead). We need
    8662-
    * to initialize the local copy of RedoRecPtr.
    8663-
    */
    8664-
    void
    8665-
    InitXLOGAccess(void)
    8666-
    {
    8667-
    XLogCtlInsert *Insert = &XLogCtl->Insert;
    8668-
    8669-
    /* set wal_segment_size */
    8670-
    wal_segment_size = ControlFile->xlog_seg_size;
    8671-
    8672-
    /* Use GetRedoRecPtr to copy the RedoRecPtr safely */
    8673-
    (void) GetRedoRecPtr();
    8674-
    /* Also update our copy of doPageWrites. */
    8675-
    doPageWrites = (Insert->fullPageWrites || Insert->forcePageWrites);
    8676-
    }
    8677-
    86788648
    /*
    86798649
    * Return the current Redo pointer from shared memory.
    86808650
    *
    @@ -8706,8 +8676,9 @@ GetRedoRecPtr(void)
    87068676
    * full-page image to be included in the WAL record.
    87078677
    *
    87088678
    * The returned values are cached copies from backend-private memory, and
    8709-
    * possibly out-of-date. XLogInsertRecord will re-check them against
    8710-
    * up-to-date values, while holding the WAL insert lock.
    8679+
    * possibly out-of-date or, indeed, uninitalized, in which case they will
    8680+
    * be InvalidXLogRecPtr and false, respectively. XLogInsertRecord will
    8681+
    * re-check them against up-to-date values, while holding the WAL insert lock.
    87118682
    */
    87128683
    void
    87138684
    GetFullPageWriteInfo(XLogRecPtr *RedoRecPtr_p, bool *doPageWrites_p)

    src/backend/postmaster/auxprocess.c

    Lines changed: 0 additions & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -154,7 +154,6 @@ AuxiliaryProcessMain(AuxProcType auxtype)
    154154
    proc_exit(1);
    155155

    156156
    case WalWriterProcess:
    157-
    InitXLOGAccess();
    158157
    WalWriterMain();
    159158
    proc_exit(1);
    160159

    src/backend/utils/init/postinit.c

    Lines changed: 5 additions & 16 deletions
    Original file line numberDiff line numberDiff line change
    @@ -624,25 +624,14 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
    624624
    }
    625625

    626626
    /*
    627-
    * Initialize local process's access to XLOG.
    627+
    * If this is either a bootstrap process nor a standalone backend, start
    628+
    * up the XLOG machinery, and register to have it closed down at exit.
    629+
    * In other cases, the startup process is responsible for starting up
    630+
    * the XLOG machinery, and the checkpointer for closing it down.
    628631
    */
    629-
    if (IsUnderPostmaster)
    630-
    {
    631-
    /*
    632-
    * The postmaster already started the XLOG machinery, but we need to
    633-
    * call InitXLOGAccess(), if the system isn't in hot-standby mode.
    634-
    * This is handled by calling RecoveryInProgress and ignoring the
    635-
    * result.
    636-
    */
    637-
    (void) RecoveryInProgress();
    638-
    }
    639-
    else
    632+
    if (!IsUnderPostmaster)
    640633
    {
    641634
    /*
    642-
    * We are either a bootstrap process or a standalone backend. Either
    643-
    * way, start up the XLOG machinery, and register to have it closed
    644-
    * down at exit.
    645-
    *
    646635
    * We don't yet have an aux-process resource owner, but StartupXLOG
    647636
    * and ShutdownXLOG will need one. Hence, create said resource owner
    648637
    * (and register a callback to clean it up after ShutdownXLOG runs).

    src/include/access/xlog.h

    Lines changed: 0 additions & 1 deletion
    Original file line numberDiff line numberDiff line change
    @@ -299,7 +299,6 @@ extern void BootStrapXLOG(void);
    299299
    extern void LocalProcessControlFile(bool reset);
    300300
    extern void StartupXLOG(void);
    301301
    extern void ShutdownXLOG(int code, Datum arg);
    302-
    extern void InitXLOGAccess(void);
    303302
    extern void CreateCheckPoint(int flags);
    304303
    extern bool CreateRestartPoint(int flags);
    305304
    extern WALAvailability GetWALAvailability(XLogRecPtr targetLSN);

    0 commit comments

    Comments
     (0)
    0