You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2019/09/09 18:31:13 UTC
[Impala-ASF-CR] IMPALA-8904: retry statestore RegisterSubscriber() RPC
Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14198
Change subject: IMPALA-8904: retry statestore RegisterSubscriber() RPC
......................................................................
IMPALA-8904: retry statestore RegisterSubscriber() RPC
Previously connection failures triggered a retry, but
failures on the actual RPC did not trigger a retry. This
change moves the retry loop to DoRpcWithRetry(), instead
of relying on the ClientCache to retry the connection.
Note that DoRpcWithRetry() for thrift was dead code since
most backend RPCs were ported to KRPC, but should still work.
Testing:
Added targeted test with debug action to inject error on first
subscribe RPC.
Change-Id: I5d4e6283b5ec83170a1d1d03075b3384a9f108b5
---
M be/src/runtime/client-cache.h
M be/src/statestore/statestore-subscriber.cc
A tests/custom_cluster/test_statestore_rpc_errors.py
3 files changed, 75 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/14198/1
--
To view, visit http://gerrit.cloudera.org:8080/14198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5d4e6283b5ec83170a1d1d03075b3384a9f108b5
Gerrit-Change-Number: 14198
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-8904: retry statestore RegisterSubscriber() RPC
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/14198 )
Change subject: IMPALA-8904: retry statestore RegisterSubscriber() RPC
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d4e6283b5ec83170a1d1d03075b3384a9f108b5
Gerrit-Change-Number: 14198
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Sep 2019 23:28:14 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8904: retry statestore RegisterSubscriber() RPC
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14198 )
Change subject: IMPALA-8904: retry statestore RegisterSubscriber() RPC
......................................................................
Patch Set 1:
Fixed the ntis
--
To view, visit http://gerrit.cloudera.org:8080/14198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d4e6283b5ec83170a1d1d03075b3384a9f108b5
Gerrit-Change-Number: 14198
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Sep 2019 19:05:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8904: retry statestore RegisterSubscriber() 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/14198 )
Change subject: IMPALA-8904: retry statestore RegisterSubscriber() RPC
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4507/ : 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/14198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d4e6283b5ec83170a1d1d03075b3384a9f108b5
Gerrit-Change-Number: 14198
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Sep 2019 19:46:44 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8904: retry statestore RegisterSubscriber() 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/14198 )
Change subject: IMPALA-8904: retry statestore RegisterSubscriber() RPC
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d4e6283b5ec83170a1d1d03075b3384a9f108b5
Gerrit-Change-Number: 14198
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Sep 2019 00:16:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8904: retry statestore RegisterSubscriber() 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/14198 )
Change subject: IMPALA-8904: retry statestore RegisterSubscriber() RPC
......................................................................
Patch Set 3: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/14198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d4e6283b5ec83170a1d1d03075b3384a9f108b5
Gerrit-Change-Number: 14198
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Sep 2019 04:21:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8904: retry statestore RegisterSubscriber() 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/14198 )
Change subject: IMPALA-8904: retry statestore RegisterSubscriber() RPC
......................................................................
Patch Set 3:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4914/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/14198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d4e6283b5ec83170a1d1d03075b3384a9f108b5
Gerrit-Change-Number: 14198
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Sep 2019 00:16:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8904: retry statestore RegisterSubscriber() 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/14198 )
Change subject: IMPALA-8904: retry statestore RegisterSubscriber() RPC
......................................................................
Patch Set 1: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/14198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d4e6283b5ec83170a1d1d03075b3384a9f108b5
Gerrit-Change-Number: 14198
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Sep 2019 22:38:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8904: retry statestore RegisterSubscriber() RPC
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Andrew Sherman, Lars Volker, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14198
to look at the new patch set (#2).
Change subject: IMPALA-8904: retry statestore RegisterSubscriber() RPC
......................................................................
IMPALA-8904: retry statestore RegisterSubscriber() RPC
Previously connection failures triggered a retry, but
failures on the actual RPC did not trigger a retry. This
change moves the retry loop to DoRpcWithRetry(), instead
of relying on the ClientCache to retry the connection.
Note that DoRpcWithRetry() for thrift was dead code since
most backend RPCs were ported to KRPC, but should still work.
Testing:
Added targeted test with debug action to inject error on first
subscribe RPC.
Change-Id: I5d4e6283b5ec83170a1d1d03075b3384a9f108b5
---
M be/src/runtime/client-cache.h
M be/src/statestore/statestore-subscriber.cc
A tests/custom_cluster/test_statestore_rpc_errors.py
3 files changed, 77 insertions(+), 16 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/14198/2
--
To view, visit http://gerrit.cloudera.org:8080/14198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5d4e6283b5ec83170a1d1d03075b3384a9f108b5
Gerrit-Change-Number: 14198
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
[Impala-ASF-CR] IMPALA-8904: retry statestore RegisterSubscriber() 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/14198 )
Change subject: IMPALA-8904: retry statestore RegisterSubscriber() RPC
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14198/1/be/src/runtime/client-cache.h
File be/src/runtime/client-cache.h:
http://gerrit.cloudera.org:8080/#/c/14198/1/be/src/runtime/client-cache.h@284
PS1, Line 284: const F& f, const Request& request, int retries, int64_t delay_ms, const DebugF& debug_fn,
line too long (96 > 90)
http://gerrit.cloudera.org:8080/#/c/14198/1/tests/custom_cluster/test_statestore_rpc_errors.py
File tests/custom_cluster/test_statestore_rpc_errors.py:
http://gerrit.cloudera.org:8080/#/c/14198/1/tests/custom_cluster/test_statestore_rpc_errors.py@21
PS1, Line 21: class TestStatestoreRpcErrors(CustomClusterTestSuite):
flake8: E302 expected 2 blank lines, found 1
http://gerrit.cloudera.org:8080/#/c/14198/1/tests/custom_cluster/test_statestore_rpc_errors.py@38
PS1, Line 38: U
flake8: E501 line too long (115 > 90 characters)
--
To view, visit http://gerrit.cloudera.org:8080/14198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d4e6283b5ec83170a1d1d03075b3384a9f108b5
Gerrit-Change-Number: 14198
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Sep 2019 18:31:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8904: retry statestore RegisterSubscriber() 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/14198 )
Change subject: IMPALA-8904: retry statestore RegisterSubscriber() RPC
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4506/ : 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/14198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d4e6283b5ec83170a1d1d03075b3384a9f108b5
Gerrit-Change-Number: 14198
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Sep 2019 19:11:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8904: retry statestore RegisterSubscriber() 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/14198 )
Change subject: IMPALA-8904: retry statestore RegisterSubscriber() RPC
......................................................................
IMPALA-8904: retry statestore RegisterSubscriber() RPC
Previously connection failures triggered a retry, but
failures on the actual RPC did not trigger a retry. This
change moves the retry loop to DoRpcWithRetry(), instead
of relying on the ClientCache to retry the connection.
Note that DoRpcWithRetry() for thrift was dead code since
most backend RPCs were ported to KRPC, but should still work.
Testing:
Added targeted test with debug action to inject error on first
subscribe RPC.
Change-Id: I5d4e6283b5ec83170a1d1d03075b3384a9f108b5
Reviewed-on: http://gerrit.cloudera.org:8080/14198
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/runtime/client-cache.h
M be/src/statestore/statestore-subscriber.cc
A tests/custom_cluster/test_statestore_rpc_errors.py
3 files changed, 77 insertions(+), 16 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/14198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I5d4e6283b5ec83170a1d1d03075b3384a9f108b5
Gerrit-Change-Number: 14198
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-8904: retry statestore RegisterSubscriber() 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/14198 )
Change subject: IMPALA-8904: retry statestore RegisterSubscriber() RPC
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4912/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/14198
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5d4e6283b5ec83170a1d1d03075b3384a9f108b5
Gerrit-Change-Number: 14198
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Mon, 09 Sep 2019 18:31:31 +0000
Gerrit-HasComments: No