The separator parameter in cfgParseCommandRoleName() was useless since it was always set to : and COLON_STR did not provide any clarity its the single other usage.
Most of the time these were not making the code any clearer.
For cases where they were used to construct Strings and Buffers, replace with constants.
Also cleanup unused Buffers and Strings.
In cases where clock skew or timezone issues are preventing backup label generation the user could see an error like this:
new backup label '20220504-152308F' is not later than latest backup label '20220504-222042F_20220504-222141I.manifest.gz'
This will happen if the most recent label is drawn from the history. It is cleaner (and probably less confusing) to strip off the extensions so the user sees:
new backup label '20220504-152308F' is not later than latest backup label '20220504-222042F_20220504-222141I'
The order of callbacks and frees meant that memory needed during a callback (for logging in all known cases) might end up being freed before a callback needed it.
Requiring callbacks and logging to check the validity of their allocations is pretty risky and it is not clear that all possible cases have been accounted for.
Instead recursively execute all the callbacks first and then come back and recursively free the context. This is safer and it removes the need to check if a context is freeing so a simple active flag (in debug builds) will do. The caller no longer needs this information at all so remove memContextFreeing() and objMemContextFreeing().
In the JSON output the percent complete is storage as an integer of the percent complete * 100. So, before display it should be converted to double and divided by 100, or split using integer mod and div.
Note that percent complete will only be displayed on the host where the backup was executed. Remote hosts will show a backup/expire running with no percent complete.
PostgreSQL 15 drops support for exclusive backup and renames the start/stop backup commands.
This is based on the pgdg-testing repo since beta1 has not been released yet, but it seems unlikely that breaking changes will be made at this point. beta1 should be tagged just before our next release so we'll retest before the release.
It also makes sense to lock old PRs. They can be manually unlocked if they are needed for some reason.
Also add output logging to make it easier to determine if thread locking is completing.
The old plugin has been defunct for some time so there are currently a lot of unlocked issues.
Running this once per week seems sufficient for now. Worst case it can be run manually if it gets behind.
This column has been removed in PostgreSQL 15. Rather than add a lot of special handling, it seems better just to update all versions to not depend on this column.
Add centralized functions to identify the type of database (i.e. system or user) by name and use FirstNormalObjectId when a name is not available.
The new query in the db module will still return the prior result for PostgreSQL <= 15, which will be stored in the manifest. This is important to preserve behavior when downgrading pgBackRest. There are no concerns here for PostgreSQL 15 since older versions of pgBackRest won't be able to restore backups for PostgreSQL 15 anyway.
Any error thrown resets execution to the last setjmp(), which means that parts of the try block need to make sure they don't get run again. FINALLY() was not doing this so if it threw an error it would end up back in the FINALLY() block, where the error would likely be thrown again, causing an infinite loop.
Fix this by tracking the state of FINALLY() and only running it once. This requires cleaning the error stack like CATCH*() and clearing the error like TRY_END() depending on the order of execution.
The archive-get/archive-push commands would not error for, .e.g permissions errors, when attempting to get a lock before launching the async process. Since the async process was not launched there would be no error status file and the user would get a generic failure message. Also, there would be no async log.
Refactor lockAcquireFile() to throw an error when failOnNoLock = false unless the file is locked by another process. This seems to be the original intent of this parameter and there may have been a mistake when porting from Perl. In any case it looks wrong enough to be considered a bug.
The mem context name is used to produce clearer debug errors but it has no purpose in production builds.
Also remove memContextName() and access the struct directly since the name is only used within the common/memContext module.
Note that a few errors that were thrown in production builds (and required the name) are now only thrown in debug builds. In practice we have not seen these errors in production builds due to extensive coverage so it does not seem worth modifying the error to work without the context name.
This saves some memory, which is worthwhile, but the goal is to refactor Strings and Variants to have their own mem contexts and this change will prevent them from using more memory than they are now, along with other changes that will be coming later.
If this error is thrown rather than a specific error returned from the async process, it means the async process is unable to write the status files for some reason and the only way to get the error is out of the async log.
This hint includes the exact async log path and name to make finding errors easier.
This doesn't solve the problem of the variant code making far too many copies, but it at least plugs the leaks.
jsonReadVarRecurse() could leak KeyValue and VariantList.
kvDup() leaked object allocations into the calling context.
kvDefault() gets a more efficient return structure.
kvGetList() leaked a Variant into the calling context.
varLstNewStrLst() leaked object allocations into the calling context. Update varLstDup() to reflect changes made in varLstNewStrLst().
Only set -DDEBUG_MEM for the modules currently being tested rather than globally.
Also run tests in a temp mem context. Running in the top context can confuse memory accounting when a new context is created in the top context.
storageS3Helper() leaked a few Strings which ended up in a long-lived context.
storageS3AuthAuto() and storageS3AuthWebId() were cleaned up by their callers but since they are not called often a temp mem context seems better.
storageS3Request() leaked an HttpRequest.
storageS3Info() leaked an HttpResponse.
storageS3PathRemoveInternal() leaked a variety of objects. Fix by freeing some of them and adding a temp mem context.
storageS3Remove() leaked an HttpResponse object.
storageWriteS3Part() leaked an HttpResponse object.
storageRemoteFilterGroup() leaked a number of objects. Use a temp mem context to prevent that.
storageRemoteProtocolInfoListCallback() leaked a PackWrite.
storageWriteRemoteFreeResource() leaked a PackWrite.
storageGcsAuthToken() memory was being cleaned up by the calling context, but seems better to keep this tidy and add a temp mem context.
storageGcsRequest() leaked an HttpRequest.
storageGcsInfo() leaked a number of objects. Use a temp mem context to prevent that.
storageGcsPathRemoveCallback() leaked an HttpResponse.
storageGcsRemove() leaked an HttpReponse.
storageWriteGcsVerify() leaked a number of objects. Use a temp mem context to prevent that.
storageWriteGcsBlock) leaked an HttpReponse.
Reuse the section/key/value Strings by truncating them instead of creating a new one every time.
Also add an error for empty sections. This function is only used for loading info files (not config files), which should never contain an empty section.
These functions allow conversion from substrings without needing to create a String or a temporary buffer.
httpDateToTime() no longer requires a temp mem context. Also improve handling of month search to avoid an allocation.
httpUriDecode() no longer requires a temp mem context.
jsonReadStr() no longer requires a temp mem context.
pgLsnFromWalSegment() no longer requires a temp mem context.
pgVersionFromStr() no longer requires a temp mem context. Also do a bit of refactoring.
storageGcsCvtTime() no longer leaks six Strings per call.
storageS3CvtTime() no longer leaks six Strings per call.
This was missed in ccc255d3 when the TLS server was introduced, probably because work on that commit preceded when the macros were introduced in 475b57c8. It would have been easy to miss in a merge.
storageAzureHelper() leaked a few Strings which ended up in a long-lived context.
storageAzureNew() failed to make a copy of the endpoint. This worked because storageAzureHelper() leaked the endpoint into the long-lived parent context.
storageAzureRequest() leaked an HttpRequest.
storageAzureInfo() leaked an HttpResponse.
storageAzurePathRemoveCallback() leaked an HttpResponse.
storageAzureRemove() leaked an HttpResponse.
pgLsnFromWalSegment() leaked two Strings.
Refactor pgLsnRangeToWalSegmentList() to create the StringList in the calling context rather than moving it later.
These leaks were not a big deal individually and there are generally few protocol objects created, but the leaks ended up in mem contexts that persist for most of execution. It makes sense to keep long-lived contexts as tidy as possible.
Refactor so that error detail is only logged in one place. This reduces calls to exitErrorDetail() and LOG_INTERNAL_FMT().
Fix minor leaks in exitErrorDetail() and exitSafe().
