moving basebackup code to its own directory
Hi,
I was thinking that it might make sense, to reduce clutter, to move
*backup*.c from src/backend/replication to a new directory, perhaps
src/backend/replication/backup or src/backend/backup.
There's no particular reason we *have* to do this, but there are 21 C
files in that directory and 11 of them are basebackup-related, so
maybe it's time, especially because I think we might end up adding
more basebackup-related stuff.
Thoughts?
--
Robert Haas
EDB: http://www.enterprisedb.com
On Tue, Aug 9, 2022 at 6:08 PM Robert Haas <robertmhaas@gmail.com> wrote:
Hi,
I was thinking that it might make sense, to reduce clutter, to move
*backup*.c from src/backend/replication to a new directory, perhaps
src/backend/replication/backup or src/backend/backup.There's no particular reason we *have* to do this, but there are 21 C
files in that directory and 11 of them are basebackup-related, so
maybe it's time, especially because I think we might end up adding
more basebackup-related stuff.Thoughts?
Those 11 files are mostly your fault, of course ;)
Anyway, I have no objection. If there'd been that many files, or plans to
have it, in the beginning we probably would've put them in
replication/basebackup or something like that from the beginning. I'm not
sure how much it's worth doing wrt effects on backpatching etc, but if
we're planning to add even more files in the future, the pain will just
become bigger once we eventually do it...
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
On Tue, Aug 9, 2022 at 12:12 PM Magnus Hagander <magnus@hagander.net> wrote:
Those 11 files are mostly your fault, of course ;)
They are. I tend to prefer smaller source files than many developers,
because I find them easier to understand and maintain. If you only
include <zlib.h> in basebackup_gzip.c, then you can be pretty sure
nothing else involved with basebackup is accidentally depending on it.
Similarly with static variables. If you just have one giant file, it's
harder to be sure about that sort of thing.
Anyway, I have no objection. If there'd been that many files, or plans to have it, in the beginning we probably would've put them in replication/basebackup or something like that from the beginning. I'm not sure how much it's worth doing wrt effects on backpatching etc, but if we're planning to add even more files in the future, the pain will just become bigger once we eventually do it...
Right.
It's not exactly clear to me what the optimal source code layout is
here. I think the placement here is under src/backend/replication
because the functionality is accessed via the replication protocol,
but I'm not sure if all backup-related code we ever add will be
related to the replication protocol. As a thought experiment, imagine
a background worker that triggers a backup periodically, or a
monitoring view that tells you about the status of your last 10 backup
attempts, or an in-memory hash table that tracks which files have been
modified since the last backup. I'm not planning on implementing any
of those things specifically, but I guess I'm a little concerned that
if we just do the obvious thing of src/backend/replication/backup it's
going to be end up being a little awkward if I or anyone else want to
add backup-related code that isn't specifically about the replication
protocol.
So maybe src/backend/backup? Or is that too grandiose for the amount
of stuff we have here?
--
Robert Haas
EDB: http://www.enterprisedb.com
On 8/9/22 12:12, Magnus Hagander wrote:
On Tue, Aug 9, 2022 at 6:08 PM Robert Haas <robertmhaas@gmail.com
<mailto:robertmhaas@gmail.com>> wrote:Hi,
I was thinking that it might make sense, to reduce clutter, to move
*backup*.c from src/backend/replication to a new directory, perhaps
src/backend/replication/backup or src/backend/backup.There's no particular reason we *have* to do this, but there are 21 C
files in that directory and 11 of them are basebackup-related, so
maybe it's time, especially because I think we might end up adding
more basebackup-related stuff.Thoughts?
Those 11 files are mostly your fault, of course ;)
Anyway, I have no objection. If there'd been that many files, or plans
to have it, in the beginning we probably would've put them in
replication/basebackup or something like that from the beginning. I'm
not sure how much it's worth doing wrt effects on backpatching etc, but
if we're planning to add even more files in the future, the pain will
just become bigger once we eventually do it...
There are big changes all around for PG15 so back-patching will be
complicated no matter what.
+1 from me and it would be great if we can get this into the PG15 branch
as well.
Regards,
-David
On 8/9/22 12:34, Robert Haas wrote:
On Tue, Aug 9, 2022 at 12:12 PM Magnus Hagander <magnus@hagander.net> wrote:
Anyway, I have no objection. If there'd been that many files, or plans to have it, in the beginning we probably would've put them in replication/basebackup or something like that from the beginning. I'm not sure how much it's worth doing wrt effects on backpatching etc, but if we're planning to add even more files in the future, the pain will just become bigger once we eventually do it...
Right.
It's not exactly clear to me what the optimal source code layout is
here. I think the placement here is under src/backend/replication
because the functionality is accessed via the replication protocol,
but I'm not sure if all backup-related code we ever add will be
related to the replication protocol. As a thought experiment, imagine
a background worker that triggers a backup periodically, or a
monitoring view that tells you about the status of your last 10 backup
attempts, or an in-memory hash table that tracks which files have been
modified since the last backup. I'm not planning on implementing any
of those things specifically, but I guess I'm a little concerned that
if we just do the obvious thing of src/backend/replication/backup it's
going to be end up being a little awkward if I or anyone else want to
add backup-related code that isn't specifically about the replication
protocol.So maybe src/backend/backup? Or is that too grandiose for the amount
of stuff we have here?
+1 for src/backend/backup. I'd also be happy to see the start/stop code
move here at some point.
Regards,
-David
On Tue, Aug 9, 2022 at 6:41 PM David Steele <david@pgmasters.net> wrote:
On 8/9/22 12:34, Robert Haas wrote:
On Tue, Aug 9, 2022 at 12:12 PM Magnus Hagander <magnus@hagander.net>
wrote:
Anyway, I have no objection. If there'd been that many files, or plans
to have it, in the beginning we probably would've put them in
replication/basebackup or something like that from the beginning. I'm not
sure how much it's worth doing wrt effects on backpatching etc, but if
we're planning to add even more files in the future, the pain will just
become bigger once we eventually do it...Right.
It's not exactly clear to me what the optimal source code layout is
here. I think the placement here is under src/backend/replication
because the functionality is accessed via the replication protocol,
but I'm not sure if all backup-related code we ever add will be
related to the replication protocol. As a thought experiment, imagine
a background worker that triggers a backup periodically, or a
monitoring view that tells you about the status of your last 10 backup
attempts, or an in-memory hash table that tracks which files have been
modified since the last backup. I'm not planning on implementing any
of those things specifically, but I guess I'm a little concerned that
if we just do the obvious thing of src/backend/replication/backup it's
going to be end up being a little awkward if I or anyone else want to
add backup-related code that isn't specifically about the replication
protocol.So maybe src/backend/backup? Or is that too grandiose for the amount
of stuff we have here?+1 for src/backend/backup. I'd also be happy to see the start/stop code
move here at some point.
Yeah, sounds reasonable. There's never an optimal source code layout, but I
agree this one is better than putting it under replication.
--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>
On Tue, Aug 9, 2022 at 12:43 PM Magnus Hagander <magnus@hagander.net> wrote:
So maybe src/backend/backup? Or is that too grandiose for the amount
of stuff we have here?+1 for src/backend/backup. I'd also be happy to see the start/stop code
move here at some point.Yeah, sounds reasonable. There's never an optimal source code layout, but I agree this one is better than putting it under replication.
OK, here's a patch.
--
Robert Haas
EDB: http://www.enterprisedb.com
Attachments:
v1-0001-Move-basebackup-code-to-new-directory-src-backend.patchapplication/octet-stream; name=v1-0001-Move-basebackup-code-to-new-directory-src-backend.patchDownload
From 7d6d3c65ff10eec79afbafe7dd0a9f7f8b3d2f29 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Tue, 9 Aug 2022 13:18:40 -0400
Subject: [PATCH v1] Move basebackup code to new directory src/backend/backup
---
.../basebackup_to_shell/basebackup_to_shell.c | 2 +-
src/backend/Makefile | 3 +-
src/backend/access/transam/xlog.c | 2 +-
src/backend/access/transam/xlogrecovery.c | 2 +-
src/backend/backup/Makefile | 30 +++++++++++++++++++
.../{replication => backup}/backup_manifest.c | 4 +--
.../{replication => backup}/basebackup.c | 10 +++----
.../{replication => backup}/basebackup_copy.c | 4 +--
.../{replication => backup}/basebackup_gzip.c | 2 +-
.../{replication => backup}/basebackup_lz4.c | 2 +-
.../basebackup_progress.c | 4 +--
.../basebackup_server.c | 4 +--
.../{replication => backup}/basebackup_sink.c | 2 +-
.../basebackup_target.c | 2 +-
.../basebackup_throttle.c | 2 +-
.../{replication => backup}/basebackup_zstd.c | 2 +-
src/backend/replication/Makefile | 11 -------
src/backend/replication/walsender.c | 2 +-
src/bin/pg_basebackup/pg_basebackup.c | 2 +-
.../{replication => backup}/backup_manifest.h | 4 +--
.../{replication => backup}/basebackup.h | 2 +-
.../{replication => backup}/basebackup_sink.h | 2 +-
.../basebackup_target.h | 4 +--
23 files changed, 62 insertions(+), 42 deletions(-)
create mode 100644 src/backend/backup/Makefile
rename src/backend/{replication => backup}/backup_manifest.c (99%)
rename src/backend/{replication => backup}/basebackup.c (99%)
rename src/backend/{replication => backup}/basebackup_copy.c (99%)
rename src/backend/{replication => backup}/basebackup_gzip.c (99%)
rename src/backend/{replication => backup}/basebackup_lz4.c (99%)
rename src/backend/{replication => backup}/basebackup_progress.c (98%)
rename src/backend/{replication => backup}/basebackup_server.c (99%)
rename src/backend/{replication => backup}/basebackup_sink.c (98%)
rename src/backend/{replication => backup}/basebackup_target.c (99%)
rename src/backend/{replication => backup}/basebackup_throttle.c (99%)
rename src/backend/{replication => backup}/basebackup_zstd.c (99%)
rename src/include/{replication => backup}/backup_manifest.h (94%)
rename src/include/{replication => backup}/basebackup.h (96%)
rename src/include/{replication => backup}/basebackup_sink.h (99%)
rename src/include/{replication => backup}/basebackup_target.h (96%)
diff --git a/contrib/basebackup_to_shell/basebackup_to_shell.c b/contrib/basebackup_to_shell/basebackup_to_shell.c
index 34188f2d9d..e2b18631e0 100644
--- a/contrib/basebackup_to_shell/basebackup_to_shell.c
+++ b/contrib/basebackup_to_shell/basebackup_to_shell.c
@@ -11,8 +11,8 @@
#include "postgres.h"
#include "access/xact.h"
+#include "backup/basebackup_target.h"
#include "miscadmin.h"
-#include "replication/basebackup_target.h"
#include "storage/fd.h"
#include "utils/acl.h"
#include "utils/guc.h"
diff --git a/src/backend/Makefile b/src/backend/Makefile
index 01d5a7448f..3f01c65592 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -17,7 +17,8 @@ subdir = src/backend
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
-SUBDIRS = access bootstrap catalog parser commands executor foreign lib libpq \
+SUBDIRS = access backup bootstrap catalog parser commands executor \
+ foreign lib libpq \
main nodes optimizer partitioning port postmaster \
regex replication rewrite \
statistics storage tcop tsearch utils $(top_builddir)/src/timezone \
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 34f0150d1e..9cedd6876f 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -63,6 +63,7 @@
#include "access/xlogreader.h"
#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
+#include "backup/basebackup.h"
#include "catalog/catversion.h"
#include "catalog/pg_control.h"
#include "catalog/pg_database.h"
@@ -77,7 +78,6 @@
#include "postmaster/bgwriter.h"
#include "postmaster/startup.h"
#include "postmaster/walwriter.h"
-#include "replication/basebackup.h"
#include "replication/logical.h"
#include "replication/origin.h"
#include "replication/slot.h"
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 21088e78f6..a59a0e826b 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -40,6 +40,7 @@
#include "access/xlogreader.h"
#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
+#include "backup/basebackup.h"
#include "catalog/pg_control.h"
#include "commands/tablespace.h"
#include "common/file_utils.h"
@@ -47,7 +48,6 @@
#include "pgstat.h"
#include "postmaster/bgwriter.h"
#include "postmaster/startup.h"
-#include "replication/basebackup.h"
#include "replication/walreceiver.h"
#include "storage/fd.h"
#include "storage/ipc.h"
diff --git a/src/backend/backup/Makefile b/src/backend/backup/Makefile
new file mode 100644
index 0000000000..b21bd8ff43
--- /dev/null
+++ b/src/backend/backup/Makefile
@@ -0,0 +1,30 @@
+#-------------------------------------------------------------------------
+#
+# Makefile--
+# Makefile for src/backend/backup
+#
+# IDENTIFICATION
+# src/backend/backup/Makefile
+#
+#-------------------------------------------------------------------------
+
+subdir = src/backend/backup
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
+
+override CPPFLAGS := -I. -I$(srcdir) $(CPPFLAGS)
+
+OBJS = \
+ backup_manifest.o \
+ basebackup.o \
+ basebackup_copy.o \
+ basebackup_gzip.o \
+ basebackup_lz4.o \
+ basebackup_zstd.o \
+ basebackup_progress.o \
+ basebackup_server.o \
+ basebackup_sink.o \
+ basebackup_target.o \
+ basebackup_throttle.o
+
+include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/replication/backup_manifest.c b/src/backend/backup/backup_manifest.c
similarity index 99%
rename from src/backend/replication/backup_manifest.c
rename to src/backend/backup/backup_manifest.c
index d47ab4c41e..358ed9a0d1 100644
--- a/src/backend/replication/backup_manifest.c
+++ b/src/backend/backup/backup_manifest.c
@@ -13,11 +13,11 @@
#include "postgres.h"
#include "access/timeline.h"
+#include "backup/backup_manifest.h"
+#include "backup/basebackup_sink.h"
#include "libpq/libpq.h"
#include "libpq/pqformat.h"
#include "mb/pg_wchar.h"
-#include "replication/backup_manifest.h"
-#include "replication/basebackup_sink.h"
#include "utils/builtins.h"
#include "utils/json.h"
diff --git a/src/backend/replication/basebackup.c b/src/backend/backup/basebackup.c
similarity index 99%
rename from src/backend/replication/basebackup.c
rename to src/backend/backup/basebackup.c
index deeddd09a9..20707b1235 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -17,9 +17,13 @@
#include <time.h>
#include "access/xlog_internal.h" /* for pg_backup_start/stop */
+#include "backup/backup_manifest.h"
+#include "backup/basebackup.h"
+#include "backup/basebackup_sink.h"
+#include "backup/basebackup_target.h"
+#include "commands/defrem.h"
#include "common/compression.h"
#include "common/file_perm.h"
-#include "commands/defrem.h"
#include "lib/stringinfo.h"
#include "miscadmin.h"
#include "nodes/pg_list.h"
@@ -27,10 +31,6 @@
#include "pgtar.h"
#include "port.h"
#include "postmaster/syslogger.h"
-#include "replication/basebackup.h"
-#include "replication/basebackup_sink.h"
-#include "replication/basebackup_target.h"
-#include "replication/backup_manifest.h"
#include "replication/walsender.h"
#include "replication/walsender_private.h"
#include "storage/bufpage.h"
diff --git a/src/backend/replication/basebackup_copy.c b/src/backend/backup/basebackup_copy.c
similarity index 99%
rename from src/backend/replication/basebackup_copy.c
rename to src/backend/backup/basebackup_copy.c
index c384d63a34..ef6097a8b9 100644
--- a/src/backend/replication/basebackup_copy.c
+++ b/src/backend/backup/basebackup_copy.c
@@ -26,12 +26,12 @@
#include "postgres.h"
#include "access/tupdesc.h"
+#include "backup/basebackup.h"
+#include "backup/basebackup_sink.h"
#include "catalog/pg_type_d.h"
#include "executor/executor.h"
#include "libpq/libpq.h"
#include "libpq/pqformat.h"
-#include "replication/basebackup.h"
-#include "replication/basebackup_sink.h"
#include "tcop/dest.h"
#include "utils/builtins.h"
#include "utils/timestamp.h"
diff --git a/src/backend/replication/basebackup_gzip.c b/src/backend/backup/basebackup_gzip.c
similarity index 99%
rename from src/backend/replication/basebackup_gzip.c
rename to src/backend/backup/basebackup_gzip.c
index ef2b954946..5acb67d04e 100644
--- a/src/backend/replication/basebackup_gzip.c
+++ b/src/backend/backup/basebackup_gzip.c
@@ -16,7 +16,7 @@
#include <zlib.h>
#endif
-#include "replication/basebackup_sink.h"
+#include "backup/basebackup_sink.h"
#ifdef HAVE_LIBZ
typedef struct bbsink_gzip
diff --git a/src/backend/replication/basebackup_lz4.c b/src/backend/backup/basebackup_lz4.c
similarity index 99%
rename from src/backend/replication/basebackup_lz4.c
rename to src/backend/backup/basebackup_lz4.c
index c9d19b6c44..bae3d91fa9 100644
--- a/src/backend/replication/basebackup_lz4.c
+++ b/src/backend/backup/basebackup_lz4.c
@@ -16,7 +16,7 @@
#include <lz4frame.h>
#endif
-#include "replication/basebackup_sink.h"
+#include "backup/basebackup_sink.h"
#ifdef USE_LZ4
diff --git a/src/backend/replication/basebackup_progress.c b/src/backend/backup/basebackup_progress.c
similarity index 98%
rename from src/backend/replication/basebackup_progress.c
rename to src/backend/backup/basebackup_progress.c
index 36671ad3fd..0cb3f59cdd 100644
--- a/src/backend/replication/basebackup_progress.c
+++ b/src/backend/backup/basebackup_progress.c
@@ -31,10 +31,10 @@
*/
#include "postgres.h"
+#include "backup/basebackup.h"
+#include "backup/basebackup_sink.h"
#include "commands/progress.h"
#include "miscadmin.h"
-#include "replication/basebackup.h"
-#include "replication/basebackup_sink.h"
#include "pgstat.h"
#include "storage/latch.h"
#include "utils/timestamp.h"
diff --git a/src/backend/replication/basebackup_server.c b/src/backend/backup/basebackup_server.c
similarity index 99%
rename from src/backend/replication/basebackup_server.c
rename to src/backend/backup/basebackup_server.c
index 9b4847d90c..5f01e49064 100644
--- a/src/backend/replication/basebackup_server.c
+++ b/src/backend/backup/basebackup_server.c
@@ -11,10 +11,10 @@
#include "postgres.h"
#include "access/xact.h"
+#include "backup/basebackup.h"
+#include "backup/basebackup_sink.h"
#include "catalog/pg_authid.h"
#include "miscadmin.h"
-#include "replication/basebackup.h"
-#include "replication/basebackup_sink.h"
#include "storage/fd.h"
#include "utils/acl.h"
#include "utils/timestamp.h"
diff --git a/src/backend/replication/basebackup_sink.c b/src/backend/backup/basebackup_sink.c
similarity index 98%
rename from src/backend/replication/basebackup_sink.c
rename to src/backend/backup/basebackup_sink.c
index 81353f8f4d..1d264ac687 100644
--- a/src/backend/replication/basebackup_sink.c
+++ b/src/backend/backup/basebackup_sink.c
@@ -12,7 +12,7 @@
#include "postgres.h"
-#include "replication/basebackup_sink.h"
+#include "backup/basebackup_sink.h"
/*
* Forward begin_backup callback.
diff --git a/src/backend/replication/basebackup_target.c b/src/backend/backup/basebackup_target.c
similarity index 99%
rename from src/backend/replication/basebackup_target.c
rename to src/backend/backup/basebackup_target.c
index 9f73457320..79d543093b 100644
--- a/src/backend/replication/basebackup_target.c
+++ b/src/backend/backup/basebackup_target.c
@@ -15,7 +15,7 @@
*/
#include "postgres.h"
-#include "replication/basebackup_target.h"
+#include "backup/basebackup_target.h"
#include "utils/memutils.h"
typedef struct BaseBackupTargetType
diff --git a/src/backend/replication/basebackup_throttle.c b/src/backend/backup/basebackup_throttle.c
similarity index 99%
rename from src/backend/replication/basebackup_throttle.c
rename to src/backend/backup/basebackup_throttle.c
index af0704c3ac..99b23e98ee 100644
--- a/src/backend/replication/basebackup_throttle.c
+++ b/src/backend/backup/basebackup_throttle.c
@@ -14,8 +14,8 @@
*/
#include "postgres.h"
+#include "backup/basebackup_sink.h"
#include "miscadmin.h"
-#include "replication/basebackup_sink.h"
#include "pgstat.h"
#include "storage/latch.h"
#include "utils/timestamp.h"
diff --git a/src/backend/replication/basebackup_zstd.c b/src/backend/backup/basebackup_zstd.c
similarity index 99%
rename from src/backend/replication/basebackup_zstd.c
rename to src/backend/backup/basebackup_zstd.c
index b23a37b29e..7442d015da 100644
--- a/src/backend/replication/basebackup_zstd.c
+++ b/src/backend/backup/basebackup_zstd.c
@@ -16,7 +16,7 @@
#include <zstd.h>
#endif
-#include "replication/basebackup_sink.h"
+#include "backup/basebackup_sink.h"
#ifdef USE_ZSTD
diff --git a/src/backend/replication/Makefile b/src/backend/replication/Makefile
index 3d8fb70c0e..2bffac58c0 100644
--- a/src/backend/replication/Makefile
+++ b/src/backend/replication/Makefile
@@ -15,17 +15,6 @@ include $(top_builddir)/src/Makefile.global
override CPPFLAGS := -I. -I$(srcdir) $(CPPFLAGS)
OBJS = \
- backup_manifest.o \
- basebackup.o \
- basebackup_copy.o \
- basebackup_gzip.o \
- basebackup_lz4.o \
- basebackup_zstd.o \
- basebackup_progress.o \
- basebackup_server.o \
- basebackup_sink.o \
- basebackup_target.o \
- basebackup_throttle.o \
repl_gram.o \
slot.o \
slotfuncs.o \
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 3a86786cc3..724010dbd9 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -57,6 +57,7 @@
#include "access/xlogreader.h"
#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
+#include "backup/basebackup.h"
#include "catalog/pg_authid.h"
#include "catalog/pg_type.h"
#include "commands/dbcommands.h"
@@ -68,7 +69,6 @@
#include "nodes/replnodes.h"
#include "pgstat.h"
#include "postmaster/interrupt.h"
-#include "replication/basebackup.h"
#include "replication/decode.h"
#include "replication/logical.h"
#include "replication/slot.h"
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 8694b05e68..0841f6636c 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -28,6 +28,7 @@
#endif
#include "access/xlog_internal.h"
+#include "backup/basebackup.h"
#include "bbstreamer.h"
#include "common/compression.h"
#include "common/file_perm.h"
@@ -37,7 +38,6 @@
#include "fe_utils/recovery_gen.h"
#include "getopt_long.h"
#include "receivelog.h"
-#include "replication/basebackup.h"
#include "streamutil.h"
#define ERRCODE_DATA_CORRUPTED "XX001"
diff --git a/src/include/replication/backup_manifest.h b/src/include/backup/backup_manifest.h
similarity index 94%
rename from src/include/replication/backup_manifest.h
rename to src/include/backup/backup_manifest.h
index 062d05024a..b15f0fa2ec 100644
--- a/src/include/replication/backup_manifest.h
+++ b/src/include/backup/backup_manifest.h
@@ -5,16 +5,16 @@
*
* Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
*
- * src/include/replication/backup_manifest.h
+ * src/include/backup/backup_manifest.h
*
*-------------------------------------------------------------------------
*/
#ifndef BACKUP_MANIFEST_H
#define BACKUP_MANIFEST_H
+#include "backup/basebackup_sink.h"
#include "common/checksum_helper.h"
#include "pgtime.h"
-#include "replication/basebackup_sink.h"
#include "storage/buffile.h"
typedef enum manifest_option
diff --git a/src/include/replication/basebackup.h b/src/include/backup/basebackup.h
similarity index 96%
rename from src/include/replication/basebackup.h
rename to src/include/backup/basebackup.h
index 1badcd45eb..593479afdc 100644
--- a/src/include/replication/basebackup.h
+++ b/src/include/backup/basebackup.h
@@ -5,7 +5,7 @@
*
* Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
*
- * src/include/replication/basebackup.h
+ * src/include/backup/basebackup.h
*
*-------------------------------------------------------------------------
*/
diff --git a/src/include/replication/basebackup_sink.h b/src/include/backup/basebackup_sink.h
similarity index 99%
rename from src/include/replication/basebackup_sink.h
rename to src/include/backup/basebackup_sink.h
index 36278cac14..a1cd24ce81 100644
--- a/src/include/replication/basebackup_sink.h
+++ b/src/include/backup/basebackup_sink.h
@@ -19,7 +19,7 @@
*
* Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
*
- * src/include/replication/basebackup_sink.h
+ * src/include/backup/basebackup_sink.h
*
*-------------------------------------------------------------------------
*/
diff --git a/src/include/replication/basebackup_target.h b/src/include/backup/basebackup_target.h
similarity index 96%
rename from src/include/replication/basebackup_target.h
rename to src/include/backup/basebackup_target.h
index 1cf3c0777d..3a359dd4d0 100644
--- a/src/include/replication/basebackup_target.h
+++ b/src/include/backup/basebackup_target.h
@@ -5,14 +5,14 @@
*
* Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
*
- * src/include/replication/basebackup_target.h
+ * src/include/backup/basebackup_target.h
*
*-------------------------------------------------------------------------
*/
#ifndef BASEBACKUP_TARGET_H
#define BASEBACKUP_TARGET_H
-#include "replication/basebackup_sink.h"
+#include "backup/basebackup_sink.h"
struct BaseBackupTargetHandle;
typedef struct BaseBackupTargetHandle BaseBackupTargetHandle;
--
2.24.3 (Apple Git-128)
On 8/9/22 13:32, Robert Haas wrote:
On Tue, Aug 9, 2022 at 12:43 PM Magnus Hagander <magnus@hagander.net> wrote:
So maybe src/backend/backup? Or is that too grandiose for the amount
of stuff we have here?+1 for src/backend/backup. I'd also be happy to see the start/stop code
move here at some point.Yeah, sounds reasonable. There's never an optimal source code layout, but I agree this one is better than putting it under replication.
OK, here's a patch.
This looks good to me.
Regards,
-David
On Tue, Aug 09, 2022 at 01:32:49PM -0400, Robert Haas wrote:
On Tue, Aug 9, 2022 at 12:43 PM Magnus Hagander <magnus@hagander.net> wrote:
So maybe src/backend/backup? Or is that too grandiose for the amount
of stuff we have here?+1 for src/backend/backup. I'd also be happy to see the start/stop code
move here at some point.Yeah, sounds reasonable. There's never an optimal source code layout, but I agree this one is better than putting it under replication.
OK, here's a patch.
It looks like this updates the header comments in the .h files but not the .c
files.
Personally, I find these to be silly boilerplate ..
--
Justin
On 8/9/22 14:40, Justin Pryzby wrote:
On Tue, Aug 09, 2022 at 01:32:49PM -0400, Robert Haas wrote:
On Tue, Aug 9, 2022 at 12:43 PM Magnus Hagander <magnus@hagander.net> wrote:
So maybe src/backend/backup? Or is that too grandiose for the amount
of stuff we have here?+1 for src/backend/backup. I'd also be happy to see the start/stop code
move here at some point.Yeah, sounds reasonable. There's never an optimal source code layout, but I agree this one is better than putting it under replication.
OK, here's a patch.
It looks like this updates the header comments in the .h files but not the .c
files.Personally, I find these to be silly boilerplate ..
Good catch. I did not notice that just looking at the diff.
Definitely agree that repeating the filename in the top comment is
mostly useless, but that seems like a separate conversation.
-David
On Tue, Aug 9, 2022 at 2:40 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
It looks like this updates the header comments in the .h files but not the .c
files.Personally, I find these to be silly boilerplate ..
Here is a version with some updates to the silly boilerplate.
--
Robert Haas
EDB: http://www.enterprisedb.com
Attachments:
v2-0001-Move-basebackup-code-to-new-directory-src-backend.patchapplication/octet-stream; name=v2-0001-Move-basebackup-code-to-new-directory-src-backend.patchDownload
From b3cf043dbd65ed766c3aa26151ecaa76fc0974ef Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Tue, 9 Aug 2022 15:24:05 -0400
Subject: [PATCH v2] Move basebackup code to new directory src/backend/backup
Reviewed by David Steele and Justin Pryzby
---
.../basebackup_to_shell/basebackup_to_shell.c | 2 +-
src/backend/Makefile | 3 +-
src/backend/access/transam/xlog.c | 2 +-
src/backend/access/transam/xlogrecovery.c | 2 +-
src/backend/backup/Makefile | 30 +++++++++++++++++++
.../{replication => backup}/backup_manifest.c | 6 ++--
.../{replication => backup}/basebackup.c | 12 ++++----
.../{replication => backup}/basebackup_copy.c | 6 ++--
.../{replication => backup}/basebackup_gzip.c | 4 +--
.../{replication => backup}/basebackup_lz4.c | 4 +--
.../basebackup_progress.c | 6 ++--
.../basebackup_server.c | 6 ++--
.../{replication => backup}/basebackup_sink.c | 4 +--
.../basebackup_target.c | 4 +--
.../basebackup_throttle.c | 4 +--
.../{replication => backup}/basebackup_zstd.c | 4 +--
src/backend/replication/Makefile | 11 -------
src/backend/replication/walsender.c | 2 +-
src/bin/pg_basebackup/pg_basebackup.c | 2 +-
.../{replication => backup}/backup_manifest.h | 4 +--
.../{replication => backup}/basebackup.h | 2 +-
.../{replication => backup}/basebackup_sink.h | 2 +-
.../basebackup_target.h | 4 +--
23 files changed, 73 insertions(+), 53 deletions(-)
create mode 100644 src/backend/backup/Makefile
rename src/backend/{replication => backup}/backup_manifest.c (98%)
rename src/backend/{replication => backup}/basebackup.c (99%)
rename src/backend/{replication => backup}/basebackup_copy.c (99%)
rename src/backend/{replication => backup}/basebackup_gzip.c (99%)
rename src/backend/{replication => backup}/basebackup_lz4.c (99%)
rename src/backend/{replication => backup}/basebackup_progress.c (98%)
rename src/backend/{replication => backup}/basebackup_server.c (98%)
rename src/backend/{replication => backup}/basebackup_sink.c (97%)
rename src/backend/{replication => backup}/basebackup_target.c (98%)
rename src/backend/{replication => backup}/basebackup_throttle.c (98%)
rename src/backend/{replication => backup}/basebackup_zstd.c (99%)
rename src/include/{replication => backup}/backup_manifest.h (94%)
rename src/include/{replication => backup}/basebackup.h (96%)
rename src/include/{replication => backup}/basebackup_sink.h (99%)
rename src/include/{replication => backup}/basebackup_target.h (96%)
diff --git a/contrib/basebackup_to_shell/basebackup_to_shell.c b/contrib/basebackup_to_shell/basebackup_to_shell.c
index 34188f2d9d..e2b18631e0 100644
--- a/contrib/basebackup_to_shell/basebackup_to_shell.c
+++ b/contrib/basebackup_to_shell/basebackup_to_shell.c
@@ -11,8 +11,8 @@
#include "postgres.h"
#include "access/xact.h"
+#include "backup/basebackup_target.h"
#include "miscadmin.h"
-#include "replication/basebackup_target.h"
#include "storage/fd.h"
#include "utils/acl.h"
#include "utils/guc.h"
diff --git a/src/backend/Makefile b/src/backend/Makefile
index 01d5a7448f..3f01c65592 100644
--- a/src/backend/Makefile
+++ b/src/backend/Makefile
@@ -17,7 +17,8 @@ subdir = src/backend
top_builddir = ../..
include $(top_builddir)/src/Makefile.global
-SUBDIRS = access bootstrap catalog parser commands executor foreign lib libpq \
+SUBDIRS = access backup bootstrap catalog parser commands executor \
+ foreign lib libpq \
main nodes optimizer partitioning port postmaster \
regex replication rewrite \
statistics storage tcop tsearch utils $(top_builddir)/src/timezone \
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 34f0150d1e..9cedd6876f 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -63,6 +63,7 @@
#include "access/xlogreader.h"
#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
+#include "backup/basebackup.h"
#include "catalog/catversion.h"
#include "catalog/pg_control.h"
#include "catalog/pg_database.h"
@@ -77,7 +78,6 @@
#include "postmaster/bgwriter.h"
#include "postmaster/startup.h"
#include "postmaster/walwriter.h"
-#include "replication/basebackup.h"
#include "replication/logical.h"
#include "replication/origin.h"
#include "replication/slot.h"
diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c
index 21088e78f6..a59a0e826b 100644
--- a/src/backend/access/transam/xlogrecovery.c
+++ b/src/backend/access/transam/xlogrecovery.c
@@ -40,6 +40,7 @@
#include "access/xlogreader.h"
#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
+#include "backup/basebackup.h"
#include "catalog/pg_control.h"
#include "commands/tablespace.h"
#include "common/file_utils.h"
@@ -47,7 +48,6 @@
#include "pgstat.h"
#include "postmaster/bgwriter.h"
#include "postmaster/startup.h"
-#include "replication/basebackup.h"
#include "replication/walreceiver.h"
#include "storage/fd.h"
#include "storage/ipc.h"
diff --git a/src/backend/backup/Makefile b/src/backend/backup/Makefile
new file mode 100644
index 0000000000..b21bd8ff43
--- /dev/null
+++ b/src/backend/backup/Makefile
@@ -0,0 +1,30 @@
+#-------------------------------------------------------------------------
+#
+# Makefile--
+# Makefile for src/backend/backup
+#
+# IDENTIFICATION
+# src/backend/backup/Makefile
+#
+#-------------------------------------------------------------------------
+
+subdir = src/backend/backup
+top_builddir = ../../..
+include $(top_builddir)/src/Makefile.global
+
+override CPPFLAGS := -I. -I$(srcdir) $(CPPFLAGS)
+
+OBJS = \
+ backup_manifest.o \
+ basebackup.o \
+ basebackup_copy.o \
+ basebackup_gzip.o \
+ basebackup_lz4.o \
+ basebackup_zstd.o \
+ basebackup_progress.o \
+ basebackup_server.o \
+ basebackup_sink.o \
+ basebackup_target.o \
+ basebackup_throttle.o
+
+include $(top_srcdir)/src/backend/common.mk
diff --git a/src/backend/replication/backup_manifest.c b/src/backend/backup/backup_manifest.c
similarity index 98%
rename from src/backend/replication/backup_manifest.c
rename to src/backend/backup/backup_manifest.c
index d47ab4c41e..a54185fdab 100644
--- a/src/backend/replication/backup_manifest.c
+++ b/src/backend/backup/backup_manifest.c
@@ -6,18 +6,18 @@
* Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/backend/replication/backup_manifest.c
+ * src/backend/backup/backup_manifest.c
*
*-------------------------------------------------------------------------
*/
#include "postgres.h"
#include "access/timeline.h"
+#include "backup/backup_manifest.h"
+#include "backup/basebackup_sink.h"
#include "libpq/libpq.h"
#include "libpq/pqformat.h"
#include "mb/pg_wchar.h"
-#include "replication/backup_manifest.h"
-#include "replication/basebackup_sink.h"
#include "utils/builtins.h"
#include "utils/json.h"
diff --git a/src/backend/replication/basebackup.c b/src/backend/backup/basebackup.c
similarity index 99%
rename from src/backend/replication/basebackup.c
rename to src/backend/backup/basebackup.c
index deeddd09a9..715428029b 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/backup/basebackup.c
@@ -6,7 +6,7 @@
* Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/backend/replication/basebackup.c
+ * src/backend/backup/basebackup.c
*
*-------------------------------------------------------------------------
*/
@@ -17,9 +17,13 @@
#include <time.h>
#include "access/xlog_internal.h" /* for pg_backup_start/stop */
+#include "backup/backup_manifest.h"
+#include "backup/basebackup.h"
+#include "backup/basebackup_sink.h"
+#include "backup/basebackup_target.h"
+#include "commands/defrem.h"
#include "common/compression.h"
#include "common/file_perm.h"
-#include "commands/defrem.h"
#include "lib/stringinfo.h"
#include "miscadmin.h"
#include "nodes/pg_list.h"
@@ -27,10 +31,6 @@
#include "pgtar.h"
#include "port.h"
#include "postmaster/syslogger.h"
-#include "replication/basebackup.h"
-#include "replication/basebackup_sink.h"
-#include "replication/basebackup_target.h"
-#include "replication/backup_manifest.h"
#include "replication/walsender.h"
#include "replication/walsender_private.h"
#include "storage/bufpage.h"
diff --git a/src/backend/replication/basebackup_copy.c b/src/backend/backup/basebackup_copy.c
similarity index 99%
rename from src/backend/replication/basebackup_copy.c
rename to src/backend/backup/basebackup_copy.c
index c384d63a34..a5ad7fa392 100644
--- a/src/backend/replication/basebackup_copy.c
+++ b/src/backend/backup/basebackup_copy.c
@@ -19,19 +19,19 @@
* Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/backend/replication/basebackup_copy.c
+ * src/backend/backup/basebackup_copy.c
*
*-------------------------------------------------------------------------
*/
#include "postgres.h"
#include "access/tupdesc.h"
+#include "backup/basebackup.h"
+#include "backup/basebackup_sink.h"
#include "catalog/pg_type_d.h"
#include "executor/executor.h"
#include "libpq/libpq.h"
#include "libpq/pqformat.h"
-#include "replication/basebackup.h"
-#include "replication/basebackup_sink.h"
#include "tcop/dest.h"
#include "utils/builtins.h"
#include "utils/timestamp.h"
diff --git a/src/backend/replication/basebackup_gzip.c b/src/backend/backup/basebackup_gzip.c
similarity index 99%
rename from src/backend/replication/basebackup_gzip.c
rename to src/backend/backup/basebackup_gzip.c
index ef2b954946..a965866ff2 100644
--- a/src/backend/replication/basebackup_gzip.c
+++ b/src/backend/backup/basebackup_gzip.c
@@ -6,7 +6,7 @@
* Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/backend/replication/basebackup_gzip.c
+ * src/backend/backup/basebackup_gzip.c
*
*-------------------------------------------------------------------------
*/
@@ -16,7 +16,7 @@
#include <zlib.h>
#endif
-#include "replication/basebackup_sink.h"
+#include "backup/basebackup_sink.h"
#ifdef HAVE_LIBZ
typedef struct bbsink_gzip
diff --git a/src/backend/replication/basebackup_lz4.c b/src/backend/backup/basebackup_lz4.c
similarity index 99%
rename from src/backend/replication/basebackup_lz4.c
rename to src/backend/backup/basebackup_lz4.c
index c9d19b6c44..d919e3dec7 100644
--- a/src/backend/replication/basebackup_lz4.c
+++ b/src/backend/backup/basebackup_lz4.c
@@ -6,7 +6,7 @@
* Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/backend/replication/basebackup_lz4.c
+ * src/backend/backup/basebackup_lz4.c
*
*-------------------------------------------------------------------------
*/
@@ -16,7 +16,7 @@
#include <lz4frame.h>
#endif
-#include "replication/basebackup_sink.h"
+#include "backup/basebackup_sink.h"
#ifdef USE_LZ4
diff --git a/src/backend/replication/basebackup_progress.c b/src/backend/backup/basebackup_progress.c
similarity index 98%
rename from src/backend/replication/basebackup_progress.c
rename to src/backend/backup/basebackup_progress.c
index 36671ad3fd..6d4b5a23d1 100644
--- a/src/backend/replication/basebackup_progress.c
+++ b/src/backend/backup/basebackup_progress.c
@@ -25,16 +25,16 @@
* Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/backend/replication/basebackup_progress.c
+ * src/backend/backup/basebackup_progress.c
*
*-------------------------------------------------------------------------
*/
#include "postgres.h"
+#include "backup/basebackup.h"
+#include "backup/basebackup_sink.h"
#include "commands/progress.h"
#include "miscadmin.h"
-#include "replication/basebackup.h"
-#include "replication/basebackup_sink.h"
#include "pgstat.h"
#include "storage/latch.h"
#include "utils/timestamp.h"
diff --git a/src/backend/replication/basebackup_server.c b/src/backend/backup/basebackup_server.c
similarity index 98%
rename from src/backend/replication/basebackup_server.c
rename to src/backend/backup/basebackup_server.c
index 9b4847d90c..d020a92bfa 100644
--- a/src/backend/replication/basebackup_server.c
+++ b/src/backend/backup/basebackup_server.c
@@ -4,17 +4,17 @@
* store basebackup archives on the server
*
* IDENTIFICATION
- * src/backend/replication/basebackup_server.c
+ * src/backend/backup/basebackup_server.c
*
*-------------------------------------------------------------------------
*/
#include "postgres.h"
#include "access/xact.h"
+#include "backup/basebackup.h"
+#include "backup/basebackup_sink.h"
#include "catalog/pg_authid.h"
#include "miscadmin.h"
-#include "replication/basebackup.h"
-#include "replication/basebackup_sink.h"
#include "storage/fd.h"
#include "utils/acl.h"
#include "utils/timestamp.h"
diff --git a/src/backend/replication/basebackup_sink.c b/src/backend/backup/basebackup_sink.c
similarity index 97%
rename from src/backend/replication/basebackup_sink.c
rename to src/backend/backup/basebackup_sink.c
index 81353f8f4d..4536029d84 100644
--- a/src/backend/replication/basebackup_sink.c
+++ b/src/backend/backup/basebackup_sink.c
@@ -5,14 +5,14 @@
*
* Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
*
- * src/backend/replication/basebackup_sink.c
+ * src/backend/backup/basebackup_sink.c
*
*-------------------------------------------------------------------------
*/
#include "postgres.h"
-#include "replication/basebackup_sink.h"
+#include "backup/basebackup_sink.h"
/*
* Forward begin_backup callback.
diff --git a/src/backend/replication/basebackup_target.c b/src/backend/backup/basebackup_target.c
similarity index 98%
rename from src/backend/replication/basebackup_target.c
rename to src/backend/backup/basebackup_target.c
index 9f73457320..83928e3205 100644
--- a/src/backend/replication/basebackup_target.c
+++ b/src/backend/backup/basebackup_target.c
@@ -9,13 +9,13 @@
* Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/backend/replication/basebackup_target.c
+ * src/backend/backup/basebackup_target.c
*
*-------------------------------------------------------------------------
*/
#include "postgres.h"
-#include "replication/basebackup_target.h"
+#include "backup/basebackup_target.h"
#include "utils/memutils.h"
typedef struct BaseBackupTargetType
diff --git a/src/backend/replication/basebackup_throttle.c b/src/backend/backup/basebackup_throttle.c
similarity index 98%
rename from src/backend/replication/basebackup_throttle.c
rename to src/backend/backup/basebackup_throttle.c
index af0704c3ac..62ba73214c 100644
--- a/src/backend/replication/basebackup_throttle.c
+++ b/src/backend/backup/basebackup_throttle.c
@@ -8,14 +8,14 @@
* Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/backend/replication/basebackup_throttle.c
+ * src/backend/backup/basebackup_throttle.c
*
*-------------------------------------------------------------------------
*/
#include "postgres.h"
+#include "backup/basebackup_sink.h"
#include "miscadmin.h"
-#include "replication/basebackup_sink.h"
#include "pgstat.h"
#include "storage/latch.h"
#include "utils/timestamp.h"
diff --git a/src/backend/replication/basebackup_zstd.c b/src/backend/backup/basebackup_zstd.c
similarity index 99%
rename from src/backend/replication/basebackup_zstd.c
rename to src/backend/backup/basebackup_zstd.c
index b23a37b29e..865067f8dc 100644
--- a/src/backend/replication/basebackup_zstd.c
+++ b/src/backend/backup/basebackup_zstd.c
@@ -6,7 +6,7 @@
* Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
*
* IDENTIFICATION
- * src/backend/replication/basebackup_zstd.c
+ * src/backend/backup/basebackup_zstd.c
*
*-------------------------------------------------------------------------
*/
@@ -16,7 +16,7 @@
#include <zstd.h>
#endif
-#include "replication/basebackup_sink.h"
+#include "backup/basebackup_sink.h"
#ifdef USE_ZSTD
diff --git a/src/backend/replication/Makefile b/src/backend/replication/Makefile
index 3d8fb70c0e..2bffac58c0 100644
--- a/src/backend/replication/Makefile
+++ b/src/backend/replication/Makefile
@@ -15,17 +15,6 @@ include $(top_builddir)/src/Makefile.global
override CPPFLAGS := -I. -I$(srcdir) $(CPPFLAGS)
OBJS = \
- backup_manifest.o \
- basebackup.o \
- basebackup_copy.o \
- basebackup_gzip.o \
- basebackup_lz4.o \
- basebackup_zstd.o \
- basebackup_progress.o \
- basebackup_server.o \
- basebackup_sink.o \
- basebackup_target.o \
- basebackup_throttle.o \
repl_gram.o \
slot.o \
slotfuncs.o \
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index 3a86786cc3..724010dbd9 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -57,6 +57,7 @@
#include "access/xlogreader.h"
#include "access/xlogrecovery.h"
#include "access/xlogutils.h"
+#include "backup/basebackup.h"
#include "catalog/pg_authid.h"
#include "catalog/pg_type.h"
#include "commands/dbcommands.h"
@@ -68,7 +69,6 @@
#include "nodes/replnodes.h"
#include "pgstat.h"
#include "postmaster/interrupt.h"
-#include "replication/basebackup.h"
#include "replication/decode.h"
#include "replication/logical.h"
#include "replication/slot.h"
diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c
index 8694b05e68..0841f6636c 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -28,6 +28,7 @@
#endif
#include "access/xlog_internal.h"
+#include "backup/basebackup.h"
#include "bbstreamer.h"
#include "common/compression.h"
#include "common/file_perm.h"
@@ -37,7 +38,6 @@
#include "fe_utils/recovery_gen.h"
#include "getopt_long.h"
#include "receivelog.h"
-#include "replication/basebackup.h"
#include "streamutil.h"
#define ERRCODE_DATA_CORRUPTED "XX001"
diff --git a/src/include/replication/backup_manifest.h b/src/include/backup/backup_manifest.h
similarity index 94%
rename from src/include/replication/backup_manifest.h
rename to src/include/backup/backup_manifest.h
index 062d05024a..b15f0fa2ec 100644
--- a/src/include/replication/backup_manifest.h
+++ b/src/include/backup/backup_manifest.h
@@ -5,16 +5,16 @@
*
* Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
*
- * src/include/replication/backup_manifest.h
+ * src/include/backup/backup_manifest.h
*
*-------------------------------------------------------------------------
*/
#ifndef BACKUP_MANIFEST_H
#define BACKUP_MANIFEST_H
+#include "backup/basebackup_sink.h"
#include "common/checksum_helper.h"
#include "pgtime.h"
-#include "replication/basebackup_sink.h"
#include "storage/buffile.h"
typedef enum manifest_option
diff --git a/src/include/replication/basebackup.h b/src/include/backup/basebackup.h
similarity index 96%
rename from src/include/replication/basebackup.h
rename to src/include/backup/basebackup.h
index 1badcd45eb..593479afdc 100644
--- a/src/include/replication/basebackup.h
+++ b/src/include/backup/basebackup.h
@@ -5,7 +5,7 @@
*
* Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
*
- * src/include/replication/basebackup.h
+ * src/include/backup/basebackup.h
*
*-------------------------------------------------------------------------
*/
diff --git a/src/include/replication/basebackup_sink.h b/src/include/backup/basebackup_sink.h
similarity index 99%
rename from src/include/replication/basebackup_sink.h
rename to src/include/backup/basebackup_sink.h
index 36278cac14..a1cd24ce81 100644
--- a/src/include/replication/basebackup_sink.h
+++ b/src/include/backup/basebackup_sink.h
@@ -19,7 +19,7 @@
*
* Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
*
- * src/include/replication/basebackup_sink.h
+ * src/include/backup/basebackup_sink.h
*
*-------------------------------------------------------------------------
*/
diff --git a/src/include/replication/basebackup_target.h b/src/include/backup/basebackup_target.h
similarity index 96%
rename from src/include/replication/basebackup_target.h
rename to src/include/backup/basebackup_target.h
index 1cf3c0777d..3a359dd4d0 100644
--- a/src/include/replication/basebackup_target.h
+++ b/src/include/backup/basebackup_target.h
@@ -5,14 +5,14 @@
*
* Portions Copyright (c) 2010-2022, PostgreSQL Global Development Group
*
- * src/include/replication/basebackup_target.h
+ * src/include/backup/basebackup_target.h
*
*-------------------------------------------------------------------------
*/
#ifndef BASEBACKUP_TARGET_H
#define BASEBACKUP_TARGET_H
-#include "replication/basebackup_sink.h"
+#include "backup/basebackup_sink.h"
struct BaseBackupTargetHandle;
typedef struct BaseBackupTargetHandle BaseBackupTargetHandle;
--
2.24.3 (Apple Git-128)
On Tue, Aug 9, 2022 at 3:28 PM Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Aug 9, 2022 at 2:40 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
It looks like this updates the header comments in the .h files but not the .c
files.Personally, I find these to be silly boilerplate ..
Here is a version with some updates to the silly boilerplate.
If there are no further comments on this I will go ahead and commit it.
David Steele voted for back-patching this on the grounds that it would
make future back-patching easier, which is an argument that seems to
me to have some merit, although on the other hand, we are already into
August so it's quite late in the day. Anyone else want to vote?
--
Robert Haas
EDB: http://www.enterprisedb.com
On Wed, Aug 10, 2022 at 10:08:02AM -0400, Robert Haas wrote:
On Tue, Aug 9, 2022 at 3:28 PM Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Aug 9, 2022 at 2:40 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
It looks like this updates the header comments in the .h files but not the .c
files.Personally, I find these to be silly boilerplate ..
Here is a version with some updates to the silly boilerplate.
If there are no further comments on this I will go ahead and commit it.
David Steele voted for back-patching this on the grounds that it would
make future back-patching easier, which is an argument that seems to
me to have some merit, although on the other hand, we are already into
August so it's quite late in the day. Anyone else want to vote?
No objection to backpatching to v15, but if you don't, git ought to handle
renamed files just fine.
These look like similar precedent for "late" renaming+backpatching: 41dae3553,
47ca48364
--
Justin
Robert Haas <robertmhaas@gmail.com> writes:
David Steele voted for back-patching this on the grounds that it would
make future back-patching easier, which is an argument that seems to
me to have some merit, although on the other hand, we are already into
August so it's quite late in the day. Anyone else want to vote?
Seems like low-risk refactoring, so +1 for keeping v15 close to HEAD.
regards, tom lane
On Wed, Aug 10, 2022 at 6:20 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
David Steele voted for back-patching this on the grounds that it would
make future back-patching easier, which is an argument that seems to
me to have some merit, although on the other hand, we are already into
August so it's quite late in the day. Anyone else want to vote?Seems like low-risk refactoring, so +1 for keeping v15 close to HEAD.
+1, but I suggest also getting a hat-tip from the RMT on it.
--
Magnus Hagander
Me: https://www.hagander.net/
Work: https://www.redpill-linpro.com/
On 8/10/22 12:32 PM, Magnus Hagander wrote:
On Wed, Aug 10, 2022 at 6:20 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
David Steele voted for back-patching this on the grounds that it would
make future back-patching easier, which is an argument that seems to
me to have some merit, although on the other hand, we are already into
August so it's quite late in the day. Anyone else want to vote?Seems like low-risk refactoring, so +1 for keeping v15 close to HEAD.
+1, but I suggest also getting a hat-tip from the RMT on it.
With RMT hat on, given a few folks who maintain backup utilities seem to
be in favor of backpatching to v15 and they are the ones to be most
affected by this, it seems to me that this is an acceptable,
noncontroversial course of action.
Jonathan
On 2022-Aug-10, Robert Haas wrote:
David Steele voted for back-patching this on the grounds that it would
make future back-patching easier, which is an argument that seems to
me to have some merit, although on the other hand, we are already into
August so it's quite late in the day. Anyone else want to vote?
Given that 10 of these 11 files are new in 15, I definitely agree with
backpatching the move.
Moving the include/ files is going to cause some pain for any
third-party code #including those files. I don't think this is a
problem.
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
On Wed, Aug 10, 2022 at 12:57 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
Given that 10 of these 11 files are new in 15, I definitely agree with
backpatching the move.
OK, done.
--
Robert Haas
EDB: http://www.enterprisedb.com