Improve error messages in md.c. When a filesystem operation like open() or

fsync() fails, say "file" rather than "relation" when printing the filename.

This makes messages that display block numbers a bit confusing. For example,
in message 'could not read block 150000 of file "base/1234/5678.1"', 150000
is the block number from the beginning of the relation, ie. segment 0, not
150000th block within that segment. Per discussion, users aren't usually
interested in the exact location within the file, so we can live with that.

To ease constructing error messages, add FilePathName(File) function to
return the pathname of a virtual fd.

diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index e0742701f1ebf20a96c3969214116a12d24f708a..b608b2c311b2beccaaafcb9135680780b2da22dc 100644 (file)
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/storage/file/fd.c,v 1.149 2009/06/11 14:49:01 momjian Exp $
+ *       $PostgreSQL: pgsql/src/backend/storage/file/fd.c,v 1.150 2009/08/05 18:01:54 heikki Exp $
  *
  * NOTES:
  *
@@ -1319,6 +1319,20 @@ FileTruncate(File file, off_t offset)
        return returnCode;
 }
 
+/*
+ * Return the pathname associated with an open file.
+ *
+ * The returned string points to an internal buffer, which is valid until
+ * the file is closed.
+ */
+char *
+FilePathName(File file)
+{
+       Assert(FileIsValid(file));
+
+       return VfdCache[file].fileName;
+}
+
 
 /*
  * Routines that want to use stdio (ie, FILE*) should use AllocateFile

diff --git a/src/backend/storage/smgr/md.c b/src/backend/storage/smgr/md.c
index 0c4861d6dbb6a8d4efd0ab859c234478e128e8ee..64cc03ba762bfbb4bc460541f1267e0d1ff4a06e 100644 (file)
--- a/src/backend/storage/smgr/md.c
+++ b/src/backend/storage/smgr/md.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/storage/smgr/md.c,v 1.148 2009/06/26 20:29:04 tgl Exp $
+ *       $PostgreSQL: pgsql/src/backend/storage/smgr/md.c,v 1.149 2009/08/05 18:01:54 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -160,6 +160,8 @@ static void register_dirty_segment(SMgrRelation reln, ForkNumber forknum,
                                           MdfdVec *seg);
 static void register_unlink(RelFileNode rnode);
 static MdfdVec *_fdvec_alloc(void);
+static char *_mdfd_segpath(SMgrRelation reln, ForkNumber forknum,
+                                                  BlockNumber segno);
 static MdfdVec *_mdfd_openseg(SMgrRelation reln, ForkNumber forkno,
                          BlockNumber segno, int oflags);
 static MdfdVec *_mdfd_getseg(SMgrRelation reln, ForkNumber forkno,
@@ -273,7 +275,7 @@ mdcreate(SMgrRelation reln, ForkNumber forkNum, bool isRedo)
                        errno = save_errno;
                        ereport(ERROR,
                                        (errcode_for_file_access(),
-                                        errmsg("could not create relation %s: %m", path)));
+                                        errmsg("could not create file \"%s\": %m", path)));
                }
        }
 
@@ -336,7 +338,16 @@ mdunlink(RelFileNode rnode, ForkNumber forkNum, bool isRedo)
         * Delete or truncate the first segment.
         */
        if (isRedo || forkNum != MAIN_FORKNUM)
+       {
                ret = unlink(path);
+               if (ret < 0)
+               {
+                       if (!isRedo || errno != ENOENT)
+                               ereport(WARNING,
+                                               (errcode_for_file_access(),
+                                                errmsg("could not remove file \"%s\": %m", path)));
+               }
+       }
        else
        {
                /* truncate(2) would be easier here, but Windows hasn't got it */
@@ -354,19 +365,16 @@ mdunlink(RelFileNode rnode, ForkNumber forkNum, bool isRedo)
                }
                else
                        ret = -1;
-       }
-       if (ret < 0)
-       {
-               if (!isRedo || errno != ENOENT)
+               if (ret < 0 && errno != ENOENT)
                        ereport(WARNING,
                                        (errcode_for_file_access(),
-                                        errmsg("could not remove relation %s: %m", path)));
+                                        errmsg("could not truncate file \"%s\": %m", path)));
        }
 
        /*
         * Delete any additional segments.
         */
-       else
+       if (ret >= 0)
        {
                char       *segpath = (char *) palloc(strlen(path) + 12);
                BlockNumber segno;
@@ -384,8 +392,7 @@ mdunlink(RelFileNode rnode, ForkNumber forkNum, bool isRedo)
                                if (errno != ENOENT)
                                        ereport(WARNING,
                                                        (errcode_for_file_access(),
-                                        errmsg("could not remove segment %u of relation %s: %m",
-                                                       segno, path)));
+                                        errmsg("could not remove file \"%s\": %m", segpath)));
                                break;
                        }
                }
@@ -429,7 +436,7 @@ mdextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
        if (blocknum == InvalidBlockNumber)
                ereport(ERROR,
                                (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
-                                errmsg("cannot extend relation %s beyond %u blocks",
+                                errmsg("cannot extend file \"%s\" beyond %u blocks",
                                                relpath(reln->smgr_rnode, forknum),
                                                InvalidBlockNumber)));
 
@@ -451,23 +458,22 @@ mdextend(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
        if (FileSeek(v->mdfd_vfd, seekpos, SEEK_SET) != seekpos)
                ereport(ERROR,
                                (errcode_for_file_access(),
-                                errmsg("could not seek to block %u of relation %s: %m",
-                                               blocknum,
-                                               relpath(reln->smgr_rnode, forknum))));
+                                errmsg("could not seek to block %u in file \"%s\": %m",
+                                               blocknum, FilePathName(v->mdfd_vfd))));
 
        if ((nbytes = FileWrite(v->mdfd_vfd, buffer, BLCKSZ)) != BLCKSZ)
        {
                if (nbytes < 0)
                        ereport(ERROR,
                                        (errcode_for_file_access(),
-                                        errmsg("could not extend relation %s: %m",
-                                                       relpath(reln->smgr_rnode, forknum)),
+                                        errmsg("could not extend file \"%s\": %m",
+                                                       FilePathName(v->mdfd_vfd)),
                                         errhint("Check free disk space.")));
                /* short write: complain appropriately */
                ereport(ERROR,
                                (errcode(ERRCODE_DISK_FULL),
-                                errmsg("could not extend relation %s: wrote only %d of %d bytes at block %u",
-                                               relpath(reln->smgr_rnode, forknum),
+                                errmsg("could not extend file \"%s\": wrote only %d of %d bytes at block %u",
+                                               FilePathName(v->mdfd_vfd),
                                                nbytes, BLCKSZ, blocknum),
                                 errhint("Check free disk space.")));
        }
@@ -523,7 +529,7 @@ mdopen(SMgrRelation reln, ForkNumber forknum, ExtensionBehavior behavior)
                        }
                        ereport(ERROR,
                                        (errcode_for_file_access(),
-                                        errmsg("could not open relation %s: %m", path)));
+                                        errmsg("could not open file \"%s\": %m", path)));
                }
        }
 
@@ -612,8 +618,8 @@ mdread(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
        if (FileSeek(v->mdfd_vfd, seekpos, SEEK_SET) != seekpos)
                ereport(ERROR,
                                (errcode_for_file_access(),
-                                errmsg("could not seek to block %u of relation %s: %m",
-                                               blocknum, relpath(reln->smgr_rnode, forknum))));
+                                errmsg("could not seek to block %u in file \"%s\": %m",
+                                               blocknum, FilePathName(v->mdfd_vfd))));
 
        nbytes = FileRead(v->mdfd_vfd, buffer, BLCKSZ);
 
@@ -629,8 +635,8 @@ mdread(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
                if (nbytes < 0)
                        ereport(ERROR,
                                        (errcode_for_file_access(),
-                                        errmsg("could not read block %u of relation %s: %m",
-                                                       blocknum, relpath(reln->smgr_rnode, forknum))));
+                                        errmsg("could not read block %u in file \"%s\": %m",
+                                                       blocknum, FilePathName(v->mdfd_vfd))));
 
                /*
                 * Short read: we are at or past EOF, or we read a partial block at
@@ -645,8 +651,8 @@ mdread(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
                else
                        ereport(ERROR,
                                        (errcode(ERRCODE_DATA_CORRUPTED),
-                                        errmsg("could not read block %u of relation %s: read only %d of %d bytes",
-                                                       blocknum, relpath(reln->smgr_rnode, forknum),
+                                        errmsg("could not read block %u in file \"%s\": read only %d of %d bytes",
+                                                       blocknum, FilePathName(v->mdfd_vfd),
                                                        nbytes, BLCKSZ)));
        }
 }
@@ -685,8 +691,8 @@ mdwrite(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
        if (FileSeek(v->mdfd_vfd, seekpos, SEEK_SET) != seekpos)
                ereport(ERROR,
                                (errcode_for_file_access(),
-                                errmsg("could not seek to block %u of relation %s: %m",
-                                               blocknum, relpath(reln->smgr_rnode, forknum))));
+                                errmsg("could not seek to block %u in file \"%s\": %m",
+                                               blocknum, FilePathName(v->mdfd_vfd))));
 
        nbytes = FileWrite(v->mdfd_vfd, buffer, BLCKSZ);
 
@@ -702,14 +708,14 @@ mdwrite(SMgrRelation reln, ForkNumber forknum, BlockNumber blocknum,
                if (nbytes < 0)
                        ereport(ERROR,
                                        (errcode_for_file_access(),
-                                        errmsg("could not write block %u of relation %s: %m",
-                                                       blocknum, relpath(reln->smgr_rnode, forknum))));
+                                        errmsg("could not write block %u in file \"%s\": %m",
+                                                       blocknum, FilePathName(v->mdfd_vfd))));
                /* short write: complain appropriately */
                ereport(ERROR,
                                (errcode(ERRCODE_DISK_FULL),
-                                errmsg("could not write block %u of relation %s: wrote only %d of %d bytes",
+                                errmsg("could not write block %u in file \"%s\": wrote only %d of %d bytes",
                                                blocknum,
-                                               relpath(reln->smgr_rnode, forknum),
+                                               FilePathName(v->mdfd_vfd),
                                                nbytes, BLCKSZ),
                                 errhint("Check free disk space.")));
        }
@@ -776,9 +782,8 @@ mdnblocks(SMgrRelation reln, ForkNumber forknum)
                        if (v->mdfd_chain == NULL)
                                ereport(ERROR,
                                                (errcode_for_file_access(),
-                                          errmsg("could not open segment %u of relation %s: %m",
-                                                         segno,
-                                                         relpath(reln->smgr_rnode, forknum))));
+                                                errmsg("could not open file \"%s\": %m",
+                                                               _mdfd_segpath(reln, forknum, segno))));
                }
 
                v = v->mdfd_chain;
@@ -807,7 +812,7 @@ mdtruncate(SMgrRelation reln, ForkNumber forknum, BlockNumber nblocks,
                if (InRecovery)
                        return;
                ereport(ERROR,
-                               (errmsg("could not truncate relation %s to %u blocks: it's only %u blocks now",
+                               (errmsg("could not truncate file \"%s\" to %u blocks: it's only %u blocks now",
                                                relpath(reln->smgr_rnode, forknum),
                                                nblocks, curnblk)));
        }
@@ -831,9 +836,9 @@ mdtruncate(SMgrRelation reln, ForkNumber forknum, BlockNumber nblocks,
                        if (FileTruncate(v->mdfd_vfd, 0) < 0)
                                ereport(ERROR,
                                                (errcode_for_file_access(),
-                                       errmsg("could not truncate relation %s to %u blocks: %m",
-                                                  relpath(reln->smgr_rnode, forknum),
-                                                  nblocks)));
+                                                errmsg("could not truncate file \"%s\": %m",
+                                                               FilePathName(v->mdfd_vfd))));
+
                        if (!isTemp)
                                register_dirty_segment(reln, forknum, v);
                        v = v->mdfd_chain;
@@ -856,8 +861,8 @@ mdtruncate(SMgrRelation reln, ForkNumber forknum, BlockNumber nblocks,
                        if (FileTruncate(v->mdfd_vfd, (off_t) lastsegblocks * BLCKSZ) < 0)
                                ereport(ERROR,
                                                (errcode_for_file_access(),
-                                       errmsg("could not truncate relation %s to %u blocks: %m",
-                                                  relpath(reln->smgr_rnode, forknum),
+                                       errmsg("could not truncate file \"%s\" to %u blocks: %m",
+                                                  FilePathName(v->mdfd_vfd),
                                                   nblocks)));
                        if (!isTemp)
                                register_dirty_segment(reln, forknum, v);
@@ -901,9 +906,8 @@ mdimmedsync(SMgrRelation reln, ForkNumber forknum)
                if (FileSync(v->mdfd_vfd) < 0)
                        ereport(ERROR,
                                        (errcode_for_file_access(),
-                                        errmsg("could not fsync segment %u of relation %s: %m",
-                                                       v->mdfd_segno,
-                                                       relpath(reln->smgr_rnode, forknum))));
+                                        errmsg("could not fsync file \"%s\": %m",
+                                                       FilePathName(v->mdfd_vfd))));
                v = v->mdfd_chain;
        }
 }
@@ -1070,18 +1074,18 @@ mdsync(void)
                                 * Don't see one at the moment, but easy to change the test
                                 * here if so.
                                 */
-                               path = relpath(entry->tag.rnode, entry->tag.forknum);
+                               path = _mdfd_segpath(reln, entry->tag.forknum,
+                                                                        entry->tag.segno);
                                if (!FILE_POSSIBLY_DELETED(errno) ||
                                        failures > 0)
                                        ereport(ERROR,
                                                        (errcode_for_file_access(),
-                                         errmsg("could not fsync segment %u of relation %s: %m",
-                                                        entry->tag.segno, path)));
+                                                        errmsg("could not fsync file \"%s\": %m", path)));
                                else
                                        ereport(DEBUG1,
                                                        (errcode_for_file_access(),
-                                                        errmsg("could not fsync segment %u of relation %s but retrying: %m",
-                                                                       entry->tag.segno, path)));
+                                                        errmsg("could not fsync file \"%s\" but retrying: %m",
+                                                                       path)));
                                pfree(path);
 
                                /*
@@ -1185,7 +1189,7 @@ mdpostckpt(void)
                        if (errno != ENOENT)
                                ereport(WARNING,
                                                (errcode_for_file_access(),
-                                                errmsg("could not remove relation %s: %m", path)));
+                                                errmsg("could not remove file \"%s\": %m", path)));
                }
                pfree(path);
 
@@ -1219,9 +1223,8 @@ register_dirty_segment(SMgrRelation reln, ForkNumber forknum, MdfdVec *seg)
                if (FileSync(seg->mdfd_vfd) < 0)
                        ereport(ERROR,
                                        (errcode_for_file_access(),
-                                        errmsg("could not fsync segment %u of relation %s: %m",
-                                                       seg->mdfd_segno,
-                                                       relpath(reln->smgr_rnode, forknum))));
+                                        errmsg("could not fsync file \"%s\": %m",
+                                                       FilePathName(seg->mdfd_vfd))));
        }
 }
 
@@ -1456,17 +1459,14 @@ _fdvec_alloc(void)
 }
 
 /*
- * Open the specified segment of the relation,
- * and make a MdfdVec object for it.  Returns NULL on failure.
+ * Return the filename for the specified segment of the relation. The
+ * returned string is palloc'd.
  */
-static MdfdVec *
-_mdfd_openseg(SMgrRelation reln, ForkNumber forknum, BlockNumber segno,
-                         int oflags)
+static char *
+_mdfd_segpath(SMgrRelation reln, ForkNumber forknum, BlockNumber segno)
 {
-       MdfdVec    *v;
-       int                     fd;
-       char       *path,
-                          *fullpath;
+       char   *path,
+                  *fullpath;
 
        path = relpath(reln->smgr_rnode, forknum);
 
@@ -1480,6 +1480,23 @@ _mdfd_openseg(SMgrRelation reln, ForkNumber forknum, BlockNumber segno,
        else
                fullpath = path;
 
+       return fullpath;
+}
+
+/*
+ * Open the specified segment of the relation,
+ * and make a MdfdVec object for it.  Returns NULL on failure.
+ */
+static MdfdVec *
+_mdfd_openseg(SMgrRelation reln, ForkNumber forknum, BlockNumber segno,
+                         int oflags)
+{
+       MdfdVec    *v;
+       int                     fd;
+       char       *fullpath;
+
+       fullpath = _mdfd_segpath(reln, forknum, segno);
+
        /* open the file */
        fd = PathNameOpenFile(fullpath, O_RDWR | PG_BINARY | oflags, 0600);
 
@@ -1566,9 +1583,8 @@ _mdfd_getseg(SMgrRelation reln, ForkNumber forknum, BlockNumber blkno,
                                        return NULL;
                                ereport(ERROR,
                                                (errcode_for_file_access(),
-                                                errmsg("could not open segment %u of relation %s (target block %u): %m",
-                                                               nextsegno,
-                                                               relpath(reln->smgr_rnode, forknum),
+                                                errmsg("could not open file \"%s\" (target block %u): %m",
+                                                               _mdfd_segpath(reln, forknum, nextsegno),
                                                                blkno)));
                        }
                }
@@ -1589,8 +1605,8 @@ _mdnblocks(SMgrRelation reln, ForkNumber forknum, MdfdVec *seg)
        if (len < 0)
                ereport(ERROR,
                                (errcode_for_file_access(),
-                        errmsg("could not seek to end of segment %u of relation %s: %m",
-                                       seg->mdfd_segno, relpath(reln->smgr_rnode, forknum))));
+                                errmsg("could not seek to end of file \"%s\": %m",
+                                               FilePathName(seg->mdfd_vfd))));
        /* note that this calculation will ignore any partial block at EOF */
        return (BlockNumber) (len / BLCKSZ);
 }

diff --git a/src/include/storage/fd.h b/src/include/storage/fd.h
index 98d091c97872db05de18c1dcb99eb6cc77d6b9e5..4924c61a6dbfb70ba2d8aa408b3df44d90ce89a9 100644 (file)
--- a/src/include/storage/fd.h
+++ b/src/include/storage/fd.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2009, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/storage/fd.h,v 1.64 2009/01/12 05:10:45 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/storage/fd.h,v 1.65 2009/08/05 18:01:54 heikki Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -68,6 +68,7 @@ extern int    FileWrite(File file, char *buffer, int amount);
 extern int     FileSync(File file);
 extern off_t FileSeek(File file, off_t offset, int whence);
 extern int     FileTruncate(File file, off_t offset);
+extern char *FilePathName(File file);
 
 /* Operations that allow use of regular stdio --- USE WITH CAUTION */
 extern FILE *AllocateFile(const char *name, const char *mode);