You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tamas Mate (Code Review)" <ge...@cloudera.org> on 2023/02/07 10:46:09 UTC
[Impala-ASF-CR] IMPALA-11834: Error reporting addendum
Tamas Mate has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19481
Change subject: IMPALA-11834: Error reporting addendum
......................................................................
IMPALA-11834: Error reporting addendum
This commit is an addendum that addresses some review comments that
arrived while the base change was merged. Code cleanup and improved
error reporting.
Testing:
- Tested the code paths manually
Change-Id: Iece31fa19b7448095462671acb2f02e8c5a405ae
---
M be/src/service/client-request-state.cc
1 file changed, 23 insertions(+), 11 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/19481/2
--
To view, visit http://gerrit.cloudera.org:8080/19481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iece31fa19b7448095462671acb2f02e8c5a405ae
Gerrit-Change-Number: 19481
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11834: Error reporting addendum
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19481 )
Change subject: IMPALA-11834: Error reporting addendum
......................................................................
Patch Set 2: Code-Review+2
Thanks.
--
To view, visit http://gerrit.cloudera.org:8080/19481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iece31fa19b7448095462671acb2f02e8c5a405ae
Gerrit-Change-Number: 19481
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Feb 2023 12:50:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11834: Error reporting addendum
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19481 )
Change subject: IMPALA-11834: Error reporting addendum
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/19481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iece31fa19b7448095462671acb2f02e8c5a405ae
Gerrit-Change-Number: 19481
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Feb 2023 18:00:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11834: Error reporting addendum
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/19481 )
Change subject: IMPALA-11834: Error reporting addendum
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19481/2/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:
http://gerrit.cloudera.org:8080/#/c/19481/2/be/src/service/client-request-state.cc@885
PS2, Line 885: errno
> Optiional: it could be more informative if, before the errno value we wrote
The strerror() call will translate the error number to an error message.
--
To view, visit http://gerrit.cloudera.org:8080/19481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iece31fa19b7448095462671acb2f02e8c5a405ae
Gerrit-Change-Number: 19481
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Feb 2023 12:48:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11834: Error reporting addendum
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19481 )
Change subject: IMPALA-11834: Error reporting addendum
......................................................................
Patch Set 2:
(1 comment)
Thanks, just a small comment.
http://gerrit.cloudera.org:8080/#/c/19481/2/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:
http://gerrit.cloudera.org:8080/#/c/19481/2/be/src/service/client-request-state.cc@885
PS2, Line 885: errno
Before this change we added "query_status.msg().msg()" to the status error message. Was it only relevant for the "!hdfs_ret.ok()" case?
See also L908.
--
To view, visit http://gerrit.cloudera.org:8080/19481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iece31fa19b7448095462671acb2f02e8c5a405ae
Gerrit-Change-Number: 19481
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Feb 2023 12:14:48 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11834: Error reporting addendum
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19481 )
Change subject: IMPALA-11834: Error reporting addendum
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iece31fa19b7448095462671acb2f02e8c5a405ae
Gerrit-Change-Number: 19481
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Feb 2023 12:51:06 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11834: Error reporting addendum
Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/19481 )
Change subject: IMPALA-11834: Error reporting addendum
......................................................................
Patch Set 2:
(1 comment)
Thank you for the review Daniel!
http://gerrit.cloudera.org:8080/#/c/19481/2/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:
http://gerrit.cloudera.org:8080/#/c/19481/2/be/src/service/client-request-state.cc@885
PS2, Line 885: errno
> Before this change we added "query_status.msg().msg()" to the status error
That was actually unnecessary in L881, because the query_status is supposed to be OK on this code path. On the other code path however it is useful to add the query_status and it is appended to revert_err.
Changed to GetDetail() because this method is better to get the error message from the Status object.
--
To view, visit http://gerrit.cloudera.org:8080/19481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iece31fa19b7448095462671acb2f02e8c5a405ae
Gerrit-Change-Number: 19481
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Feb 2023 08:11:32 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11834: Error reporting addendum
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19481 )
Change subject: IMPALA-11834: Error reporting addendum
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12325/ : 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/19481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iece31fa19b7448095462671acb2f02e8c5a405ae
Gerrit-Change-Number: 19481
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Feb 2023 11:08:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11834: Error reporting addendum
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/19481 )
Change subject: IMPALA-11834: Error reporting addendum
......................................................................
IMPALA-11834: Error reporting addendum
This commit is an addendum that addresses some review comments that
arrived while the base change was merged. Code cleanup and improved
error reporting.
Testing:
- Tested the code paths manually
Change-Id: Iece31fa19b7448095462671acb2f02e8c5a405ae
Reviewed-on: http://gerrit.cloudera.org:8080/19481
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/service/client-request-state.cc
1 file changed, 23 insertions(+), 11 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/19481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iece31fa19b7448095462671acb2f02e8c5a405ae
Gerrit-Change-Number: 19481
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11834: Error reporting addendum
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19481 )
Change subject: IMPALA-11834: Error reporting addendum
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9036/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/19481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iece31fa19b7448095462671acb2f02e8c5a405ae
Gerrit-Change-Number: 19481
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Feb 2023 12:51:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11834: Error reporting addendum
Posted by "Daniel Becker (Code Review)" <ge...@cloudera.org>.
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/19481 )
Change subject: IMPALA-11834: Error reporting addendum
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19481/2/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:
http://gerrit.cloudera.org:8080/#/c/19481/2/be/src/service/client-request-state.cc@885
PS2, Line 885: errno
> That was actually unnecessary in L881, because the query_status is supposed
Optiional: it could be more informative if, before the errno value we wrote something like "error number".
--
To view, visit http://gerrit.cloudera.org:8080/19481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iece31fa19b7448095462671acb2f02e8c5a405ae
Gerrit-Change-Number: 19481
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Feb 2023 12:43:59 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11834: Error reporting addendum
Posted by "Noemi Pap-Takacs (Code Review)" <ge...@cloudera.org>.
Noemi Pap-Takacs has posted comments on this change. ( http://gerrit.cloudera.org:8080/19481 )
Change subject: IMPALA-11834: Error reporting addendum
......................................................................
Patch Set 2: Code-Review+1
LGTM
--
To view, visit http://gerrit.cloudera.org:8080/19481
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iece31fa19b7448095462671acb2f02e8c5a405ae
Gerrit-Change-Number: 19481
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Daniel Becker <da...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <np...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@apache.org>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Wed, 08 Feb 2023 12:35:28 +0000
Gerrit-HasComments: No