On 2016-05-02 12:44:53 -0400, Robert Haas wrote:
> On Mon, May 2, 2016 at 12:41 PM, Andres Freund <[email protected]> wrote:
> > On 2016-05-02 12:29:45 -0400, Robert Haas wrote:
> >> On Fri, Apr 29, 2016 at 7:58 PM, Andres Freund <[email protected]> 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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
