On 2020-03-02 07:53, Michael Paquier wrote:
+ * For fixed-size files, the caller may pass the expected size as an
+ * additional crosscheck on successful recovery. If the file size is
not
+ * known, set expectedSize = 0.
+ */
+int
+RestoreArchivedWALFile(const char *path, const char *xlogfname,
+ off_t expectedSize, const char
*restoreCommand)
Actually, expectedSize is IMO a bad idea, because any caller of this
routine passing down zero could be trapped with an incorrect file
size. So let's remove the behavior where it is possible to bypass
this sanity check. We don't need it in pg_rewind either.
OK, sounds reasonable, but just to be clear. I will remove only a
possibility to bypass this sanity check (with 0), but leave expectedSize
argument intact. We still need it, since pg_rewind takes WalSegSz from
ControlFile and should pass it further, am I right?
+ /* Remove trailing newline */
+ if (strchr(cmd_output, '\n') != NULL)
+ *strchr(cmd_output, '\n') = '\0';
It seems to me that what you are looking here is to use
pg_strip_crlf(). Thinking harder, we have pipe_read_line() in
src/common/exec.c which does the exact same job..
pg_strip_crlf fits well, but would you mind if I also make
pipe_read_line external in this patch?
- /*
- * construct the command to be executed
- */
Perhaps you meant "build" here.
Actually, the verb 'construct' is used historically applied to
archive/restore commands (see also xlogarchive.c and pgarch.c), but it
should be 'build' in (fe_)archive.c, since we have BuildRestoreCommand
there now.
All other remarks look clear for me, so I fix them in the next patch
version, thanks.
Regards
--
Alexey Kondratov
Postgres Professional https://www.postgrespro.com
The Russian Postgres Company
