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/07/02 18:40:24 UTC
[Impala-ASF-CR] IMPALA-8543: More diagnostics for TAcceptQueueServer
Michael Ho has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13790
Change subject: IMPALA-8543: More diagnostics for TAcceptQueueServer
......................................................................
IMPALA-8543: More diagnostics for TAcceptQueueServer
This change adds more logging information in TAcceptQueueServer
to help diagnose issues at various stages of client connections
establishment.
Two new metrics are also added to measure the connection setup time
and the wait time for service threads to be available.
Change-Id: I33b32352b457a2c8ec7bae6da46bb9c555dc9c36
---
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M common/thrift/metrics.json
3 files changed, 180 insertions(+), 22 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/13790/1
--
To view, visit http://gerrit.cloudera.org:8080/13790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I33b32352b457a2c8ec7bae6da46bb9c555dc9c36
Gerrit-Change-Number: 13790
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
[Impala-ASF-CR] IMPALA-8543: More diagnostics for TAcceptQueueServer
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13790 )
Change subject: IMPALA-8543: More diagnostics for TAcceptQueueServer
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I33b32352b457a2c8ec7bae6da46bb9c555dc9c36
Gerrit-Change-Number: 13790
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: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jul 2019 00:37:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8543: More diagnostics for TAcceptQueueServer
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13790 )
Change subject: IMPALA-8543: More diagnostics for TAcceptQueueServer
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/13790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I33b32352b457a2c8ec7bae6da46bb9c555dc9c36
Gerrit-Change-Number: 13790
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: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jul 2019 06:55:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8543: More diagnostics for TAcceptQueueServer
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13790 )
Change subject: IMPALA-8543: More diagnostics for TAcceptQueueServer
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I33b32352b457a2c8ec7bae6da46bb9c555dc9c36
Gerrit-Change-Number: 13790
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: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Jul 2019 17:45:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8543: More diagnostics for TAcceptQueueServer
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/13790 )
Change subject: IMPALA-8543: More diagnostics for TAcceptQueueServer
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13790/1/be/src/rpc/TAcceptQueueServer.cpp
File be/src/rpc/TAcceptQueueServer.cpp:
http://gerrit.cloudera.org:8080/#/c/13790/1/be/src/rpc/TAcceptQueueServer.cpp@255
PS1, Line 255: // Insert thre
> Maybe make this two separate variables, eg. setup_timer and thread_wait_tim
Done
http://gerrit.cloudera.org:8080/#/c/13790/1/be/src/rpc/TAcceptQueueServer.cpp@293
PS1, Line 293: ient $
> Can this one also be made const?
Done
http://gerrit.cloudera.org:8080/#/c/13790/1/be/src/rpc/TAcceptQueueServer.cpp@406
PS1, Line 406:
> maybe label these, eg: /* highest_trackable_value */
Done
--
To view, visit http://gerrit.cloudera.org:8080/13790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I33b32352b457a2c8ec7bae6da46bb9c555dc9c36
Gerrit-Change-Number: 13790
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: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Jul 2019 01:26:50 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8543: More diagnostics for TAcceptQueueServer
Posted by "Michael Ho (Code Review)" <ge...@cloudera.org>.
Michael Ho has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/13790 )
Change subject: IMPALA-8543: More diagnostics for TAcceptQueueServer
......................................................................
IMPALA-8543: More diagnostics for TAcceptQueueServer
This change adds more logging information in TAcceptQueueServer
to help diagnose issues at various stages of client connections
establishment.
Two new metrics are also added to measure the connection setup time
and the wait time for service threads to be available.
Change-Id: I33b32352b457a2c8ec7bae6da46bb9c555dc9c36
---
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M common/thrift/metrics.json
3 files changed, 189 insertions(+), 28 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/90/13790/2
--
To view, visit http://gerrit.cloudera.org:8080/13790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I33b32352b457a2c8ec7bae6da46bb9c555dc9c36
Gerrit-Change-Number: 13790
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-8543: More diagnostics for TAcceptQueueServer
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13790 )
Change subject: IMPALA-8543: More diagnostics for TAcceptQueueServer
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3805/ : 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/13790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I33b32352b457a2c8ec7bae6da46bb9c555dc9c36
Gerrit-Change-Number: 13790
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 Jul 2019 19:19:54 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8543: More diagnostics for TAcceptQueueServer
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13790 )
Change subject: IMPALA-8543: More diagnostics for TAcceptQueueServer
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13790/1/be/src/rpc/TAcceptQueueServer.h
File be/src/rpc/TAcceptQueueServer.h:
http://gerrit.cloudera.org:8080/#/c/13790/1/be/src/rpc/TAcceptQueueServer.h@119
PS1, Line 119: cnxns_setup_time_metric_
cnxns_setup_time_us_metric_ or note the units in the comment
http://gerrit.cloudera.org:8080/#/c/13790/1/be/src/rpc/TAcceptQueueServer.cpp
File be/src/rpc/TAcceptQueueServer.cpp:
http://gerrit.cloudera.org:8080/#/c/13790/1/be/src/rpc/TAcceptQueueServer.cpp@255
PS1, Line 255: timer.Reset();
Maybe make this two separate variables, eg. setup_timer and thread_wait_timer, or at least leave a comment here and at the call to Start() eg:
// Start timing for connection setup
// Start timing for getting the service thread
http://gerrit.cloudera.org:8080/#/c/13790/1/be/src/rpc/TAcceptQueueServer.cpp@293
PS1, Line 293: string
Can this one also be made const?
http://gerrit.cloudera.org:8080/#/c/13790/1/common/thrift/metrics.json
File common/thrift/metrics.json:
http://gerrit.cloudera.org:8080/#/c/13790/1/common/thrift/metrics.json@955
PS1, Line 955: connection
client connection? here and elsewhere
--
To view, visit http://gerrit.cloudera.org:8080/13790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I33b32352b457a2c8ec7bae6da46bb9c555dc9c36
Gerrit-Change-Number: 13790
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 08 Jul 2019 19:57:08 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8543: More diagnostics for TAcceptQueueServer
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13790 )
Change subject: IMPALA-8543: More diagnostics for TAcceptQueueServer
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3851/ : 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/13790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I33b32352b457a2c8ec7bae6da46bb9c555dc9c36
Gerrit-Change-Number: 13790
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: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 10 Jul 2019 02:07:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8543: More diagnostics for TAcceptQueueServer
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13790 )
Change subject: IMPALA-8543: More diagnostics for TAcceptQueueServer
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13790/1/be/src/rpc/TAcceptQueueServer.cpp
File be/src/rpc/TAcceptQueueServer.cpp:
http://gerrit.cloudera.org:8080/#/c/13790/1/be/src/rpc/TAcceptQueueServer.cpp@406
PS1, Line 406: 5 * 60 * MICROS_PER_SEC, 1
maybe label these, eg: /* highest_trackable_value */
http://gerrit.cloudera.org:8080/#/c/13790/1/common/thrift/metrics.json
File common/thrift/metrics.json:
http://gerrit.cloudera.org:8080/#/c/13790/1/common/thrift/metrics.json@955
PS1, Line 955: connection
> client connection? here and elsewhere
In fact, rather than having the label always match the description on these, it might be helpful to be a little more explicit, eg:
"Stats on the amount of time client connections to the Beeswax API spend waiting for the connection to be setup" or similar
--
To view, visit http://gerrit.cloudera.org:8080/13790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I33b32352b457a2c8ec7bae6da46bb9c555dc9c36
Gerrit-Change-Number: 13790
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 09 Jul 2019 00:18:24 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8543: More diagnostics for TAcceptQueueServer
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/13790 )
Change subject: IMPALA-8543: More diagnostics for TAcceptQueueServer
......................................................................
IMPALA-8543: More diagnostics for TAcceptQueueServer
This change adds more logging information in TAcceptQueueServer
to help diagnose issues at various stages of client connections
establishment.
Two new metrics are also added to measure the connection setup time
and the wait time for service threads to be available.
Change-Id: I33b32352b457a2c8ec7bae6da46bb9c555dc9c36
Reviewed-on: http://gerrit.cloudera.org:8080/13790
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/rpc/TAcceptQueueServer.cpp
M be/src/rpc/TAcceptQueueServer.h
M common/thrift/metrics.json
3 files changed, 189 insertions(+), 28 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/13790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I33b32352b457a2c8ec7bae6da46bb9c555dc9c36
Gerrit-Change-Number: 13790
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: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-8543: More diagnostics for TAcceptQueueServer
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13790 )
Change subject: IMPALA-8543: More diagnostics for TAcceptQueueServer
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4594/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/13790
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I33b32352b457a2c8ec7bae6da46bb9c555dc9c36
Gerrit-Change-Number: 13790
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: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 11 Jul 2019 00:37:02 +0000
Gerrit-HasComments: No