8000 Another fix to relmapper race condition. · postgres/postgres@9b8ed0f · GitHub
[go: up one dir, main page]

Skip to content
{"payload":{"commit":{"oid":"9b8ed0f52bffceaccf6fa650ffe482e7da20fbf2","url":"/postgres/postgres/commit/9b8ed0f52bffceaccf6fa650ffe482e7da20fbf2","authoredDate":"2021-06-24T11:19:03.000+03:00","committedDate":"2021-06-24T11:19:03.000+03:00","shortMessage":null,"shortMessageMarkdown":"\u003cdiv\u003eAnother fix to relmapper race condition.\u003c/div\u003e","shortMessageMarkdownLink":null,"bodyMessageHtml":"In previous commit, I missed that relmap_redo() was also not acquiring the\nRelationMappingLock. Thanks to Thomas Munro for pointing that out.\n\nBackpatch-through: 9.6, like previous commit.\nDiscussion: \u003ca href=\"https://www.postgresql.org/message-id/CA%2BhUKGLev%3DPpOSaL3WRZgOvgk217et%2BbxeJcRr4eR-NttP1F6Q%40mail.gmail.com\" rel=\"nofollow\"\u003ehttps://www.postgresql.org/message-id/CA%2BhUKGLev%3DPpOSaL3WRZgOvgk217et%2BbxeJcRr4eR-NttP1F6Q%40mail.gmail.com\u003c/a\u003e","authors":[{"login":"hlinnaka","displayName":"Heikki Linnakangas","avatarUrl":"https://avatars.githubusercontent.com/u/191602?v=4","path":"/hlinnaka","isGitHub":false}],"committerAttribution":false,"committer":{"login":"hlinnaka","displayName":"Heikki Linnakangas","avatarUrl":"https://avatars.githubusercontent.com/u/191602?v=4","path":"/hlinnaka","isGitHub":false},"parents":["b6d8d2073f228d9f7198f1f9826d8f6ab643c219"],"globalRelayId":"MDY6Q29tbWl0OTI3NDQyOjliOGVkMGY1MmJmZmNlYWNjZjZmYTY1MGZmZTQ4MmU3ZGEyMGZiZjI=","sha1":"b6d8d2073f228d9f7198f1f9826d8f6ab643c219","sha2":"9b8ed0f52bffceaccf6fa650ffe482e7da20fbf2"},"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":1029,"text":"@@ -1030,12 +1030,13 @@ relmap_redo(XLogReaderState *record)","html":"@@ -1030,12 +1030,13 @@ relmap_redo(XLogReaderState *record)","displayNoNewLineWarning":false,"position":0,"left":1029,"right":1029},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":1030,"text":" \t\t * preserve files, either.","html":" \u003cspan class=pl-c\u003e\t\t * preserve files, either.\u003c/span\u003e","displayNoNewLineWarning":false,"position":1,"left":1030,"right":1030},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":1031,"text":" \t\t *","html":" \u003cspan class=pl-c\u003e\t\t *\u003c/span\u003e","displayNoNewLineWarning":false,"position":2,"left":1031,"right":1031},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":1032,"text":" \t\t * There shouldn't be anyone else updating relmaps during WAL replay,","html":" \u003cspan class=pl-c\u003e\t\t * There shouldn\u0026#39;t be anyone else updating relmaps during WAL replay,\u003c/span\u003e","displayNoNewLineWarning":false,"position":3,"left":1032,"right":1032},{"stylingDirective":null,"type":"DELETION","blobLineNumber":1033,"text":"-\t\t * so we don't bother to take the RelationMappingLock. We would need","html":"-\u003cspan class=pl-c\u003e\t\t * so we don\u0026#39;t bother to take the RelationMappingLock. We would need\u003c/span\u003e","displayNoNewLineWarning":false,"position":4,"left":1033,"right":1032},{"stylingDirective":null,"type":"DELETION","blobLineNumber":1034,"text":"-\t\t * to do so if load_relmap_file needed to interlock against writers.","html":"-\u003cspan class=pl-c\u003e\t\t * to do so if load_relmap_file needed to interlock against writers.\u003c/span\u003e","displayNoNewLineWarning":false,"position":5,"left":1034,"right":1032},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":1033,"text":"+\t\t * but grab the lock to interlock against load_relmap_file().","html":"+\u003cspan class=pl-c\u003e\t\t * but grab the lock to interlock against load_relmap_file().\u003c/span\u003e","displayNoNewLineWarning":false,"position":6,"left":1034,"right":1033},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":1034,"text":" \t\t */","html":" \u003cspan class=pl-c\u003e\t\t */\u003c/span\u003e","displayNoNewLineWarning":false,"position":7,"left":1035,"right":1034},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":1035,"text":"+\t\tLWLockAcquire(RelationMappingLock, LW_EXCLUSIVE);","html":"+\t\t\u003cspan class=pl-en\u003eLWLockAcquire\u003c/span\u003e(\u003cspan class=pl-s1\u003eRelationMappingLock\u003c/span\u003e, \u003cspan class=pl-c1\u003eLW_EXCLUSIVE\u003c/span\u003e);","displayNoNewLineWarning":false,"position":8,"left":1035,"right":1035},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":1036,"text":" \t\twrite_relmap_file((xlrec-\u003edbid == InvalidOid), \u0026newmap,","html":" \t\t\u003cspan class=pl-en\u003ewrite_relmap_file\u003c/span\u003e((\u003cspan class=pl-s1\u003exlrec\u003c/span\u003e\u003cspan class=pl-c1\u003e-\u0026gt;\u003c/span\u003e\u003cspan class=pl-c1\u003edbid\u003c/span\u003e \u003cspan class=pl-c1\u003e==\u003c/span\u003e \u003cspan class=pl-s1\u003eInvalidOid\u003c/span\u003e), \u003cspan class=pl-c1\u003e\u0026amp;\u003c/span\u003e\u003cspan class=pl-s1\u003enewmap\u003c/span\u003e,","displayNoNewLineWarning":false,"position":9,"left":1036,"right":1036},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":1037,"text":" \t\t\t\t\t\t false, true, false,","html":" \t\t\t\t\t\t false, true, false,","displayNoNewLineWarning":false,"position":10,"left":1037,"right":1037},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":1038,"text":" \t\t\t\t\t\t xlrec-\u003edbid, xlrec-\u003etsid, dbpath);","html":" \t\t\t\t\t\t \u003cspan class=pl-s1\u003exlrec\u003c/span\u003e\u003cspan class=pl-c1\u003e-\u0026gt;\u003c/span\u003e\u003cspan class=pl-c1\u003edbid\u003c/span\u003e, \u003cspan class=pl-s1\u003exlrec\u003c/span\u003e\u003cspan class=pl-c1\u003e-\u0026gt;\u003c/span\u003e\u003cspan class=pl-c1\u003etsid\u003c/span\u003e, \u003cspan class=pl-s1\u003edbpath\u003c/span\u003e);","displayNoNewLineWarning":false,"position":11,"left":1038,"right":1038},{"stylingDirective":null,"type":"ADDITION","blobLineNumber":1039,"text":"+\t\tLWLockRelease(RelationMappingLock);","html":"+\t\t\u003cspan class=pl-en\u003eLWLockRelease\u003c/span\u003e(\u003cspan class=pl-s1\u003eRelationMappingLock\u003c/span\u003e);","displayNoNewLineWarning":false,"position":12,"left":1038,"right":1039},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":1040,"text":" ","html":"\u003cbr\u003e","displayNoNewLineWarning":false,"position":13,"left":1039,"right":1040},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":1041,"text":" \t\tpfree(dbpath);","html":" \t\t\u003cspan class=pl-en\u003epfree\u003c/span\u003e(\u003cspan class=pl-s1\u003edbpath\u003c/span\u003e);","displayNoNewLineWarning":false,"position":14,"left":1040,"right":1041},{"stylingDirective":null,"type":"CONTEXT","blobLineNumber":1042,"text":" \t}","html":" \t}","displayNoNewLineWarning":false,"position":15,"left":1041,"right":1042}],"diffNumber":0,"diffSize":"0 Bytes","isBinary":false,"isTooBig":false,"collapsed":false,"isSubmodule":false,"lineCount":1045,"linesChanged":5,"newTreeEntry":{"lineCount":1045,"path":"src/backend/utils/cache/relmapper.c","mode":100644,"isGenerated":false},"oldTreeEntry":{"lineCount":0,"path":"src/backend/utils/cache/relmapper.c","mode":100644},"linesAdded":3,"linesDeleted":2,"path":"src/backend/utils/cache/relmapper.c","pathDigest":"6551429a4e6761569bc87c773801b443d9092791c5d561e49df4eb39b79bd6dc","status":"MODIFIED","truncatedReason":null,"oldOid":"b6d8d2073f228d9f7198f1f9826d8f6ab643c219","newOid":"9b8ed0f52bffceaccf6fa650ffe482e7da20fbf2","copilotChatReference":null,"deletedSha":"b6d8d2073f228d9f7198f1f9826d8f6ab643c219","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/9b8ed0f52bffceaccf6fa650ffe482e7da20fbf2","fileTreeExpanded":true,"headerInfo":{"additions":3,"deletions":2,"filesChanged":1,"filesChangedString":"1"},"moreDiffsToLoad":false,"asyncDiffLoadInfo":{"startIndex":1,"truncated":false,"byteCount":634,"lineShownCount":16},"commentInfo":{"canComment":false,"locked":false,"canLock":false,"repoArchived":false},"csrf_tokens":{"/users/diffview?diff=split":{"post":"Irf3fXrcW_iDVA3BP7dDn38wDpcpIOeCIAJwoKLkD6vFfhe8PLw1djJZWEBEgm6wrVsI5Uct7QmFnuCjtonajA"},"/users/diffview?diff=unified":{"post":"m5MdLGWeJrQzX9GQPWOmzEqY60skhxLRnev-Uyib0BF8Wv3tI_5IOoJShBFGVovjmPPtOUqKGFo4d25QPPYFNg"},"/notifications/thread":{"post":"WaGhA_tX1CfEVxB995KCTbXS7H0drKfaLL0gmFbkcY7SmILC_Kxu1TLR67bapXILvbKvs_otEG9v0fsuFjLROA"}}},"title":"Another fix to relmapper race condition. · postgres/postgres@9b8ed0f","appPayload":{"helpUrl":"https://docs.github.com","findInDiffWorkerPath":"/assets-cdn/worker/find-in-diff-worker-f6b2312e7da9.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":false}}}

Commit 9b8ed0f

Browse files
committed
Another fix to relmapper race condition.
In previous commit, I missed that relmap_redo() was also not acquiring the RelationMappingLock. Thanks to Thomas Munro for pointing that out. Backpatch-through: 9.6, like previous commit. Discussion: https://www.postgresql.org/message-id/CA%2BhUKGLev%3DPpOSaL3WRZgOvgk217et%2BbxeJcRr4eR-NttP1F6Q%40mail.gmail.com
1 parent b6d8d20 commit 9b8ed0f

File tree

1 file changed

+3
-2
lines changed

1 file changed

+3
-2
lines changed

src/backend/utils/cache/relmapper.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1030,12 +1030,13 @@ relmap_redo(XLogReaderState *record)
10301030
* preserve files, either.
10311031
*
10321032
* There shouldn't be anyone else updating relmaps during WAL replay,
1033-
* so we don't bother to take the RelationMappingLock. We would need
1034-
* to do so if load_relmap_file needed to interlock against writers.
1033+
* but grab the lock to interlock against load_relmap_file().
10351034
*/
1035+
LWLockAcquire(RelationMappingLock, LW_EXCLUSIVE 5E03 );
10361036
write_relmap_file((xlrec->dbid == InvalidOid), &newmap,
10371037
false, true, false,
10381038
xlrec->dbid, xlrec->tsid, dbpath);
1039+
LWLockRelease(RelationMappingLock);
10391040

10401041
pfree(dbpath);
10411042
}

0 commit comments

Comments
 (0)
0