Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11052 )
Change subject: IMPALA-6644: Add last heartbeat timestamp into Statestore metric ...................................................................... Patch Set 4: (12 comments) looks good, mostly just nits http://gerrit.cloudera.org:8080/#/c/11052/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11052/4//COMMIT_MSG@9 PS4, Line 9: This patch updates the last heartbeat timestamp of a subscriber : everytime the subscriber heartbeats successfully. It also launches : a monitoring thread which periodically checks the last heartbeat : timestamp for all the subscribers has been updated and logs the : slow subscribers. After this patch, the statestore keeps track of the last successful heartbeat timestamp which is exposed as a subscriber metric on the statestore debug page. Also adds a monitoring thread that periodically checks the last heartbeat timestamp for all the subscribers and logs the IDs of those that have not been updated since the last periodic check. http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.h File be/src/statestore/statestore.h: http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.h@418 PS4, Line 418: seconds milliseconds or ms http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.h@442 PS4, Line 442: /// The timestamp of the last heartbeat in milliseconds. This timestamp is updated : /// everytime the subscirber heartbeats successfully. The timestamp of the last successful heartbeat in milliseconds. http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.h@444 PS4, Line 444: update frequency heartbeat frequency http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.h@532 PS4, Line 532: Thread that monitors the subscriber's heartbeats. Thread that monitors the heartbeats of all subscribers. http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.h@684 PS4, Line 684: the subscriber's heartbeats every the heartbeats of all subscribers every http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.h@686 PS4, Line 686: the that http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.h@686 PS4, Line 686: subscriber's : /// information to the logs. subscriber's Id. http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.cc File be/src/statestore/statestore.cc: http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.cc@424 PS4, Line 424: Status status = Thread::Create("statestore-heartbeat", "heartbeat-monitoring-thread", : &Statestore::MonitorSubscriberHeartbeat, this, &heartbeat_monitoring_thread_); : if (!status.ok()) LOG(WARNING) << "Unable to start heartbeat monitoring thread."; move this to Init() and return status http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.cc@1003 PS4, Line 1003: %s $0 http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.cc@1004 PS4, Line 1004: , nit: add a space after comma http://gerrit.cloudera.org:8080/#/c/11052/4/www/statestore_subscribers.tmpl File www/statestore_subscribers.tmpl: http://gerrit.cloudera.org:8080/#/c/11052/4/www/statestore_subscribers.tmpl@31 PS4, Line 31: <th>Last heartbeat timestamp</th> nit: add " (ms)" -- 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: 4 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: Fri, 03 Aug 2018 01:38:45 +0000 Gerrit-HasComments: Yes