On Tue, Nov 19, 2013 at 12:01 AM, Haribabu kommi <haribabu.ko...@huawei.com> wrote: > On 18 November 2013 18:45 Fujii Masao wrote: >> On Mon, Nov 18, 2013 at 6:31 PM, Haribabu kommi >> <haribabu.ko...@huawei.com> wrote: >> > >> > On 18 November 2013 11:19 Haribabu kommi wrote: >> >> On 17 November 2013 00:55 Fujii Masao wrote: >> >> > On Sat, Nov 16, 2013 at 2:27 PM, Haribabu kommi >> >> > <haribabu.ko...@huawei.com> wrote: >> >> > > on 15 November 2013 17:26 Magnus Hagander wrote: >> >> > > >> >> > >>On Fri, Nov 15, 2013 at 12:10 PM, Haribabu kommi >> >> > >><haribabu.ko...@huawei.com> wrote: >> >> > > >> >> > >>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: >> >> > >>>> 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. >> >> > > >> >> > > >> >> > > >> >> > >>Given how easy it would be to prevent that, I think we should. >> It >> >> > >>would be an easy misunderstanding to specify that when you >> >> actually >> >> > >>want it in <wherever>/pg_xlog. Specifying that would be >> >> > >>redundant in the first place, but people ca do that, but it >> >> > > >> >> > >>would also be very easy to do it by mistake, and you'd end up >> >> > >>with something that's really bad, including a recursive symlink. >> >> > > >> >> > > >> >> > > >> >> > > Presently with initdb also user can specify both data and xlog >> >> > > directories as same. >> >> > > >> >> > > To prevent the data directory and xlog directory as same, there >> >> > > is >> >> a >> >> > > way in windows (_fullpath api) to get absolute path from a >> >> > > relative path, but I didn't get any such possibilities in linux. >> >> > > >> >> > > I didn't find any other way to check it, if anyone have any idea >> >> > > regarding this please let me know. >> >> > >> >> > What about make_absolute_path() in miscinit.c? >> >> >> >> The make_absoulte_patch() function gets the current working >> directory >> >> and adds The relative path to CWD, this is not giving proper >> absolute >> >> path. >> >> >> >> I have added a new function verify_data_and_xlog_dir_same() which >> >> will change the Current working directory to data directory and gets >> >> the CWD and the same way for transaction log directory. Compare the >> >> both data and xlog directories and throw an error. Please check it >> once. >> >> >> >> Is there any other way to identify that both data and xlog >> >> directories are pointing to the same Instead of comparing both >> absolute paths? >> >> >> >> Updated patch attached in the mail. >> > >> > Failure when the xlogdir doesn't exist is fixed in the updated patch >> attached in the mail. >> >> Thanks for updating the patch! >> >> With the patch, when I specify the same directory in both -D and -- >> xlogdir, I got the following error. >> >> $ cd /tmp >> $ pg_basebackup -D hoge --xlogdir=/tmp/hoge -X fetch >> pg_basebackup: could not change directory to "hoge": No such file or >> directory > > Thanks. After finding the xlog directory absolute path returning back to > current working > Directory is missed, because of this reason it is failing while finding the > absolute > Path for data directory. Apart from this change the code is reorganized a bit. > > Updated patch is attached in the mail. Please review it once.
Thanks for newer version of the patch! I found that the empty base directory is created and remains even when the same directory is specified in both -D and --xlogdir and the error occurs. I think that it's better to throw an error in that case before creating any new directory. Thought? + xlogdir = get_absolute_path(xlog_dir); xlog_dir must be an absolute path. ISTM we don't do the above. No? Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
