Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/10946 )
Change subject: [logging] fix logging init if linking in kudu client library ...................................................................... Patch Set 1: > Instead of this fix, could we just change the client library to not > call InitGoogleLoggingSafeBasic from a module constructor, but > rather to lazily call it on first use of one of the library entry > points? > > Or as that code suggests, limit the ctor call to the 'exported' > client build, since the CLI tool uses the non-exported variant? I read those comments about 'multiple entry points' around that c-tor in client.cc and decided to go this way (i.e. the way how PS1 of this patch is built). Yep, the latter one looks like a good alternative, however I have some concerns with that. If changing the way how it works as you suggested, the problem of 'wrong logging init' can happen again. I.e., imagine someone has linked against the exported library and then called InitGoogleLoggingSafe(), expecting all that extra initialization to happen. However, that would not be the case and that's very frustrating. And the approach in PS1 was built to address exactly that, which I think is the main issue with InitGoogleLoggingSafe() and InitGoogleLoggingSafeBasic() as I see it. -- To view, visit http://gerrit.cloudera.org:8080/10946 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9438248e7f9699cf14d648160d9d516ab27d6ca2 Gerrit-Change-Number: 10946 Gerrit-PatchSet: 1 Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Mon, 16 Jul 2018 19:42:19 +0000 Gerrit-HasComments: No
