db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2025-09-16 09:06:18 +02:00
Code Issues Releases Activity

Add noTruncate flag to storageNewWriteP().

Browse Source 
This flag skips truncation when opening a file for write on drivers that support it, currently Posix and CIFS. This is convenient for cases where the file needs to be manipulated directly using the file descriptor. Using the file descriptor is not ideal and additional functionality should be added to the storage interface, but for now at least this avoids code duplication, especially on close which updates owners, the timestamp, syncs, etc.

The remote driver forbids no truncate because a file descriptor is never available for a remote storage write object.

Update two instances in the current code which benefit from this new functionality, but the primary reason for the change is to support more complex restore deltas in the upcoming block incremental feature.
This commit is contained in:
David Steele
2022-10-18 11:33:19 +13:00
committed by GitHub
parent 7967c750d8
commit 1730ef4ac3
25 changed files with 84 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
doc/xml/release.xml
View File

@@ -87,6 +87,9 @@
<commit subject="Update ManifestFile booleans to bit fields."/>
<commit subject="Use read-only storage to calculate checksum in restoreFile()."/>
<commit subject="Swap command/backup and command/restore unit tests."/>
<commit subject="Add noTruncate flag to storageNewWriteP().">
<github-pull-request id="1898"/>
</commit>
<release-item-contributor-list>
<release-item-contributor id="david.steele"/>

13
src/command/control/stop.c
View File

@@ -35,14 +35,11 @@ cmdStop(void)
// Create the lock path (ignore if already created)
storagePathCreateP(storageLocalWrite(), strPath(stopFile), .mode = 0770);
// Create the stop file with Read/Write and Create only - do not use Truncate
int fd = -1;
THROW_ON_SYS_ERROR_FMT(
((fd = open(strZ(stopFile), O_WRONLY | O_CREAT, STORAGE_MODE_FILE_DEFAULT)) == -1), FileOpenError,
"unable to open stop file '%s'", strZ(stopFile));
// Close the file
close(fd);
// Create the stop file with without truncating an existing file
IoWrite *const stopWrite = storageWriteIo(
storageNewWriteP(storageLocalWrite(), stopFile, .noAtomic = true, .noTruncate = true, .modePath = 0770));
ioWriteOpen(stopWrite);
ioWriteClose(stopWrite);
// If --force was specified then send term signals to running processes
if (cfgOptionBool(cfgOptForce))

30
src/command/restore/file.c
View File

@@ -88,26 +88,20 @@ List *restoreFile(
// this using the checksum below)
if (info.size > file->size)
{
// Open the file for write
int fd = open(fileName, O_WRONLY, 0);
THROW_ON_SYS_ERROR_FMT(fd == -1, FileReadError, STORAGE_ERROR_WRITE_OPEN, fileName);
// Open file for write
IoWrite *const pgWriteTruncate = storageWriteIo(
storageNewWriteP(
storagePgWrite(), file->name, .noAtomic = true, .noCreatePath = true,
.noSyncPath = true, .noTruncate = true));
ioWriteOpen(pgWriteTruncate);
TRY_BEGIN()
{
// Truncate to original size
THROW_ON_SYS_ERROR_FMT(
ftruncate(fd, (off_t)file->size) == -1, FileWriteError, "unable to truncate file '%s'",
fileName);
// Truncate to original size
THROW_ON_SYS_ERROR_FMT(
ftruncate(ioWriteFd(pgWriteTruncate), (off_t)file->size) == -1, FileWriteError,
"unable to truncate file '%s'", fileName);
// Sync
THROW_ON_SYS_ERROR_FMT(fsync(fd) == -1, FileSyncError, STORAGE_ERROR_WRITE_SYNC, fileName);
}
FINALLY()
{
THROW_ON_SYS_ERROR_FMT(
close(fd) == -1, FileCloseError, STORAGE_ERROR_WRITE_CLOSE, fileName);
}
TRY_END();
// Close file
ioWriteClose(pgWriteTruncate);
// Update info
info = storageInfoP(storagePg(), file->name, .followLink = true);

1
src/storage/azure/storage.c
View File

@@ -579,6 +579,7 @@ storageAzureNewWrite(THIS_VOID, const String *file, StorageInterfaceNewWritePara
ASSERT(this != NULL);
ASSERT(file != NULL);
ASSERT(param.createPath);
ASSERT(param.truncate);
ASSERT(param.user == NULL);
ASSERT(param.group == NULL);
ASSERT(param.timeModified == 0);

1
src/storage/azure/write.c
View File

@@ -292,6 +292,7 @@ storageWriteAzureNew(StorageAzure *storage, const String *name, uint64_t fileId,
.createPath = true,
.syncFile = true,
.syncPath = true,
.truncate = true,
.ioInterface = (IoWriteInterface)
{

1
src/storage/gcs/storage.c
View File

@@ -817,6 +817,7 @@ storageGcsNewWrite(THIS_VOID, const String *file, StorageInterfaceNewWriteParam
ASSERT(this != NULL);
ASSERT(file != NULL);
ASSERT(param.createPath);
ASSERT(param.truncate);
ASSERT(param.user == NULL);
ASSERT(param.group == NULL);
ASSERT(param.timeModified == 0);

1
src/storage/gcs/write.c
View File

@@ -350,6 +350,7 @@ storageWriteGcsNew(StorageGcs *storage, const String *name, size_t chunkSize)
.createPath = true,
.syncFile = true,
.syncPath = true,
.truncate = true,
.ioInterface = (IoWriteInterface)
{

3
src/storage/posix/storage.c
View File

@@ -374,6 +374,7 @@ storagePosixNewWrite(THIS_VOID, const String *file, StorageInterfaceNewWritePara
FUNCTION_LOG_PARAM(BOOL, param.syncFile);
FUNCTION_LOG_PARAM(BOOL, param.syncPath);
FUNCTION_LOG_PARAM(BOOL, param.atomic);
FUNCTION_LOG_PARAM(BOOL, param.truncate);
FUNCTION_LOG_END();
ASSERT(this != NULL);
@@ -383,7 +384,7 @@ storagePosixNewWrite(THIS_VOID, const String *file, StorageInterfaceNewWritePara
STORAGE_WRITE,
storageWritePosixNew(
this, file, param.modeFile, param.modePath, param.user, param.group, param.timeModified, param.createPath,
param.syncFile, this->interface.pathSync != NULL ? param.syncPath : false, param.atomic));
param.syncFile, this->interface.pathSync != NULL ? param.syncPath : false, param.atomic, param.truncate));
}
/**********************************************************************************************************************************/

14
src/storage/posix/write.c
View File

@@ -43,7 +43,6 @@ File open constants
Since open is called more than once use constants to make sure these parameters are always the same
***********************************************************************************************************************************/
#define FILE_OPEN_FLAGS (O_CREAT | O_TRUNC | O_WRONLY)
#define FILE_OPEN_PURPOSE "write"
/***********************************************************************************************************************************
@@ -81,7 +80,9 @@ storageWritePosixOpen(THIS_VOID)
ASSERT(this->fd == -1);
// Open the file
this->fd = open(strZ(this->nameTmp), FILE_OPEN_FLAGS, this->interface.modeFile);
const int flags = O_CREAT | O_WRONLY | (this->interface.truncate ? O_TRUNC : 0);
this->fd = open(strZ(this->nameTmp), flags, this->interface.modeFile);
// Attempt to create the path if it is missing
if (this->fd == -1 && errno == ENOENT && this->interface.createPath) // {vm_covered}
@@ -90,7 +91,7 @@ storageWritePosixOpen(THIS_VOID)
storageInterfacePathCreateP(this->storage, this->path, false, false, this->interface.modePath);
// Open file again
this->fd = open(strZ(this->nameTmp), FILE_OPEN_FLAGS, this->interface.modeFile);
this->fd = open(strZ(this->nameTmp), flags, this->interface.modeFile);
}
// Handle errors
@@ -221,8 +222,9 @@ storageWritePosixFd(const THIS_VOID)
/**********************************************************************************************************************************/
StorageWrite *
storageWritePosixNew(
StoragePosix *storage, const String *name, mode_t modeFile, mode_t modePath, const String *user, const String *group,
time_t timeModified, bool createPath, bool syncFile, bool syncPath, bool atomic)
StoragePosix *const storage, const String *const name, const mode_t modeFile, const mode_t modePath, const String *const user,
const String *const group, const time_t timeModified, const bool createPath, const bool syncFile, const bool syncPath,
const bool atomic, const bool truncate)
{
FUNCTION_LOG_BEGIN(logLevelTrace);
FUNCTION_LOG_PARAM(STORAGE_POSIX, storage);
@@ -236,6 +238,7 @@ storageWritePosixNew(
FUNCTION_LOG_PARAM(BOOL, syncFile);
FUNCTION_LOG_PARAM(BOOL, syncPath);
FUNCTION_LOG_PARAM(BOOL, atomic);
FUNCTION_LOG_PARAM(BOOL, truncate);
FUNCTION_LOG_END();
ASSERT(storage != NULL);
@@ -266,6 +269,7 @@ storageWritePosixNew(
.modePath = modePath,
.syncFile = syncFile,
.syncPath = syncPath,
.truncate = truncate,
.user = strDup(user),
.timeModified = timeModified,

2
src/storage/posix/write.h
View File

@@ -12,6 +12,6 @@ Constructors
***********************************************************************************************************************************/
StorageWrite *storageWritePosixNew(
StoragePosix *storage, const String *name, mode_t modeFile, mode_t modePath, const String *user, const String *group,
time_t timeModified, bool createPath, bool syncFile, bool syncPath, bool atomic);
time_t timeModified, bool createPath, bool syncFile, bool syncPath, bool atomic, bool truncate);
#endif

2
src/storage/remote/protocol.c
View File

@@ -455,7 +455,7 @@ storageRemoteOpenWriteProtocol(PackRead *const param, ProtocolServer *const serv
storageInterfaceNewWriteP(
storageRemoteProtocolLocal.driver, file, .modeFile = modeFile, .modePath = modePath, .user = user, .group = group,
.timeModified = timeModified, .createPath = createPath, .syncFile = syncFile, .syncPath = syncPath,
.atomic = atomic));
.atomic = atomic, .truncate = true));
// Set filter group based on passed filters
storageRemoteFilterGroup(ioWriteFilterGroup(fileWrite), filter);

1
src/storage/remote/storage.c
View File

@@ -315,6 +315,7 @@ storageRemoteNewWrite(
ASSERT(this != NULL);
ASSERT(file != NULL);
ASSERT(param.truncate);
FUNCTION_LOG_RETURN(
STORAGE_WRITE,

1
src/storage/remote/write.c
View File

@@ -233,6 +233,7 @@ storageWriteRemoteNew(
.modePath = modePath,
.syncFile = syncFile,
.syncPath = syncPath,
.truncate = true,
.user = strDup(user),
.timeModified = timeModified,

1
src/storage/s3/storage.c
View File

@@ -886,6 +886,7 @@ storageS3NewWrite(THIS_VOID, const String *file, StorageInterfaceNewWriteParam p
ASSERT(this != NULL);
ASSERT(file != NULL);
ASSERT(param.createPath);
ASSERT(param.truncate);
ASSERT(param.user == NULL);
ASSERT(param.group == NULL);
ASSERT(param.timeModified == 0);

1
src/storage/s3/write.c
View File

@@ -295,6 +295,7 @@ storageWriteS3New(StorageS3 *storage, const String *name, size_t partSize)
.createPath = true,
.syncFile = true,
.syncPath = true,
.truncate = true,
.ioInterface = (IoWriteInterface)
{

6
src/storage/storage.c
View File

@@ -478,11 +478,14 @@ storageNewWrite(const Storage *this, const String *fileExp, StorageNewWriteParam
FUNCTION_LOG_PARAM(BOOL, param.noSyncFile);
FUNCTION_LOG_PARAM(BOOL, param.noSyncPath);
FUNCTION_LOG_PARAM(BOOL, param.noAtomic);
FUNCTION_LOG_PARAM(BOOL, param.noTruncate);
FUNCTION_LOG_PARAM(BOOL, param.compressible);
FUNCTION_LOG_END();
ASSERT(this != NULL);
ASSERT(this->write);
// noTruncate does not work with atomic writes because a new file is always created for atomic writes
ASSERT(!param.noTruncate || param.noAtomic);
StorageWrite *result = NULL;
@@ -493,7 +496,8 @@ storageNewWrite(const Storage *this, const String *fileExp, StorageNewWriteParam
storageDriver(this), storagePathP(this, fileExp), .modeFile = param.modeFile != 0 ? param.modeFile : this->modeFile,
.modePath = param.modePath != 0 ? param.modePath : this->modePath, .user = param.user, .group = param.group,
.timeModified = param.timeModified, .createPath = !param.noCreatePath, .syncFile = !param.noSyncFile,
.syncPath = !param.noSyncPath, .atomic = !param.noAtomic, .compressible = param.compressible),
.syncPath = !param.noSyncPath, .atomic = !param.noAtomic, .truncate = !param.noTruncate,
.compressible = param.compressible),
memContextPrior());
}
MEM_CONTEXT_TEMP_END();

5
src/storage/storage.h
View File

@@ -171,6 +171,11 @@ typedef struct StorageNewWriteParam
bool noSyncFile;
bool noSyncPath;
bool noAtomic;
// Do not truncate file if it exists. Use this only in cases where the file will be manipulated directly through the file
// handle, which should always be the exception and indicates functionality that should be added to the storage interface.
bool noTruncate;
bool compressible;
mode_t modeFile;
mode_t modePath;

4
src/storage/storage.intern.h
View File

@@ -156,6 +156,10 @@ typedef struct StorageInterfaceNewWriteParam
// (e.g. S3). Non-atomic writes are used in some places where there is a performance advantage and atomicity is not needed.
bool atomic;
// Truncate file if it exists. Disable this only in cases where the file will be manipulated directly through the file handle,
// which should always be the exception and shows functionality that should be added to the storage interface.
bool truncate;
// Is the file compressible? This is used when the file must be moved across a network and temporary compression is helpful.
bool compressible;
} StorageInterfaceNewWriteParam;

7
src/storage/write.h
View File

@@ -93,6 +93,13 @@ storageWriteSyncPath(const StorageWrite *const this)
return THIS_PUB(StorageWrite)->interface->syncPath;
}
// Will the file be truncated if it exists?
FN_INLINE_ALWAYS bool
storageWriteTruncate(const StorageWrite *const this)
{
return THIS_PUB(StorageWrite)->interface->truncate;
}
// File type
FN_INLINE_ALWAYS StringId
storageWriteType(const StorageWrite *const this)

1
src/storage/write.intern.h
View File

@@ -21,6 +21,7 @@ typedef struct StorageWriteInterface
const String *name;
bool atomic;
bool truncate; // Truncate file if it exists
bool createPath;
bool compressible; // Is this file compressible?
unsigned int compressLevel; // Level to use for compression

1
test/src/module/storage/azureTest.c
View File

@@ -590,6 +590,7 @@ testRun(void)
TEST_RESULT_STR_Z(storageWriteName(write), "/file.txt", "check file name");
TEST_RESULT_BOOL(storageWriteSyncFile(write), true, "file is synced");
TEST_RESULT_BOOL(storageWriteSyncPath(write), true, "path is synced");
TEST_RESULT_BOOL(storageWriteTruncate(write), true, "file will be truncated");
TEST_RESULT_VOID(storageWriteAzureClose(write->driver), "close file again");

1
test/src/module/storage/gcsTest.c
View File

@@ -540,6 +540,7 @@ testRun(void)
TEST_RESULT_STR_Z(storageWriteName(write), "/file.txt", "check file name");
TEST_RESULT_BOOL(storageWriteSyncFile(write), true, "file is synced");
TEST_RESULT_BOOL(storageWriteSyncPath(write), true, "path is synced");
TEST_RESULT_BOOL(storageWriteTruncate(write), true, "file will be truncated");
TEST_RESULT_VOID(storageWriteGcsClose(write->driver), "close file again");

17
test/src/module/storage/posixTest.c
View File

@@ -1260,6 +1260,7 @@ testRun(void)
TEST_RESULT_STR(storageWriteName(file), fileNoPerm, "check name");
TEST_RESULT_BOOL(storageWriteSyncPath(file), false, "check sync path");
TEST_RESULT_BOOL(storageWriteSyncFile(file), false, "check sync file");
TEST_RESULT_BOOL(storageWriteTruncate(file), true, "file will be truncated");
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("permission denied");
@@ -1374,6 +1375,22 @@ testRun(void)
TEST_RESULT_INT(storageInfoP(storageTest, fileName).mode, 0600, "check file mode");
storageRemoveP(storageTest, fileName, .errorOnMissing = true);
// -------------------------------------------------------------------------------------------------------------------------
TEST_TITLE("no truncate");
HRN_STORAGE_PUT_Z(storageTest, "no-truncate", "ABC", .modeFile = 0600);
TEST_ASSIGN(
file, storageNewWriteP(storageTest, STRDEF("no-truncate"), .modeFile = 0660, .timeModified = 77777, .noAtomic = true,
.noTruncate = true), "new write file");
TEST_RESULT_BOOL(storageWriteTruncate(file), false, "file will not be truncated");
TEST_RESULT_VOID(ioWriteOpen(storageWriteIo(file)), "open file");
TEST_RESULT_VOID(ioWriteClose(storageWriteIo(file)), "close file");
TEST_STORAGE_GET(storageTest, "no-truncate", "ABC");
TEST_RESULT_UINT(storageInfoP(storageTest, STRDEF("no-truncate")).mode, 0600, "check mode");
TEST_RESULT_INT(storageInfoP(storageTest, STRDEF("no-truncate")).timeModified, 77777, "check time");
}
// *****************************************************************************************************************************

1
test/src/module/storage/remoteTest.c
View File

@@ -399,6 +399,7 @@ testRun(void)
TEST_RESULT_STR_Z(storageWriteName(write), TEST_PATH "/repo128/test.txt", "check file name");
TEST_RESULT_BOOL(storageWriteSyncFile(write), true, "file is synced");
TEST_RESULT_BOOL(storageWriteSyncPath(write), true, "path is synced");
TEST_RESULT_BOOL(storageWriteTruncate(write), true, "file will be truncated");
TEST_RESULT_VOID(storagePutP(write, contentBuf), "write file");
TEST_RESULT_UINT(((StorageWriteRemote *)write->driver)->protocolWriteBytes, bufSize(contentBuf), "check write size");

1
test/src/module/storage/s3Test.c
View File

@@ -709,6 +709,7 @@ testRun(void)
TEST_RESULT_STR_Z(storageWriteName(write), "/file.txt", "check file name");
TEST_RESULT_BOOL(storageWriteSyncFile(write), true, "file is synced");
TEST_RESULT_BOOL(storageWriteSyncPath(write), true, "path is synced");
TEST_RESULT_BOOL(storageWriteTruncate(write), true, "file will be truncated");
TEST_RESULT_VOID(storageWriteS3Close(write->driver), "close file again");