Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/8490 )
Change subject: IMPALA-2248: Make idle_session_timeout a query option ...................................................................... Patch Set 14: (1 comment) http://gerrit.cloudera.org:8080/#/c/8490/14/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/8490/14/be/src/service/impala-server.h@452 PS14, Line 452: friend class SessionState; Well, I feel here that making SessionState a friend class so that the Register..() and Unregister..() functions can be private doesn't add any further value. Actually we grant access to SessionState for the whole private part of ImpalaServer even though it only needs to access those 2 functions. I feel that in this case keeping these functions public and not making SessionState a friend class would be slightly better. I don't insist though just sharing my thoughts :) Zoli, Michael, what do you think? -- To view, visit http://gerrit.cloudera.org:8080/8490 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I32e2775f80da387b0df4195fe2c5435b3f8e585e Gerrit-Change-Number: 8490 Gerrit-PatchSet: 14 Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Laszlo Gaal <laszlo.g...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Thu, 14 Dec 2017 07:54:11 +0000 Gerrit-HasComments: Yes
