[Postgres-xl-developers] removing PGXC/XC ifdefs from XL code

Pavan Deolasee pavan.deolasee at gmail.com
Fri Sep 2 21:22:14 PDT 2016


On Sat, Sep 3, 2016 at 9:09 AM, Andrei Martsinchyk <
andrei.martsinchyk at gmail.com> wrote:

>
>
>> I believe you are writing this as a result of doing a painful merge.
>> Would the merge really have been all that much easier without the ifdefs?
>> I am worried about hidden bugs that get merged in and won't be so easily
>> noticed.
>>
>> I also see Andrei chimed in to support you, so you may have a good point.
>> OTOH, I know of another maintained private fork where I have been told
>> that the ifdefs have been very helpful.
>>
>
>
> Indeed, I never liked that idea with ifdefs, but I was a minority and gave
> up.
>
>
Even I never liked ifdefs and did not put in much efforts to preserve them
in the last couple of years since I started hacking XL. But I understand
that there are folks who find them useful and I am ok with that. While I
did not like them, they did not cause significant inconvenience either to
me.

I probably also have distinction of doing most of the large merges for
XC/XL, the most recent being 9.2 to 9.5 merge, but I also carried out some
large merges for XC. TBH there were only a handful conflicts where ifdefs
helped me in resolution. There were also downsides when code blocks are
duplicated between #ifdef /#else #endif because git merge may just merge in
one block and leave the other one intact. If the blocks are large and
changes are small, there may not even be conflict and you'll need to
manually fix things later.

The opinion seems to be divided here. But if we must keep ifdefs, I would
at the very least clean up so that there is just one macro (XCP or PGXC)
used in the code. That will also mean removing XC code that's no longer
relevant in XL.

I also concur with Tomas that those who find #ifdefs useful probably do not
use some powerful utilities like gtit mergetool. As I said, I've been
(un)fortunate enough to deal with some very large merges and mergetool has
been my friend even since I discovered it.

There is another lessons that Ive learned while maintaining such a large
fork and something I'd to correct at least twice, once in XC and then in XL
again. We based our code on a minor release of PG and that caused large,
unnecessary conflicts while merging with PG master branch. So on both
occasions I'd to first rebase the code on a more appropriate commit point
(this was a non trivial effort both times) and then carry out the merge.
Some other things that we can do to reduce conflicts is to avoid making
lots of inline changes to PG code and instead add new code towards the end
of the file or even better to a new file. Similarly, we should not remove
PG code even if its currently not used in XL, say because certain feature
is blocked or implemented in a different way. Sometimes conflicts are good
since they make use think hard during the merge process. Finding and fixing
bugs later can also be equally painful.

Finally, if decide to remove ifdefs, a strong +1 to add enough comments at
every such place explaining what and why it changed.

Thanks,
Pavan
-- 
 Pavan Deolasee                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.postgres-xl.org/private.cgi/postgres-xl-developers-postgres-xl.org/attachments/20160903/68250ee7/attachment.htm>


More information about the Postgres-xl-developers mailing list