You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{"payload":{"commit":{"oid":"1d919de5eb3fffa7cc9479ed6d2915fb89794459","url":"/postgres/postgres/commit/1d919de5eb3fffa7cc9479ed6d2915fb89794459","authoredDate":"2021-07-30T08:33:33.000-04:00","committedDate":"2021-07-30T08:35:13.000-04:00","shortMessage":null,"shortMessageMarkdown":"\u003cdiv\u003eRemove unnecessary call to ReadCheckpointRecord().\u003c/div\u003e","shortMessageMarkdownLink":null,"bodyMessageHtml":"It should always be the case that the last checkpoint record is still\nreadable, because otherwise, a crash would leave us in a situation\nfrom which we can't recover. Therefore the test removed by this patch\nshould always succeed. For it to fail, either there has to be a serious\nbug in the code someplace, or the user has to be manually modifying\npg_wal while crash recovery is running. If it's the first one, we\nshould fix the bug. If it's the second one, they should stop, or\nanyway they're doing so at their own risk. In neither case does\na full checkpoint instead of an end-of-recovery record seem like a\nclear winner. Furthermore, rarely-taken code paths are particularly\nvulnerable to bugs, so let's simplify by getting rid of this one.\n\nDiscussion: \u003ca href=\"http://postgr.es/m/CA+TgmoYmw==TOJ6EzYb_vcjyS09NkzrVKSyBKUUyo1zBEaJASA@mail.gmail.com\" rel=\"nofollow\"\u003ehttp://postgr.es/m/CA+TgmoYmw==TOJ6EzYb_vcjyS09NkzrVKSyBKUUyo1zBEaJASA@mail.gmail.com\u003c/a\u003e","authors":[{"login":"robertmhaas","displayName":"Robert Haas","avatarUrl":"https://avatars.githubusercontent.com/u/886678?v=4","path":"/robertmhaas","isGitHub":false}],"committerAttribution":false,"committer":{"login":"robertmhaas","displayName":"Robert Haas","avatarUrl":"https://avatars.githubusercontent.com/u/886678?v=4","path":"/robertmhaas","isGitHub":false},"parents":["3df93a66593c344e6298e618df3fa5090fca4309"],"globalRelayId":"MDY6Q29tbWl0OTI3NDQyOjFkOTE5ZGU1ZWIzZmZmYTdjYzk0NzllZDZkMjkxNWZiODk3OTQ0NTk=","sha1":"3df93a66593c344e6298e618df3fa5090fca4309","sha2":"1d919de5eb3fffa7cc9479ed6d2915fb89794459"},"currentUser":null,"repo":{"id":927442,"defaultBranch":"master","name":"postgres","ownerLogin":"postgres","currentUserCanPush":false,"isFork":false,"isEmpty":false,"createdAt":"2010-09-21T11:35:45.000Z","ownerAvatar":"https://avatars.githubusercontent.com/u/177543?v=4","public":true,"private":false,"isOrgOwned":true},"diffEntryData":[{"diffLines":[{"stylingDirective":null,"type":"HUNK","blobLineNumber":7916,"text":"@@ -7917,33 +7917,21 @@ StartupXLOG(void)","html":"@@ -7917,33 +7917,21 @@ StartupXLOG(void)","displayNoNewLineWarning":false,"position":0,"left":7916,"right":7916},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":7917,"text":" \t\t{","html":" \t\t{","displayNoNewLineWarning":false,"position":1,"left":7917,"right":7917},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":7918,"text":" \t\t\tif (LocalPromoteIsTriggered)","html":" \t\t\t\u003cspan class=pl-k\u003eif\u003c/span\u003e (\u003cspan class=pl-s1\u003eLocalPromoteIsTriggered\u003c/span\u003e)","displayNoNewLineWarning":false,"position":2,"left":7918,"right":7918},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":7919,"text":" \t\t\t{","html":" \t\t\t{","displayNoNewLineWarning":false,"position":3,"left":7919,"right":7919},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7920,"text":"-\t\t\t\tcheckPointLoc = ControlFile-\u003echeckPoint;","html":"-\t\t\t\t\u003cspan class=\"pl-s1 x x-first x-last\"\u003echeckPointLoc\u003c/span\u003e \u003cspan class=\"pl-c1\"\u003e=\u003c/span\u003e \u003cspan class=\"pl-s1 x x-first\"\u003eControlFile\u003c/span\u003e\u003cspan class=\"pl-c1 x\"\u003e-\u0026gt;\u003c/span\u003e\u003cspan class=\"pl-c1 x x-last\"\u003echeckPoint\u003c/span\u003e;","displayNoNewLineWarning":false,"position":4,"left":7920,"right":7919},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":7920,"text":"+\t\t\t\tpromoted = true;","html":"+\t\t\t\t\u003cspan class=\"pl-s1 x x-first x-last\"\u003epromoted\u003c/span\u003e \u003cspan class=\"pl-c1\"\u003e=\u003c/span\u003e \u003cspan class=\"x x-first x-last\"\u003etrue\u003c/span\u003e;","displayNoNewLineWarning":false,"position":5,"left":7920,"right":7920},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":7921,"text":" ","html":"\u003cbr\u003e","displayNoNewLineWarning":false,"position":6,"left":7921,"right":7921},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":7922,"text":" \t\t\t\t/*","html":" \t\t\t\t\u003cspan class=pl-c\u003e/*\u003c/span\u003e","displayNoNewLineWarning":false,"position":7,"left":7922,"right":7922},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7923,"text":"-\t\t\t\t * Confirm the last checkpoint is available for us to recover","html":"-\u003cspan class=pl-c\u003e\t\t\t\t * Confirm the last checkpoint is available for us to recover\u003c/span\u003e","displayNoNewLineWarning":false,"position":8,"left":7923,"right":7922},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7924,"text":"-\t\t\t\t * from if we fail.","html":"-\u003cspan class=pl-c\u003e\t\t\t\t * from if we fail.\u003c/span\u003e","displayNoNewLineWarning":false,"position":9,"left":7924,"right":7922},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":7923,"text":"+\t\t\t\t * Insert a special WAL record to mark the end of recovery,","html":"+\u003cspan class=pl-c\u003e\t\t\t\t * Insert a special WAL record to mark the end of recovery,\u003c/span\u003e","displayNoNewLineWarning":false,"position":10,"left":7924,"right":7923},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":7924,"text":"+\t\t\t\t * since we aren't doing a checkpoint. That means that the","html":"+\u003cspan class=pl-c\u003e\t\t\t\t * since we aren\u0026#39;t doing a checkpoint. That means that the\u003c/span\u003e","displayNoNewLineWarning":false,"position":11,"left":7924,"right":7924},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":7925,"text":"+\t\t\t\t * checkpointer process may likely be in the middle of a","html":"+\u003cspan class=pl-c\u003e\t\t\t\t * checkpointer process may likely be in the middle of a\u003c/span\u003e","displayNoNewLineWarning":false,"position":12,"left":7924,"right":7925},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":7926,"text":"+\t\t\t\t * time-smoothed restartpoint and could continue to be for","html":"+\u003cspan class=pl-c\u003e\t\t\t\t * time-smoothed restartpoint and could continue to be for\u003c/span\u003e","displayNoNewLineWarning":false,"position":13,"left":7924,"right":7926},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":7927,"text":"+\t\t\t\t * minutes after this. That sounds strange, but the effect is","html":"+\u003cspan class=pl-c\u003e\t\t\t\t * minutes after this. That sounds strange, but the effect is\u003c/span\u003e","displayNoNewLineWarning":false,"position":14,"left":7924,"right":7927},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":7928,"text":"+\t\t\t\t * roughly the same and it would be stranger to try to come","html":"+\u003cspan class=pl-c\u003e\t\t\t\t * roughly the same and it would be stranger to try to come\u003c/span\u003e","displayNoNewLineWarning":false,"position":15,"left":7924,"right":7928},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":7929,"text":"+\t\t\t\t * out of the restartpoint and then checkpoint. We request a","html":"+\u003cspan class=pl-c\u003e\t\t\t\t * out of the restartpoint and then checkpoint. We request a\u003c/span\u003e","displayNoNewLineWarning":false,"position":16,"left":7924,"right":7929},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":7930,"text":"+\t\t\t\t * checkpoint later anyway, just for safety.","html":"+\u003cspan class=pl-c\u003e\t\t\t\t * checkpoint later anyway, just for safety.\u003c/span\u003e","displayNoNewLineWarning":false,"position":17,"left":7924,"right":7930},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":7931,"text":" \t\t\t\t */","html":" \u003cspan class=pl-c\u003e\t\t\t\t */\u003c/span\u003e","displayNoNewLineWarning":false,"position":18,"left":7925,"right":7931},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7926,"text":"-\t\t\t\trecord = ReadCheckpointRecord(xlogreader, checkPointLoc, 1, false);","html":"-\t\t\t\t\u003cspan class=pl-s1\u003erecord\u003c/span\u003e \u003cspan class=pl-c1\u003e=\u003c/span\u003e \u003cspan class=pl-en\u003eReadCheckpointRecord\u003c/span\u003e(\u003cspan class=pl-s1\u003exlogreader\u003c/span\u003e, \u003cspan class=pl-s1\u003echeckPointLoc\u003c/span\u003e, \u003cspan class=pl-c1\u003e1\u003c/span\u003e, false);","displayNoNewLineWarning":false,"position":19,"left":7926,"right":7931},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7927,"text":"-\t\t\t\tif (record != NULL)","html":"-\t\t\t\t\u003cspan class=pl-k\u003eif\u003c/span\u003e (\u003cspan class=pl-s1\u003erecord\u003c/span\u003e \u003cspan class=pl-c1\u003e!=\u003c/span\u003e \u003cspan class=pl-c1\u003eNULL\u003c/span\u003e)","displayNoNewLineWarning":false,"position":20,"left":7927,"right":7931},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7928,"text":"-\t\t\t\t{","html":"-\t\t\t\t{","displayNoNewLineWarning":false,"position":21,"left":7928,"right":7931},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7929,"text":"-\t\t\t\t\tpromoted = true;","html":"-\t\t\t\t\t\u003cspan class=pl-s1\u003epromoted\u003c/span\u003e \u003cspan class=pl-c1\u003e=\u003c/span\u003e true;","displayNoNewLineWarning":false,"position":22,"left":7929,"right":7931},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7930,"text":"-","html":"-","displayNoNewLineWarning":false,"position":23,"left":7930,"right":7931},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7931,"text":"-\t\t\t\t\t/*","html":"-\t\t\t\t\t\u003cspan class=pl-c\u003e/*\u003c/span\u003e","displayNoNewLineWarning":false,"position":24,"left":7931,"right":7931},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7932,"text":"-\t\t\t\t\t * Insert a special WAL record to mark the end of","html":"-\u003cspan class=pl-c\u003e\t\t\t\t\t * Insert a special WAL record to mark the end of\u003c/span\u003e","displayNoNewLineWarning":false,"position":25,"left":7932,"right":7931},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7933,"text":"-\t\t\t\t\t * recovery, since we aren't doing a checkpoint. That","html":"-\u003cspan class=pl-c\u003e\t\t\t\t\t * recovery, since we aren\u0026#39;t doing a checkpoint. That\u003c/span\u003e","displayNoNewLineWarning":false,"position":26,"left":7933,"right":7931},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7934,"text":"-\t\t\t\t\t * means that the checkpointer process may likely be in","html":"-\u003cspan class=pl-c\u003e\t\t\t\t\t * means that the checkpointer process may likely be in\u003c/span\u003e","displayNoNewLineWarning":false,"position":27,"left":7934,"right":7931},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7935,"text":"-\t\t\t\t\t * the middle of a time-smoothed restartpoint and could","html":"-\u003cspan class=pl-c\u003e\t\t\t\t\t * the middle of a time-smoothed restartpoint and could\u003c/span\u003e","displayNoNewLineWarning":false,"position":28,"left":7935,"right":7931},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7936,"text":"-\t\t\t\t\t * continue to be for minutes after this. That sounds","html":"-\u003cspan class=pl-c\u003e\t\t\t\t\t * continue to be for minutes after this. That sounds\u003c/span\u003e","displayNoNewLineWarning":false,"position":29,"left":7936,"right":7931},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7937,"text":"-\t\t\t\t\t * strange, but the effect is roughly the same and it","html":"-\u003cspan class=pl-c\u003e\t\t\t\t\t * strange, but the effect is roughly the same and it\u003c/span\u003e","displayNoNewLineWarning":false,"position":30,"left":7937,"right":7931},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7938,"text":"-\t\t\t\t\t * would be stranger to try to come out of the","html":"-\u003cspan class=pl-c\u003e\t\t\t\t\t * would be stranger to try to come out of the\u003c/span\u003e","displayNoNewLineWarning":false,"position":31,"left":7938,"right":7931},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7939,"text":"-\t\t\t\t\t * restartpoint and then checkpoint. We request a","html":"-\u003cspan class=pl-c\u003e\t\t\t\t\t * restartpoint and then checkpoint. We request a\u003c/span\u003e","displayNoNewLineWarning":false,"position":32,"left":7939,"right":7931},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7940,"text":"-\t\t\t\t\t * checkpoint later anyway, just for safety.","html":"-\u003cspan class=pl-c\u003e\t\t\t\t\t * checkpoint later anyway, just for safety.\u003c/span\u003e","displayNoNewLineWarning":false,"position":33,"left":7940,"right":7931},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7941,"text":"-\t\t\t\t\t */","html":"-\u003cspan class=pl-c\u003e\t\t\t\t\t */\u003c/span\u003e","displayNoNewLineWarning":false,"position":34,"left":7941,"right":7931},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7942,"text":"-\t\t\t\t\tCreateEndOfRecoveryRecord();","html":"-\t\t\t\t\t\u003cspan class=pl-en\u003eCreateEndOfRecoveryRecord\u003c/span\u003e();","displayNoNewLineWarning":false,"position":35,"left":7942,"right":7931},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7943,"text":"-\t\t\t\t}","html":"-\t\t\t\t}","displayNoNewLineWarning":false,"position":36,"left":7943,"right":7931},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":7932,"text":"+\t\t\t\tCreateEndOfRecoveryRecord();","html":"+\t\t\t\t\u003cspan class=pl-en\u003eCreateEndOfRecoveryRecord\u003c/span\u003e();","displayNoNewLineWarning":false,"position":37,"left":7943,"right":7932},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":7933,"text":" \t\t\t}","html":" \t\t\t}","displayNoNewLineWarning":false,"position":38,"left":7944,"right":7933},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7945,"text":"-","html":"-","displayNoNewLineWarning":false,"position":39,"left":7945,"right":7933},{"stylingDirective":null,"type":"DELETION","blobLineNumber":7946,"text":"-\t\t\tif (!promoted)","html":"-\t\t\t\u003cspan class=pl-k\u003eif\u003c/span\u003e (!\u003cspan class=pl-s1\u003epromoted\u003c/span\u003e)","displayNoNewLineWarning":false,"position":40,"left":7946,"right":7933},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":7934,"text":"+\t\t\telse","html":"+\t\t\t\u003cspan class=pl-k\u003eelse\u003c/span\u003e","displayNoNewLineWarning":false,"position":41,"left":7946,"right":7934},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":7935,"text":" \t\t\t\tRequestCheckpoint(CHECKPOINT_END_OF_RECOVERY |","html":" \t\t\t\t\u003cspan class=pl-en\u003eRequestCheckpoint\u003c/span\u003e(\u003cspan class=pl-c1\u003eCHECKPOINT_END_OF_RECOVERY\u003c/span\u003e |","displayNoNewLineWarning":false,"position":42,"left":7947,"right":7935},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":7936,"text":" \t\t\t\t\t\t\t\t CHECKPOINT_IMMEDIATE |","html":" \t\t\t\t\t\t\t\t \u003cspan class=pl-c1\u003eCHECKPOINT_IMMEDIATE\u003c/span\u003e |","displayNoNewLineWarning":false,"position":43,"left":7948,"right":7936},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":7937,"text":" \t\t\t\t\t\t\t\t CHECKPOINT_WAIT);","html":" \t\t\t\t\t\t\t\t \u003cspan class=pl-c1\u003eCHECKPOINT_WAIT\u003c/span\u003e);","displayNoNewLineWarning":false,"position":44,"left":7949,"right":7937}],"diffNumber":0,"diffSize":"0 Bytes","isBinary":false,"isTooBig":false,"collapsed":false,"isSubmodule":false,"lineCount":13011,"linesChanged":34,"newTreeEntry":{"lineCount":13011,"path":"src/backend/access/transam/xlog.c","mode":100644,"isGenerated":false},"oldTreeEntry":{"lineCount":0,"path":"src/backend/access/transam/xlog.c","mode":100644},"linesAdded":11,"linesDeleted":23,"path":"src/backend/access/transam/xlog.c","pathDigest":"c1cb3ab2a19606390c1a7ed00ffe5a45531702ca5faf999d401c548f8951c65b","status":"MODIFIED","truncatedReason":null,"oldOid":"3df93a66593c344e6298e618df3fa5090fca4309","newOid":"1d919de5eb3fffa7cc9479ed6d2915fb89794459","copilotChatReference":null,"deletedSha":"3df93a66593c344e6298e618df3fa5090fca4309","canToggleRichDiff":false,"defaultToRichDiff":false,"proseDifffHtml":null,"renderInfo":null,"dependencyDiffPath":null,"submodule":null}],"splitViewPreference":"unified","ignoreWhitespace":false,"repoOwnerGlobalRelayId":"MDEyOk9yZ2FuaXphdGlvbjE3NzU0Mw==","commentsPreference":"visible","diffLineSpacingPreference":"relaxed","useMonospaceFont":false,"pasteUrlLinkAsPlainText":false,"userNotices":[],"path":"/postgres/postgres/commit/1d919de5eb3fffa7cc9479ed6d2915fb89794459","fileTreeExpanded":true,"headerInfo":{"additions":11,"deletions":23,"filesChanged":1,"filesChangedString":"1"},"moreDiffsToLoad":false,"asyncDiffLoadInfo":{"startIndex":1,"truncated":false,"byteCount":1663,"lineShownCount":45},"commentInfo":{"canComment":false,"locked":false,"canLock":false,"repoArchived":false},"csrf_tokens":{"/users/diffview?diff=split":{"post":"9DsF_-jquEqLP5VARJvFPjLiNEwx1ZilIAjYwKkfsVYIDpwt_PgzkgWg2eHTffTCmjaDrjPmNdtN_ihX7e8bcA"},"/users/diffview?diff=unified":{"post":"Uf-cVic8y0uWXi63uFDGz8avYV2_CnfSqoOHI-ce9fOtygWEMy5AkxjBYhYvtvczbnvWv7052qzHdXe0o-5f1Q"},"/notifications/thread":{"post":"Gbva8c0ci_oTFkJ0V30bM7-gLaD-AuhJZhNfbJlvHr_fRdnNh1cV47L_UdNYTi_W1q21gGEgVf6gvzwOeUqzyQ"}}},"title":"Remove unnecessary call to ReadCheckpointRecord(). · postgres/postgres@1d919de","appPayload":{"helpUrl":"https://docs.github.com","findInDiffWorkerPath":"/assets-cdn/worker/find-in-diff-worker-2bfe39677d14.js","enabled_features":{"diff_ux_refresh_beta":false,"diff_inline_comments":true,"diff_ux_refresh_ssr_five":false,"diff_ux_refresh_ssr_ten":false,"react_diff_line_type_character_correction":true}}}
Remove unnecessary call to ReadCheckpointRecord().
It should always be the case that the last checkpoint record is still
readable, because otherwise, a crash would leave us in a situation
from which we can't recover. Therefore the test removed by this patch
should always succeed. For it to fail, either there has to be a serious
bug in the code someplace, or the user has to be manually modifying
pg_wal while crash recovery is running. If it's the first one, we
should fix the bug. If it's the second one, they should stop, or
anyway they're doing so at their own risk. In neither case does
a full checkpoint instead of an end-of-recovery record seem like a
clear winner. Furthermore, rarely-taken code paths are particularly
vulnerable to bugs, so let's simplify by getting rid of this one.
Discussion: http://postgr.es/m/CA+TgmoYmw==TOJ6EzYb_vcjyS09NkzrVKSyBKUUyo1zBEaJASA@mail.gmail.com
0 commit comments