You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Michael Ho (Code Review)" <ge...@cloudera.org> on 2019/03/04 08:15:10 UTC

[Impala-ASF-CR] IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()

Michael Ho has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12651


Change subject: IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()
......................................................................

IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()

The coordinator skips over any stale or duplicated status
reports of fragment instances. In the previous implementation,
the index pointing into the vector of Thrift profiles wasn't
updated when skipping over a status report. This breaks the
assumption that the status reports and thrift profiles vectors
have one-to-one correspondence. Consequently, we may end up
passing the wrong profile to InstanceStats::Update(), leading
to random crashes.

This change fixes the problem above by using iterators to
iterate through the status reports and thrift profiles vectors
and ensures that both iterators are updated on every iteration
of the loop.

Change-Id: I8bce426c7d08ffbf0f8cd26889262243a52cc752
---
M be/src/runtime/coordinator-backend-state.cc
M tests/custom_cluster/test_rpc_timeout.py
2 files changed, 32 insertions(+), 17 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8bce426c7d08ffbf0f8cd26889262243a52cc752
Gerrit-Change-Number: 12651
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho <kw...@cloudera.com>

[Impala-ASF-CR] IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()

Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Hello Thomas Marshall, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()
......................................................................

IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()

The coordinator skips over any stale or duplicated status
reports of fragment instances. In the previous implementation,
the index pointing into the vector of Thrift profiles wasn't
updated when skipping over a status report. This breaks the
assumption that the status reports and thrift profiles vectors
have one-to-one correspondence. Consequently, we may end up
passing the wrong profile to InstanceStats::Update(), leading
to random crashes.

This change fixes the problem above by using iterators to
iterate through the status reports and thrift profiles vectors
and ensures that both iterators are updated on every iteration
of the loop.

Change-Id: I8bce426c7d08ffbf0f8cd26889262243a52cc752
---
M be/src/runtime/coordinator-backend-state.cc
M tests/custom_cluster/test_rpc_timeout.py
2 files changed, 33 insertions(+), 17 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8bce426c7d08ffbf0f8cd26889262243a52cc752
Gerrit-Change-Number: 12651
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()

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

Change subject: IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()
......................................................................

IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()

The coordinator skips over any stale or duplicated status
reports of fragment instances. In the previous implementation,
the index pointing into the vector of Thrift profiles wasn't
updated when skipping over a status report. This breaks the
assumption that the status reports and thrift profiles vectors
have one-to-one correspondence. Consequently, we may end up
passing the wrong profile to InstanceStats::Update(), leading
to random crashes.

This change fixes the problem above by using iterators to
iterate through the status reports and thrift profiles vectors
and ensures that both iterators are updated on every iteration
of the loop.

Change-Id: I8bce426c7d08ffbf0f8cd26889262243a52cc752
Reviewed-on: http://gerrit.cloudera.org:8080/12651
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/runtime/coordinator-backend-state.cc
M tests/custom_cluster/test_rpc_timeout.py
2 files changed, 33 insertions(+), 17 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I8bce426c7d08ffbf0f8cd26889262243a52cc752
Gerrit-Change-Number: 12651
Gerrit-PatchSet: 4
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()

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

Change subject: IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()
......................................................................


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8bce426c7d08ffbf0f8cd26889262243a52cc752
Gerrit-Change-Number: 12651
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Mar 2019 19:45:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()

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

Change subject: IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()
......................................................................


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8bce426c7d08ffbf0f8cd26889262243a52cc752
Gerrit-Change-Number: 12651
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Mar 2019 23:56:12 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()

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

Change subject: IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8bce426c7d08ffbf0f8cd26889262243a52cc752
Gerrit-Change-Number: 12651
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Mar 2019 08:58:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()

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

Change subject: IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8bce426c7d08ffbf0f8cd26889262243a52cc752
Gerrit-Change-Number: 12651
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Mar 2019 19:45:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()

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

Change subject: IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()
......................................................................


Patch Set 2: Code-Review+2

Added a line of comment. Carry Thomas' +2.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8bce426c7d08ffbf0f8cd26889262243a52cc752
Gerrit-Change-Number: 12651
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Mar 2019 19:34:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()

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

Change subject: IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8bce426c7d08ffbf0f8cd26889262243a52cc752
Gerrit-Change-Number: 12651
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Mar 2019 20:18:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()

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

Change subject: IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()
......................................................................


Patch Set 1: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8bce426c7d08ffbf0f8cd26889262243a52cc752
Gerrit-Change-Number: 12651
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Mar 2019 18:09:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()

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

Change subject: IMPALA-8274: Fix iteration of profiles in ApplyExecStatusReport()
......................................................................


Patch Set 2:

Waiting for https://gerrit.cloudera.org/#/c/12649/ to merge.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8bce426c7d08ffbf0f8cd26889262243a52cc752
Gerrit-Change-Number: 12651
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 04 Mar 2019 19:35:53 +0000
Gerrit-HasComments: No