[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add last heartbeat timestamp into Statestore metric .. IMPALA-6644: Add last heartbeat timestamp into Statestore metric After this patch, the statestore keeps track of the time since the last heartbeat for each subscriber. It is exposed as a subscriber metric on the statestore debug page. It also adds a monitoring thread that periodically checks the last heartbeat timestamp for all subscribers and logs the IDs of those that have not been updated since the last periodic check. Testing: Added an end to end test to verify the 'sec_since_heartbeat' metric of a slow subscriber. Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Reviewed-on: http://gerrit.cloudera.org:8080/11052 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/statestore/statestore.cc M be/src/statestore/statestore.h M tests/statestore/test_statestore.py M www/statestore_subscribers.tmpl 4 files changed, 104 insertions(+), 3 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- 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: merged Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 Gerrit-PatchSet: 16 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Impala Public Jenkins 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 15: Verified+1 -- 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: 15 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 29 Aug 2018 22:05:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Impala Public Jenkins 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 15: Code-Review+2 -- 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: 15 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 29 Aug 2018 18:55:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Impala Public Jenkins 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 15: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3091/ DRY_RUN=false -- 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: 15 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 29 Aug 2018 18:55:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
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 14: Code-Review+2 Carrying Todd's +2 -- 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: 14 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 29 Aug 2018 18:54:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Impala Public Jenkins 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 14: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/522/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 14 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 29 Aug 2018 18:46:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Pooja Nilangekar has uploaded a new patch set (#14). ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add last heartbeat timestamp into Statestore metric .. IMPALA-6644: Add last heartbeat timestamp into Statestore metric After this patch, the statestore keeps track of the time since the last heartbeat for each subscriber. It is exposed as a subscriber metric on the statestore debug page. It also adds a monitoring thread that periodically checks the last heartbeat timestamp for all subscribers and logs the IDs of those that have not been updated since the last periodic check. Testing: Added an end to end test to verify the 'sec_since_heartbeat' metric of a slow subscriber. Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 --- M be/src/statestore/statestore.cc M be/src/statestore/statestore.h M tests/statestore/test_statestore.py M www/statestore_subscribers.tmpl 4 files changed, 104 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/11052/14 -- 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: newpatchset Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 Gerrit-PatchSet: 14 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Todd Lipcon 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 13: Code-Review+2 -- 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: 13 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 29 Aug 2018 17:32:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Impala Public Jenkins 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 13: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/519/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 13 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 29 Aug 2018 16:59:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Pooja Nilangekar 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 13: (1 comment) > Patch Set 12: > > (1 comment) http://gerrit.cloudera.org:8080/#/c/11052/12/be/src/statestore/statestore.cc File be/src/statestore/statestore.cc: http://gerrit.cloudera.org:8080/#/c/11052/12/be/src/statestore/statestore.cc@78 PS12, Line 78: 6000 > this went from 1 minute to 6 seconds here, was that intentional? It was a mistake. Fixed it. -- 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: 13 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 29 Aug 2018 16:15:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Pooja Nilangekar has uploaded a new patch set (#13). ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add last heartbeat timestamp into Statestore metric .. IMPALA-6644: Add last heartbeat timestamp into Statestore metric After this patch, the statestore keeps track of the time since the last heartbeat for each subscriber. It is exposed as a subscriber metric on the statestore debug page. It also adds a monitoring thread that periodically checks the last heartbeat timestamp for all subscribers and logs the IDs of those that have not been updated since the last periodic check. Testing: Added an end to end test to verify the 'sec_since_heartbeat' metric of a slow subscriber. Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 --- M be/src/statestore/statestore.cc M be/src/statestore/statestore.h M tests/statestore/test_statestore.py M www/statestore_subscribers.tmpl 4 files changed, 104 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/11052/13 -- 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: newpatchset Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 Gerrit-PatchSet: 13 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Impala Public Jenkins 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 12: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/513/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 12 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 29 Aug 2018 01:07:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Todd Lipcon 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 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/11052/12/be/src/statestore/statestore.cc File be/src/statestore/statestore.cc: http://gerrit.cloudera.org:8080/#/c/11052/12/be/src/statestore/statestore.cc@78 PS12, Line 78: 6000 this went from 1 minute to 6 seconds here, was that intentional? -- 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: 12 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 29 Aug 2018 00:44:39 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Pooja Nilangekar 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 12:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11052/11/be/src/statestore/statestore.h
File be/src/statestore/statestore.h:
http://gerrit.cloudera.org:8080/#/c/11052/11/be/src/statestore/statestore.h@393
PS11, Line 393: tatic_
> nit: maybe use a static_cast
Done
http://gerrit.cloudera.org:8080/#/c/11052/11/be/src/statestore/statestore.h@466
PS11, Line 466: /// older than the heartbeat frequency implies an
unresponsive subscriber.
> nit: AtomicInt64 last_heartbeat_ts_{0}
Done
http://gerrit.cloudera.org:8080/#/c/11052/11/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:
http://gerrit.cloudera.org:8080/#/c/11052/11/be/src/statestore/statestore.cc@1001
PS11, Line 1001: for (const a
> nit: you can use SleepForMs instead and get rid of extra includes
Done
--
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: 12
Gerrit-Owner: Pooja Nilangekar
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Reviewer: Sailesh Mukil
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Wed, 29 Aug 2018 00:32:58 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Pooja Nilangekar has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add last heartbeat timestamp into Statestore metric .. IMPALA-6644: Add last heartbeat timestamp into Statestore metric After this patch, the statestore keeps track of the time since the last heartbeat for each subscriber. It is exposed as a subscriber metric on the statestore debug page. It also adds a monitoring thread that periodically checks the last heartbeat timestamp for all subscribers and logs the IDs of those that have not been updated since the last periodic check. Testing: Added an end to end test to verify the 'sec_since_heartbeat' metric of a slow subscriber. Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 --- M be/src/statestore/statestore.cc M be/src/statestore/statestore.h M tests/statestore/test_statestore.py M www/statestore_subscribers.tmpl 4 files changed, 104 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/11052/12 -- 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: newpatchset Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 Gerrit-PatchSet: 12 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Todd Lipcon 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 10: lgtm, thanks for being patient with the requested changes! -- 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: 10 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 28 Aug 2018 02:52:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Todd Lipcon 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 10: Code-Review+2 -- 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: 10 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 28 Aug 2018 02:51:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Impala Public Jenkins 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 10: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/491/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 10 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 27 Aug 2018 18:26:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Pooja Nilangekar has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add last heartbeat timestamp into Statestore metric .. IMPALA-6644: Add last heartbeat timestamp into Statestore metric After this patch, the statestore keeps track of the time since the last heartbeat for each subscriber. It is exposed as a subscriber metric on the statestore debug page. It also adds a monitoring thread that periodically checks the last heartbeat timestamp for all subscribers and logs the IDs of those that have not been updated since the last periodic check. Testing: Added an end to end test to verify the 'sec_since_heartbeat' metric of a slow subscriber. Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 --- M be/src/statestore/statestore.cc M be/src/statestore/statestore.h M tests/statestore/test_statestore.py M www/statestore_subscribers.tmpl 4 files changed, 109 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/11052/10 -- 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: newpatchset Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 Gerrit-PatchSet: 10 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Impala Public Jenkins 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 9: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/478/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 9 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 Aug 2018 23:18:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Todd Lipcon 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 9: (3 comments) http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.h File be/src/statestore/statestore.h: http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.h@447 PS8, Line 447: Topics priority_subscribed_topics_; > To incorporate the changes I changes the last_heartbeat_ts_ to kudu::MonoTi Does the worker thread which calls RefreshHeartbeatTimestamp also hold subscriber_lock_ while doing so? If not, I think it's technically required to make it atomic (or ensure that the caller does hold that lock). That's the case even though the underlying variable here happens to be int64 and therefore atomic "naturally" on x86. Accessing it without synchronization ends up in murky territory around what optimizations compilers are allowed to do, etc, and also means that if we ever implement TSAN support for running tests we'd see a bunch of warnings about incorrect synchronization. http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.cc File be/src/statestore/statestore.cc: http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.cc@389 PS8, Line 389: > Yes, it does occur (especially during initialization). I changed it from mi Even though the resolution stored is in nanoseconds it's possible that the clock source doesn't have the same resolution (ie it may step with some "coarseness" rather than being accurate to the nano). I think the particular clock implementation we use on Linux are fine, but baking that kind of timing assumption in here seems like we might just get a really surprising failure at some point down the road. I'd also think about this from the other direction: let's say we did accidentally update it twice at the same microsecond. Would it be a problem? Since we're just using this for reporting freshness of heartbeats, and not relying on it being increasing-only or something, I don't think it's worth being strict. http://gerrit.cloudera.org:8080/#/c/11052/9/be/src/statestore/statestore.cc File be/src/statestore/statestore.cc: http://gerrit.cloudera.org:8080/#/c/11052/9/be/src/statestore/statestore.cc@451 PS9, Line 451: initialized_ = true; shouldn't this actually go at the end of the function? ie if the server fails to start and returns a bad Status here, it's acceptable to destruct the Statestore - it's only once we've started the new thread that it enters the "unstoppable" state. I think with it in this position, you might trigger the CHECK failure if the SSL configuration were incorrect or the port was already bound, whereas maybe we'd want to edit in a more graceful fashion. -- 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: 9 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 Aug 2018 22:40:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Pooja Nilangekar has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add last heartbeat timestamp into Statestore metric .. IMPALA-6644: Add last heartbeat timestamp into Statestore metric After this patch, the statestore keeps track of the time since the last heartbeat for each subscriber. It is exposed as a subscriber metric on the statestore debug page. It also adds a monitoring thread that periodically checks the last heartbeat timestamp for all subscribers and logs the IDs of those that have not been updated since the last periodic check. Testing: Added an end to end test to verify the 'sec_since_heartbeat' metric of a slow subscriber. Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 --- M be/src/statestore/statestore.cc M be/src/statestore/statestore.h M tests/statestore/test_statestore.py M www/statestore_subscribers.tmpl 4 files changed, 107 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/11052/9 -- 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: newpatchset Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 Gerrit-PatchSet: 9 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Todd Lipcon 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 8: (10 comments) http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.h File be/src/statestore/statestore.h: http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.h@420 PS8, Line 420: /// Sets the subscriber's last heartbeat timestamp to the given value. nit: can you clarify what the reference point for this timestamp is? ie is this a wall time or monotonic time? You might consider using the kudu::MonoTime class here to make that clear in lieu of a comment (not sure if you have a similar wrapper, but that should already be imported in kudu/util) http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.h@445 PS8, Line 445: microseconds same as above http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.h@538 PS8, Line 538: AtomicBool why does this need to be atomic? shouldn't we assume that initialization is single-threaded and the object should not be exposed to other threads until after initialization? http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.cc File be/src/statestore/statestore.cc: http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.cc@77 PS8, Line 77: DEFINE_int32 perhaps this should be _hidden unless we have some use case in mind where an admin would want to tune it? http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.cc@389 PS8, Line 389: DCHECK_GT(timestamp_us, last_heartbeat_ts_.Load()); should this be _GE? it's possible (though unlikely) that we get called in rapid succession. http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.cc@445 PS8, Line 445: if (initialized_.Load()) LOG(FATAL) this can just be CHECK(!initialized_.Load()) http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.cc@552 PS8, Line 552: time_since_heartbeat can you name this secs_since_heartbeat so the unit is clear? http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.cc@916 PS8, Line 916: SetLastHeartbeatTimestamp(MonotonicMicros()); Given the argument to this method is always going to be MonotonicMicros(), what if the method was just called 'RefreshHeartbeatTimestamp()' or something and internally it called MonotonicMicros? Then, instead of the getter, you could have 'double TimeSinceHeartbeaat()' which calculates the subtraction. That would encapsulate the timing mechanism and units internal to the Subscriber class and only expose a minimal interface to callers http://gerrit.cloudera.org:8080/#/c/11052/8/be/src/statestore/statestore.cc@1002 PS8, Line 1002: .size() == 0 empty http://gerrit.cloudera.org:8080/#/c/11052/8/www/statestore_subscribers.tmpl File www/statestore_subscribers.tmpl: http://gerrit.cloudera.org:8080/#/c/11052/8/www/statestore_subscribers.tmpl@31 PS8, Line 31: Time since last heartbeat (seconds) more concise: "Seconds since last 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: 8 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 Aug 2018 16:15:13 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Impala Public Jenkins 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 7: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/383/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 7 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 17 Aug 2018 00:57:09 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Pooja Nilangekar 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 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/11052/6/tests/statestore/test_statestore.py File tests/statestore/test_statestore.py: http://gerrit.cloudera.org:8080/#/c/11052/6/tests/statestore/test_statestore.py@473 PS6, Line 473: > flake8: E501 line too long (92 > 90 characters) Done -- 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: 7 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 17 Aug 2018 00:23:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Pooja Nilangekar has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add last heartbeat timestamp into Statestore metric .. IMPALA-6644: Add last heartbeat timestamp into Statestore metric After this patch, the statestore keeps track of the time since the last heartbeat for each subscriber. It is exposed as a subscriber metric on the statestore debug page. It also adds a monitoring thread that periodically checks the last heartbeat timestamp for all subscribers and logs the IDs of those that have not been updated since the last periodic check. Testing: Added an end to end test to verify the 'time_since_heartbeat' metric of a slow subscriber. Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 --- M be/src/statestore/statestore.cc M be/src/statestore/statestore.h M tests/statestore/test_statestore.py M www/statestore_subscribers.tmpl 4 files changed, 101 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/11052/7 -- 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: newpatchset Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 Gerrit-PatchSet: 7 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Impala Public Jenkins 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 6: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/378/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 6 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 16 Aug 2018 23:27:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Impala Public Jenkins 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 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/11052/6/tests/statestore/test_statestore.py File tests/statestore/test_statestore.py: http://gerrit.cloudera.org:8080/#/c/11052/6/tests/statestore/test_statestore.py@473 PS6, Line 473: " flake8: E501 line too long (92 > 90 characters) -- 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: 6 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 16 Aug 2018 22:55:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Pooja Nilangekar 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 6:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/11052/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11052/5//COMMIT_MSG@16
PS5, Line 16: Testing: Added an end to end test to verify the
'time_since_heartbeat'
: metric of a slow subscriber.
:
> sorry, one more thing I forgot to mention: can we add an automated e2e test
Done
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.h
File be/src/statestore/statestore.h:
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.h@444
PS5, Line 444:
> nit: technically this should be atomic, right? The "monitor" thread is acce
Done
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.h@687
PS5, Line 687: rapidjson::Document* document);
> the 'MainLoop' call seems to indicate that it would run until some exit fla
Done.
That comment was left around from quiet some time ago. The Statestore class no
longer contains the exit_flag_ nor does it provide any API for graceful
decommissioning. I have checked the current codebase and I can confirm that the
statestore is never destructed. I have also added the destructor.
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.cc@78
PS5, Line 78: s from a
> nit: typo
Done
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.cc@315
PS5, Line 315: last_heartbeat_ts_(MonotonicMicros()) {
> is there any race here when a subscriber first registers before it sends a
Done
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.cc@389
PS5, Line 389: DCHECK_GT(timestamp_us, last_heartbeat_ts_.Load());
> if these are wall times, it's possible for them to go backwards if the host
Done
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.cc@542
PS5, Line 542:
> for the purposes of the user understanding this data, I'd think it would be
Done
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.cc@984
PS5, Line 984: }
> You don't gain any real performance by declaring these outside of the loop,
Done
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.cc@990
PS5, Line 990: int num_subscribers;
> maybe it would be clearer to not track this incrementally, but instead, jus
Done
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.cc@992
PS5, Line 992: AGS_heartbeat_monitoring_
> nit: i think using 'const auto&' is just as good here
Done
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.cc@998
PS5, Line 998:
inactive_subscribers.push_back(subscriber.second->id());
> is it worth escalating this to WARN level if there are any inactive?
Done
--
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: 6
Gerrit-Owner: Pooja Nilangekar
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Reviewer: Sailesh Mukil
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Thu, 16 Aug 2018 22:54:48 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Pooja Nilangekar has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add last heartbeat timestamp into Statestore metric .. IMPALA-6644: Add last heartbeat timestamp into Statestore metric After this patch, the statestore keeps track of the time since the last heartbeat for each subscriber. It is exposed as a subscriber metric on the statestore debug page. It also adds a monitoring thread that periodically checks the last heartbeat timestamp for all subscribers and logs the IDs of those that have not been updated since the last periodic check. Testing: Added an end to end test to verify the 'time_since_heartbeat' metric of a slow subscriber. Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 --- M be/src/statestore/statestore.cc M be/src/statestore/statestore.h M tests/statestore/test_statestore.py M www/statestore_subscribers.tmpl 4 files changed, 101 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/11052/6 -- 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: newpatchset Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 Gerrit-PatchSet: 6 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Todd Lipcon 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 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/11052/5//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11052/5//COMMIT_MSG@16 PS5, Line 16: Testing: Manually inspected the Web UI and statestore logs to : verify that the last heartbeat timestamp for each subscriber is : updated periodically. sorry, one more thing I forgot to mention: can we add an automated e2e test to test_statestore.py that ensures that this data shows up and is reasonable? If you want to get fancy you could even kill -STOP one of the subscribers and make sure that it eventually shows up as inactive, but I'd be OK if you just do the basic thing of checking that some reasonable looking strings show up in the JSON. -- 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: 5 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 08 Aug 2018 00:50:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Todd Lipcon 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 5:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.h
File be/src/statestore/statestore.h:
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.h@444
PS5, Line 444: int64_t last_heartbeat_ts_;
nit: technically this should be atomic, right? The "monitor" thread is
accessing this variable only under 'subscribers_lock_', but this variable gets
*set* from the heartbeat-sending thread which doesn't hold that lock, if I read
the code correctly.
In practice it probably doesn't matter since it's just an int and x86 has a
strong enough memory model, but I think it would be better to use a
std::atomic here so that tools like TSAN don't complain.
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.h@687
PS5, Line 687: [[noreturn]] void MonitorSubscriberHeartbeat();
the 'MainLoop' call seems to indicate that it would run until some exit flag is
set, but I don't see the exit flag. So, i'm not 100% sure: is it possible that
a StateStore object ever destructs? If it did, I think this thread would keep
on running and then crash as it accessed the freed memory of the StateStore
object.
If we believe that StateStore can never destruct, can we add a destructor which
does something like a LOG(FATAL) << "Cannot shut down StateStore object once
started" in the case that it sees that Init() was called? That way if someone
tries to destruct a StateStore we'll get an obvious crash instead of a
tricky-to-debug one
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.cc
File be/src/statestore/statestore.cc:
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.cc@78
PS5, Line 78: hearbeats
nit: typo
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.cc@315
PS5, Line 315: last_heartbeat_ts_(0) {
is there any race here when a subscriber first registers before it sends a
heartbeat where it will report as something like 48 years since heartbeat?
Perhaps this should be set to the current time instead?
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.cc@389
PS5, Line 389: DCHECK_GT(timestamp_ms, last_heartbeat_ts_);
if these are wall times, it's possible for them to go backwards if the host
clock resets. Perhaps we should be using monotonic times here to avoid this
issue?
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.cc@542
PS5, Line 542: Value last_heartbeat_timestamp(
for the purposes of the user understanding this data, I'd think it would be
more usable to output the number of seconds since the heartbeat, instead of the
last heartbeat time. It's much easier to scan down a list of 100 subscribers
and find the one with a big value, instead of trying to scan down a list of
timestamps to find the one that is farthest in the past.
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.cc@984
PS5, Line 984: int num_subscribers, num_active_subscribers;
You don't gain any real performance by declaring these outside of the loop, and
I think you lose some clarity. For the ints, I don't think it makes a
performance difference at all, since the compiler will just allocate stack
slots for them either way, and there is no constructor/destructor. For the
vector, in theory there might be some gain, but given this loop only runs once
a minute I don't think saving potentially 1 tiny allocation matters.
Instead, could you move these down to inside the for loop so it's clear they
don't cross from one loop iteration to the next? I think doing so will remove
some lines of code and make it more obvious that only 'previous_timestamp' is
carried from one iteration to the next.
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.cc@990
PS5, Line 990: num_subscribers = subscribers_.size();
maybe it would be clearer to not track this incrementally, but instead, just
before the log statement, do:
int num_active_subscrbers = num_subscribers - inactive_subscribers.size();
since it would reduce some extra branching and mental overhead
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.cc@992
PS5, Line 992: SubscriberMap::value_type
nit: i think using 'const auto&' is just as good here
http://gerrit.cloudera.org:8080/#/c/11052/5/be/src/statestore/statestore.cc@998
PS5, Line 998: LOG(INFO) << num_active_subscribers << "/" << num_subscribers
is it worth escalating this to WARN level if there are any inactive?
--
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
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
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 5: Code-Review+1 -- 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: 5 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 07 Aug 2018 22:18:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Impala Public Jenkins 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 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/178/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 5 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 03 Aug 2018 19:23:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Pooja Nilangekar 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 5: (12 comments) 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: 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 l > After this patch, the statestore keeps track of the last successful heartbe Done 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: ms); > milliseconds or ms Done http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.h@442 PS4, Line 442: /// The timestamp of the last successful heartbeat in milliseconds. A timestamp much : /// older than the heartbeat frequency implies an unr > The timestamp of the last successful heartbeat in milliseconds. Done http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.h@444 PS4, Line 444: 4_t last_heartbe > heartbeat frequency Done http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.h@532 PS4, Line 532: :unique_ptr heartbeat_monitoring_thread_; > Thread that monitors the heartbeats of all subscribers. Done http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.h@684 PS4, Line 684: rtbeat_monitoring_frequency_ms mi > the heartbeats of all subscribers every Done http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.h@686 PS4, Line 686: > that Done http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.h@686 PS4, Line 686: : [[noreturn]] void MonitorSub > subscriber's Id. Done 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: DCHECK(metrics != NULL); : metrics_ = metrics; : num_subscribers_metric_ = metrics->AddGauge(STATESTORE_LIVE_SUBSCRIBERS, 0); > move this to Init() and return status Done http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.cc@1003 PS4, Line 1003: s, > $0 Done http://gerrit.cloudera.org:8080/#/c/11052/4/be/src/statestore/statestore.cc@1004 PS4, Line 1004: > nit: add a space after comma Done 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: Last heartbeat timestamp > nit: add " (ms)" The timestamp is printed in the following format: -mm-dd hh:mm:ss.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: 5 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 03 Aug 2018 18:47:54 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Pooja Nilangekar has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add last heartbeat timestamp into Statestore metric .. IMPALA-6644: Add last heartbeat timestamp into Statestore metric 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. Testing: Manually inspected the Web UI and statestore logs to verify that the last heartbeat timestamp for each subscriber is updated periodically. Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 --- M be/src/statestore/statestore.cc M be/src/statestore/statestore.h M www/statestore_subscribers.tmpl 3 files changed, 67 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/11052/5 -- 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: newpatchset Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 Gerrit-PatchSet: 5 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
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",
: ::MonitorSubscriberHeartbeat, this,
_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: Last heartbeat timestamp
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
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Reviewer: Sailesh Mukil
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Fri, 03 Aug 2018 01:38:45 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Pooja Nilangekar 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:
(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: else if (status.code() == TErrorCode::RPC_RECV_TIMEOUT) {
: // Add details to status to make it more useful, while
preserving the stack
: status.AddDetail(Substitute(
> Makes sense. I will change it.
Done
--
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
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Pooja Nilangekar
Gerrit-Reviewer: Sailesh Mukil
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Wed, 01 Aug 2018 01:10:14 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6644: Add last heartbeat timestamp into Statestore metric
Pooja Nilangekar has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/11052 ) Change subject: IMPALA-6644: Add last heartbeat timestamp into Statestore metric .. IMPALA-6644: Add last heartbeat timestamp into Statestore metric 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. Testing: Manually inspected the Web UI and statestore logs to verify that the last heartbeat timestamp for each subscriber is updated periodically. Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 --- M be/src/statestore/statestore.cc M be/src/statestore/statestore.h M www/statestore_subscribers.tmpl 3 files changed, 69 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/52/11052/4 -- 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: newpatchset Gerrit-Change-Id: I754adccc4569e8219d5d01500cccdfc8782953f7 Gerrit-Change-Number: 11052 Gerrit-PatchSet: 4 Gerrit-Owner: Pooja Nilangekar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Pooja Nilangekar Gerrit-Reviewer: Sailesh Mukil Gerrit-Reviewer: Todd Lipcon
