On Fri, Mar 31, 2017 at 10:40 AM, Beena Emerson <memissemer...@gmail.com> wrote: > On 30 Mar 2017 15:10, "Kuntal Ghosh" <kuntalghosh.2...@gmail.com> wrote:
>> 03-modify-tools.patch - Makes XLogSegSize into a variable, currently set >> as >> XLOG_SEG_SIZE and modifies the tools to fetch the size instead of using >> inbuilt value. > Several methods are declared and defined in different tools to fetch > the size of wal-seg-size. > In pg_standby.c, > RetrieveXLogSegSize() - /* Set XLogSegSize from the WAL file header */ > > In pg_basebackup/streamutil.c, > on behaRetrieveXLogSegSize(PGconn *conn) - /* set XLogSegSize using > SHOW wal_segment_size */ > > In pg_waldump.c, > ReadXLogFromDir(char *archive_loc) > RetrieveXLogSegSize(char *archive_path) /* Scan through the archive > location to set XLogSegsize from the first WAL file */ > > IMHO, it's better to define a single method in xlog.c and based on the > different strategy, it can retrieve the XLogSegsize on behalf of > different modules. I've suggested the same in my first set review and > I'll still vote for it. For example, in xlog.c, you can define > something as following: > bool RetrieveXLogSegSize(RetrieveStrategy rs, void* ptr) > > Now based on the RetrieveStrategy(say Conn, File, Dir), you can cast > the void pointer to the appropriate type. So, when a new tool needs to > retrieve XLogSegSize, it can just call this function instead of > defining a new RetrieveXLogSegSize method. > > It's just a suggestion from my side. Is there anything I'm missing > which can cause the aforesaid approach not to be working? > Apart from that, I've nothing to add here. > > > > I do not think a generalised function is a good idea. Besides, I feel the > respective approaches are best kept in the modules used also because the > internal code is not easily accessible by utils. > Ahh, I wonder what the reason can be. Anyway, I'll leave that decision for the committer. I'm moving the status to Ready for committer. I've only tested the patch in my 64-bit linux system. It needs some testing on other environment settings. -- Thanks & Regards, Kuntal Ghosh EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
