Re: Refactor pg_rewind code and make it work against a standby

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Refactor pg_rewind code and make it work against a standby
Date: 2020-08-25 13:32:02
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 20/08/2020 11:32, Kyotaro Horiguchi wrote:
> 0002: Rewording that old->target and new->source makes the meaning far
> clearer. Moving decisions core code into filemap_finalize is
> reasonable.
> By the way, some of the rules are remaining in
> process_source/target_file. For example, pg_wal that is a symlink,
> or tmporary directories. and excluded files. The number of
> excluded files doesn't seem so large so it doesn't seem that the
> exclusion give advantage so much. They seem to me movable to
> filemap_finalize, and we can get rid of the callbacks by doing
> so. Is there any reason that the remaining rules should be in the
> callbacks?

Good idea! I changed the patch that way.

> 0003: Thomas is propsing sort template. It could be used if committed.
> 0004:
> The names of many of the functions gets an additional word "local"
> but I don't get the meaning clearly. but its about linguistic sense
> and I'm not fit to that..
> -rewind_copy_file_range(const char *path, off_t begin, off_t end, bool trunc)
> +local_fetch_file_range(rewind_source *source, const char *path, uint64 off,
> The function actually copying the soruce range to the target file. So
> "fetch" might give somewhat different meaning, but its about
> linguistic (omitted..).

Hmm. It is "fetching" the range from the source server, and writing it
to the target. The term makes more sense with a libpq source. Perhaps
this function should be called "local_copy_range" or something, but it'd
also be nice to have "fetch" in the name because the function pointer
it's assigned to is called "queue_fetch_range".

> I'm going to continue reviewing this later.

Thanks! Attached is a new set of patches. The only meaningful change is
in the 2nd patch, which I modified per your suggestion. Also, I moved
the logic to decide each file's fate into a new subroutine called

- Heikki

Attachment Content-Type Size
v2-0001-pg_rewind-Move-syncTargetDirectory-to-file_ops.c.patch text/x-patch 4.4 KB
v2-0002-Refactor-pg_rewind-for-more-clear-decision-making.patch text/x-patch 30.2 KB
v2-0003-pg_rewind-Replace-the-hybrid-list-array-data-stru.patch text/x-patch 21.3 KB
v2-0004-pg_rewind-Refactor-the-abstraction-to-fetch-from-.patch text/x-patch 42.0 KB
v2-0005-Allow-pg_rewind-to-use-a-standby-server-as-the-so.patch text/x-patch 17.5 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Laurenz Albe 2020-08-25 13:32:18 Re: passwordcheck: Log cracklib diagnostics
Previous Message Stephen Frost 2020-08-25 13:21:48 Re: Problems with the FSM, heap fillfactor, and temporal locality