Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/8202 )
Change subject: IMPALA-4704: Turns on client connections when local catalog initialized. ...................................................................... Patch Set 17: (7 comments) http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@81 PS17, Line 81: /// whitespace http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@85 PS17, Line 85: /// by clients at the same time. Might want to weave in something like this to motivate the specific startup sequence: The Impala server is considered 'ready' iff it can successfully process requests in all of its roles. The goal is make the state of the service easy to reason about. http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@87 PS17, Line 87: /// 1. Init Init() http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@89 PS17, Line 89: /// - Wait (indefinitelt) for local catalog to be initialized from statestore (if coordinator) typo: indefinitely http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@93 PS17, Line 93: /// 2. Start Start() http://gerrit.cloudera.org:8080/#/c/8202/17/be/src/service/impala-server.h@98 PS17, Line 98: /// Membership callback thread: When is the 'is_ready' metric set? http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/8202/16/be/src/service/impala-server.h@1014 PS16, Line 1014: ExpirationQueue queries_by_timestamp_; > clarified the comment. Fair point. My preference would be to use 'is_ready' to avoid redundant state. If metrics are seen as a view on the internal state (which I agree with!), then 'is_ready' should report the value of this 'services_started_' flag and not store a separate value. -- To view, visit http://gerrit.cloudera.org:8080/8202 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I52b881cba18a7e4533e21a78751c2e35c3d4c8a6 Gerrit-Change-Number: 8202 Gerrit-PatchSet: 17 Gerrit-Owner: Vuk Ercegovac <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Balazs Jeszenszky <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Michael Brown <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Tue, 31 Oct 2017 16:21:23 +0000 Gerrit-HasComments: Yes
