On Thu, Sep 28, 2017 at 10:52 PM, chenhj <chjis...@163.com> wrote: > On 2017-09-29 00:43:18，"Alexander Korotkov" <a.korot...@postgrespro.ru> > wrote: > > On Thu, Sep 28, 2017 at 6:44 PM, chenhj <chjis...@163.com> wrote: > >> On 2017-09-28 01:29:29，"Alexander Korotkov" <a.korot...@postgrespro.ru> >> wrote: >> >> It appears that your patch conflicts with fc49e24f. Please, rebase it. >> >> >> Yes, i had rebased it, Please check the new patch. >> > > Good, now it applies cleanly. > > else if (strncmp(path, XLOGDIR"/", strlen(XLOGDIR"/")) == 0 && >> IsXLogFileName(path + strlen(XLOGDIR"/")) && >> (strcmp(path + strlen(XLOGDIR"/") + 8, divergence_wal_filename + 8) < 0 >> || >> strcmp(path + strlen(XLOGDIR"/") + 8, last_source_wal_filename + 8) > >> 0)) > > > According to our conding style, you should leave a space betwen XLOGDIF > and "/". > Also, you do a trick by comparison xlog segment numbers using strcmp(). > It's nice, but I would prefer seeing XLogFromFileName() here. It would > improve code readability and be less error prone during further > modifications. > > > Thanks for advice! > I had modified it. >
OK. Patch becomes better. I also have more general question. Why do we need upper bound for segment number (last_source_segno)? I understand the purpose of lower bound (divergence_segno) which save us from copying extra WAL files, but what is upper bound for? As I understood, we anyway need to replay most recent WAL records to reach consistent state after pg_rewind. I propose to remove last_source_segno unless I'm missing something. ------ Alexander Korotkov Postgres Professional: http://www.postgrespro.com The Russian Postgres Company
