[Impala-ASF-CR] IMPALA-7183: Include remote host when logging unknown execution status.

2018-12-17 Thread Andrew Sherman (Code Review)
Andrew Sherman has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/12102


Change subject: IMPALA-7183: Include remote host when logging unknown execution 
status.
..

IMPALA-7183: Include remote host when logging unknown execution status.

ControlService::ReportExecStatus reports the execution status of
a fragment to the coordinator. When the coordinator receives a
ReportExecStatus call, it looks up the ClientRequestState for the query
id. If this cannot be found, usually because the query is in the process
of being canceled, then it logs a message before responding to the
RPC. Add the remote host name to this message. We try to use the DNS
reverse-lookup name, if that fails we use the IP address instead.

Example output: "ReportExecStatus(): Received report for unknown query
ID (probably closed or cancelled): 9a437a10d83cfd86:3c961136
remote host=localhost"

TESTING
 Ran end-to-end tests.
 Forced code to execute by hand to check output is correct.

Change-Id: I79b223e22096e2d964b72654a34dc0d88e8c6422
---
M be/src/service/control-service.cc
1 file changed, 9 insertions(+), 1 deletion(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I79b223e22096e2d964b72654a34dc0d88e8c6422
Gerrit-Change-Number: 12102
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Thomas Marshall 


[Impala-ASF-CR] IMPALA-7183: Include remote host when logging unknown execution status.

2018-12-17 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12102 )

Change subject: IMPALA-7183: Include remote host when logging unknown execution 
status.
..


Patch Set 1:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I79b223e22096e2d964b72654a34dc0d88e8c6422
Gerrit-Change-Number: 12102
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Tue, 18 Dec 2018 00:38:21 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7183: Include remote host when logging unknown execution status.

2018-12-17 Thread Thomas Marshall (Code Review)
Thomas Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12102 )

Change subject: IMPALA-7183: Include remote host when logging unknown execution 
status.
..


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/12102/1/be/src/service/control-service.cc
File be/src/service/control-service.cc:

http://gerrit.cloudera.org:8080/#/c/12102/1/be/src/service/control-service.cc@117
PS1, Line 117: LookupHostname
I'm concerned that this call may be relatively expensive as it performs a 
reverse DNS lookup - even if its only a few ms, that's a lot to pay for making 
a logged error slightly more convenient, especially since a user can generally 
perform the lookup themselves if they see this error and are concerned about it.

As the comment above says, this is expected to only happen occasionally, but 
you can imagine a scenario where this starts happening frequently due to 
excessive load on the cluster, and the added work here exacerbates the problem.


http://gerrit.cloudera.org:8080/#/c/12102/1/be/src/service/control-service.cc@120
PS1, Line 120: remote_host_name = rpc_context->remote_address().host();
Probably worth including the port too, i.e. by calling 
remote_address().ToString()



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I79b223e22096e2d964b72654a34dc0d88e8c6422
Gerrit-Change-Number: 12102
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Tue, 18 Dec 2018 00:41:38 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7183: Include remote host when logging unknown execution status.

2018-12-18 Thread Andrew Sherman (Code Review)
Andrew Sherman has uploaded a new patch set (#2). ( 
http://gerrit.cloudera.org:8080/12102 )

Change subject: IMPALA-7183: Include remote host when logging unknown execution 
status.
..

IMPALA-7183: Include remote host when logging unknown execution status.

ControlService::ReportExecStatus reports the execution status of
a fragment to the coordinator. When the coordinator receives a
ReportExecStatus call, it looks up the ClientRequestState for the query
id. If this cannot be found, usually because the query is in the process
of being canceled, then it logs a message before responding to the
RPC. Add the IP address and port of the remote host to this message.

Example output (from minicluster): "ReportExecStatus(): Received report
for unknown query ID (probably closed or cancelled):
174dbb16aa00a5c2:382e5cc8 remote host=127.0.0.1:47112"

TESTING
 Ran end-to-end tests.
 Forced code to execute by hand to check output is correct.

Change-Id: I79b223e22096e2d964b72654a34dc0d88e8c6422
---
M be/src/service/control-service.cc
1 file changed, 3 insertions(+), 1 deletion(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I79b223e22096e2d964b72654a34dc0d88e8c6422
Gerrit-Change-Number: 12102
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 


[Impala-ASF-CR] IMPALA-7183: Include remote host when logging unknown execution status.

2018-12-18 Thread Thomas Marshall (Code Review)
Thomas Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12102 )

Change subject: IMPALA-7183: Include remote host when logging unknown execution 
status.
..


Patch Set 2: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I79b223e22096e2d964b72654a34dc0d88e8c6422
Gerrit-Change-Number: 12102
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Tue, 18 Dec 2018 18:55:20 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7183: Include remote host when logging unknown execution status.

2018-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12102 )

Change subject: IMPALA-7183: Include remote host when logging unknown execution 
status.
..


Patch Set 3:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I79b223e22096e2d964b72654a34dc0d88e8c6422
Gerrit-Change-Number: 12102
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Tue, 18 Dec 2018 18:55:50 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7183: Include remote host when logging unknown execution status.

2018-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12102 )

Change subject: IMPALA-7183: Include remote host when logging unknown execution 
status.
..


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I79b223e22096e2d964b72654a34dc0d88e8c6422
Gerrit-Change-Number: 12102
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Tue, 18 Dec 2018 18:55:49 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7183: Include remote host when logging unknown execution status.

2018-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12102 )

Change subject: IMPALA-7183: Include remote host when logging unknown execution 
status.
..


Patch Set 2:

Build Successful

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I79b223e22096e2d964b72654a34dc0d88e8c6422
Gerrit-Change-Number: 12102
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Tue, 18 Dec 2018 19:24:22 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7183: Include remote host when logging unknown execution status.

2018-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/12102 )

Change subject: IMPALA-7183: Include remote host when logging unknown execution 
status.
..

IMPALA-7183: Include remote host when logging unknown execution status.

ControlService::ReportExecStatus reports the execution status of
a fragment to the coordinator. When the coordinator receives a
ReportExecStatus call, it looks up the ClientRequestState for the query
id. If this cannot be found, usually because the query is in the process
of being canceled, then it logs a message before responding to the
RPC. Add the IP address and port of the remote host to this message.

Example output (from minicluster): "ReportExecStatus(): Received report
for unknown query ID (probably closed or cancelled):
174dbb16aa00a5c2:382e5cc8 remote host=127.0.0.1:47112"

TESTING
 Ran end-to-end tests.
 Forced code to execute by hand to check output is correct.

Change-Id: I79b223e22096e2d964b72654a34dc0d88e8c6422
Reviewed-on: http://gerrit.cloudera.org:8080/12102
Reviewed-by: Impala Public Jenkins 
Tested-by: Impala Public Jenkins 
---
M be/src/service/control-service.cc
1 file changed, 3 insertions(+), 1 deletion(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I79b223e22096e2d964b72654a34dc0d88e8c6422
Gerrit-Change-Number: 12102
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 


[Impala-ASF-CR] IMPALA-7183: Include remote host when logging unknown execution status.

2018-12-18 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/12102 )

Change subject: IMPALA-7183: Include remote host when logging unknown execution 
status.
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I79b223e22096e2d964b72654a34dc0d88e8c6422
Gerrit-Change-Number: 12102
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Thomas Marshall 
Gerrit-Comment-Date: Tue, 18 Dec 2018 23:01:09 +
Gerrit-HasComments: No