Fix some sloppiness in the new BufFileSize() and BufFileAppend() functions.
There were three related issues: * BufFileAppend() incorrectly reset the seek position on the 'source' file. As a result, if you had called BufFileRead() on the file before calling BufFileAppend(), it got confused, and subsequent calls would read/write at wrong position. * BufFileSize() did not work with files opened with BufFileOpenShared(). * FileGetSize() only worked on temporary files. To fix, change the way BufFileSize() works so that it works on shared files. Remove FileGetSize() altogether, as it's no longer needed. Remove buffilesize from TapeShare struct, as the leader process can simply call BufFileSize() to get the tape's size, there's no need to pass it through shared memory anymore. Discussion: https://www.postgresql.org/message-id/CAH2-WznEDYe_NZXxmnOfsoV54oFkTdMy7YLE2NPBLuttO96vTQ@mail.gmail.com
This commit is contained in:
parent
7f6570b3a8
commit
445e31bdc7
|
@ -802,14 +802,24 @@ BufFileTellBlock(BufFile *file)
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Return the current file size. Counts any holes left behind by
|
* Return the current file size.
|
||||||
* BufFileViewAppend as part of the size.
|
*
|
||||||
|
* Counts any holes left behind by BufFileAppend as part of the size.
|
||||||
|
* Returns -1 on error.
|
||||||
*/
|
*/
|
||||||
off_t
|
off_t
|
||||||
BufFileSize(BufFile *file)
|
BufFileSize(BufFile *file)
|
||||||
{
|
{
|
||||||
|
off_t lastFileSize;
|
||||||
|
|
||||||
|
/* Get the size of the last physical file by seeking to end. */
|
||||||
|
lastFileSize = FileSeek(file->files[file->numFiles - 1], 0, SEEK_END);
|
||||||
|
if (lastFileSize < 0)
|
||||||
|
return -1;
|
||||||
|
file->offsets[file->numFiles - 1] = lastFileSize;
|
||||||
|
|
||||||
return ((file->numFiles - 1) * (off_t) MAX_PHYSICAL_FILESIZE) +
|
return ((file->numFiles - 1) * (off_t) MAX_PHYSICAL_FILESIZE) +
|
||||||
FileGetSize(file->files[file->numFiles - 1]);
|
lastFileSize;
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
@ -853,7 +863,7 @@ BufFileAppend(BufFile *target, BufFile *source)
|
||||||
for (i = target->numFiles; i < newNumFiles; i++)
|
for (i = target->numFiles; i < newNumFiles; i++)
|
||||||
{
|
{
|
||||||
target->files[i] = source->files[i - target->numFiles];
|
target->files[i] = source->files[i - target->numFiles];
|
||||||
target->offsets[i] = 0L;
|
target->offsets[i] = source->offsets[i - target->numFiles];
|
||||||
}
|
}
|
||||||
target->numFiles = newNumFiles;
|
target->numFiles = newNumFiles;
|
||||||
|
|
||||||
|
|
|
@ -2255,16 +2255,6 @@ FileGetRawMode(File file)
|
||||||
return VfdCache[file].fileMode;
|
return VfdCache[file].fileMode;
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
|
||||||
* FileGetSize - returns the size of file
|
|
||||||
*/
|
|
||||||
off_t
|
|
||||||
FileGetSize(File file)
|
|
||||||
{
|
|
||||||
Assert(FileIsValid(file));
|
|
||||||
return VfdCache[file].fileSize;
|
|
||||||
}
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Make room for another allocatedDescs[] array entry if needed and possible.
|
* Make room for another allocatedDescs[] array entry if needed and possible.
|
||||||
* Returns true if an array element is available.
|
* Returns true if an array element is available.
|
||||||
|
|
|
@ -426,11 +426,17 @@ ltsConcatWorkerTapes(LogicalTapeSet *lts, TapeShare *shared,
|
||||||
{
|
{
|
||||||
char filename[MAXPGPATH];
|
char filename[MAXPGPATH];
|
||||||
BufFile *file;
|
BufFile *file;
|
||||||
|
off_t filesize;
|
||||||
|
|
||||||
lt = <s->tapes[i];
|
lt = <s->tapes[i];
|
||||||
|
|
||||||
pg_itoa(i, filename);
|
pg_itoa(i, filename);
|
||||||
file = BufFileOpenShared(fileset, filename);
|
file = BufFileOpenShared(fileset, filename);
|
||||||
|
filesize = BufFileSize(file);
|
||||||
|
if (filesize < 0)
|
||||||
|
ereport(ERROR,
|
||||||
|
(errcode_for_file_access(),
|
||||||
|
errmsg("could not determine size of temporary file \"%s\"", filename)));
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* Stash first BufFile, and concatenate subsequent BufFiles to that.
|
* Stash first BufFile, and concatenate subsequent BufFiles to that.
|
||||||
|
@ -447,8 +453,8 @@ ltsConcatWorkerTapes(LogicalTapeSet *lts, TapeShare *shared,
|
||||||
lt->offsetBlockNumber = BufFileAppend(lts->pfile, file);
|
lt->offsetBlockNumber = BufFileAppend(lts->pfile, file);
|
||||||
}
|
}
|
||||||
/* Don't allocate more for read buffer than could possibly help */
|
/* Don't allocate more for read buffer than could possibly help */
|
||||||
lt->max_size = Min(MaxAllocSize, shared[i].buffilesize);
|
lt->max_size = Min(MaxAllocSize, filesize);
|
||||||
tapeblocks = shared[i].buffilesize / BLCKSZ;
|
tapeblocks = filesize / BLCKSZ;
|
||||||
nphysicalblocks += tapeblocks;
|
nphysicalblocks += tapeblocks;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -938,7 +944,6 @@ LogicalTapeFreeze(LogicalTapeSet *lts, int tapenum, TapeShare *share)
|
||||||
{
|
{
|
||||||
BufFileExportShared(lts->pfile);
|
BufFileExportShared(lts->pfile);
|
||||||
share->firstblocknumber = lt->firstBlockNumber;
|
share->firstblocknumber = lt->firstBlockNumber;
|
||||||
share->buffilesize = BufFileSize(lts->pfile);
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -4395,7 +4395,6 @@ tuplesort_initialize_shared(Sharedsort *shared, int nWorkers, dsm_segment *seg)
|
||||||
for (i = 0; i < nWorkers; i++)
|
for (i = 0; i < nWorkers; i++)
|
||||||
{
|
{
|
||||||
shared->tapes[i].firstblocknumber = 0L;
|
shared->tapes[i].firstblocknumber = 0L;
|
||||||
shared->tapes[i].buffilesize = 0;
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -78,7 +78,6 @@ extern char *FilePathName(File file);
|
||||||
extern int FileGetRawDesc(File file);
|
extern int FileGetRawDesc(File file);
|
||||||
extern int FileGetRawFlags(File file);
|
extern int FileGetRawFlags(File file);
|
||||||
extern mode_t FileGetRawMode(File file);
|
extern mode_t FileGetRawMode(File file);
|
||||||
extern off_t FileGetSize(File file);
|
|
||||||
|
|
||||||
/* Operations used for sharing named temporary files */
|
/* Operations used for sharing named temporary files */
|
||||||
extern File PathNameCreateTemporaryFile(const char *name, bool error_on_failure);
|
extern File PathNameCreateTemporaryFile(const char *name, bool error_on_failure);
|
||||||
|
|
|
@ -44,13 +44,10 @@ typedef struct LogicalTapeSet LogicalTapeSet;
|
||||||
typedef struct TapeShare
|
typedef struct TapeShare
|
||||||
{
|
{
|
||||||
/*
|
/*
|
||||||
* firstblocknumber is first block that should be read from materialized
|
* Currently, all the leader process needs is the location of the
|
||||||
* tape.
|
* materialized tape's first block.
|
||||||
*
|
|
||||||
* buffilesize is the size of associated BufFile following freezing.
|
|
||||||
*/
|
*/
|
||||||
long firstblocknumber;
|
long firstblocknumber;
|
||||||
off_t buffilesize;
|
|
||||||
} TapeShare;
|
} TapeShare;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
|
Loading…
Reference in New Issue