You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tianyi Wang (Code Review)" <ge...@cloudera.org> on 2018/10/06 01:33:46 UTC

[Impala-ASF-CR] IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC

Tianyi Wang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11603


Change subject: IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC
......................................................................

IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC

UpdateTableUsage() is logically a one-way RPC and the status object in
TUpdateTableUsageResponse is set only if there is an error at RPC
layer. This patch fixes the incorrect error handling that leads to
NullPointerException in ImpaladTableUsageTracer.

Change-Id: Iccba4c6f4696ef08bc8a614ae13f62b5e445917b
---
M be/src/service/fe-support.cc
M fe/src/main/java/org/apache/impala/catalog/ImpaladTableUsageTracker.java
2 files changed, 2 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iccba4c6f4696ef08bc8a614ae13f62b5e445917b
Gerrit-Change-Number: 11603
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC

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

Change subject: IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC
......................................................................

IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC

UpdateTableUsage() is logically a one-way RPC and the status object in
TUpdateTableUsageResponse is set only if there is an error at RPC
layer. This patch fixes the incorrect error handling that leads to
NullPointerException in ImpaladTableUsageTracer.

Change-Id: Iccba4c6f4696ef08bc8a614ae13f62b5e445917b
---
M be/src/service/fe-support.cc
M fe/src/main/java/org/apache/impala/catalog/ImpaladTableUsageTracker.java
2 files changed, 3 insertions(+), 4 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iccba4c6f4696ef08bc8a614ae13f62b5e445917b
Gerrit-Change-Number: 11603
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC

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

Change subject: IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iccba4c6f4696ef08bc8a614ae13f62b5e445917b
Gerrit-Change-Number: 11603
Gerrit-PatchSet: 3
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Oct 2018 02:13:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC

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

Change subject: IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC
......................................................................

IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC

UpdateTableUsage() is logically a one-way RPC and the status object in
TUpdateTableUsageResponse is set only if there is an error at RPC
layer. This patch fixes the incorrect error handling that leads to
NullPointerException in ImpaladTableUsageTracer.

Change-Id: Iccba4c6f4696ef08bc8a614ae13f62b5e445917b
Reviewed-on: http://gerrit.cloudera.org:8080/11603
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/service/fe-support.cc
M fe/src/main/java/org/apache/impala/catalog/ImpaladTableUsageTracker.java
2 files changed, 3 insertions(+), 4 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iccba4c6f4696ef08bc8a614ae13f62b5e445917b
Gerrit-Change-Number: 11603
Gerrit-PatchSet: 4
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC

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

Change subject: IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/11603/1/be/src/service/fe-support.cc
File be/src/service/fe-support.cc:

http://gerrit.cloudera.org:8080/#/c/11603/1/be/src/service/fe-support.cc@543
PS1, Line 543: status.ToThrift(&result.status);
             :     result.__isset.status = true;
Use  Status::SetTStatus(&result)? There are other places that miss this in this file.. For ex: TGetPartitionStatsResponse. I'm afraid we are silently swallowing errors since the caller thinks the RPC is not successful if the 'status' is not set.


http://gerrit.cloudera.org:8080/#/c/11603/1/fe/src/main/java/org/apache/impala/catalog/ImpaladTableUsageTracker.java
File fe/src/main/java/org/apache/impala/catalog/ImpaladTableUsageTracker.java:

http://gerrit.cloudera.org:8080/#/c/11603/1/fe/src/main/java/org/apache/impala/catalog/ImpaladTableUsageTracker.java@110
PS1, Line 110: resp.status.isSetStatus_code(
This looks like a required field?

struct TStatus {
  1: required ErrorCodes.TErrorCode status_code
  2: list<string> error_msgs
}



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iccba4c6f4696ef08bc8a614ae13f62b5e445917b
Gerrit-Change-Number: 11603
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Oct 2018 22:03:02 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC

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

Change subject: IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iccba4c6f4696ef08bc8a614ae13f62b5e445917b
Gerrit-Change-Number: 11603
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Sat, 06 Oct 2018 02:06:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC

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

Change subject: IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iccba4c6f4696ef08bc8a614ae13f62b5e445917b
Gerrit-Change-Number: 11603
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Oct 2018 01:07:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC

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

Change subject: IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC
......................................................................


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iccba4c6f4696ef08bc8a614ae13f62b5e445917b
Gerrit-Change-Number: 11603
Gerrit-PatchSet: 3
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Oct 2018 06:09:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC

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

Change subject: IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC
......................................................................


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iccba4c6f4696ef08bc8a614ae13f62b5e445917b
Gerrit-Change-Number: 11603
Gerrit-PatchSet: 2
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Oct 2018 02:11:33 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC

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

Change subject: IMPALA-7675: Fix the error handling of UpdateTableUsage() RPC
......................................................................


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iccba4c6f4696ef08bc8a614ae13f62b5e445917b
Gerrit-Change-Number: 11603
Gerrit-PatchSet: 3
Gerrit-Owner: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Oct 2018 02:13:39 +0000
Gerrit-HasComments: No