On Sat, Apr 18, 2020 at 08:37:58AM -0400, Robert Haas wrote: > Despite the fact that we are after feature freeze, I think it would be > a good idea to commit this to PG 13. It could be saved for PG 14, but > that will make future back-patching substantially harder. Also, a > patch that's just moving code is pretty low-risk.
Makes sense to move this code around, so that's fine by me to do it even after the feature freeze as it means less back-patching pain in the future. +static inline bool +IsManifestEnabled(manifest_info *manifest) +{ + return (manifest->buffile != NULL); +} I would keep this one static and located within backup_manifest.c as it is only used there. +extern void InitializeManifest(manifest_info *manifest, + manifest_option want_manifest, + pg_checksum_type manifest_checksum_type); +extern void AppendStringToManifest(manifest_info *manifest, char *s); +extern void AddFileToManifest(manifest_info *manifest, const char *spcoid, + const char *pathname, size_t size, + pg_time_t mtime, + pg_checksum_context *checksum_ctx); +extern void AddWALInfoToManifest(manifest_info *manifest, XLogRecPtr startptr, + TimeLineID starttli, XLogRecPtr endptr, + TimeLineID endtli); +extern void SendBackupManifest(manifest_info *manifest); Now the names of those routines is not really consistent if you wish to make one single family. Here is a suggestion: - BackupManifestInit() - BackupManifestAppendString() - BackupManifestAddFile() - BackupManifestAddWALInfo() - BackupManifestSend() + * Portions Copyright (c) 2010-2020, PostgreSQL Global Development Group I would vote for some more consistency. Personally I just use that all the time: * Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California +typedef enum manifest_option +{ [...] +typedef struct manifest_info +{ These ought to be named backup_manifest_info and backup_manifest_option? -- Michael
signature.asc
Description: PGP signature