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;
 }