You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2019/02/21 01:34:04 UTC
[Impala-ASF-CR] IMPALA-8235: avoid TIME MS in runtime profile
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12544
Change subject: IMPALA-8235: avoid TIME_MS in runtime profile
......................................................................
IMPALA-8235: avoid TIME_MS in runtime profile
There are issues with using new TUnit values in runtime profiles that
areare consumed by readers with an older version of the thrift
definitions.
Work around the issue by using the older TIME_NS type for this
counter.
Testing:
Confirmed manually that a tool with an old version of the thrift API
(Cloudera Manager) could parse the profile.
Change-Id: Ia6a0846f401ced1947187b8581563de9dbb4fe53
---
M be/src/scheduling/admission-controller.cc
1 file changed, 3 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/12544/1
--
To view, visit http://gerrit.cloudera.org:8080/12544
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia6a0846f401ced1947187b8581563de9dbb4fe53
Gerrit-Change-Number: 12544
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-8235: avoid TIME MS in runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12544 )
Change subject: IMPALA-8235: avoid TIME_MS in runtime profile
......................................................................
IMPALA-8235: avoid TIME_MS in runtime profile
There are issues with using new TUnit values in runtime profiles that
are consumed by readers with an older version of the thrift
definitions.
Work around the issue by using the older TIME_NS type for this
counter.
Testing:
Confirmed manually that a tool with an old version of the thrift API
(Cloudera Manager) could parse the profile.
Change-Id: Ia6a0846f401ced1947187b8581563de9dbb4fe53
Reviewed-on: http://gerrit.cloudera.org:8080/12544
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/scheduling/admission-controller.cc
1 file changed, 3 insertions(+), 1 deletion(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/12544
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia6a0846f401ced1947187b8581563de9dbb4fe53
Gerrit-Change-Number: 12544
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-8235: avoid TIME MS in runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12544 )
Change subject: IMPALA-8235: avoid TIME_MS in runtime profile
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2192/ : 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/12544
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6a0846f401ced1947187b8581563de9dbb4fe53
Gerrit-Change-Number: 12544
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Feb 2019 02:05:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8235: avoid TIME MS in runtime profile
Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/12544 )
Change subject: IMPALA-8235: avoid TIME_MS in runtime profile
......................................................................
Patch Set 2: Code-Review+2
Thanks for fixing this!
--
To view, visit http://gerrit.cloudera.org:8080/12544
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6a0846f401ced1947187b8581563de9dbb4fe53
Gerrit-Change-Number: 12544
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Feb 2019 02:18:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8235: avoid TIME MS in runtime profile
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Michael Ho, Lars Volker, Joe McDonnell, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/12544
to look at the new patch set (#2).
Change subject: IMPALA-8235: avoid TIME_MS in runtime profile
......................................................................
IMPALA-8235: avoid TIME_MS in runtime profile
There are issues with using new TUnit values in runtime profiles that
are consumed by readers with an older version of the thrift
definitions.
Work around the issue by using the older TIME_NS type for this
counter.
Testing:
Confirmed manually that a tool with an old version of the thrift API
(Cloudera Manager) could parse the profile.
Change-Id: Ia6a0846f401ced1947187b8581563de9dbb4fe53
---
M be/src/scheduling/admission-controller.cc
1 file changed, 3 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/12544/2
--
To view, visit http://gerrit.cloudera.org:8080/12544
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia6a0846f401ced1947187b8581563de9dbb4fe53
Gerrit-Change-Number: 12544
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-8235: avoid TIME MS in runtime profile
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12544 )
Change subject: IMPALA-8235: avoid TIME_MS in runtime profile
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12544/1/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:
http://gerrit.cloudera.org:8080/#/c/12544/1/be/src/scheduling/admission-controller.cc@1108
PS1, Line 1108: static_cast<int64_t>(
> NANOS_PER_MICRO and MICROS_PER_MILLI are a "long long" and the compiler can
Here's a minimal example of the problem: https://godbolt.org/z/cfFzvs
--
To view, visit http://gerrit.cloudera.org:8080/12544
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6a0846f401ced1947187b8581563de9dbb4fe53
Gerrit-Change-Number: 12544
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Feb 2019 02:06:58 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8235: avoid TIME MS in runtime profile
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12544 )
Change subject: IMPALA-8235: avoid TIME_MS in runtime profile
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12544
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6a0846f401ced1947187b8581563de9dbb4fe53
Gerrit-Change-Number: 12544
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Feb 2019 02:33:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8235: avoid TIME MS in runtime profile
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12544 )
Change subject: IMPALA-8235: avoid TIME_MS in runtime profile
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12544/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12544/1//COMMIT_MSG@10
PS1, Line 10: areare
are
http://gerrit.cloudera.org:8080/#/c/12544/1//COMMIT_MSG@13
PS1, Line 13: Work around the issue by using the older TIME_NS type for this
: counter.
What's the plan going forward ? Do we need to identify a set of interfaces which need to be backward compatible ?
http://gerrit.cloudera.org:8080/#/c/12544/1/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:
http://gerrit.cloudera.org:8080/#/c/12544/1/be/src/scheduling/admission-controller.cc@1108
PS1, Line 1108: static_cast<int64_t>(
Why is this cast necessary ? Isn't time_since_update_ms already int64_t ?
--
To view, visit http://gerrit.cloudera.org:8080/12544
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6a0846f401ced1947187b8581563de9dbb4fe53
Gerrit-Change-Number: 12544
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Feb 2019 01:58:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8235: avoid TIME MS in runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12544 )
Change subject: IMPALA-8235: avoid TIME_MS in runtime profile
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3811/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12544
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6a0846f401ced1947187b8581563de9dbb4fe53
Gerrit-Change-Number: 12544
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Feb 2019 04:51:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8235: avoid TIME MS in runtime profile
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12544 )
Change subject: IMPALA-8235: avoid TIME_MS in runtime profile
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/12544/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/12544/1//COMMIT_MSG@10
PS1, Line 10: areare
> are
Done
http://gerrit.cloudera.org:8080/#/c/12544/1//COMMIT_MSG@13
PS1, Line 13: Work around the issue by using the older TIME_NS type for this
: counter.
> What's the plan going forward ? Do we need to identify a set of interfaces
Lars is documenting this a bit better here: https://gerrit.cloudera.org/#/c/12543/
I'm not sure what to do about evolution of TUnit going forward - thrift doesn't really seem to support evolution of required enums very well. IMPALA-8236 tracks that issue.
http://gerrit.cloudera.org:8080/#/c/12544/1/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:
http://gerrit.cloudera.org:8080/#/c/12544/1/be/src/scheduling/admission-controller.cc@1108
PS1, Line 1108: static_cast<int64_t>(
> Why is this cast necessary ? Isn't time_since_update_ms already int64_t ?
NANOS_PER_MICRO and MICROS_PER_MILLI are a "long long" and the compiler can't figure out whether to use the Set(int) or Set(int64_t) overload because int64_t is a typedef of long and it considers long and "long long" to be different types.
--
To view, visit http://gerrit.cloudera.org:8080/12544
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6a0846f401ced1947187b8581563de9dbb4fe53
Gerrit-Change-Number: 12544
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Feb 2019 02:04:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8235: avoid TIME MS in runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12544 )
Change subject: IMPALA-8235: avoid TIME_MS in runtime profile
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12544
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6a0846f401ced1947187b8581563de9dbb4fe53
Gerrit-Change-Number: 12544
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Feb 2019 09:09:59 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8235: avoid TIME MS in runtime profile
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12544 )
Change subject: IMPALA-8235: avoid TIME_MS in runtime profile
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12544/1/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:
http://gerrit.cloudera.org:8080/#/c/12544/1/be/src/scheduling/admission-controller.cc@1108
PS1, Line 1108: static_cast<int64_t>(
> Here's a minimal example of the problem: https://godbolt.org/z/cfFzvs
I see. Thanks for the explanation.
--
To view, visit http://gerrit.cloudera.org:8080/12544
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6a0846f401ced1947187b8581563de9dbb4fe53
Gerrit-Change-Number: 12544
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Feb 2019 02:33:06 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8235: avoid TIME MS in runtime profile
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12544 )
Change subject: IMPALA-8235: avoid TIME_MS in runtime profile
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12544
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6a0846f401ced1947187b8581563de9dbb4fe53
Gerrit-Change-Number: 12544
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Feb 2019 04:51:18 +0000
Gerrit-HasComments: No