Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/11052 )
Change subject: IMPALA-6644: Add recent heartbeat timestamp into Statestore metric ...................................................................... Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/11052/3/be/src/statestore/statestore.cc File be/src/statestore/statestore.cc: http://gerrit.cloudera.org:8080/#/c/11052/3/be/src/statestore/statestore.cc@913 PS3, Line 913: LOG(INFO) << "Recently logged heartbeat for subscriber " << subscriber->id() : << " at " << ToStringFromUnix(current_timestamp); : subscriber->SetRecentHeartbeatTimestamp(current_timestamp); just passing through here: I'm not entirely following the logic. It makes sense that you don't want to issue a LOG() on every heartbeat, but for the purposes of updating the timestamp member of the subscriber, why not update it every time and reduce some complexity? Also the wording of the log message might not make much sense outside the context of this patch. Maybe just "successfully heartbeat with subscriber X" since the timestamp itself is already going to be included in the log format? Also once a minute times every host could end up being a lot of logs. Maybe it makes sense to do the logging from some separate thread which iterates over all subscribers, and just lists a summary like "99/100 hosts successfully heartbeat within the last 60 seconds. Slow hosts: foo.blah.com (83 seconds since heartbeat)" -- To view, visit http://gerrit.cloudera.org:8080/11052 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 Gerrit-PatchSet: 3 Gerrit-Owner: Pooja Nilangekar <pooja.nilange...@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Pooja Nilangekar <pooja.nilange...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Mon, 30 Jul 2018 22:19:07 +0000 Gerrit-HasComments: Yes
