You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org> on 2020/12/23 17:21:21 UTC

[Impala-ASF-CR] IMPALA-10259: Fixed DCHECK error for backend in terminal state

Wenzhe Zhou has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16900


Change subject: IMPALA-10259: Fixed DCHECK error for backend in terminal state
......................................................................

IMPALA-10259: Fixed DCHECK error for backend in terminal state

This issue happened for cdpd-master core ASAN build. According to
log message, one backend sent status report with instance_exec_status
as done for all assigned instances, then it sent last status report
with error. The coordinator marked the backend state as done after
it processed the status report with instance_exec_status as done,
but did not apply last status report with error to the backend state.
This caused backend to receive a response with status as OK for the
last status report, hence hit DCHECK error.
This issue could not be re-produced with ASAN build on local machine.
To fix the bug, the coordinator need to check the overall-status
of exec status report even if the num_remaining_instances_ of the
BackendState reach to 0.

Testing:
 - Passed exhaustive test.

Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
---
M be/src/runtime/coordinator-backend-state.cc
1 file changed, 17 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
Gerrit-Change-Number: 16900
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-10259: Fixed DCHECK error for backend in terminal state

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16900 )

Change subject: IMPALA-10259: Fixed DCHECK error for backend in terminal state
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7902/ : 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/16900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
Gerrit-Change-Number: 16900
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 24 Dec 2020 00:39:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10259: Fixed DCHECK error for backend in terminal state

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16900 )

Change subject: IMPALA-10259: Fixed DCHECK error for backend in terminal state
......................................................................


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6853/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
Gerrit-Change-Number: 16900
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Jan 2021 20:36:35 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10259: Fixed DCHECK error for backend in terminal state

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16900 )

Change subject: IMPALA-10259: Fixed DCHECK error for backend in terminal state
......................................................................


Patch Set 4: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
Gerrit-Change-Number: 16900
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Jan 2021 02:19:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10259: Fixed DCHECK error for backend in terminal state

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/16900 )

Change subject: IMPALA-10259: Fixed DCHECK error for backend in terminal state
......................................................................

IMPALA-10259: Fixed DCHECK error for backend in terminal state

This issue happened for core ASAN build.
According to log message, one backend sent status report with
instance_exec_status as done for all assigned instances without
error, then it sent last status report with error. The coordinator
treat the backend state as done after it processed the status report
with instance_exec_status as done, but did not apply last status
report with error to the overall backend state.
This caused backend to receive a response with status as OK for the
last status report, hence hit DCHECK error.

This patch fix the race for updating the 'Query State' and updating
the fragment instance state when hitting error during execution of
fragment instance. The backends will not send status report with
fragment instance state as "completed" without error after hitting
error.

Testing:
 - Manual tests
   I could only reproduce the situation by adding some artificial
   delays in the beginning of QueryState::ErrorDuringExecute()
   when repeatedly running test case test_spilling.py::
   TestSpillingDebugActionDimensions::test_spilling_naaj for
   Impala ASAN build.
   Verified that the issue did not happen after applying this
   patch.
 - Passed exhaustive test.

Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
Reviewed-on: http://gerrit.cloudera.org:8080/16900
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/fragment-instance-state.h
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
4 files changed, 38 insertions(+), 12 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
Gerrit-Change-Number: 16900
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-10259: Fixed DCHECK error for backend in terminal state

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16900 )

Change subject: IMPALA-10259: Fixed DCHECK error for backend in terminal state
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7961/ : 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/16900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
Gerrit-Change-Number: 16900
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Jan 2021 01:47:50 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10259: Fixed DCHECK error for backend in terminal state

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/16900 )

Change subject: IMPALA-10259: Fixed DCHECK error for backend in terminal state
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
Gerrit-Change-Number: 16900
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Jan 2021 20:35:33 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10259: Fixed DCHECK error for backend in terminal state

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16900 )

Change subject: IMPALA-10259: Fixed DCHECK error for backend in terminal state
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7901/ : 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/16900
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
Gerrit-Change-Number: 16900
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Dec 2020 17:43:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10259: Fixed DCHECK error for backend in terminal state

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/16900 )

Change subject: IMPALA-10259: Fixed DCHECK error for backend in terminal state
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16900/3/be/src/runtime/fragment-instance-state.cc
File be/src/runtime/fragment-instance-state.cc:

http://gerrit.cloudera.org:8080/#/c/16900/3/be/src/runtime/fragment-instance-state.cc@120
PS3, Line 120:       query_state_->ErrorDuringPrepare(status, instance_id());
> Is the same issue possible here?
No. In QueryState::MonitorFInstances(), the executor wait for preparation to be finished before it start to periodically send status report. If preparation failed, the executor only send the last status report. So there is no race.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
Gerrit-Change-Number: 16900
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Jan 2021 19:35:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10259: Fixed DCHECK error for backend in terminal state

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/16900 )

Change subject: IMPALA-10259: Fixed DCHECK error for backend in terminal state
......................................................................

IMPALA-10259: Fixed DCHECK error for backend in terminal state

This issue happened for cdpd-master core ASAN build. According to
log message, one backend sent status report with instance_exec_status
as done for all assigned instances, then it sent last status report
with error. The coordinator marked the backend state as done after
it processed the status report with instance_exec_status as done,
but did not apply last status report with error to the backend state.
This caused backend to receive a response with status as OK for the
last status report, hence hit DCHECK error.

To fix the bug, the coordinator need to check the overall-status
of exec status report even if the num_remaining_instances_ of the
BackendState reach to 0.

Testing:
 - Manual tests
   I could only reproduce the situation by adding some artificial
   delays in the beginning of QueryState::ErrorDuringExecute()
   when repeatedly running test case test_spilling.py::
   TestSpillingDebugActionDimensions::test_spilling_naaj.
   Verified that the issue did not happen after applying this
   patch.
 - Passed exhaustive test.

Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
---
M be/src/runtime/coordinator-backend-state.cc
1 file changed, 17 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
Gerrit-Change-Number: 16900
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-10259: Fixed DCHECK error for backend in terminal state

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16900 )

Change subject: IMPALA-10259: Fixed DCHECK error for backend in terminal state
......................................................................


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
Gerrit-Change-Number: 16900
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Jan 2021 20:36:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10259: Fixed DCHECK error for backend in terminal state

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/16900 )

Change subject: IMPALA-10259: Fixed DCHECK error for backend in terminal state
......................................................................

IMPALA-10259: Fixed DCHECK error for backend in terminal state

This issue happened for core ASAN build.
According to log message, one backend sent status report with
instance_exec_status as done for all assigned instances without
error, then it sent last status report with error. The coordinator
treat the backend state as done after it processed the status report
with instance_exec_status as done, but did not apply last status
report with error to the overall backend state.
This caused backend to receive a response with status as OK for the
last status report, hence hit DCHECK error.

This patch fix the race for updating the 'Query State' and updating
the fragment instance state when hitting error during execution of
fragment instance. The backends will not send status report with
fragment instance state as "completed" without error after hitting
error.

Testing:
 - Manual tests
   I could only reproduce the situation by adding some artificial
   delays in the beginning of QueryState::ErrorDuringExecute()
   when repeatedly running test case test_spilling.py::
   TestSpillingDebugActionDimensions::test_spilling_naaj for
   Impala ASAN build.
   Verified that the issue did not happen after applying this
   patch.
 - Passed exhaustive test.

Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
---
M be/src/runtime/fragment-instance-state.cc
M be/src/runtime/fragment-instance-state.h
M be/src/runtime/query-state.cc
M be/src/runtime/query-state.h
4 files changed, 38 insertions(+), 12 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
Gerrit-Change-Number: 16900
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>

[Impala-ASF-CR] IMPALA-10259: Fixed DCHECK error for backend in terminal state

Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/16900 )

Change subject: IMPALA-10259: Fixed DCHECK error for backend in terminal state
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16900/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16900/2//COMMIT_MSG@9
PS2, Line 9: cdpd-master
> This is Apache Impala, please don't reference vendor-specific things like t
will remove it.


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

http://gerrit.cloudera.org:8080/#/c/16900/2/be/src/runtime/coordinator-backend-state.cc@412
PS2, Line 412:     // It's possible to receive status report from backend even num_remaining_instances_
> I'm concerned that this approach will have unintended consequences, as ther
I will make change on executor side: executor will not report any error if it already reported all assigned fragment instances done.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
Gerrit-Change-Number: 16900
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Jan 2021 22:16:10 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10259: Fixed DCHECK error for backend in terminal state

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/16900 )

Change subject: IMPALA-10259: Fixed DCHECK error for backend in terminal state
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16900/3/be/src/runtime/fragment-instance-state.cc
File be/src/runtime/fragment-instance-state.cc:

http://gerrit.cloudera.org:8080/#/c/16900/3/be/src/runtime/fragment-instance-state.cc@120
PS3, Line 120:       query_state_->ErrorDuringPrepare(status, instance_id());
Is the same issue possible here?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
Gerrit-Change-Number: 16900
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 19 Jan 2021 19:12:20 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10259: Fixed DCHECK error for backend in terminal state

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/16900 )

Change subject: IMPALA-10259: Fixed DCHECK error for backend in terminal state
......................................................................


Patch Set 2:

(2 comments)

So something I'm not clear on here: is the query that causes the issue supposed to fail the error? If so, why is it that the backend sent a 'done' status report with an OK status at all?

It seems to me that is the actual problem - that the backend was apparently not actually done executing (since it hadn't yet hit the error it was supposed to) yet it reported that it was done.

http://gerrit.cloudera.org:8080/#/c/16900/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16900/2//COMMIT_MSG@9
PS2, Line 9: cdpd-master
This is Apache Impala, please don't reference vendor-specific things like this, as others looking at this review may not know what you're talking about.


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

http://gerrit.cloudera.org:8080/#/c/16900/2/be/src/runtime/coordinator-backend-state.cc@412
PS2, Line 412:     // It's possible to receive status report from backend even num_remaining_instances_
I'm concerned that this approach will have unintended consequences, as there may be other places in Coordinator where we assume that a backend that has reached to 'done' state will no longer have any changes to its state.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic12a80e20ddc11e32349edfec2bd16338c24b841
Gerrit-Change-Number: 16900
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Jan 2021 21:57:58 +0000
Gerrit-HasComments: Yes