diff options
| author | Noah Misch <noah@leadboat.com> | 2020-01-10 18:31:22 -0800 |
|---|---|---|
| committer | Noah Misch <noah@leadboat.com> | 2020-01-10 18:31:25 -0800 |
| commit | 93078e63f77979df3752e8ef5392dd384117e0ef (patch) | |
| tree | 888483f57f7729e0253bb3f78f234f479ab2546a /src | |
| parent | 2e89a1248db51bebd01f41011d439cdf9832eca8 (diff) | |
| download | postgresql-93078e63f77979df3752e8ef5392dd384117e0ef.tar.gz postgresql-93078e63f77979df3752e8ef5392dd384117e0ef.zip | |
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
Diffstat (limited to 'src')
| -rw-r--r-- | src/backend/storage/smgr/md.c | 20 |
1 files 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], |