From b198a3d98ce754c265096c6cbdf67f0700558afb Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Sat, 30 Nov 2019 13:49:01 +1300 Subject: [PATCH 3/3] Align BufFileWrite()'s error reporting with BufFileRead()'s. BufFileRead() now uses ereport() for I/O errors, and otherwise returns the number of bytes read. Let's use ereport for BufFileWrite() too, for symmetry. While this removes the ability of the caller to report a more informative error message, in practice the messages were all pretty generic anyway. Since error detection was the only use for the return value, change the return type to void. --- src/backend/access/gist/gistbuildbuffers.c | 12 +---------- src/backend/executor/nodeHashjoin.c | 14 ++---------- src/backend/storage/file/buffile.c | 25 +++++++++------------- src/backend/utils/sort/logtape.c | 6 +++--- src/backend/utils/sort/sharedtuplestore.c | 7 +----- src/backend/utils/sort/tuplestore.c | 18 +++------------- src/include/storage/buffile.h | 2 +- 7 files changed, 21 insertions(+), 63 deletions(-) diff --git a/src/backend/access/gist/gistbuildbuffers.c b/src/backend/access/gist/gistbuildbuffers.c index 2feec247cf..5d376258fa 100644 --- a/src/backend/access/gist/gistbuildbuffers.c +++ b/src/backend/access/gist/gistbuildbuffers.c @@ -768,15 +768,5 @@ WriteTempFileBlock(BufFile *file, long blknum, void *ptr) { if (BufFileSeekBlock(file, blknum) != 0) elog(ERROR, "could not seek temporary file: %m"); - if (BufFileWrite(file, ptr, BLCKSZ) != BLCKSZ) - { - /* - * the other errors in Read/WriteTempFileBlock shouldn't happen, but - * an error at write can easily happen if you run out of disk space. - */ - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not write block %ld of temporary file: %m", - blknum))); - } + BufFileWrite(file, ptr, BLCKSZ); } diff --git a/src/backend/executor/nodeHashjoin.c b/src/backend/executor/nodeHashjoin.c index 35a181632b..d62b6e8954 100644 --- a/src/backend/executor/nodeHashjoin.c +++ b/src/backend/executor/nodeHashjoin.c @@ -1200,7 +1200,6 @@ ExecHashJoinSaveTuple(MinimalTuple tuple, uint32 hashvalue, BufFile **fileptr) { BufFile *file = *fileptr; - size_t written; if (file == NULL) { @@ -1209,17 +1208,8 @@ ExecHashJoinSaveTuple(MinimalTuple tuple, uint32 hashvalue, *fileptr = file; } - written = BufFileWrite(file, (void *) &hashvalue, sizeof(uint32)); - if (written != sizeof(uint32)) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not write to hash-join temporary file: %m"))); - - written = BufFileWrite(file, (void *) tuple, tuple->t_len); - if (written != tuple->t_len) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not write to hash-join temporary file: %m"))); + BufFileWrite(file, (void *) &hashvalue, sizeof(uint32)); + BufFileWrite(file, (void *) tuple, tuple->t_len); } /* diff --git a/src/backend/storage/file/buffile.c b/src/backend/storage/file/buffile.c index 1706f9d4d5..e7f7c9e253 100644 --- a/src/backend/storage/file/buffile.c +++ b/src/backend/storage/file/buffile.c @@ -497,7 +497,10 @@ BufFileDumpBuffer(BufFile *file) file->curOffset, WAIT_EVENT_BUFFILE_WRITE); if (bytestowrite <= 0) - return; /* failed to write */ + ereport(ERROR, + (errcode_for_file_access(), + errmsg("could not write to \"%s\" : %m", + FilePathName(thisfile)))); file->curOffset += bytestowrite; wpos += bytestowrite; @@ -576,9 +579,10 @@ BufFileRead(BufFile *file, void *ptr, size_t size) /* * BufFileWrite * - * Like fwrite() except we assume 1-byte element size. + * Like fwrite() except we assume 1-byte element size and report errors via + * ereport(). */ -size_t +void BufFileWrite(BufFile *file, void *ptr, size_t size) { size_t nwritten = 0; @@ -592,11 +596,7 @@ BufFileWrite(BufFile *file, void *ptr, size_t size) { /* Buffer full, dump it out */ if (file->dirty) - { BufFileDumpBuffer(file); - if (file->dirty) - break; /* I/O error */ - } else { /* Hmm, went directly from reading to writing? */ @@ -621,8 +621,6 @@ BufFileWrite(BufFile *file, void *ptr, size_t size) size -= nthistime; nwritten += nthistime; } - - return nwritten; } /* @@ -634,13 +632,9 @@ static void BufFileFlush(BufFile *file) { if (file->dirty) - { BufFileDumpBuffer(file); - if (file->dirty) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not write to temporary file: %m"))); - } + + Assert(!file->dirty); } /* @@ -649,6 +643,7 @@ BufFileFlush(BufFile *file) * Like fseek(), except that target position needs two values in order to * work when logical filesize exceeds maximum value representable by off_t. * We do not support relative seeks across more than that, however. + * I/O errors are reported by ereport(). * * Result is 0 if OK, EOF if not. Logical position is not moved if an * impossible seek is attempted. diff --git a/src/backend/utils/sort/logtape.c b/src/backend/utils/sort/logtape.c index a410ff82f8..085c203944 100644 --- a/src/backend/utils/sort/logtape.c +++ b/src/backend/utils/sort/logtape.c @@ -248,12 +248,12 @@ ltsWriteBlock(LogicalTapeSet *lts, long blocknum, void *buffer) } /* Write the requested block */ - if (BufFileSeekBlock(lts->pfile, blocknum) != 0 || - BufFileWrite(lts->pfile, buffer, BLCKSZ) != BLCKSZ) + if (BufFileSeekBlock(lts->pfile, blocknum) != 0) ereport(ERROR, (errcode_for_file_access(), - errmsg("could not write block %ld of temporary file: %m", + errmsg("could not seek to block %ld of temporary file: %m", blocknum))); + BufFileWrite(lts->pfile, buffer, BLCKSZ); /* Update nBlocksWritten, if we extended the file */ if (blocknum == lts->nBlocksWritten) diff --git a/src/backend/utils/sort/sharedtuplestore.c b/src/backend/utils/sort/sharedtuplestore.c index 7765a445c0..c6d09b118f 100644 --- a/src/backend/utils/sort/sharedtuplestore.c +++ b/src/backend/utils/sort/sharedtuplestore.c @@ -196,14 +196,9 @@ static void sts_flush_chunk(SharedTuplestoreAccessor *accessor) { size_t size; - size_t written; size = STS_CHUNK_PAGES * BLCKSZ; - written = BufFileWrite(accessor->write_file, accessor->write_chunk, size); - if (written != size) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not write to temporary file: %m"))); + BufFileWrite(accessor->write_file, accessor->write_chunk, size); memset(accessor->write_chunk, 0, size); accessor->write_pointer = &accessor->write_chunk->data[0]; accessor->sts->participants[accessor->participant].npages += diff --git a/src/backend/utils/sort/tuplestore.c b/src/backend/utils/sort/tuplestore.c index 2761b063a6..b57d90c53b 100644 --- a/src/backend/utils/sort/tuplestore.c +++ b/src/backend/utils/sort/tuplestore.c @@ -1511,22 +1511,10 @@ writetup_heap(Tuplestorestate *state, void *tup) /* total on-disk footprint: */ unsigned int tuplen = tupbodylen + sizeof(int); - if (BufFileWrite(state->myfile, (void *) &tuplen, - sizeof(tuplen)) != sizeof(tuplen)) - 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) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not write to tuplestore temporary file: %m"))); + BufFileWrite(state->myfile, (void *) &tuplen, sizeof(tuplen)); + BufFileWrite(state->myfile, (void *) tupbody, tupbodylen); if (state->backward) /* need trailing length word? */ - if (BufFileWrite(state->myfile, (void *) &tuplen, - sizeof(tuplen)) != sizeof(tuplen)) - ereport(ERROR, - (errcode_for_file_access(), - errmsg("could not write to tuplestore temporary file: %m"))); + BufFileWrite(state->myfile, (void *) &tuplen, sizeof(tuplen)); FREEMEM(state, GetMemoryChunkSpace(tuple)); heap_free_minimal_tuple(tuple); diff --git a/src/include/storage/buffile.h b/src/include/storage/buffile.h index 1fba404fe2..bfb41f5bd6 100644 --- a/src/include/storage/buffile.h +++ b/src/include/storage/buffile.h @@ -39,7 +39,7 @@ typedef struct BufFile BufFile; extern BufFile *BufFileCreateTemp(bool interXact); extern void BufFileClose(BufFile *file); extern size_t BufFileRead(BufFile *file, void *ptr, size_t size); -extern size_t BufFileWrite(BufFile *file, void *ptr, size_t size); +extern void BufFileWrite(BufFile *file, void *ptr, size_t size); extern int BufFileSeek(BufFile *file, int fileno, off_t offset, int whence); extern void BufFileTell(BufFile *file, int *fileno, off_t *offset); extern int BufFileSeekBlock(BufFile *file, long blknum); -- 2.23.0