You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Joe McDonnell (Code Review)" <ge...@cloudera.org> on 2017/09/01 21:57:41 UTC

[Impala-ASF-CR] IMPALA-5892: Allow reporting status independent of fragment instance

Joe McDonnell has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/7943

Change subject: IMPALA-5892: Allow reporting status independent of fragment instance
......................................................................

IMPALA-5892: Allow reporting status independent of fragment instance

Queries can hit an error that is not specific to a
particular fragment instance. For example, QueryState::StartFInstances()
calls DescriptorTbl::Create() before any fragment instances
start. This location has no reason to report status via a
particular fragment, and there is currently no way to report
status otherwise. This leads to a query hang, because the
status is never propagated back to the coordinator.

This adds the ability to report status that is not associated
with a particular fragment instance. By reporting status,
the coordinator will now correctly abort the query in the
case of the QueryState::StartFInstances() scenario described.

Change-Id: I4cd98022f1d62a999c7c80ff5474fa8d069eb12c
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/runtime/query-state.cc
M common/thrift/ImpalaInternalService.thrift
6 files changed, 58 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/7943/1
-- 
To view, visit http://gerrit.cloudera.org:8080/7943
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I4cd98022f1d62a999c7c80ff5474fa8d069eb12c
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>

[Impala-ASF-CR] IMPALA-5892: Allow reporting status independent of fragment instance

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Dan Hecht,

I'd like you to reexamine a change.  Please visit

    http://gerrit.cloudera.org:8080/7943

to look at the new patch set (#3).

Change subject: IMPALA-5892: Allow reporting status independent of fragment instance
......................................................................

IMPALA-5892: Allow reporting status independent of fragment instance

Queries can hit an error that is not specific to a
particular fragment instance. For example, QueryState::StartFInstances()
calls DescriptorTbl::Create() before any fragment instances
start. This location has no reason to report status via a
particular fragment, and there is currently no way to report
status otherwise. This leads to a query hang, because the
status is never propagated back to the coordinator.

This adds the ability to report status that is not associated
with a particular fragment instance. By reporting status,
the coordinator will now correctly abort the query in the
case of the QueryState::StartFInstances() scenario described.

Change-Id: I4cd98022f1d62a999c7c80ff5474fa8d069eb12c
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/runtime/query-state.cc
M common/thrift/ImpalaInternalService.thrift
6 files changed, 73 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/7943/3
-- 
To view, visit http://gerrit.cloudera.org:8080/7943
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4cd98022f1d62a999c7c80ff5474fa8d069eb12c
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>

[Impala-ASF-CR] IMPALA-5892: Allow reporting status independent of fragment instance

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: IMPALA-5892: Allow reporting status independent of fragment instance
......................................................................


Patch Set 1:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/7943/1/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

PS1, Line 219: *failed_instance_id = failed_instance_id_;
see header question -- this is why it seems we should require that the caller can only ask for the failed_instance_id if it also asks whether there is a failed instance. otherwise, this assignment is kinda meaningless.


PS1, Line 309: independently
independent of an instance
(to clarify what it's independent of)


PS1, Line 310: incorporated
incorporated to where?  and what is "here" referring to, given that you just said that the cumulative status includes fragment instance status.

I had to read through the code to understand what the comment was saying, so I think the comment would be clearer if it said something like:

So far 'status_' incorporates the fragment instances' status. Now incorporate the overall exec status into 'status_'.

or something along those lines.


PS1, Line 312: cumulative_status
that name confused me because it's doesn't seem like this is the cumulative status, but instead status_ is the thing we are accumulating.

I think in a comment later you refer to this as the "general status". That might be clearer to call this general_status.


http://gerrit.cloudera.org:8080/#/c/7943/1/be/src/runtime/coordinator-backend-state.h
File be/src/runtime/coordinator-backend-state.h:

PS1, Line 100: has_failed_instance == false
what does that mean? do you mean '*has_failed_instance == false' or something different?

should we require that failed_instance_id != nullptr implies has_failed_instance != nullptr?


http://gerrit.cloudera.org:8080/#/c/7943/1/be/src/runtime/coordinator.h
File be/src/runtime/coordinator.h:

PS1, Line 375:   Status UpdateStatus(const Status& status, const TUniqueId& failed_fragment,
             :       const std::string& instance_hostname,
             :       bool has_failed_instance = true) WARN_UNUSED_RESULT;
> Might be cleaner to add two functions UpdateFragmentStatus(status, failed_f
If we keep this one method, let's be careful with our use of default parameter values. sometimes they are convenient but i think we should be careful to use them since they can often lead to errors due to forgetting to set the value. And in this case, it seems more intuitive to have has_failed_instance come before failed_fragement since the meaning of failed_fragment is dependent on it. And to be consistent in parameter order of GetStatus(), which is tightly related.


http://gerrit.cloudera.org:8080/#/c/7943/1/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:

PS1, Line 628: This incorporates status from
             :   // TFragmentInstanceExecStatus as well as errors that occur independently
given today's implementation it doesn't matter, but from a protocol standpoint, if there are multiple instances with error status, which one is this set to? i.e. i think we should be very clear on our specification of our protocols.


-- 
To view, visit http://gerrit.cloudera.org:8080/7943
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4cd98022f1d62a999c7c80ff5474fa8d069eb12c
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5892: Allow reporting status independent of fragment instance

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has uploaded a new patch set (#2).

Change subject: IMPALA-5892: Allow reporting status independent of fragment instance
......................................................................

IMPALA-5892: Allow reporting status independent of fragment instance

Queries can hit an error that is not specific to a
particular fragment instance. For example, QueryState::StartFInstances()
calls DescriptorTbl::Create() before any fragment instances
start. This location has no reason to report status via a
particular fragment, and there is currently no way to report
status otherwise. This leads to a query hang, because the
status is never propagated back to the coordinator.

This adds the ability to report status that is not associated
with a particular fragment instance. By reporting status,
the coordinator will now correctly abort the query in the
case of the QueryState::StartFInstances() scenario described.

Change-Id: I4cd98022f1d62a999c7c80ff5474fa8d069eb12c
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/runtime/query-state.cc
M common/thrift/ImpalaInternalService.thrift
6 files changed, 79 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/7943/2
-- 
To view, visit http://gerrit.cloudera.org:8080/7943
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4cd98022f1d62a999c7c80ff5474fa8d069eb12c
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>

[Impala-ASF-CR] IMPALA-5892: Allow reporting status independent of fragment instance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5892: Allow reporting status independent of fragment instance
......................................................................


Patch Set 3: Verified+1

-- 
To view, visit http://gerrit.cloudera.org:8080/7943
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4cd98022f1d62a999c7c80ff5474fa8d069eb12c
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5892: Allow reporting status independent of fragment instance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged.

Change subject: IMPALA-5892: Allow reporting status independent of fragment instance
......................................................................


IMPALA-5892: Allow reporting status independent of fragment instance

Queries can hit an error that is not specific to a
particular fragment instance. For example, QueryState::StartFInstances()
calls DescriptorTbl::Create() before any fragment instances
start. This location has no reason to report status via a
particular fragment, and there is currently no way to report
status otherwise. This leads to a query hang, because the
status is never propagated back to the coordinator.

This adds the ability to report status that is not associated
with a particular fragment instance. By reporting status,
the coordinator will now correctly abort the query in the
case of the QueryState::StartFInstances() scenario described.

Change-Id: I4cd98022f1d62a999c7c80ff5474fa8d069eb12c
Reviewed-on: http://gerrit.cloudera.org:8080/7943
Reviewed-by: Lars Volker <lv...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/runtime/coordinator-backend-state.cc
M be/src/runtime/coordinator-backend-state.h
M be/src/runtime/coordinator.cc
M be/src/runtime/coordinator.h
M be/src/runtime/query-state.cc
M common/thrift/ImpalaInternalService.thrift
6 files changed, 73 insertions(+), 22 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Lars Volker: Looks good to me, approved



-- 
To view, visit http://gerrit.cloudera.org:8080/7943
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I4cd98022f1d62a999c7c80ff5474fa8d069eb12c
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>

[Impala-ASF-CR] IMPALA-5892: Allow reporting status independent of fragment instance

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change.

Change subject: IMPALA-5892: Allow reporting status independent of fragment instance
......................................................................


Patch Set 1:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/7943/1/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

PS1, Line 219: *failed_instance_id = failed_instance_id_;
> see header question -- this is why it seems we should require that the call
Changed the semantic so that the caller must specify both or neither.


PS1, Line 309: independently
> independent of an instance
Rewrote this comment.


PS1, Line 310: incorporated
> incorporated to where?  and what is "here" referring to, given that you jus
Rewrote this comment.


PS1, Line 312: cumulative_status
> that name confused me because it's doesn't seem like this is the cumulative
Changed this to "overall_backend_status" and rewrote the comment.


http://gerrit.cloudera.org:8080/#/c/7943/1/be/src/runtime/coordinator-backend-state.h
File be/src/runtime/coordinator-backend-state.h:

Line 95:   /// (with not specific fragment responsible).
> nit: s/not/no/?
Done


PS1, Line 100: has_failed_instance == false
> what does that mean? do you mean '*has_failed_instance == false' or somethi
Changed the semantics so that the caller must specify both or neither. Also updated the comment to make the semantics clear.


http://gerrit.cloudera.org:8080/#/c/7943/1/be/src/runtime/coordinator.h
File be/src/runtime/coordinator.h:

PS1, Line 375:   Status UpdateStatus(const Status& status, const TUniqueId& failed_fragment,
             :       const std::string& instance_hostname,
             :       bool has_failed_instance = true) WARN_UNUSED_RESULT;
> If we keep this one method, let's be careful with our use of default parame
Changed the ordering and made the arguments required.


http://gerrit.cloudera.org:8080/#/c/7943/1/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:

PS1, Line 628: This incorporates status from
             :   // TFragmentInstanceExecStatus as well as errors that occur independently
> given today's implementation it doesn't matter, but from a protocol standpo
Updated the comment to be clear about the priorities. We don't currently report multiple errors, but I specified what I think is a reasonable semantic for that case.


-- 
To view, visit http://gerrit.cloudera.org:8080/7943
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4cd98022f1d62a999c7c80ff5474fa8d069eb12c
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5892: Allow reporting status independent of fragment instance

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-5892: Allow reporting status independent of fragment instance
......................................................................


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1200/

-- 
To view, visit http://gerrit.cloudera.org:8080/7943
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4cd98022f1d62a999c7c80ff5474fa8d069eb12c
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5892: Allow reporting status independent of fragment instance

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change.

Change subject: IMPALA-5892: Allow reporting status independent of fragment instance
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7943/1/be/src/runtime/coordinator.h
File be/src/runtime/coordinator.h:

PS1, Line 375:   Status UpdateStatus(const Status& status, const TUniqueId& failed_fragment,
             :       const std::string& instance_hostname,
             :       bool has_failed_instance = true) WARN_UNUSED_RESULT;
Might be cleaner to add two functions UpdateFragmentStatus(status, failed_fragment, instance_hostname) and UpdateGeneralStatus(status, instance_hostname).


-- 
To view, visit http://gerrit.cloudera.org:8080/7943
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4cd98022f1d62a999c7c80ff5474fa8d069eb12c
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5892: Allow reporting status independent of fragment instance

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change.

Change subject: IMPALA-5892: Allow reporting status independent of fragment instance
......................................................................


Patch Set 2:

(3 comments)

Also rebased all the way.

http://gerrit.cloudera.org:8080/#/c/7943/2/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

Line 221:   DCHECK(requests_fragment_detail || omits_fragment_args);
> how about just:
Done


http://gerrit.cloudera.org:8080/#/c/7943/2/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

Line 873:   // in that case).  Coordinator fragment failed in this case so we log the query_id.
> not your change (and doesn't have to be addressed now) but this comment doe
I think the first sentence makes some sense. If there is already an error in query_status_, then UpdateStatus() will return that rather than the status passed in. If a fragment hit an error and called CancelInternal(), that would call ReleaseResources(), which closes the coord_sink_. That should cause GetNext() to return an error, but not the original error.

The second sentence doesn't match anything that I can see.


http://gerrit.cloudera.org:8080/#/c/7943/2/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:

PS2, Line 613: =cdfklnoru
> some random characters snuck in
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/7943
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4cd98022f1d62a999c7c80ff5474fa8d069eb12c
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5892: Allow reporting status independent of fragment instance

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change.

Change subject: IMPALA-5892: Allow reporting status independent of fragment instance
......................................................................


Patch Set 3: Code-Review+2

Carrying Dan's +2.

-- 
To view, visit http://gerrit.cloudera.org:8080/7943
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4cd98022f1d62a999c7c80ff5474fa8d069eb12c
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5892: Allow reporting status independent of fragment instance

Posted by "Dan Hecht (Code Review)" <ge...@cloudera.org>.
Dan Hecht has posted comments on this change.

Change subject: IMPALA-5892: Allow reporting status independent of fragment instance
......................................................................


Patch Set 2: Code-Review+2

(3 comments)

http://gerrit.cloudera.org:8080/#/c/7943/2/be/src/runtime/coordinator-backend-state.cc
File be/src/runtime/coordinator-backend-state.cc:

Line 221:   DCHECK(requests_fragment_detail || omits_fragment_args);
how about just:

DCHECK_EQ(is_fragment_failure == nullptr, failed_instance_id == nullptr);

seems simpler to reason about rather than why the || condition is really xor.
(nit: If you do keep these bool's, remove the extraneous parenthesis from the rhs).


http://gerrit.cloudera.org:8080/#/c/7943/2/be/src/runtime/coordinator.cc
File be/src/runtime/coordinator.cc:

Line 873:   // in that case).  Coordinator fragment failed in this case so we log the query_id.
not your change (and doesn't have to be addressed now) but this comment doesn't seem to match the code, right?


http://gerrit.cloudera.org:8080/#/c/7943/2/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:

PS2, Line 613: =cdfklnoru
some random characters snuck in


-- 
To view, visit http://gerrit.cloudera.org:8080/7943
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4cd98022f1d62a999c7c80ff5474fa8d069eb12c
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5892: Allow reporting status independent of fragment instance

Posted by "Philip Zeyliger (Code Review)" <ge...@cloudera.org>.
Philip Zeyliger has posted comments on this change.

Change subject: IMPALA-5892: Allow reporting status independent of fragment instance
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/7943/1/be/src/runtime/coordinator-backend-state.h
File be/src/runtime/coordinator-backend-state.h:

Line 95:   /// (with not specific fragment responsible).
nit: s/not/no/?


-- 
To view, visit http://gerrit.cloudera.org:8080/7943
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I4cd98022f1d62a999c7c80ff5474fa8d069eb12c
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Philip Zeyliger <ph...@cloudera.com>
Gerrit-HasComments: Yes