postgres.git / summary / log / commit / refs
commit 93078e63f77979df3752e8ef5392dd384117e0ef
Author: Noah Misch <noah@leadboat.com>
Date: Sat Jan 11 02:31:22 2020 +0000
First released in: 13.0, 10.12, 11.7, 12.2
Maintain valid md.c state when FileClose() fails.
FileClose() failure ordinarily causes a PANIC. Suppose the user
disables that PANIC via data_sync_retry=on. After mdclose() issued a
FileClose() that failed, calls into md.c raised SIGSEGV. This fix adds
repalloc() calls during mdclose(); update a comment about ignoring
repalloc() cost. The rate of relation segment count change is a minor
factor; more relevant to overall performance is the rate of mdclose()
and subsequent re-opening of segments. Back-patch to v10, where commit
45e191e3aa62d47a8bc1a33f784286b2051f45cb introduced the bug.
Reviewed by Kyotaro Horiguchi.
Discussion: https://postgr.es/m/20191222091930.GA1280238@rfd.leadboat.com
src/backend/storage/smgr/md.c | 20 ++++++--------------
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index 101fe6c399d..050cee5f9a9 100644
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -491,18 +491,10 @@ mdclose(SMgrRelation reln, ForkNumber forknum)
{
MdfdVec *v = &reln->md_seg_fds[forknum][nopensegs - 1];
- /* if not closed already */
- if (v->mdfd_vfd >= 0)
- {
- FileClose(v->mdfd_vfd);
- v->mdfd_vfd = -1;
- }
-
+ FileClose(v->mdfd_vfd);
+ _fdvec_resize(reln, forknum, nopensegs - 1);
nopensegs--;
}
-
- /* resize just once, avoids pointless reallocations */
- _fdvec_resize(reln, forknum, 0);
}
/*
@@ -1025,10 +1017,10 @@ _fdvec_resize(SMgrRelation reln,
else
{
/*
- * It doesn't seem worthwhile complicating the code by having a more
- * aggressive growth strategy here; the number of segments doesn't
- * grow that fast, and the memory context internally will sometimes
- * avoid doing an actual reallocation.
+ * It doesn't seem worthwhile complicating the code to amortize
+ * repalloc() calls. Those are far faster than PathNameOpenFile() or
+ * FileClose(), and the memory context internally will sometimes avoid
+ * doing an actual reallocation.
*/
reln->md_seg_fds[forknum] =
repalloc(reln->md_seg_fds[forknum],
[parent: 2e89a1248db5]