On 14 November 2013 23:59 Fujii Masao wrote:
> On Thu, Nov 14, 2013 at 9:08 PM, Haribabu kommi
> <haribabu.ko...@huawei.com> wrote:
> > Please find attached the patch, for adding a new option for
> > pg_basebackup, to specify a different directory for pg_xlog.
>
> Sounds good! Here are the review comments:
>
> + printf(_(" --xlogdir=XLOGDIR location for the
> transaction log directory\n"));
>
> This message is not aligned well.
Fixed.
> - if (!streamwal || strcmp(filename +
> strlen(filename) - 8, "/pg_xlog") != 0)
> + if ((!streamwal && (strcmp(xlog_dir, "") == 0))
> + || strcmp(filename + strlen(filename) -
> 8, "/pg_xlog") != 0)
>
> You need to update the source code comment.
Corrected the source code comment. Please check once.
> +#ifdef HAVE_SYMLINK
> + if (symlink(xlog_dir, linkloc) != 0)
> + {
> + fprintf(stderr, _("%s: could not create symbolic link
> \"%s\": %s\n"),
> + progname, linkloc, strerror(errno));
> + exit(1);
> + }
> +#else
> + fprintf(stderr, _("%s: symlinks are not supported on this
> platform "
> + "cannot use xlogdir"));
> + exit(1);
> +#endif
> + }
>
> Is it better to call pg_free() at the end? Even if we don't do that, it
> would be almost harmless, though.
Added pg_free to free up the linkloc.
> Don't we need to prevent users from specifying the same directory in
> both --pgdata and --xlogdir?
I feel no need to prevent, even if user specifies both --pgdata and --xlogdir
as same directory
all the transaction log files will be created in the base directory instead of
pg_xlog directory.
Regards,
Hari babu.
UserSpecifiedxlogDir_v2.patch
Description: UserSpecifiedxlogDir_v2.patch
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
