Matthew Jacobs has posted comments on this change. Change subject: IMPALA-4669: [KUTIL] Add kudu_util library to the build. ......................................................................
Patch Set 5: > (5 comments) > > > 1) Brining in all the gflags is maybe concerning. will any > conflict with ours? even if not, they will crowd our already large > number of gflags. perhaps we need to look more closely at them. > > I agree, it's a lot of new (unused) gflags. I filed > https://issues.apache.org/jira/browse/IMPALA-5174 to see if > > > 2) I'm curious to know how this (and more particularly the actual > code import change) affects the binary size. > > Before: 391865544 > After: 391795120 > > So somehow it's smaller (?). I'll take a closer look, but nothing > to worry about right now. > > > 3) It'd be good if we could push some of these changes upstream > if possible. > > Will do. The issue is that every upstream update brings in all the > changes since the last rebase, which leads to more compilation or > even functionality issues to resolve. So rather than try and make > that process converge, I figured I'd draw a line under a known good > version of kutil, and add the few changes needed to Impala. > > I should upstream these changes soon, though, so that next upgrade > we bring them all in. Thanks for looking into these. Weird that the binary got smaller. I do think the gflags thing is a problem we need to address though, there are 86 flags defined in util/ alone. Maybe we can augment the gflag macros in the directories we import to prepend "_kudu_import" or such - that wouldn't get rid of them but at least group them and keep them from making Impala's flags hard to find. Perhaps we should run this by some other folks to see if (a) others maybe aren't concerned about this and (b) anyone has some ideas for dealing with this gracefully. -- To view, visit http://gerrit.cloudera.org:8080/5715 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I8da798ee55506d6f969416b17c191eb03cb215f5 Gerrit-PatchSet: 5 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Henry Robinson <he...@cloudera.com> Gerrit-Reviewer: Henry Robinson <he...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-HasComments: No
