Skip to content

Commit 9c14d60

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 aeb785c commit 9c14d60

File tree

6 files changed

+95
-105
lines changed

6 files changed

+95
-105
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
@@ -1056,7 +1056,7 @@ ExecHashJoinNewBatch(HashJoinState *hjstate)
10561056
if (BufFileSeek(innerFile, 0, 0L, SEEK_SET))
10571057
ereport(ERROR,
10581058
(errcode_for_file_access(),
1059-
errmsg("could not rewind hash-join temporary file: %m")));
1059+
errmsg("could not rewind hash-join temporary file")));
10601060

10611061
while ((slot = ExecHashJoinGetSavedTuple(hjstate,
10621062
innerFile,
@@ -1086,7 +1086,7 @@ ExecHashJoinNewBatch(HashJoinState *hjstate)
10861086
if (BufFileSeek(hashtable->outerBatchFile[curbatch], 0, 0L, SEEK_SET))
10871087
ereport(ERROR,
10881088
(errcode_for_file_access(),
1089-
errmsg("could not rewind hash-join temporary file: %m")));
1089+
errmsg("could not rewind hash-join temporary file")));
10901090
}
10911091

10921092
return true;
@@ -1231,7 +1231,6 @@ ExecHashJoinSaveTuple(MinimalTuple tuple, uint32 hashvalue,
12311231
BufFile **fileptr)
12321232
{
12331233
BufFile *file = *fileptr;
1234-
size_t written;
12351234

12361235
if (file == NULL)
12371236
{
@@ -1240,17 +1239,8 @@ ExecHashJoinSaveTuple(MinimalTuple tuple, uint32 hashvalue,
12401239
*fileptr = file;
12411240
}
12421241

1243-
written = BufFileWrite(file, (void *) &hashvalue, sizeof(uint32));
1244-
if (written != sizeof(uint32))
1245-
ereport(ERROR,
1246-
(errcode_for_file_access(),
1247-
errmsg("could not write to hash-join temporary file: %m")));
1248-
1249-
written = BufFileWrite(file, (void *) tuple, tuple->t_len);
1250-
if (written != tuple->t_len)
1251-
ereport(ERROR,
1252-
(errcode_for_file_access(),
1253-
errmsg("could not write to hash-join temporary file: %m")));
1242+
BufFileWrite(file, (void *) &hashvalue, sizeof(uint32));
1243+
BufFileWrite(file, (void *) tuple, tuple->t_len);
12541244
}
12551245

12561246
/*
@@ -1291,7 +1281,8 @@ ExecHashJoinGetSavedTuple(HashJoinState *hjstate,
12911281
if (nread != sizeof(header))
12921282
ereport(ERROR,
12931283
(errcode_for_file_access(),
1294-
errmsg("could not read from hash-join temporary file: %m")));
1284+
errmsg("could not read from hash-join temporary file: read only %zu of %zu bytes",
1285+
nread, sizeof(header))));
12951286
*hashvalue = header[0];
12961287
tuple = (MinimalTuple) palloc(header[1]);
12971288
tuple->t_len = header[1];
@@ -1301,7 +1292,8 @@ ExecHashJoinGetSavedTuple(HashJoinState *hjstate,
13011292
if (nread != header[1] - sizeof(uint32))
13021293
ereport(ERROR,
13031294
(errcode_for_file_access(),
1304-
errmsg("could not read from hash-join temporary file: %m")));
1295+
errmsg("could not read from hash-join temporary file: read only %zu of %zu bytes",
1296+
nread, header[1] - sizeof(uint32))));
13051297
return ExecStoreMinimalTuple(tuple, tupleSlot, true);
13061298
}
13071299

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

Lines changed: 31 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ static BufFile *makeBufFile(File firstfile);
104104
static void extendBufFile(BufFile *file);
105105
static void BufFileLoadBuffer(BufFile *file);
106106
static void BufFileDumpBuffer(BufFile *file);
107-
static int BufFileFlush(BufFile *file);
107+
static void BufFileFlush(BufFile *file);
108108
static File MakeNewSharedSegment(BufFile *file, int segment);
109109

110110
/*
@@ -430,7 +430,10 @@ BufFileLoadBuffer(BufFile *file)
430430
if (file->curOffset != file->offsets[file->curFile])
431431
{
432432
if (FileSeek(thisfile, file->curOffset, SEEK_SET) != file->curOffset)
433-
return; /* seek failed, read nothing */
433+
ereport(ERROR,
434+
(errcode_for_file_access(),
435+
errmsg("could not seek in file \"%s\": %m",
436+
FilePathName(thisfile))));
434437
file->offsets[file->curFile] = file->curOffset;
435438
}
436439

@@ -442,7 +445,14 @@ BufFileLoadBuffer(BufFile *file)
442445
sizeof(file->buffer),
443446
WAIT_EVENT_BUFFILE_READ);
444447
if (file->nbytes < 0)
448+
{
445449
file->nbytes = 0;
450+
ereport(ERROR,
451+
(errcode_for_file_access(),
452+
errmsg("could not read file \"%s\": %m",
453+
FilePathName(thisfile))));
454+
}
455+
446456
file->offsets[file->curFile] += file->nbytes;
447457
/* we choose not to advance curOffset here */
448458

@@ -499,15 +509,22 @@ BufFileDumpBuffer(BufFile *file)
499509
if (file->curOffset != file->offsets[file->curFile])
500510
{
501511
if (FileSeek(thisfile, file->curOffset, SEEK_SET) != file->curOffset)
502-
return; /* seek failed, give up */
512+
ereport(ERROR,
513+
(errcode_for_file_access(),
514+
errmsg("could not seek in file \"%s\": %m",
515+
FilePathName(thisfile))));
503516
file->offsets[file->curFile] = file->curOffset;
504517
}
505518
bytestowrite = FileWrite(thisfile,
506519
file->buffer.data + wpos,
507520
bytestowrite,
508521
WAIT_EVENT_BUFFILE_WRITE);
509522
if (bytestowrite <= 0)
510-
return; /* failed to write */
523+
ereport(ERROR,
524+
(errcode_for_file_access(),
525+
errmsg("could not write to file \"%s\" : %m",
526+
FilePathName(thisfile))));
527+
511528
file->offsets[file->curFile] += bytestowrite;
512529
file->curOffset += bytestowrite;
513530
wpos += bytestowrite;
@@ -540,20 +557,16 @@ BufFileDumpBuffer(BufFile *file)
540557
/*
541558
* BufFileRead
542559
*
543-
* Like fread() except we assume 1-byte element size.
560+
* Like fread() except we assume 1-byte element size and report I/O errors via
561+
* ereport().
544562
*/
545563
size_t
546564
BufFileRead(BufFile *file, void *ptr, size_t size)
547565
{
548566
size_t nread = 0;
549567
size_t nthistime;
550568

551-
if (file->dirty)
552-
{
553-
if (BufFileFlush(file) != 0)
554-
return 0; /* could not flush... */
555-
Assert(!file->dirty);
556-
}
569+
BufFileFlush(file);
557570

558571
while (size > 0)
559572
{
@@ -587,7 +600,8 @@ BufFileRead(BufFile *file, void *ptr, size_t size)
587600
/*
588601
* BufFileWrite
589602
*
590-
* Like fwrite() except we assume 1-byte element size.
603+
* Like fwrite() except we assume 1-byte element size and report errors via
604+
* ereport().
591605
*/
592606
size_t
593607
BufFileWrite(BufFile *file, void *ptr, size_t size)
@@ -603,11 +617,7 @@ BufFileWrite(BufFile *file, void *ptr, size_t size)
603617
{
604618
/* Buffer full, dump it out */
605619
if (file->dirty)
606-
{
607620
BufFileDumpBuffer(file);
608-
if (file->dirty)
609-
break; /* I/O error */
610-
}
611621
else
612622
{
613623
/* Hmm, went directly from reading to writing? */
@@ -639,19 +649,15 @@ BufFileWrite(BufFile *file, void *ptr, size_t size)
639649
/*
640650
* BufFileFlush
641651
*
642-
* Like fflush()
652+
* Like fflush(), except that I/O errors are reported with ereport().
643653
*/
644-
static int
654+
static void
645655
BufFileFlush(BufFile *file)
646656
{
647657
if (file->dirty)
648-
{
649658
BufFileDumpBuffer(file);
650-
if (file->dirty)
651-
return EOF;
652-
}
653659

654-
return 0;
660+
Assert(!file->dirty);
655661
}
656662

657663
/*
@@ -660,6 +666,7 @@ BufFileFlush(BufFile *file)
660666
* Like fseek(), except that target position needs two values in order to
661667
* work when logical filesize exceeds maximum value representable by off_t.
662668
* We do not support relative seeks across more than that, however.
669+
* I/O errors are reported by ereport().
663670
*
664671
* Result is 0 if OK, EOF if not. Logical position is not moved if an
665672
* impossible seek is attempted.
@@ -717,8 +724,7 @@ BufFileSeek(BufFile *file, int fileno, off_t offset, int whence)
717724
return 0;
718725
}
719726
/* Otherwise, must reposition buffer, so flush any dirty data */
720-
if (BufFileFlush(file) != 0)
721-
return EOF;
727+
BufFileFlush(file);
722728

723729
/*
724730
* 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)