On Tue, Aug 25, 2020 at 04:32:02PM +0300, Heikki Linnakangas wrote: > 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().
The patch set fails to apply from 0002~, so this needs a rebase. I have not looked at all that in details, but no objections to apply 0001 from me. It makes sense to move the sync subroutine for the target folder to file_ops.c. -- Michael
signature.asc
Description: PGP signature
