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 2018/04/11 22:44:49 UTC
[Impala-ASF-CR] IMPALA-4631: loosen monotonic clock DCHECK
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10026
Change subject: IMPALA-4631: loosen monotonic clock DCHECK
......................................................................
IMPALA-4631: loosen monotonic clock DCHECK
We saw another build failure due to hitting one of these DCHECKs.
Let's further loosen the check to avoid the failures on misbehaving
systems.
Change-Id: I72d314518087aede16e8d702c2f904b679a55f6d
---
M be/src/runtime/fragment-instance-state.cc
M be/src/util/runtime-profile-counters.h
2 files changed, 7 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/10026/1
--
To view, visit http://gerrit.cloudera.org:8080/10026
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I72d314518087aede16e8d702c2f904b679a55f6d
Gerrit-Change-Number: 10026
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4631: loosen monotonic clock DCHECK
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10026 )
Change subject: IMPALA-4631: loosen monotonic clock DCHECK
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/10026/1/be/src/runtime/fragment-instance-state.cc
File be/src/runtime/fragment-instance-state.cc:
http://gerrit.cloudera.org:8080/#/c/10026/1/be/src/runtime/fragment-instance-state.cc@323
PS1, Line 323: // TODO: IMPALA-4631: Occasionally we see other_time = total_time + ε where ε is 1,
> I'm worried that the epsilon will kill some editor or compiler somewhere. I
You're probably right that we can't have nice things :)
http://gerrit.cloudera.org:8080/#/c/10026/1/be/src/util/runtime-profile-counters.h
File be/src/util/runtime-profile-counters.h:
http://gerrit.cloudera.org:8080/#/c/10026/1/be/src/util/runtime-profile-counters.h@306
PS1, Line 306: // (start_time_ns - ε), where ε is 1, 2 or 3 even though 'start_time_ns' was
> same here
Done
--
To view, visit http://gerrit.cloudera.org:8080/10026
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72d314518087aede16e8d702c2f904b679a55f6d
Gerrit-Change-Number: 10026
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Apr 2018 23:16:28 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4631: loosen monotonic clock DCHECK
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10026 )
Change subject: IMPALA-4631: loosen monotonic clock DCHECK
......................................................................
Patch Set 2:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2289/
--
To view, visit http://gerrit.cloudera.org:8080/10026
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72d314518087aede16e8d702c2f904b679a55f6d
Gerrit-Change-Number: 10026
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Apr 2018 23:16:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4631: loosen monotonic clock DCHECK
Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/10026 )
Change subject: IMPALA-4631: loosen monotonic clock DCHECK
......................................................................
Patch Set 1: Code-Review+2
(2 comments)
Weird. +2 but see my s/e/eps/ suggestion.
http://gerrit.cloudera.org:8080/#/c/10026/1/be/src/runtime/fragment-instance-state.cc
File be/src/runtime/fragment-instance-state.cc:
http://gerrit.cloudera.org:8080/#/c/10026/1/be/src/runtime/fragment-instance-state.cc@323
PS1, Line 323: // TODO: IMPALA-4631: Occasionally we see other_time = total_time + ε where ε is 1,
I'm worried that the epsilon will kill some editor or compiler somewhere. If you know we do this in .cc files already, sure, but if this is the first time, I'd replace with "eps" or "epsilon".
http://gerrit.cloudera.org:8080/#/c/10026/1/be/src/util/runtime-profile-counters.h
File be/src/util/runtime-profile-counters.h:
http://gerrit.cloudera.org:8080/#/c/10026/1/be/src/util/runtime-profile-counters.h@306
PS1, Line 306: // (start_time_ns - ε), where ε is 1, 2 or 3 even though 'start_time_ns' was
same here
--
To view, visit http://gerrit.cloudera.org:8080/10026
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72d314518087aede16e8d702c2f904b679a55f6d
Gerrit-Change-Number: 10026
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Apr 2018 22:53:57 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4631: loosen monotonic clock DCHECK
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10026 )
Change subject: IMPALA-4631: loosen monotonic clock DCHECK
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/10026
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72d314518087aede16e8d702c2f904b679a55f6d
Gerrit-Change-Number: 10026
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Apr 2018 05:29:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4631: loosen monotonic clock DCHECK
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10026 )
Change subject: IMPALA-4631: loosen monotonic clock DCHECK
......................................................................
IMPALA-4631: loosen monotonic clock DCHECK
We saw another build failure due to hitting one of these DCHECKs.
Let's further loosen the check to avoid the failures on misbehaving
systems.
Change-Id: I72d314518087aede16e8d702c2f904b679a55f6d
Reviewed-on: http://gerrit.cloudera.org:8080/10026
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
---
M be/src/runtime/fragment-instance-state.cc
M be/src/util/runtime-profile-counters.h
2 files changed, 7 insertions(+), 6 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Tim Armstrong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/10026
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I72d314518087aede16e8d702c2f904b679a55f6d
Gerrit-Change-Number: 10026
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4631: loosen monotonic clock DCHECK
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/10026 )
Change subject: IMPALA-4631: loosen monotonic clock DCHECK
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/10026
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I72d314518087aede16e8d702c2f904b679a55f6d
Gerrit-Change-Number: 10026
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Apr 2018 03:27:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4631: loosen monotonic clock DCHECK
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Philip Zeyliger,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/10026
to look at the new patch set (#2).
Change subject: IMPALA-4631: loosen monotonic clock DCHECK
......................................................................
IMPALA-4631: loosen monotonic clock DCHECK
We saw another build failure due to hitting one of these DCHECKs.
Let's further loosen the check to avoid the failures on misbehaving
systems.
Change-Id: I72d314518087aede16e8d702c2f904b679a55f6d
---
M be/src/runtime/fragment-instance-state.cc
M be/src/util/runtime-profile-counters.h
2 files changed, 7 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/26/10026/2
--
To view, visit http://gerrit.cloudera.org:8080/10026
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I72d314518087aede16e8d702c2f904b679a55f6d
Gerrit-Change-Number: 10026
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>