db/pgbackrest
1
0
Fork 0
mirror of https://github.com/pgbackrest/pgbackrest.git synced 2024-12-12 10:04:14 +02:00
Code Issues Releases Activity

Preserve Variant parsed from JSON in iniLoad().

Browse Source 
The fix for = characters in info files (039d314) added JSON validation but discarded the resulting Variant which means the JSON is being parsed twice. This nearly doubles the time to load a manifest since a lot of complex JSON is involved.

Time to load a million file manifest:
Before 039d314: 7.8s
039d314: 15.5s
This patch: 7.5s

To fix this regression return the Variant in the callback so the caller does not have to parse it again. The new code appears slightly more efficient overall, probably because there are fewer operations against Strings.
This commit is contained in:
David Steele 2020-07-30 10:59:50 -04:00 committed by GitHub
parent 3e9dce0d76
commit ac72e1f193
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 89 additions and 68 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
doc/xml/release.xml
View File

@ -27,6 +27,9 @@
</release-item>
<release-item>
<commit subject="Fix issue with = character in file or database names."/>
<commit subject="Preserve Variant parsed from JSON in iniLoad()."/>
<release-item-contributor-list>
<release-item-ideator id="brad.nicholson"/>
<release-item-ideator id="bastian.wegge"/>

8
src/common/ini.c
View File

@ -332,7 +332,8 @@ iniSet(Ini *this, const String *section, const String *key, const String *value)
/**********************************************************************************************************************************/
void
iniLoad(
IoRead *read, void (*callbackFunction)(void *data, const String *section, const String *key, const String *value),
IoRead *read,
void (*callbackFunction)(void *data, const String *section, const String *key, const String *value, const Variant *valueVar),
void *callbackData)
{
FUNCTION_LOG_BEGIN(logLevelDebug);
@ -395,6 +396,7 @@ iniLoad(
// then an error is thrown.
String *key;
String *value;
Variant *valueVar = NULL;
bool retry;
@ -409,7 +411,7 @@ iniLoad(
// Check that the value is valid JSON
TRY_BEGIN()
{
jsonToVar(value);
valueVar = jsonToVar(value);
}
CATCH(JsonFormatError)
{
@ -431,7 +433,7 @@ iniLoad(
THROW_FMT(FormatError, "key is zero-length at line %u: %s", lineIdx++, linePtr);
// Callback with the section/key/value
callbackFunction(callbackData, section, key, value);
callbackFunction(callbackData, section, key, value, valueVar);
}
}

3
src/common/ini.h
View File

@ -64,7 +64,8 @@ Helper Functions
***********************************************************************************************************************************/
// Load an ini file and return data to a callback
void iniLoad(
IoRead *read, void (*callbackFunction)(void *data, const String *section, const String *key, const String *value),
IoRead *read,
void (*callbackFunction)(void *data, const String *section, const String *key, const String *value, const Variant *valueVar),
void *callbackData);
/***********************************************************************************************************************************

19
src/info/info.c
View File

@ -165,13 +165,14 @@ typedef struct InfoLoadData
} InfoLoadData;
static void
infoLoadCallback(void *data, const String *section, const String *key, const String *value)
infoLoadCallback(void *data, const String *section, const String *key, const String *value, const Variant *valueVar)
{
FUNCTION_TEST_BEGIN();
FUNCTION_TEST_PARAM_P(VOID, data);
FUNCTION_TEST_PARAM(STRING, section);
FUNCTION_TEST_PARAM(STRING, key);
FUNCTION_TEST_PARAM(STRING, value);
FUNCTION_TEST_PARAM(VARIANT, valueVar);
FUNCTION_TEST_END();
ASSERT(data != NULL);
@ -206,18 +207,20 @@ infoLoadCallback(void *data, const String *section, const String *key, const Str
// Process backrest section
if (strEq(section, INFO_SECTION_BACKREST_STR))
{
ASSERT(valueVar != NULL);
// Validate format
if (strEq(key, INFO_KEY_FORMAT_STR))
{
if (jsonToUInt(value) != REPOSITORY_FORMAT)
THROW_FMT(FormatError, "expected format %d but found %d", REPOSITORY_FORMAT, cvtZToInt(strZ(value)));
if (varUInt64(valueVar) != REPOSITORY_FORMAT)
THROW_FMT(FormatError, "expected format %d but found %" PRIu64, REPOSITORY_FORMAT, varUInt64(valueVar));
}
// Store pgBackRest version
else if (strEq(key, INFO_KEY_VERSION_STR))
{
MEM_CONTEXT_BEGIN(loadData->info->memContext)
{
loadData->info->backrestVersion = jsonToStr(value);
loadData->info->backrestVersion = strDup(varStr(valueVar));
}
MEM_CONTEXT_END();
}
@ -226,7 +229,7 @@ infoLoadCallback(void *data, const String *section, const String *key, const Str
{
MEM_CONTEXT_BEGIN(loadData->memContext)
{
loadData->checksumExpected = jsonToStr(value);
loadData->checksumExpected = strDup(varStr(valueVar));
}
MEM_CONTEXT_END();
}
@ -234,19 +237,21 @@ infoLoadCallback(void *data, const String *section, const String *key, const Str
// Process cipher section
else if (strEq(section, INFO_SECTION_CIPHER_STR))
{
ASSERT(valueVar != NULL);
// No validation needed for cipher-pass, just store it
if (strEq(key, INFO_KEY_CIPHER_PASS_STR))
{
MEM_CONTEXT_BEGIN(loadData->info->memContext)
{
loadData->info->cipherPass = jsonToStr(value);
loadData->info->cipherPass = strDup(varStr(valueVar));
}
MEM_CONTEXT_END();
}
}
// Else pass to callback for processing
else
loadData->callbackFunction(loadData->callbackData, section, key, value);
loadData->callbackFunction(loadData->callbackData, section, key, valueVar);
FUNCTION_TEST_RETURN_VOID();
}

2
src/info/info.h
View File

@ -34,7 +34,7 @@ Function types for loading and saving
// start at 0 and be incremented on each call.
typedef bool InfoLoadCallback(void *data, unsigned int try);
typedef void InfoLoadNewCallback(void *data, const String *section, const String *key, const String *value);
typedef void InfoLoadNewCallback(void *data, const String *section, const String *key, const Variant *value);
typedef void InfoSaveCallback(void *data, const String *sectionNext, InfoSave *infoSaveData);
/***********************************************************************************************************************************

6
src/info/infoBackup.c
View File

@ -116,13 +116,13 @@ infoBackupNew(unsigned int pgVersion, uint64_t pgSystemId, const String *cipherP
Create new object and load contents from a file
***********************************************************************************************************************************/
static void
infoBackupLoadCallback(void *data, const String *section, const String *key, const String *value)
infoBackupLoadCallback(void *data, const String *section, const String *key, const Variant *value)
{
FUNCTION_TEST_BEGIN();
FUNCTION_TEST_PARAM_P(VOID, data);
FUNCTION_TEST_PARAM(STRING, section);
FUNCTION_TEST_PARAM(STRING, key);
FUNCTION_TEST_PARAM(STRING, value);
FUNCTION_TEST_PARAM(VARIANT, value);
FUNCTION_TEST_END();
ASSERT(data != NULL);
@ -135,7 +135,7 @@ infoBackupLoadCallback(void *data, const String *section, const String *key, con
// Process current backup list
if (strEq(section, INFO_BACKUP_SECTION_BACKUP_CURRENT_STR))
{
const KeyValue *backupKv = jsonToKv(value);
const KeyValue *backupKv = varKv(value);
MEM_CONTEXT_BEGIN(lstMemContext(infoBackup->backup))
{

11
src/info/infoPg.c
View File

@ -101,13 +101,13 @@ typedef struct InfoPgLoadData
} InfoPgLoadData;
static void
infoPgLoadCallback(void *data, const String *section, const String *key, const String *value)
infoPgLoadCallback(void *data, const String *section, const String *key, const Variant *value)
{
FUNCTION_TEST_BEGIN();
FUNCTION_TEST_PARAM_P(VOID, data);
FUNCTION_TEST_PARAM(STRING, section);
FUNCTION_TEST_PARAM(STRING, key);
FUNCTION_TEST_PARAM(STRING, value);
FUNCTION_TEST_PARAM(VARIANT, value);
FUNCTION_TEST_END();
ASSERT(data != NULL);
@ -121,15 +121,14 @@ infoPgLoadCallback(void *data, const String *section, const String *key, const S
if (strEq(section, INFO_SECTION_DB_STR))
{
if (strEq(key, INFO_KEY_DB_ID_STR))
loadData->currentId = jsonToUInt(value);
loadData->currentId = varUIntForce(value);
}
// Process db:history section
else if (strEq(section, INFO_SECTION_DB_HISTORY_STR))
{
// Load JSON loadData into a KeyValue
const KeyValue *pgDataKv = jsonToKv(value);
// Get db values that are common to all info files
const KeyValue *pgDataKv = varKv(value);
InfoPgData infoPgData =
{
.id = cvtZToUInt(strZ(key)),

89
src/info/manifest.c
View File

@ -1333,10 +1333,9 @@ manifestOwnerGet(const Variant *owner)
FUNCTION_TEST_RETURN(varStr(owner));
}
// Helper to convert default owner to a variant. Input could be boolean false (meaning there is no owner) or a string (there is an
// owner).
// Helper to check the variant type of owner and duplicate (call in the containing context)
static const Variant *
manifestOwnerDefaultGet(const String *ownerDefault)
manifestOwnerDefaultGet(const Variant *ownerDefault)
{
FUNCTION_TEST_BEGIN();
FUNCTION_TEST_PARAM(STRING, ownerDefault);
@ -1344,23 +1343,31 @@ manifestOwnerDefaultGet(const String *ownerDefault)
ASSERT(ownerDefault != NULL);
FUNCTION_TEST_RETURN(strEq(ownerDefault, FALSE_STR) ? BOOL_FALSE_VAR : varNewStr(jsonToStr(ownerDefault)));
// Bool = false means the owner was not mapped to a name
if (varType(ownerDefault) == varTypeBool)
{
// Value must be false
CHECK(!varBool(ownerDefault));
FUNCTION_TEST_RETURN(BOOL_FALSE_VAR);
}
// Return a duplicate of the owner passed in
FUNCTION_TEST_RETURN(varDup(ownerDefault));
}
static void
manifestLoadCallback(void *callbackData, const String *section, const String *key, const String *value)
manifestLoadCallback(void *callbackData, const String *section, const String *key, const Variant *value)
{
FUNCTION_TEST_BEGIN();
FUNCTION_TEST_PARAM_P(VOID, callbackData);
FUNCTION_TEST_PARAM(STRING, section);
FUNCTION_TEST_PARAM(STRING, key);
FUNCTION_TEST_PARAM(STRING, value);
FUNCTION_TEST_PARAM(VARIANT, value);
FUNCTION_TEST_END();
ASSERT(callbackData != NULL);
ASSERT(section != NULL);
ASSERT(key != NULL);
ASSERT(value != NULL);
ManifestLoadData *loadData = (ManifestLoadData *)callbackData;
Manifest *manifest = loadData->manifest;
@ -1368,7 +1375,7 @@ manifestLoadCallback(void *callbackData, const String *section, const String *ke
// -----------------------------------------------------------------------------------------------------------------------------
if (strEq(section, MANIFEST_SECTION_TARGET_FILE_STR))
{
KeyValue *fileKv = varKv(jsonToVar(value));
KeyValue *fileKv = varKv(value);
MEM_CONTEXT_BEGIN(lstMemContext(manifest->fileList))
{
@ -1458,7 +1465,7 @@ manifestLoadCallback(void *callbackData, const String *section, const String *ke
// -----------------------------------------------------------------------------------------------------------------------------
else if (strEq(section, MANIFEST_SECTION_TARGET_PATH_STR))
{
KeyValue *pathKv = varKv(jsonToVar(value));
KeyValue *pathKv = varKv(value);
MEM_CONTEXT_BEGIN(lstMemContext(manifest->pathList))
{
@ -1496,7 +1503,7 @@ manifestLoadCallback(void *callbackData, const String *section, const String *ke
// -----------------------------------------------------------------------------------------------------------------------------
else if (strEq(section, MANIFEST_SECTION_TARGET_LINK_STR))
{
KeyValue *linkKv = varKv(jsonToVar(value));
KeyValue *linkKv = varKv(value);
MEM_CONTEXT_BEGIN(lstMemContext(manifest->linkList))
{
@ -1534,9 +1541,9 @@ manifestLoadCallback(void *callbackData, const String *section, const String *ke
if (strEq(key, MANIFEST_KEY_GROUP_STR))
loadData->fileGroupDefault = manifestOwnerDefaultGet(value);
else if (strEq(key, MANIFEST_KEY_MODE_STR))
loadData->fileModeDefault = cvtZToMode(strZ(jsonToStr(value)));
loadData->fileModeDefault = cvtZToMode(strZ(varStr(value)));
else if (strEq(key, MANIFEST_KEY_PRIMARY_STR))
loadData->filePrimaryDefault = jsonToBool(value);
loadData->filePrimaryDefault = varBool(value);
else if (strEq(key, MANIFEST_KEY_USER_STR))
loadData->fileUserDefault = manifestOwnerDefaultGet(value);
}
@ -1551,7 +1558,7 @@ manifestLoadCallback(void *callbackData, const String *section, const String *ke
if (strEq(key, MANIFEST_KEY_GROUP_STR))
loadData->pathGroupDefault = manifestOwnerDefaultGet(value);
else if (strEq(key, MANIFEST_KEY_MODE_STR))
loadData->pathModeDefault = cvtZToMode(strZ(jsonToStr(value)));
loadData->pathModeDefault = cvtZToMode(strZ(varStr(value)));
else if (strEq(key, MANIFEST_KEY_USER_STR))
loadData->pathUserDefault = manifestOwnerDefaultGet(value);
}
@ -1574,7 +1581,7 @@ manifestLoadCallback(void *callbackData, const String *section, const String *ke
// -----------------------------------------------------------------------------------------------------------------------------
else if (strEq(section, MANIFEST_SECTION_BACKUP_TARGET_STR))
{
KeyValue *targetKv = varKv(jsonToVar(value));
KeyValue *targetKv = varKv(value);
const String *targetType = varStr(kvGet(targetKv, MANIFEST_KEY_TYPE_VAR));
ASSERT(strEq(targetType, MANIFEST_TARGET_TYPE_LINK_STR) || strEq(targetType, MANIFEST_TARGET_TYPE_PATH_STR));
@ -1595,7 +1602,7 @@ manifestLoadCallback(void *callbackData, const String *section, const String *ke
// -----------------------------------------------------------------------------------------------------------------------------
else if (strEq(section, MANIFEST_SECTION_DB_STR))
{
KeyValue *dbKv = varKv(jsonToVar(value));
KeyValue *dbKv = varKv(value);
MEM_CONTEXT_BEGIN(lstMemContext(manifest->dbList))
{
@ -1617,25 +1624,25 @@ manifestLoadCallback(void *callbackData, const String *section, const String *ke
MEM_CONTEXT_BEGIN(manifest->memContext)
{
if (strEq(key, MANIFEST_KEY_BACKUP_ARCHIVE_START_STR))
manifest->data.archiveStart = jsonToStr(value);
manifest->data.archiveStart = strDup(varStr(value));
else if (strEq(key, MANIFEST_KEY_BACKUP_ARCHIVE_STOP_STR))
manifest->data.archiveStop = jsonToStr(value);
manifest->data.archiveStop = strDup(varStr(value));
else if (strEq(key, MANIFEST_KEY_BACKUP_LABEL_STR))
manifest->data.backupLabel = jsonToStr(value);
manifest->data.backupLabel = strDup(varStr(value));
else if (strEq(key, MANIFEST_KEY_BACKUP_LSN_START_STR))
manifest->data.lsnStart = jsonToStr(value);
manifest->data.lsnStart = strDup(varStr(value));
else if (strEq(key, MANIFEST_KEY_BACKUP_LSN_STOP_STR))
manifest->data.lsnStop = jsonToStr(value);
manifest->data.lsnStop = strDup(varStr(value));
else if (strEq(key, MANIFEST_KEY_BACKUP_PRIOR_STR))
manifest->data.backupLabelPrior = jsonToStr(value);
manifest->data.backupLabelPrior = strDup(varStr(value));
else if (strEq(key, MANIFEST_KEY_BACKUP_TIMESTAMP_COPY_START_STR))
manifest->data.backupTimestampCopyStart = (time_t)jsonToUInt64(value);
manifest->data.backupTimestampCopyStart = (time_t)varUInt64(value);
else if (strEq(key, MANIFEST_KEY_BACKUP_TIMESTAMP_START_STR))
manifest->data.backupTimestampStart = (time_t)jsonToUInt64(value);
manifest->data.backupTimestampStart = (time_t)varUInt64(value);
else if (strEq(key, MANIFEST_KEY_BACKUP_TIMESTAMP_STOP_STR))
manifest->data.backupTimestampStop = (time_t)jsonToUInt64(value);
manifest->data.backupTimestampStop = (time_t)varUInt64(value);
else if (strEq(key, MANIFEST_KEY_BACKUP_TYPE_STR))
manifest->data.backupType = backupType(jsonToStr(value));
manifest->data.backupType = backupType(varStr(value));
}
MEM_CONTEXT_END();
}
@ -1644,11 +1651,11 @@ manifestLoadCallback(void *callbackData, const String *section, const String *ke
else if (strEq(section, MANIFEST_SECTION_BACKUP_DB_STR))
{
if (strEq(key, MANIFEST_KEY_DB_ID_STR))
manifest->data.pgId = jsonToUInt(value);
manifest->data.pgId = varUIntForce(value);
else if (strEq(key, MANIFEST_KEY_DB_SYSTEM_ID_STR))
manifest->data.pgSystemId = jsonToUInt64(value);
manifest->data.pgSystemId = varUInt64(value);
else if (strEq(key, MANIFEST_KEY_DB_VERSION_STR))
manifest->data.pgVersion = pgVersionFromStr(jsonToStr(value));
manifest->data.pgVersion = pgVersionFromStr(varStr(value));
}
// -----------------------------------------------------------------------------------------------------------------------------
@ -1658,36 +1665,36 @@ manifestLoadCallback(void *callbackData, const String *section, const String *ke
{
// Required options
if (strEq(key, MANIFEST_KEY_OPTION_ARCHIVE_CHECK_STR))
manifest->data.backupOptionArchiveCheck = jsonToBool(value);
manifest->data.backupOptionArchiveCheck = varBool(value);
else if (strEq(key, MANIFEST_KEY_OPTION_ARCHIVE_COPY_STR))
manifest->data.backupOptionArchiveCopy = jsonToBool(value);
manifest->data.backupOptionArchiveCopy = varBool(value);
// Historically this option meant to add gz compression
else if (strEq(key, MANIFEST_KEY_OPTION_COMPRESS_STR))
manifest->data.backupOptionCompressType = jsonToBool(value) ? compressTypeGz : compressTypeNone;
manifest->data.backupOptionCompressType = varBool(value) ? compressTypeGz : compressTypeNone;
// This new option allows any type of compression to be specified. It must be parsed after the option above so the
// value does not get overwritten. Since options are stored in alpha order this should always be true.
else if (strEq(key, MANIFEST_KEY_OPTION_COMPRESS_TYPE_STR))
manifest->data.backupOptionCompressType = compressTypeEnum(jsonToStr(value));
manifest->data.backupOptionCompressType = compressTypeEnum(varStr(value));
else if (strEq(key, MANIFEST_KEY_OPTION_HARDLINK_STR))
manifest->data.backupOptionHardLink = jsonToBool(value);
manifest->data.backupOptionHardLink = varBool(value);
else if (strEq(key, MANIFEST_KEY_OPTION_ONLINE_STR))
manifest->data.backupOptionOnline = jsonToBool(value);
manifest->data.backupOptionOnline = varBool(value);
// Options that were added after v1.00 and may not be present in every manifest
else if (strEq(key, MANIFEST_KEY_OPTION_BACKUP_STANDBY_STR))
manifest->data.backupOptionStandby = varNewBool(jsonToBool(value));
manifest->data.backupOptionStandby = varNewBool(varBool(value));
else if (strEq(key, MANIFEST_KEY_OPTION_BUFFER_SIZE_STR))
manifest->data.backupOptionBufferSize = varNewUInt(jsonToUInt(value));
manifest->data.backupOptionBufferSize = varNewUInt(varUIntForce(value));
else if (strEq(key, MANIFEST_KEY_OPTION_CHECKSUM_PAGE_STR))
manifest->data.backupOptionChecksumPage = varNewBool(jsonToBool(value));
manifest->data.backupOptionChecksumPage = varDup(value);
else if (strEq(key, MANIFEST_KEY_OPTION_COMPRESS_LEVEL_STR))
manifest->data.backupOptionCompressLevel = varNewUInt(jsonToUInt(value));
manifest->data.backupOptionCompressLevel = varNewUInt(varUIntForce(value));
else if (strEq(key, MANIFEST_KEY_OPTION_COMPRESS_LEVEL_NETWORK_STR))
manifest->data.backupOptionCompressLevelNetwork = varNewUInt(jsonToUInt(value));
manifest->data.backupOptionCompressLevelNetwork = varNewUInt(varUIntForce(value));
else if (strEq(key, MANIFEST_KEY_OPTION_DELTA_STR))
manifest->data.backupOptionDelta = varNewBool(jsonToBool(value));
manifest->data.backupOptionDelta = varDup(value);
else if (strEq(key, MANIFEST_KEY_OPTION_PROCESS_MAX_STR))
manifest->data.backupOptionProcessMax = varNewUInt(jsonToUInt(value));
manifest->data.backupOptionProcessMax = varNewUInt(varUIntForce(value));
}
MEM_CONTEXT_END();
}

8
test/src/common/harnessInfo.c
View File

@ -28,9 +28,11 @@ typedef struct HarnessInfoChecksumData
} HarnessInfoChecksumData;
static void
harnessInfoChecksumCallback(void *callbackData, const String *section, const String *key, const String *value)
harnessInfoChecksumCallback(
void *callbackData, const String *section, const String *key, const String *value, const Variant *valueVar)
{
HarnessInfoChecksumData *data = (HarnessInfoChecksumData *)callbackData;
(void)valueVar;
// Calculate checksum
if (data->sectionLast == NULL || !strEq(section, data->sectionLast))
@ -115,8 +117,8 @@ harnessInfoChecksumZ(const char *info)
Test callback that logs the results to a string
***********************************************************************************************************************************/
void
harnessInfoLoadNewCallback(void *callbackData, const String *section, const String *key, const String *value)
harnessInfoLoadNewCallback(void *callbackData, const String *section, const String *key, const Variant *value)
{
if (callbackData != NULL)
strCatFmt((String *)callbackData, "[%s] %s=%s\n", strZ(section), strZ(key), strZ(value));
strCatFmt((String *)callbackData, "[%s] %s=%s\n", strZ(section), strZ(key), strZ(jsonFromVar(value)));
}

2
test/src/common/harnessInfo.h
View File

@ -10,4 +10,4 @@ Functions
Buffer *harnessInfoChecksum(const String *info);
Buffer *harnessInfoChecksumZ(const char *info);
void harnessInfoLoadNewCallback(void *callbackData, const String *section, const String *key, const String *value);
void harnessInfoLoadNewCallback(void *callbackData, const String *section, const String *key, const Variant *value);

3
test/src/module/common/iniTest.c
View File

@ -9,8 +9,9 @@ Test Ini
Test callback to accumulate ini load results
***********************************************************************************************************************************/
static void
testIniLoadCallback(void *data, const String *section, const String *key, const String *value)
testIniLoadCallback(void *data, const String *section, const String *key, const String *value, const Variant *valueVar)
{
ASSERT(strEq(value, jsonFromVar(valueVar)));
strCatFmt((String *)data, "%s:%s:%s\n", strZ(section), strZ(key), strZ(value));
}

3
test/src/module/performance/typeTest.c
View File

@ -42,12 +42,13 @@ testComparator(const void *item1, const void *item2)
Test callback to count ini load results
***********************************************************************************************************************************/
static void
testIniLoadCountCallback(void *data, const String *section, const String *key, const String *value)
testIniLoadCountCallback(void *data, const String *section, const String *key, const String *value, const Variant *valueVar)
{
(*(unsigned int *)data)++;
(void)section;
(void)key;
(void)value;
(void)valueVar;
}
/***********************************************************************************************************************************