You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org> on 2018/07/25 21:40:04 UTC
[Impala-ASF-CR] [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
Pooja Nilangekar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11005
Change subject: [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
[WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
This change ensures that coordinator::UpdateFilter is executed
only for queries which are in the EXECUTING state. Additionally,
it also guarantees that coordinator::ReleaseExecResources is
executed only when no other thread is executing UpdateFilter.
Testing: Ran all back-end tests.
[TODO] Add a test to verify that this change guarantees that
UpdateFilter and ReleaseExecResources never execute concurrently.
Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
---
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
2 files changed, 40 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/11005/4
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 )
Change subject: [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
Patch Set 4:
Build Started https://jenkins.impala.io/job/gerrit-code-review-checks/66/
Running initial code review checks. This is experimental - please report any issues to tarmstrong@cloudera.com or on this JIRA: IMPALA-7317
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Jul 2018 21:40:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6153: Execute UpdateFilter() only for executing queries
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/11005 )
Change subject: IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
IMPALA-6153: Execute UpdateFilter() only for executing queries
This change ensures that coordinator::UpdateFilter is executed
only for queries which are in the EXECUTING state. Additionally,
it also guarantees that coordinator::ReleaseExecResources is
executed only when no other thread is executing UpdateFilter.
Testing: Ran all back-end tests.
Additionally, ran the exhaustive tests with DCHECKs which would
be triggered if the ReleaseExecResources() is executed while any
other thread is executing UpdateFilter().
Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Reviewed-on: http://gerrit.cloudera.org:8080/11005
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
2 files changed, 45 insertions(+), 16 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 11
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 )
Change subject: IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
Patch Set 8: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11005/6/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:
http://gerrit.cloudera.org:8080/#/c/11005/6/be/src/runtime/coordinator.cc@265
PS6, Line 265: lock_guard<shared_mutex> lock(filter_lock_);
: for (const FragmentExecParams& fragment_params: schedule_.fragment_exec_params()) {
: int num_instances = fragment_params.instance_exec_par
> Make sense. I have removed the comment. I have kept the lock_guard for the
keeping the lock seems ok to me
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 8
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Aug 2018 22:13:02 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 )
Change subject: IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
Patch Set 10: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 10
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Aug 2018 21:47:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/11005 )
Change subject: [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
[WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
This change ensures that coordinator::UpdateFilter is executed
only for queries which are in the EXECUTING state. Additionally,
it also guarantees that coordinator::ReleaseExecResources is
executed only when no other thread is executing UpdateFilter.
Testing: Ran all back-end tests.
[TODO] Add a test to verify that this change guarantees that
UpdateFilter and ReleaseExecResources never execute concurrently.
Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
---
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
2 files changed, 45 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/11005/7
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 7
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 )
Change subject: IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
Patch Set 9: Code-Review+1
Carry forward Bikram's +1.
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 9
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Aug 2018 22:15:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Sailesh Mukil (Code Review)" <ge...@cloudera.org>.
Sailesh Mukil has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 )
Change subject: [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
Patch Set 4:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.h
File be/src/runtime/coordinator.h:
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.h@302
PS4, Line 302: it
nit: this lock
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.h@309
PS4, Line 309: releases the memory associated with
: /// filter_routing_table_
nit: alters the filter_routing_table_
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.h@311
PS4, Line 311: boost::shared_mutex
We have to ensure that this is a 'writer preferred' shared mutex. It turns out that boost doesn't have an option to specify a preference, and uses a "fair" algorithm instead to decide which waiter to schedule next, so I guess this is fine.
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.cc@79
PS4, Line 79: exec_state_.Load(), ExecState::EXECUTING
not your change, but replace with:
!IsExecuting()
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.cc@397
PS4, Line 397: lock_guard<SpinLock> l(filter_update_lock_);
Why not just get exclusive access to 'filter_lock_' here?
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.cc@454
PS4, Line 454: exec_state_.Load() != ExecState::EXECUTING
not your change, but replace with:
!IsExecuting()
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.cc@714
PS4, Line 714: exec_state_.Load() == ExecState::EXECUTING
not your change, but replace with:
IsExecuting()
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.cc@871
PS4, Line 871: if (!IsExecuting()) return;
Instead of doing this here, I would say you can replace the IsDone() check inside CoordinatorBackendState::PublishFilter() with this IsExecuting() check.
CoordinatorBackendState has a reverse reference to the Coordinator object.
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 27 Jul 2018 21:17:37 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 )
Change subject: IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
Patch Set 9: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 9
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Aug 2018 21:47:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Hello Sailesh Mukil, Tim Armstrong, Bikramjeet Vig, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11005
to look at the new patch set (#9).
Change subject: IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
IMPALA-6153: Execute UpdateFilter() only for executing queries
This change ensures that coordinator::UpdateFilter is executed
only for queries which are in the EXECUTING state. Additionally,
it also guarantees that coordinator::ReleaseExecResources is
executed only when no other thread is executing UpdateFilter.
Testing: Ran all back-end tests.
Additionally, ran the exhaustive tests with DCHECKs which would
be triggered if the ReleaseExecResources() is executed while any
other thread is executing UpdateFilter().
Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
---
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
2 files changed, 45 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/11005/9
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 9
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 )
Change subject: IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
Patch Set 10:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2958/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 10
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Aug 2018 21:47:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 )
Change subject: [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/109/ : 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/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 6
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 31 Jul 2018 00:34:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 )
Change subject: [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/66/ : 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/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Jul 2018 22:14:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/11005 )
Change subject: [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
[WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
This change ensures that coordinator::UpdateFilter is executed
only for queries which are in the EXECUTING state. Additionally,
it also guarantees that coordinator::ReleaseExecResources is
executed only when no other thread is executing UpdateFilter.
Testing: Ran all back-end tests.
[TODO] Add a test to verify that this change guarantees that
UpdateFilter and ReleaseExecResources never execute concurrently.
Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
---
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
2 files changed, 56 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/11005/6
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 6
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 )
Change subject: IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
Patch Set 10: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 10
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 09 Aug 2018 01:04:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 )
Change subject: [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
Patch Set 4:
I have published the patch.
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 25 Jul 2018 21:40:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 )
Change subject: [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
Patch Set 4:
(7 comments)
I have modified the use of the filter_lock_.
While addressing comments by Bikram and Sailesh, I noticed that the `FilterDebugString()` function has two invocations (called in InitFilterRoutingTable() and ReleaseExecResources()). Acquiring the filter_lock_ in FilterDebugString() was incorrect because the lock was already acquired by the caller in some cases (ReleaseExecResources()). On the other hand, not acquiring the filter_lock_ in the InitFilterRoutingTable() breaks the control flow that a thread always acquires filter_lock_ before altering the filter_routing_table_. So I modified InitFilterRoutingTable to reflect this logic.
@Bikram @Sailesh, could you please review this part of the code and verify that the comments make sense?
Thanks,
Pooja
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.h
File be/src/runtime/coordinator.h:
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.h@302
PS4, Line 302: it
> nit: this lock
Done
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.h@309
PS4, Line 309: releases the memory associated with
: /// filter_routing_table_
> nit: alters the filter_routing_table_
Done
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.h@311
PS4, Line 311: boost::shared_mutex
> We have to ensure that this is a 'writer preferred' shared mutex. It turns
Ack
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.cc@79
PS4, Line 79: exec_state_.Load(), ExecState::EXECUTING
> not your change, but replace with:
Done
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.cc@397
PS4, Line 397: lock_guard<SpinLock> l(filter_update_lock_);
> Why not just get exclusive access to 'filter_lock_' here?
I have actually removed this line from the function.
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.cc@454
PS4, Line 454: exec_state_.Load() != ExecState::EXECUTING
> not your change, but replace with:
Done
http://gerrit.cloudera.org:8080/#/c/11005/4/be/src/runtime/coordinator.cc@714
PS4, Line 714: exec_state_.Load() == ExecState::EXECUTING
> not your change, but replace with:
Done
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 19:30:36 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 )
Change subject: IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
Patch Set 10:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2959/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 10
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Aug 2018 21:48:51 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 )
Change subject: [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
Patch Set 7:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11005/6/be/src/runtime/coordinator.h
File be/src/runtime/coordinator.h:
http://gerrit.cloudera.org:8080/#/c/11005/6/be/src/runtime/coordinator.h@301
PS6, Line 301: /// Synchronizes updates to the filter_routing_table_.
: SpinLock filter_update_lock_;
:
: /// Protects filter_routing_table_.
: /// Usage pattern:
: /// 1. To update filter_routing_table_: Acquire shared access on filter_lock_ and
: /// upgrade to exclusive access by subsequently acquiring filter_update_lock_.
: /// 2. To read, initialize/destroy filter_routing_table: Directly acquire exclusive
: /// access on filter_lock_.
: boost::shared_mutex filter_lock_;
:
> maybe write a consolidated comment to better explain the need for two locks
Done
http://gerrit.cloudera.org:8080/#/c/11005/6/be/src/runtime/coordinator.h@334
PS6, Line 334:
: /// Called when the query is done executing due to reaching EOS or client
: /// cancellation. If 'exec_state_' != EXECUTING, does nothing.
> nit: "Caller must have exclusive access to filter_lock_"
Done
http://gerrit.cloudera.org:8080/#/c/11005/6/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:
http://gerrit.cloudera.org:8080/#/c/11005/6/be/src/runtime/coordinator.cc@265
PS6, Line 265: lock_guard<shared_mutex> lock(filter_lock_);
: for (const FragmentExecParams& fragment_params: schedule_.fragment_exec_params()) {
: int num_instances = fragment_params.instance_exec_par
> fyi: this method is called in exec before any fragments are started. Also,
Make sense. I have removed the comment. I have kept the lock_guard for the sake of keeping a consistent code flow across all accesses. Acquiring the lock should not be expensive if there is no contention on it. Or would you recommend entirely removing it form here?
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 7
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Aug 2018 20:24:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 )
Change subject: [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
Patch Set 5:
No Builds Executed
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 5
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 30 Jul 2018 22:34:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Hello Sailesh Mukil, Tim Armstrong, Bikramjeet Vig, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11005
to look at the new patch set (#8).
Change subject: IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
IMPALA-6153: Execute UpdateFilter() only for executing queries
This change ensures that coordinator::UpdateFilter is executed
only for queries which are in the EXECUTING state. Additionally,
it also guarantees that coordinator::ReleaseExecResources is
executed only when no other thread is executing UpdateFilter.
Testing: Ran all back-end tests.
Additionally, ran the exhaustive tests with DCHECKs which would
triggered if the ReleaseExecResources() is executed while any
other thread is executing UpdateFilter().
Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
---
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
2 files changed, 45 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/11005/8
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 8
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 )
Change subject: [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/179/ : 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/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 7
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Aug 2018 20:48:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/11005 )
Change subject: [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
[WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
This change ensures that coordinator::UpdateFilter is executed
only for queries which are in the EXECUTING state. Additionally,
it also guarantees that coordinator::ReleaseExecResources is
executed only when no other thread is executing UpdateFilter.
Testing: Ran all back-end tests.
[TODO] Add a test to verify that this change guarantees that
UpdateFilter and ReleaseExecResources never execute concurrently.
Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
---
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
2 files changed, 48 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/05/11005/5
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 5
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11005 )
Change subject: [WIP] IMPALA-6153: Execute UpdateFilter() only for executing queries
......................................................................
Patch Set 6:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11005/6/be/src/runtime/coordinator.h
File be/src/runtime/coordinator.h:
http://gerrit.cloudera.org:8080/#/c/11005/6/be/src/runtime/coordinator.h@301
PS6, Line 301: /// Synchronizes updates to the filter_routing_table_.
: /// Any thread which attempts to update the filter_routing_table_ must acquire
: /// this lock after acquiring the filter_lock_ in shared mode. However, if a thread
: /// has exclusive access to filter_lock_, it need not acquire the
: /// filter_update_lock_.
: SpinLock filter_update_lock_;
:
: /// Protects filter_routing_table_.
: /// Any thread which attempts to access the filter_routing_table must acquire the lock
: /// in shared mode. Additionally, any thread which alters the filter_routing_table_
: /// must acquire the lock in exclusive mode.
maybe write a consolidated comment to better explain the need for two locks. This would also help to avoid writing explanations in the .cc file every time we acquire these locks:
Protects filter_routing_table_.
Usage Pattern:
1. To update filter_routing_table_: Acquire shared access on filter_lock_ and upgrade to exclusive access by subsequently acquiring filter_update_lock_.
2. To read, initialize/destroy filter_routing_table_: Directly acquire exclusive access on filter_lock_
http://gerrit.cloudera.org:8080/#/c/11005/6/be/src/runtime/coordinator.h@334
PS6, Line 334: /// This function must only be invoked by threads which have acquired
: /// exclusive access to the filter_lock_. If the caller has not acquired
: /// the filter_lock_. the results produced would be undefined.
nit: "Caller must have exclusive access to filter_lock_"
http://gerrit.cloudera.org:8080/#/c/11005/6/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:
http://gerrit.cloudera.org:8080/#/c/11005/6/be/src/runtime/coordinator.cc@265
PS6, Line 265: // Since this function alters the filter_routing_table_ by allocating memory for the
: // filter objects, acquire the filter_lock_ in exclusive mode to ensure that no other
: // thread accesses the memory before it is initialized.
fyi: this method is called in exec before any fragments are started. Also, no other coord methods are called before exec returns.
--
To view, visit http://gerrit.cloudera.org:8080/11005
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I669db217f86db5ff2802335e7b1ae8027ea7161c
Gerrit-Change-Number: 11005
Gerrit-PatchSet: 6
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sa...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Aug 2018 00:55:17 +0000
Gerrit-HasComments: Yes