Skip to content

Commit 28ee126

Browse files
macdicemacdice
committed
Fix buffile.c error handling.
Convert buffile.c error handling to use ereport. This fixes cases where I/O errors were indistinguishable from EOF or not reported. Also remove "%m" from error messages where errno would be bogus. While we're modifying those strings, add block numbers and short read byte counts where appropriate. Back-patch to all supported releases. Reported-by: Amit Khandekar <amitdkhan.pg@gmail.com> Reviewed-by: Melanie Plageman <melanieplageman@gmail.com> Reviewed-by: Alvaro Herrera <alvherre@2ndquadrant.com> Reviewed-by: Robert Haas <robertmhaas@gmail.com> Reviewed-by: Ibrar Ahmed <ibrar.ahmad@gmail.com> Reviewed-by: Michael Paquier <michael@paquier.xyz> Discussion: https://postgr.es/m/CA%2BhUKGJE04G%3D8TLK0DLypT_27D9dR8F1RQgNp0jK6qR0tZGWOw%40mail.gmail.com
1 parent 8e93359 commit 28ee126

File tree

6 files changed

+86
-103
lines changed

6 files changed

+86
-103
lines changed

‎src/backend/access/gist/gistbuildbuffers.c

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -757,26 +757,20 @@ gistRelocateBuildBuffersOnSplit(GISTBuildBuffers *gfbb, GISTSTATE *giststate,
757757
static void
758758
ReadTempFileBlock(BufFile *file, long blknum, void *ptr)
759759
{
760+
size_t nread;
761+
760762
if (BufFileSeekBlock(file, blknum) != 0)
761-
elog(ERROR, "could not seek temporary file: %m");
762-
if (BufFileRead(file, ptr, BLCKSZ) != BLCKSZ)
763-
elog(ERROR, "could not read temporary file: %m");
763+
elog(ERROR, "could not seek to block %ld in temporary file", blknum);
764+
nread = BufFileRead(file, ptr, BLCKSZ);
765+
if (nread != BLCKSZ)
766+
elog(ERROR, "could not read temporary file: read only %zu of %zu bytes",
767+
nread, (size_t) BLCKSZ);
764768
}
765769

766770
static void
767771
WriteTempFileBlock(BufFile *file, long blknum, void *ptr)
768772
{
769773
if (BufFileSeekBlock(file, blknum) != 0)
770-
elog(ERROR, "could not seek temporary file: %m");
771-
if (BufFileWrite(file, ptr, BLCKSZ) != BLCKSZ)
772-
{
773-
/*
774-
* the other errors in Read/WriteTempFileBlock shouldn't happen, but
775-
* an error at write can easily happen if you run out of disk space.
776-
*/
777-
ereport(ERROR,
778-
(errcode_for_file_access(),
779-
errmsg("could not write block %ld of temporary file: %m",
780-
blknum)));
781-
}
774+
elog(ERROR, "could not seek to block %ld in temporary file", blknum);
775+
BufFileWrite(file, ptr, BLCKSZ);
782776
}

‎src/backend/executor/nodeHashjoin.c

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1043,7 +1043,7 @@ ExecHashJoinNewBatch(HashJoinState *hjstate)
10431043
if (BufFileSeek(innerFile, 0, 0L, SEEK_SET))
10441044
ereport(ERROR,
10451045
(errcode_for_file_access(),
1046-
errmsg("could not rewind hash-join temporary file: %m")));
1046+
errmsg("could not rewind hash-join temporary file")));
10471047

10481048
while ((slot = ExecHashJoinGetSavedTuple(hjstate,
10491049
innerFile,
@@ -1073,7 +1073,7 @@ ExecHashJoinNewBatch(HashJoinState *hjstate)
10731073
if (BufFileSeek(hashtable->outerBatchFile[curbatch], 0, 0L, SEEK_SET))
10741074
ereport(ERROR,
10751075
(errcode_for_file_access(),
1076-
errmsg("could not rewind hash-join temporary file: %m")));
1076+
errmsg("could not rewind hash-join temporary file")));
10771077
}
10781078

10791079
return true;
@@ -1219,7 +1219,6 @@ ExecHashJoinSaveTuple(MinimalTuple tuple, uint32 hashvalue,
12191219
BufFile **fileptr)
12201220
{
12211221
BufFile *file = *fileptr;
1222-
size_t written;
12231222

12241223
if (file == NULL)
12251224
{
@@ -1228,17 +1227,8 @@ ExecHashJoinSaveTuple(MinimalTuple tuple, uint32 hashvalue,
12281227
*fileptr = file;
12291228
}
12301229

1231-
written = BufFileWrite(file, (void *) &hashvalue, sizeof(uint32));
1232-
if (written != sizeof(uint32))
1233-
ereport(ERROR,
1234-
(errcode_for_file_access(),
1235-
errmsg("could not write to hash-join temporary file: %m")));
1236-
1237-
written = BufFileWrite(file, (void *) tuple, tuple->t_len);
1238-
if (written != tuple->t_len)
1239-
ereport(ERROR,
1240-
(errcode_for_file_access(),
1241-
errmsg("could not write to hash-join temporary file: %m")));
1230+
BufFileWrite(file, (void *) &hashvalue, sizeof(uint32));
1231+
BufFileWrite(file, (void *) tuple, tuple->t_len);
12421232
}
12431233

12441234
/*
@@ -1279,7 +1269,8 @@ ExecHashJoinGetSavedTuple(HashJoinState *hjstate,
12791269
if (nread != sizeof(header))
12801270
ereport(ERROR,
12811271
(errcode_for_file_access(),
1282-
errmsg("could not read from hash-join temporary file: %m")));
1272+
errmsg("could not read from hash-join temporary file: read only %zu of %zu bytes",
1273+
nread, sizeof(header))));
12831274
*hashvalue = header[0];
12841275
tuple = (MinimalTuple) palloc(header[1]);
12851276
tuple->t_len = header[1];
@@ -1289,7 +1280,8 @@ ExecHashJoinGetSavedTuple(HashJoinState *hjstate,
12891280
if (nread != header[1] - sizeof(uint32))
12901281
ereport(ERROR,
12911282
(errcode_for_file_access(),
1292-
errmsg("could not read from hash-join temporary file: %m")));
1283+
errmsg("could not read from hash-join temporary file: read only %zu of %zu bytes",
1284+
nread, header[1] - sizeof(uint32))));
12931285
ExecForceStoreMinimalTuple(tuple, tupleSlot, true);
12941286
return tupleSlot;
12951287
}

‎src/backend/storage/file/buffile.c

Lines changed: 22 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ static BufFile *makeBufFile(File firstfile);
9999
static void extendBufFile(BufFile *file);
100100
static void BufFileLoadBuffer(BufFile *file);
101101
static void BufFileDumpBuffer(BufFile *file);
102-
static int BufFileFlush(BufFile *file);
102+
static void BufFileFlush(BufFile *file);
103103
static File MakeNewSharedSegment(BufFile *file, int segment);
104104

105105
/*
@@ -434,7 +434,14 @@ BufFileLoadBuffer(BufFile *file)
434434
file->curOffset,
435435
WAIT_EVENT_BUFFILE_READ);
436436
if (file->nbytes < 0)
437+
{
437438
file->nbytes = 0;
439+
ereport(ERROR,
440+
(errcode_for_file_access(),
441+
errmsg("could not read file \"%s\": %m",
442+
FilePathName(thisfile))));
443+
}
444+
438445
/* we choose not to advance curOffset here */
439446

440447
if (file->nbytes > 0)
@@ -490,7 +497,10 @@ BufFileDumpBuffer(BufFile *file)
490497
file->curOffset,
491498
WAIT_EVENT_BUFFILE_WRITE);
492499
if (bytestowrite <= 0)
493-
return; /* failed to write */
500+
ereport(ERROR,
501+
(errcode_for_file_access(),
502+
errmsg("could not write to file \"%s\" : %m",
503+
FilePathName(thisfile))));
494504
file->curOffset += bytestowrite;
495505
wpos += bytestowrite;
496506

@@ -522,20 +532,16 @@ BufFileDumpBuffer(BufFile *file)
522532
/*
523533
* BufFileRead
524534
*
525-
* Like fread() except we assume 1-byte element size.
535+
* Like fread() except we assume 1-byte element size and report I/O errors via
536+
* ereport().
526537
*/
527538
size_t
528539
BufFileRead(BufFile *file, void *ptr, size_t size)
529540
{
530541
size_t nread = 0;
531542
size_t nthistime;
532543

533-
if (file->dirty)
534-
{
535-
if (BufFileFlush(file) != 0)
536-
return 0; /* could not flush... */
537-
Assert(!file->dirty);
538-
}
544+
BufFileFlush(file);
539545

540546
while (size > 0)
541547
{
@@ -569,7 +575,8 @@ BufFileRead(BufFile *file, void *ptr, size_t size)
569575
/*
570576
* BufFileWrite
571577
*
572-
* Like fwrite() except we assume 1-byte element size.
578+
* Like fwrite() except we assume 1-byte element size and report errors via
579+
* ereport().
573580
*/
574581
size_t
575582
BufFileWrite(BufFile *file, void *ptr, size_t size)
@@ -585,11 +592,7 @@ BufFileWrite(BufFile *file, void *ptr, size_t size)
585592
{
586593
/* Buffer full, dump it out */
587594
if (file->dirty)
588-
{
589595
BufFileDumpBuffer(file);
590-
if (file->dirty)
591-
break; /* I/O error */
592-
}
593596
else
594597
{
595598
/* Hmm, went directly from reading to writing? */
@@ -621,19 +624,15 @@ BufFileWrite(BufFile *file, void *ptr, size_t size)
621624
/*
622625
* BufFileFlush
623626
*
624-
* Like fflush()
627+
* Like fflush(), except that I/O errors are reported with ereport().
625628
*/
626-
static int
629+
static void
627630
BufFileFlush(BufFile *file)
628631
{
629632
if (file->dirty)
630-
{
631633
BufFileDumpBuffer(file);
632-
if (file->dirty)
633-
return EOF;
634-
}
635634

636-
return 0;
635+
Assert(!file->dirty);
637636
}
638637

639638
/*
@@ -642,6 +641,7 @@ BufFileFlush(BufFile *file)
642641
* Like fseek(), except that target position needs two values in order to
643642
* work when logical filesize exceeds maximum value representable by off_t.
644643
* We do not support relative seeks across more than that, however.
644+
* I/O errors are reported by ereport().
645645
*
646646
* Result is 0 if OK, EOF if not. Logical position is not moved if an
647647
* impossible seek is attempted.
@@ -699,8 +699,7 @@ BufFileSeek(BufFile *file, int fileno, off_t offset, int whence)
699699
return 0;
700700
}
701701
/* Otherwise, must reposition buffer, so flush any dirty data */
702-
if (BufFileFlush(file) != 0)
703-
return EOF;
702+
BufFileFlush(file);
704703

705704
/*
706705
* At this point and no sooner, check for seek past last segment. The

‎src/backend/utils/sort/logtape.c

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -248,12 +248,12 @@ ltsWriteBlock(LogicalTapeSet *lts, long blocknum, void *buffer)
248248
}
249249

250250
/* Write the requested block */
251-
if (BufFileSeekBlock(lts->pfile, blocknum) != 0 ||
252-
BufFileWrite(lts->pfile, buffer, BLCKSZ) != BLCKSZ)
251+
if (BufFileSeekBlock(lts->pfile, blocknum) != 0)
253252
ereport(ERROR,
254253
(errcode_for_file_access(),
255-
errmsg("could not write block %ld of temporary file: %m",
254+
errmsg("could not seek to block %ld of temporary file",
256255
blocknum)));
256+
BufFileWrite(lts->pfile, buffer, BLCKSZ);
257257

258258
/* Update nBlocksWritten, if we extended the file */
259259
if (blocknum == lts->nBlocksWritten)
@@ -269,12 +269,19 @@ ltsWriteBlock(LogicalTapeSet *lts, long blocknum, void *buffer)
269269
static void
270270
ltsReadBlock(LogicalTapeSet *lts, long blocknum, void *buffer)
271271
{
272-
if (BufFileSeekBlock(lts->pfile, blocknum) != 0 ||
273-
BufFileRead(lts->pfile, buffer, BLCKSZ) != BLCKSZ)
272+
size_t nread;
273+
274+
if (BufFileSeekBlock(lts->pfile, blocknum) != 0)
274275
ereport(ERROR,
275276
(errcode_for_file_access(),
276-
errmsg("could not read block %ld of temporary file: %m",
277+
errmsg("could not seek to block %ld of temporary file",
277278
blocknum)));
279+
nread = BufFileRead(lts->pfile, buffer, BLCKSZ);
280+
if (nread != BLCKSZ)
281+
ereport(ERROR,
282+
(errcode_for_file_access(),
283+
errmsg("could not read block %ld of temporary file: read only %zu of %zu bytes",
284+
blocknum, nread, (size_t) BLCKSZ)));
278285
}
279286

280287
/*

‎src/backend/utils/sort/sharedtuplestore.c

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -198,14 +198,9 @@ static void
198198
sts_flush_chunk(SharedTuplestoreAccessor *accessor)
199199
{
200200
size_t size;
201-
size_t written;
202201

203202
size = STS_CHUNK_PAGES * BLCKSZ;
204-
written = BufFileWrite(accessor->write_file, accessor->write_chunk, size);
205-
if (written != size)
206-
ereport(ERROR,
207-
(errcode_for_file_access(),
208-
errmsg("could not write to temporary file: %m")));
203+
BufFileWrite(accessor->write_file, accessor->write_chunk, size);
209204
memset(accessor->write_chunk, 0, size);
210205
accessor->write_pointer = &accessor->write_chunk->data[0];
211206
accessor->sts->participants[accessor->participant].npages +=
@@ -557,6 +552,7 @@ sts_parallel_scan_next(SharedTuplestoreAccessor *accessor, void *meta_data)
557552
if (!eof)
558553
{
559554
SharedTuplestoreChunk chunk_header;
555+
size_t nread;
560556

561557
/* Make sure we have the file open. */
562558
if (accessor->read_file == NULL)
@@ -572,14 +568,15 @@ sts_parallel_scan_next(SharedTuplestoreAccessor *accessor, void *meta_data)
572568
if (BufFileSeekBlock(accessor->read_file, read_page) != 0)
573569
ereport(ERROR,
574570
(errcode_for_file_access(),
575-
errmsg("could not read from shared tuplestore temporary file"),
576-
errdetail_internal("Could not seek to next block.")));
577-
if (BufFileRead(accessor->read_file, &chunk_header,
578-
STS_CHUNK_HEADER_SIZE) != STS_CHUNK_HEADER_SIZE)
571+
errmsg("could not seek block %u in shared tuplestore temporary file",
572+
read_page)));
573+
nread = BufFileRead(accessor->read_file, &chunk_header,
574+
STS_CHUNK_HEADER_SIZE);
575+
if (nread != STS_CHUNK_HEADER_SIZE)
579576
ereport(ERROR,
580577
(errcode_for_file_access(),
581-
errmsg("could not read from shared tuplestore temporary file"),
582-
errdetail_internal("Short read while reading chunk header.")));
578+
errmsg("could not read from shared tuplestore temporary file: read only %zu of %zu bytes",
579+
nread, STS_CHUNK_HEADER_SIZE)));
583580

584581
/*
585582
* If this is an overflow chunk, we skip it and any following

0 commit comments

Comments
 (0)