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
decide_file_action().
- Heikki