Improve tuplestore's error messages for I/O failures.

We should report the errno when we get a failure from functions like
BufFileWrite.  "ERROR: write failed" is unreasonably taciturn for a
case that's well within the realm of possibility; I've seen it a
couple times in the buildfarm recently, in situations that were
probably out-of-disk-space, but it'd be good to see the errno
to confirm it.

I think this code was originally written without assuming that
the buffile.c functions would return useful errno; but most other
callers *are* assuming that, and a quick look at the buffile code
gives no reason to suppose otherwise.

Also, a couple of the old messages were phrased on the assumption
that a short read might indicate a logic bug in tuplestore itself;
but that code's pretty well tested by now, so a filesystem-level
problem seems much more likely.

diff --git a/src/backend/utils/sort/logtape.c b/src/backend/utils/sort/logtape.c
index 106b917f72fc367c5377ec6d8b7555c4cf7e8227..62726c7a63ace1c0b3c420025c33c9849fca43ca 100644 (file)
--- a/src/backend/utils/sort/logtape.c
+++ b/src/backend/utils/sort/logtape.c
@@ -208,11 +208,9 @@ ltsWriteBlock(LogicalTapeSet *lts, long blocknum, void *buffer)
        if (BufFileSeekBlock(lts->pfile, blocknum) != 0 ||
                BufFileWrite(lts->pfile, buffer, BLCKSZ) != BLCKSZ)
                ereport(ERROR,
-               /* XXX is it okay to assume errno is correct? */
                                (errcode_for_file_access(),
                                 errmsg("could not write block %ld of temporary file: %m",
-                                               blocknum),
-                                errhint("Perhaps out of disk space?")));
+                                               blocknum)));
 }
 
 /*
@@ -227,7 +225,6 @@ ltsReadBlock(LogicalTapeSet *lts, long blocknum, void *buffer)
        if (BufFileSeekBlock(lts->pfile, blocknum) != 0 ||
                BufFileRead(lts->pfile, buffer, BLCKSZ) != BLCKSZ)
                ereport(ERROR,
-               /* XXX is it okay to assume errno is correct? */
                                (errcode_for_file_access(),
                                 errmsg("could not read block %ld of temporary file: %m",
                                                blocknum)));

diff --git a/src/backend/utils/sort/tuplestore.c b/src/backend/utils/sort/tuplestore.c
index 8b968a8b62f9eda3c7ca690a5efe8ac1aedcbb84..1d6fe869944d90bbca7db424bc788377b355bd41 100644 (file)
--- a/src/backend/utils/sort/tuplestore.c
+++ b/src/backend/utils/sort/tuplestore.c
@@ -501,7 +501,9 @@ tuplestore_select_read_pointer(Tuplestorestate *state, int ptr)
                                                                state->writepos_file,
                                                                state->writepos_offset,
                                                                SEEK_SET) != 0)
-                                       elog(ERROR, "tuplestore seek failed");
+                                       ereport(ERROR,
+                                                       (errcode_for_file_access(),
+                                                        errmsg("could not seek in tuplestore temporary file: %m")));
                        }
                        else
                        {
@@ -509,7 +511,9 @@ tuplestore_select_read_pointer(Tuplestorestate *state, int ptr)
                                                                readptr->file,
                                                                readptr->offset,
                                                                SEEK_SET) != 0)
-                                       elog(ERROR, "tuplestore seek failed");
+                                       ereport(ERROR,
+                                                       (errcode_for_file_access(),
+                                                        errmsg("could not seek in tuplestore temporary file: %m")));
                        }
                        break;
                default:
@@ -834,7 +838,9 @@ tuplestore_puttuple_common(Tuplestorestate *state, void *tuple)
                        if (BufFileSeek(state->myfile,
                                                        state->writepos_file, state->writepos_offset,
                                                        SEEK_SET) != 0)
-                               elog(ERROR, "tuplestore seek to EOF failed");
+                               ereport(ERROR,
+                                               (errcode_for_file_access(),
+                                errmsg("could not seek in tuplestore temporary file: %m")));
                        state->status = TSS_WRITEFILE;
 
                        /*
@@ -936,7 +942,9 @@ tuplestore_gettuple(Tuplestorestate *state, bool forward,
                                if (BufFileSeek(state->myfile,
                                                                readptr->file, readptr->offset,
                                                                SEEK_SET) != 0)
-                                       elog(ERROR, "tuplestore seek failed");
+                                       ereport(ERROR,
+                                                       (errcode_for_file_access(),
+                                                        errmsg("could not seek in tuplestore temporary file: %m")));
                        state->status = TSS_READFILE;
                        /* FALL THRU into READFILE case */
 
@@ -998,7 +1006,9 @@ tuplestore_gettuple(Tuplestorestate *state, bool forward,
                                        if (BufFileSeek(state->myfile, 0,
                                                                        -(long) (tuplen + sizeof(unsigned int)),
                                                                        SEEK_CUR) != 0)
-                                               elog(ERROR, "bogus tuple length in backward scan");
+                                               ereport(ERROR,
+                                                               (errcode_for_file_access(),
+                                                                errmsg("could not seek in tuplestore temporary file: %m")));
                                        Assert(!state->truncated);
                                        return NULL;
                                }
@@ -1013,7 +1023,9 @@ tuplestore_gettuple(Tuplestorestate *state, bool forward,
                        if (BufFileSeek(state->myfile, 0,
                                                        -(long) tuplen,
                                                        SEEK_CUR) != 0)
-                               elog(ERROR, "bogus tuple length in backward scan");
+                               ereport(ERROR,
+                                               (errcode_for_file_access(),
+                                errmsg("could not seek in tuplestore temporary file: %m")));
                        tup = READTUP(state, tuplen);
                        return tup;
 
@@ -1213,7 +1225,9 @@ tuplestore_rescan(Tuplestorestate *state)
                case TSS_READFILE:
                        readptr->eof_reached = false;
                        if (BufFileSeek(state->myfile, 0, 0L, SEEK_SET) != 0)
-                               elog(ERROR, "tuplestore seek to start failed");
+                               ereport(ERROR,
+                                               (errcode_for_file_access(),
+                                errmsg("could not seek in tuplestore temporary file: %m")));
                        break;
                default:
                        elog(ERROR, "invalid tuplestore state");
@@ -1276,14 +1290,18 @@ tuplestore_copy_read_pointer(Tuplestorestate *state,
                                                                        state->writepos_file,
                                                                        state->writepos_offset,
                                                                        SEEK_SET) != 0)
-                                               elog(ERROR, "tuplestore seek failed");
+                                               ereport(ERROR,
+                                                               (errcode_for_file_access(),
+                                                                errmsg("could not seek in tuplestore temporary file: %m")));
                                }
                                else
                                {
                                        if (BufFileSeek(state->myfile,
                                                                        dptr->file, dptr->offset,
                                                                        SEEK_SET) != 0)
-                                               elog(ERROR, "tuplestore seek failed");
+                                               ereport(ERROR,
+                                                               (errcode_for_file_access(),
+                                                                errmsg("could not seek in tuplestore temporary file: %m")));
                                }
                        }
                        else if (srcptr == state->activeptr)
@@ -1427,10 +1445,10 @@ getlen(Tuplestorestate *state, bool eofOK)
        nbytes = BufFileRead(state->myfile, (void *) &len, sizeof(len));
        if (nbytes == sizeof(len))
                return len;
-       if (nbytes != 0)
-               elog(ERROR, "unexpected end of tape");
-       if (!eofOK)
-               elog(ERROR, "unexpected end of data");
+       if (nbytes != 0 || !eofOK)
+               ereport(ERROR,
+                               (errcode_for_file_access(),
+                          errmsg("could not read from tuplestore temporary file: %m")));
        return 0;
 }
 
@@ -1469,14 +1487,20 @@ writetup_heap(Tuplestorestate *state, void *tup)
 
        if (BufFileWrite(state->myfile, (void *) &tuplen,
                                         sizeof(tuplen)) != sizeof(tuplen))
-               elog(ERROR, "write failed");
+               ereport(ERROR,
+                               (errcode_for_file_access(),
+                                errmsg("could not write to tuplestore temporary file: %m")));
        if (BufFileWrite(state->myfile, (void *) tupbody,
                                         tupbodylen) != (size_t) tupbodylen)
-               elog(ERROR, "write failed");
+               ereport(ERROR,
+                               (errcode_for_file_access(),
+                                errmsg("could not write to tuplestore temporary file: %m")));
        if (state->backward)            /* need trailing length word? */
                if (BufFileWrite(state->myfile, (void *) &tuplen,
                                                 sizeof(tuplen)) != sizeof(tuplen))
-                       elog(ERROR, "write failed");
+                       ereport(ERROR,
+                                       (errcode_for_file_access(),
+                               errmsg("could not write to tuplestore temporary file: %m")));
 
        FREEMEM(state, GetMemoryChunkSpace(tuple));
        heap_free_minimal_tuple(tuple);
@@ -1495,10 +1519,14 @@ readtup_heap(Tuplestorestate *state, unsigned int len)
        tuple->t_len = tuplen;
        if (BufFileRead(state->myfile, (void *) tupbody,
                                        tupbodylen) != (size_t) tupbodylen)
-               elog(ERROR, "unexpected end of data");
+               ereport(ERROR,
+                               (errcode_for_file_access(),
+                          errmsg("could not read from tuplestore temporary file: %m")));
        if (state->backward)            /* need trailing length word? */
                if (BufFileRead(state->myfile, (void *) &tuplen,
                                                sizeof(tuplen)) != sizeof(tuplen))
-                       elog(ERROR, "unexpected end of data");
+                       ereport(ERROR,
+                                       (errcode_for_file_access(),
+                          errmsg("could not read from tuplestore temporary file: %m")));
        return (void *) tuple;
 }