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

diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index 199c7221dd6429bdf62e7edf2ed62f7966e2bdfe..85b7115400621307182d058202f0cbc16bea4798 100644 (file)
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -516,18 +516,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);
 }
 
 /*
@@ -1050,10 +1042,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],