Skip to content

Commit 7897e3b

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 4c5cf54 commit 7897e3b

File tree

7 files changed

+88
-110
lines changed

7 files changed

+88
-110
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/replication/backup_manifest.c

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ SendBackupManifest(backup_manifest_info *manifest)
319319
if (BufFileSeek(manifest->buffile, 0, 0L, SEEK_SET))
320320
ereport(ERROR,
321321
(errcode_for_file_access(),
322-
errmsg("could not rewind temporary file: %m")));
322+
errmsg("could not rewind temporary file")));
323323

324324
/* Send CopyOutResponse message */
325325
pq_beginmessage(&protobuf, 'H');
@@ -365,15 +365,10 @@ static void
365365
AppendStringToManifest(backup_manifest_info *manifest, char *s)
366366
{
367367
int len = strlen(s);
368-
size_t written;
369368

370369
Assert(manifest != NULL);
371370
if (manifest->still_checksumming)
372371
pg_sha256_update(&manifest->manifest_ctx, (uint8 *) s, len);
373-
written = BufFileWrite(manifest->buffile, s, len);
374-
if (written != len)
375-
ereport(ERROR,
376-
(errcode_for_file_access(),
377-
errmsg("could not write to temporary file: %m")));
372+
BufFileWrite(manifest->buffile, s, len);
378373
manifest->manifest_size += len;
379374
}

‎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
@@ -262,12 +262,12 @@ ltsWriteBlock(LogicalTapeSet *lts, long blocknum, void *buffer)
262262
}
263263

264264
/* Write the requested block */
265-
if (BufFileSeekBlock(lts->pfile, blocknum) != 0 ||
266-
BufFileWrite(lts->pfile, buffer, BLCKSZ) != BLCKSZ)
265+
if (BufFileSeekBlock(lts->pfile, blocknum) != 0)
267266
ereport(ERROR,
268267
(errcode_for_file_access(),
269-
errmsg("could not write block %ld of temporary file: %m",
268+
errmsg("could not seek to block %ld of temporary file",
270269
blocknum)));
270+
BufFileWrite(lts->pfile, buffer, BLCKSZ);
271271

272272
/* Update nBlocksWritten, if we extended the file */
273273
if (blocknum == lts->nBlocksWritten)
@@ -283,12 +283,19 @@ ltsWriteBlock(LogicalTapeSet *lts, long blocknum, void *buffer)
283283
static void
284284
ltsReadBlock(LogicalTapeSet *lts, long blocknum, void *buffer)
285285
{
286-
if (BufFileSeekBlock(lts->pfile, blocknum) != 0 ||
287-
BufFileRead(lts->pfile, buffer, BLCKSZ) != BLCKSZ)
286+
size_t nread;
287+
288+
if (BufFileSeekBlock(lts->pfile, blocknum) != 0)
288289
ereport(ERROR,
289290
(errcode_for_file_access(),
290-
errmsg("could not read block %ld of temporary file: %m",
291+
errmsg("could not seek to block %ld of temporary file",
291292
blocknum)));
293+
nread = BufFileRead(lts->pfile, buffer, BLCKSZ);
294+
if (nread != BLCKSZ)
295+
ereport(ERROR,
296+
(errcode_for_file_access(),
297+
errmsg("could not read block %ld of temporary file: read only %zu of %zu bytes",
298+
blocknum, nread, (size_t) BLCKSZ)));
292299
}
293300

294301
/*

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

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -196,14 +196,9 @@ static void
196196
sts_flush_chunk(SharedTuplestoreAccessor *accessor)
197197
{
198198
size_t size;
199-
size_t written;
200199

201200
size = STS_CHUNK_PAGES * BLCKSZ;
202-
written = BufFileWrite(accessor->write_file, accessor->write_chunk, size);
203-
if (written != size)
204-
ereport(ERROR,
205-
(errcode_for_file_access(),
206-
errmsg("could not write to temporary file: %m")));
201+
BufFileWrite(accessor->write_file, accessor->write_chunk, size);
207202
memset(accessor->write_chunk, 0, size);
208203
accessor->write_pointer = &accessor->write_chunk->data[0];
209204
accessor->sts->participants[accessor->participant].npages +=
@@ -555,6 +550,7 @@ sts_parallel_scan_next(SharedTuplestoreAccessor *accessor, void *meta_data)
555550
if (!eof)
556551
{
557552
SharedTuplestoreChunk chunk_header;
553+
size_t nread;
558554

559555
/* Make sure we have the file open. */
560556
if (accessor->read_file == NULL)
@@ -570,14 +566,15 @@ sts_parallel_scan_next(SharedTuplestoreAccessor *accessor, void *meta_data)
570566
if (BufFileSeekBlock(accessor->read_file, read_page) != 0)
571567
ereport(ERROR,
572568
(errcode_for_file_access(),
573-
errmsg("could not read from shared tuplestore temporary file"),
574-
errdetail_internal("Could not seek to next block.")));
575-
if (BufFileRead(accessor->read_file, &chunk_header,
576-
STS_CHUNK_HEADER_SIZE) != STS_CHUNK_HEADER_SIZE)
569+
errmsg("could not seek block %u in shared tuplestore temporary file",
570+
read_page)));
571+
nread = BufFileRead(accessor->read_file, &chunk_header,
572+
STS_CHUNK_HEADER_SIZE);
573+
if (nread != STS_CHUNK_HEADER_SIZE)
577574
ereport(ERROR,
578575
(errcode_for_file_access(),
579-
errmsg("could not read from shared tuplestore temporary file"),
580-
errdetail_internal("Short read while reading chunk header.")));
576+
errmsg("could not read from shared tuplestore temporary file: read only %zu of %zu bytes",
577+
nread, STS_CHUNK_HEADER_SIZE)));
581578

582579
/*
583580
* If this is an overflow chunk, we skip it and any following

0 commit comments

Comments
 (0)