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 2016/10/20 15:43:00 UTC
[Impala-ASF-CR] IMPALA-4241: remove spurious child queries event
Tim Armstrong has uploaded a new change for review.
http://gerrit.cloudera.org:8080/4768
Change subject: IMPALA-4241: remove spurious child queries event
......................................................................
IMPALA-4241: remove spurious child queries event
"IMPALA-4037,IMPALA-4038: fix locking during query
cancellation" accidentally added the "Child queries
finished" event unconditionally. We should only do
this if there are actually child queries.
Change-Id: I3881d032622750444d750f161ad6843bdbd16c30
---
M be/src/service/query-exec-state.cc
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/68/4768/1
--
To view, visit http://gerrit.cloudera.org:8080/4768
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3881d032622750444d750f161ad6843bdbd16c30
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4241: remove spurious child queries event
Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.
Change subject: IMPALA-4241: remove spurious child queries event
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4768/1/be/src/service/query-exec-state.cc
File be/src/service/query-exec-state.cc:
PS1, Line 623: child_queries
remind me why we don't put line 616->622 in this kind of conditional? ISTR there was a reason we wanted to run child_query_executor_->WaitForAll() in all cases, but it escapes me now.
--
To view, visit http://gerrit.cloudera.org:8080/4768
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3881d032622750444d750f161ad6843bdbd16c30
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4241: remove spurious child queries event
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4241: remove spurious child queries event
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4768/1/be/src/service/query-exec-state.cc
File be/src/service/query-exec-state.cc:
PS1, Line 623: child_queries
> remind me why we don't put line 616->622 in this kind of conditional? ISTR
We would need to add more state to QueryExecState that is redundant with ChildQueryExecutor's state (since ChildQueryExecutor obviously knows whether any child queries were running). Normally this wouldn't be too bad but currently the locking rules around QueryExecState's members are so confusing I think we should avoid adding state unless it's necessary.
--
To view, visit http://gerrit.cloudera.org:8080/4768
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3881d032622750444d750f161ad6843bdbd16c30
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4241: remove spurious child queries event
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has submitted this change and it was merged.
Change subject: IMPALA-4241: remove spurious child queries event
......................................................................
IMPALA-4241: remove spurious child queries event
"IMPALA-4037,IMPALA-4038: fix locking during query
cancellation" accidentally added the "Child queries
finished" event unconditionally. We should only do
this if there are actually child queries.
Change-Id: I3881d032622750444d750f161ad6843bdbd16c30
Reviewed-on: http://gerrit.cloudera.org:8080/4768
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Internal Jenkins
---
M be/src/service/query-exec-state.cc
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
Internal Jenkins: Verified
Tim Armstrong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/4768
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I3881d032622750444d750f161ad6843bdbd16c30
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-4241: remove spurious child queries event
Posted by "Internal Jenkins (Code Review)" <ge...@cloudera.org>.
Internal Jenkins has posted comments on this change.
Change subject: IMPALA-4241: remove spurious child queries event
......................................................................
Patch Set 2: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/4768
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3881d032622750444d750f161ad6843bdbd16c30
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Internal Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4241: remove spurious child queries event
Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.
Change subject: IMPALA-4241: remove spurious child queries event
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/4768/1/be/src/service/query-exec-state.cc
File be/src/service/query-exec-state.cc:
PS1, Line 623: child_queries
> We would need to add more state to QueryExecState that is redundant with Ch
Oh, right, of course child_queries doesn't get populated until WaitForAll(). Thanks.
--
To view, visit http://gerrit.cloudera.org:8080/4768
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3881d032622750444d750f161ad6843bdbd16c30
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4241: remove spurious child queries event
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change.
Change subject: IMPALA-4241: remove spurious child queries event
......................................................................
Patch Set 2: Code-Review+2
carry +2
--
To view, visit http://gerrit.cloudera.org:8080/4768
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3881d032622750444d750f161ad6843bdbd16c30
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: No