From 3ba1776c568e36423ee66892481b6d1703f4fbd8 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 12 Jun 2014 18:59:06 -0400 Subject: [PATCH] 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. --- src/backend/utils/sort/logtape.c | 5 +-- src/backend/utils/sort/tuplestore.c | 64 +++++++++++++++++++++-------- 2 files changed, 47 insertions(+), 22 deletions(-) diff --git a/src/backend/utils/sort/logtape.c b/src/backend/utils/sort/logtape.c index 106b917f72..62726c7a63 100644 --- 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 8b968a8b62..1d6fe86994 100644 --- 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; } -- 2.40.0