On 2020-03-26 10:34, Michael Paquier wrote:
On Tue, Mar 24, 2020 at 12:22:16PM +0900, Michael Paquier wrote:
Thanks Alvaro and Alexander.  0001 has been applied as of e09ad07.
Now for 0002, let's see about it later.  Attached is a rebased
version, with no actual changes.

I was looking at this patch again today and I am rather fine with the
existing semantics.  Still I don't like much to name the frontend-side
routine FrontendRestoreArchivedFile() and use a different name than
the backend counterpart because we have to include xlog_internal.h in
fe_archive.c to be able to grab XLOGDIR.

So here is an idea: let's move the declaration of the routines part of
xlogarchive.c to a new header, called xlogarchive.h, and then let's
use the same routine name for the frontend and the backend in this
second patch.  We include xlog_internal.h already in many frontend
tools, so that would clean up things a bit.  Two extra things are the
routines for the checkpointer as well as the variables like
ArchiveRecoveryRequested.  It may be nice to move those while on it,
but I am not sure where and that's not actually required for this
patch set so that could be addressed later if need be.

Any thoughts?


The block of function declarations for xlogarchive.c inside xlog_internal.h looks a bit dangling already, since all other functions and variables defined/initialized in xlog.c. That way, it looks good to me to move it outside.

The only one concern about using the same name I have is that later someone may introduce a new variable or typedef inside xlogarchive.h. So someone else would be required to include both fe_archive.h and xlogarchive.h at once. But probably there should be a warning in the header comment section against doing so.

Anyway, I have tried to do what you proposed and attached is a small patch, that introduces xlogarchive.h.


Regards
--
Alexey Kondratov

Postgres Professional https://www.postgrespro.com
Russian Postgres Company
diff --git a/src/backend/access/transam/timeline.c b/src/backend/access/transam/timeline.c
index 860a996414..d683af377f 100644
--- a/src/backend/access/transam/timeline.c
+++ b/src/backend/access/transam/timeline.c
@@ -36,6 +36,7 @@
 
 #include "access/timeline.h"
 #include "access/xlog.h"
+#include "access/xlogarchive.h"
 #include "access/xlog_internal.h"
 #include "access/xlogdefs.h"
 #include "pgstat.h"
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 7621fc05e2..242427f815 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -32,6 +32,7 @@
 #include "access/transam.h"
 #include "access/twophase.h"
 #include "access/xact.h"
+#include "access/xlogarchive.h"
 #include "access/xlog_internal.h"
 #include "access/xloginsert.h"
 #include "access/xlogreader.h"
diff --git a/src/backend/access/transam/xlogarchive.c b/src/backend/access/transam/xlogarchive.c
index 914ad340ea..04104b55ea 100644
--- a/src/backend/access/transam/xlogarchive.c
+++ b/src/backend/access/transam/xlogarchive.c
@@ -20,6 +20,7 @@
 #include <unistd.h>
 
 #include "access/xlog.h"
+#include "access/xlogarchive.h"
 #include "access/xlog_internal.h"
 #include "common/archive.h"
 #include "miscadmin.h"
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index 25e0333c9e..cc91954ac1 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -48,6 +48,7 @@
 #include "access/htup_details.h"
 #include "access/timeline.h"
 #include "access/transam.h"
+#include "access/xlogarchive.h"
 #include "access/xlog_internal.h"
 #include "catalog/pg_authid.h"
 #include "catalog/pg_type.h"
diff --git a/src/include/access/xlog_internal.h b/src/include/access/xlog_internal.h
index 27ded593ab..8e3cfcf83e 100644
--- a/src/include/access/xlog_internal.h
+++ b/src/include/access/xlog_internal.h
@@ -320,22 +320,4 @@ extern bool InArchiveRecovery;
 extern bool StandbyMode;
 extern char *recoveryRestoreCommand;
 
-/*
- * Prototypes for functions in xlogarchive.c
- */
-extern bool RestoreArchivedFile(char *path, const char *xlogfname,
-								const char *recovername, off_t expectedSize,
-								bool cleanupEnabled);
-extern void ExecuteRecoveryCommand(const char *command, const char *commandName,
-								   bool failOnSignal);
-extern void KeepFileRestoredFromArchive(const char *path, const char *xlogfname);
-extern void XLogArchiveNotify(const char *xlog);
-extern void XLogArchiveNotifySeg(XLogSegNo segno);
-extern void XLogArchiveForceDone(const char *xlog);
-extern bool XLogArchiveCheckDone(const char *xlog);
-extern bool XLogArchiveIsBusy(const char *xlog);
-extern bool XLogArchiveIsReady(const char *xlog);
-extern bool XLogArchiveIsReadyOrDone(const char *xlog);
-extern void XLogArchiveCleanup(const char *xlog);
-
 #endif							/* XLOG_INTERNAL_H */
diff --git a/src/include/access/xlogarchive.h b/src/include/access/xlogarchive.h
new file mode 100644
index 0000000000..3406fb2800
--- /dev/null
+++ b/src/include/access/xlogarchive.h
@@ -0,0 +1,31 @@
+/*
+ * xlogarchive.h
+ *		Prototypes of functions for archiving WAL files and restoring
+ *		from the archive.
+ *
+ * Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group
+ * Portions Copyright (c) 1994, Regents of the University of California
+ *
+ * src/include/access/xlogarchive.h
+ */
+#ifndef XLOG_ARCHIVE_H
+#define XLOG_ARCHIVE_H
+
+#include "access/xlogdefs.h"
+
+extern bool RestoreArchivedFile(char *path, const char *xlogfname,
+								const char *recovername, off_t expectedSize,
+								bool cleanupEnabled);
+extern void ExecuteRecoveryCommand(const char *command, const char *commandName,
+								   bool failOnSignal);
+extern void KeepFileRestoredFromArchive(const char *path, const char *xlogfname);
+extern void XLogArchiveNotify(const char *xlog);
+extern void XLogArchiveNotifySeg(XLogSegNo segno);
+extern void XLogArchiveForceDone(const char *xlog);
+extern bool XLogArchiveCheckDone(const char *xlog);
+extern bool XLogArchiveIsBusy(const char *xlog);
+extern bool XLogArchiveIsReady(const char *xlog);
+extern bool XLogArchiveIsReadyOrDone(const char *xlog);
+extern void XLogArchiveCleanup(const char *xlog);
+
+#endif							/* XLOG_ARCHIVE_H */

Reply via email to