Hello hackers,
I think we should extend the "log" directory the same courtesy as was
done for pg_wal (pg_xlog) in 0e42397f42b.
Today, even if BOTH source and target servers have symlinked "log"
directories, pg_rewind fails with:
file "log" is of different type in source and target.
Attached is a repro patch using the 004_pg_xlog_symlink.pl test to
demonstrate the failure.
Running make check PROVE_TESTS='t/004_pg_xlog_symlink.pl'
in src/bin/pg_rewind should suffice after applying.
This is because when we use the libpq query to fetch the filemap from
the source server, we consider the log directory as a directory, even if
it is a symlink. This is because pg_stat_file() is used in that query in
libpq_traverse_files() and pg_stat_file() returns isdir=t for symlinks
to directories.
This shortcoming is somewhat called out:
* XXX: There is no backend function to get a symbolic link's target in
* general, so if the admin has put any custom symbolic links in the data
* directory, they won't be copied correctly.
We could fix the query and/or pg_stat_file(). However, we would also
like to support deployments where only one of the primaries and/or
standbys have the symlink. That is not hard to conceive, given primaries
and standbys can have drastically disparate log volume and/or log
collection requirements.
Attached is a patch that treats "log" like we treat "pg_wal".
Regards,
Soumyadeep (VMware)
From 697414d2b630efdad0a9137ea9cc93f8576a9792 Mon Sep 17 00:00:00 2001
From: Soumyadeep Chakraborty <soumyadeep2...@gmail.com>
Date: Sun, 5 Mar 2023 17:57:55 -0800
Subject: [PATCH v1 1/1] Fix pg_rewind when log is a symlink
The log directory can often be symlinked in the same way the pg_wal
directory is. Today, even if BOTH the source and target servers have
their log directories symlinked, pg_rewind will run into the error:
"log" is of different type in source and target
This is because when we use the libpq query to fetch the filemap from
the source server, we consider the log directory as a directory, even if
it is a symlink. This is because pg_stat_file() is used in that query in
libpq_traverse_files() and pg_stat_file() returns isdir=t for symlinks
to directories.
This shortcoming is somewhat called out:
* XXX: There is no backend function to get a symbolic link's target in
* general, so if the admin has put any custom symbolic links in the data
* directory, they won't be copied correctly.
We could fix the query and/or pg_stat_file(). However, we would also
like to support deployments where only one of the primaries and/or
standbys have the symlink. That is not hard to conceive, given primaries
and standbys can have drastically disparate log volume and/or log
collection requirements.
So, we decide to extend the log directory the same courtesy as was done
for pg_wal (pg_xlog) in 0e42397f42b.
---
src/bin/pg_rewind/filemap.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c
index bd5c598e200..a076bb33996 100644
--- a/src/bin/pg_rewind/filemap.c
+++ b/src/bin/pg_rewind/filemap.c
@@ -221,11 +221,11 @@ process_source_file(const char *path, file_type_t type, size_t size,
file_entry_t *entry;
/*
- * Pretend that pg_wal is a directory, even if it's really a symlink. We
+ * Pretend that pg_wal/log is a directory, even if it's really a symlink. We
* don't want to mess with the symlink itself, nor complain if it's a
* symlink in source but not in target or vice versa.
*/
- if (strcmp(path, "pg_wal") == 0 && type == FILE_TYPE_SYMLINK)
+ if (((strcmp(path, "pg_wal") == 0 || strcmp(path, "log") == 0)) && type == FILE_TYPE_SYMLINK)
type = FILE_TYPE_DIRECTORY;
/*
@@ -263,9 +263,9 @@ process_target_file(const char *path, file_type_t type, size_t size,
*/
/*
- * Like in process_source_file, pretend that pg_wal is always a directory.
+ * Like in process_source_file, pretend that pg_wal/log is always a directory.
*/
- if (strcmp(path, "pg_wal") == 0 && type == FILE_TYPE_SYMLINK)
+ if (((strcmp(path, "pg_wal") == 0 || strcmp(path, "log") == 0)) && type == FILE_TYPE_SYMLINK)
type = FILE_TYPE_DIRECTORY;
/* Remember this target file */
--
2.34.1
diff --git a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
index 5fb7fa9077c..f95ba1a1486 100644
--- a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
+++ b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
@@ -2,7 +2,7 @@
# Copyright (c) 2021-2023, PostgreSQL Global Development Group
#
-# Test pg_rewind when the target's pg_wal directory is a symlink.
+# Test pg_rewind when the target's log directory is a symlink.
#
use strict;
use warnings;
@@ -27,11 +27,12 @@ sub run_test
RewindTest::setup_cluster($test_mode);
my $test_primary_datadir = $node_primary->data_dir;
+ mkdir("$test_primary_datadir/log") or die;
# turn pg_wal into a symlink
- print("moving $test_primary_datadir/pg_wal to $primary_xlogdir\n");
- move("$test_primary_datadir/pg_wal", $primary_xlogdir) or die;
- dir_symlink($primary_xlogdir, "$test_primary_datadir/pg_wal") or die;
+ print("moving $test_primary_datadir/log to $primary_xlogdir\n");
+ move("$test_primary_datadir/log", $primary_xlogdir) or die;
+ dir_symlink($primary_xlogdir, "$test_primary_datadir/log") or die;
RewindTest::start_primary();
@@ -43,6 +44,16 @@ sub run_test
RewindTest::create_standby($test_mode);
+ my $test_standby_datadir = $node_standby->data_dir;
+ mkdir("$test_standby_datadir/log") or die;
+
+ my $standby_xlogdir =
+ "${PostgreSQL::Test::Utils::tmp_check}/xlog_standby";
+ print("moving $test_standby_datadir/log to $standby_xlogdir\n");
+ move("$test_standby_datadir/log", $standby_xlogdir) or die;
+ dir_symlink($standby_xlogdir, "$test_standby_datadir/log") or die;
+
+
# Insert additional data on primary that will be replicated to standby
primary_psql("INSERT INTO tbl1 values ('in primary, before promotion')");
