[pg_rewind] use the passing callback instead of global function

Started by Junwang Zhaoover 2 years ago3 messages
#1Junwang Zhao
zhjwpku@gmail.com
1 attachment(s)

`local_traverse_files` and `libpq_traverse_files` both have a
callback parameter but instead use the global process_source_file
which is no good for function encapsulation.

--
Regards
Junwang Zhao

Attachments:

0001-pg_rewind-use-the-passing-callback-instead-of-global.patchapplication/octet-stream; name=0001-pg_rewind-use-the-passing-callback-instead-of-global.patchDownload
From caecd1f199eec79d4b79a53bc3431fefb7405f4b Mon Sep 17 00:00:00 2001
From: Junwang Zhao <zhjwpku@gmail.com>
Date: Wed, 26 Apr 2023 09:43:56 +0800
Subject: [PATCH v1] [pg_rewind] use the passing callback instead of global
 function

`local_traverse_files` and `libpq_traverse_files` both have a
callback parameter but instead use the global process_source_file
which is no good for function encapsulation.

Signed-off-by: Junwang Zhao <zhjwpku@gmail.com>
---
 src/bin/pg_rewind/libpq_source.c | 2 +-
 src/bin/pg_rewind/local_source.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/bin/pg_rewind/libpq_source.c b/src/bin/pg_rewind/libpq_source.c
index 5f486b2a61..0d8e9ee2d1 100644
--- a/src/bin/pg_rewind/libpq_source.c
+++ b/src/bin/pg_rewind/libpq_source.c
@@ -304,7 +304,7 @@ libpq_traverse_files(rewind_source *source, process_file_callback_t callback)
 		else
 			type = FILE_TYPE_REGULAR;
 
-		process_source_file(path, type, filesize, link_target);
+		callback(path, type, filesize, link_target);
 	}
 	PQclear(res);
 }
diff --git a/src/bin/pg_rewind/local_source.c b/src/bin/pg_rewind/local_source.c
index 4e2a1376c6..9bd43cba74 100644
--- a/src/bin/pg_rewind/local_source.c
+++ b/src/bin/pg_rewind/local_source.c
@@ -59,7 +59,7 @@ init_local_source(const char *datadir)
 static void
 local_traverse_files(rewind_source *source, process_file_callback_t callback)
 {
-	traverse_datadir(((local_source *) source)->datadir, &process_source_file);
+	traverse_datadir(((local_source *) source)->datadir, callback);
 }
 
 static char *
-- 
2.33.0

#2Richard Guo
guofenglinux@gmail.com
In reply to: Junwang Zhao (#1)
Re: [pg_rewind] use the passing callback instead of global function

On Wed, Apr 26, 2023 at 9:51 AM Junwang Zhao <zhjwpku@gmail.com> wrote:

`local_traverse_files` and `libpq_traverse_files` both have a
callback parameter but instead use the global process_source_file
which is no good for function encapsulation.

Nice catch. This should be a typo introduced by 37d2ff38.

While this patch is doing it correctly, I'm wondering that since both
kinds of source server (libpq and local) are using the same function
(i.e. process_source_file) to process source file list for
traverse_files operations, do we really need to provide a callback? Or
will there be some kind of source server that may use different source
file processing function?

Thanks
Richard

#3Daniel Gustafsson
daniel@yesql.se
In reply to: Richard Guo (#2)
Re: [pg_rewind] use the passing callback instead of global function

On 26 Apr 2023, at 10:33, Richard Guo <guofenglinux@gmail.com> wrote:

On Wed, Apr 26, 2023 at 9:51 AM Junwang Zhao <zhjwpku@gmail.com <mailto:zhjwpku@gmail.com>> wrote:
`local_traverse_files` and `libpq_traverse_files` both have a
callback parameter but instead use the global process_source_file
which is no good for function encapsulation.

Nice catch. This should be a typo introduced by 37d2ff38.

Agreed, I'll look at applying this after some testing.

While this patch is doing it correctly, I'm wondering that since both
kinds of source server (libpq and local) are using the same function
(i.e. process_source_file) to process source file list for
traverse_files operations, do we really need to provide a callback? Or
will there be some kind of source server that may use different source
file processing function?

While there isn't one right now, removing the callback seems like imposing a
restriction that the refactoring in 37d2ff38 aimed to avoid.

--
Daniel Gustafsson