On 2016-05-02 12:44:53 -0400, Robert Haas wrote: > On Mon, May 2, 2016 at 12:41 PM, Andres Freund <and...@anarazel.de> wrote: > > On 2016-05-02 12:29:45 -0400, Robert Haas wrote: > >> On Fri, Apr 29, 2016 at 7:58 PM, Andres Freund <and...@anarazel.de> wrote: > >> > Basically the reason for the problem is that mdsync() needs to access > >> > "formally non-existant segments" (as in ones where previous segments are > >> > < RELSEG_SIZE), because we queue (and the might be preexistant) fsync > >> > requests via register_dirty_segment() in mdtruncate(). > >> > >> Shouldn't we just throw those flush requests away? > > > > Well, we explicity make them for truncations (register_dirty_segment() > > calls in mdtruncate()). There's no comment as to why - I suspect the > > idea is that you want to make sure the truncation sticks in case of > > crash? > > I dunno, I don't understand this well enough yet. > > > FWIW, falling back to _mdfd_openseg() fixes the issue. > > Can you post a patch?
Sure, attached. I'm not sure this is the best way to go about this. I can see valid arguments for *always* using _mdfd_openseg() in mdsync(); and I'm wondering whether we shouldn't make EXTENSION_* into a bitmask (extend,extend_recovery,return_null,open_deleted). Andres
diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c index 2981b41..3774fb0 100644 --- a/src/backend/storage/smgr/md.c +++ b/src/backend/storage/smgr/md.c @@ -1159,6 +1159,7 @@ mdsync(void) while ((segno = bms_first_member(requests)) >= 0) { int failures; + bool opened_directly = false; /* * If fsync is off then we don't have to bother opening the @@ -1223,6 +1224,23 @@ mdsync(void) INSTR_TIME_SET_CURRENT(sync_start); + /* + * _mdfd_getseg() will only open segments which aren't + * preceded by non-truncated segments (c.f. notes about + * RELSEG_SIZE at the top of this file). But there are + * some cases, e.g. mdtruncate, where truncated segments + * are to be fsynced: Thus open those explicitly here. We + * avoid always using _mdfd_openseg() because using + * _mdfd_getseg() avoids some open()/close() calls if the + * underlying files are already open. + */ + if (seg == NULL) + { + seg = _mdfd_openseg(reln, forknum, segno, + EXTENSION_RETURN_NULL); + opened_directly = true; + } + if (seg != NULL && FileSync(seg->mdfd_vfd) >= 0) { @@ -1241,6 +1259,13 @@ mdsync(void) FilePathName(seg->mdfd_vfd), (double) elapsed / 1000); + /* free resources if explicitly opened above */ + if (opened_directly) + { + if (seg->mdfd_vfd >= 0) + FileClose(seg->mdfd_vfd); + pfree(seg); + } break; /* out of retry loop */ }
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers