You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2022/10/19 00:37:57 UTC
[kudu-CR] [tserver] validate scanner TTL vs RPC connection timeout
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19152
Change subject: [tserver] validate scanner TTL vs RPC connection timeout
......................................................................
[tserver] validate scanner TTL vs RPC connection timeout
This patch adds a group validator for --scanner_ttl_ms and
--rpc_default_keepalive_time_ms flags. The validator outputs a warning
(not an error though) if the TTL for an idle scanner is greater than the
timeout for an idle RPC connection.
Even if an idle scanner is kept alive at the server side for some time,
Kudu servers periodically close connections that have been idle at least
for --rpc_default_keepalive_time_ms time interval. So, setting the
--rpc_default_keepalive_time_ms flag to a greater or equal value than
--scanner_ttl_ms helps keeping yet-to-be-used connections to idle
scanners open, avoiding inadvertent closure and re-opening connections
to scanners that might yet be sent continuation scan requests. The new
constraint also helps to work around one particular bug [1] in the Kudu
Java client.
I didn't add a test for the newly added validator, but I manually
verified that the warning is output as expected when necessary.
[1] https://issues.apache.org/jira/browse/KUDU-3169
Change-Id: If1439dfb6eb82ba2be0472547b04e5a692879535
---
M src/kudu/tserver/scanners.cc
1 file changed, 28 insertions(+), 6 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/19152/1
--
To view, visit http://gerrit.cloudera.org:8080/19152
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If1439dfb6eb82ba2be0472547b04e5a692879535
Gerrit-Change-Number: 19152
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
[kudu-CR] [tserver] validate scanner TTL vs RPC connection timeout
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19152 )
Change subject: [tserver] validate scanner TTL vs RPC connection timeout
......................................................................
Patch Set 1: Verified+1
unrelated test failure in Java test (TSAN)
--
To view, visit http://gerrit.cloudera.org:8080/19152
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If1439dfb6eb82ba2be0472547b04e5a692879535
Gerrit-Change-Number: 19152
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 19 Oct 2022 18:41:52 +0000
Gerrit-HasComments: No
[kudu-CR] [tserver] validate scanner TTL vs RPC connection timeout
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19152 )
Change subject: [tserver] validate scanner TTL vs RPC connection timeout
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19152
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If1439dfb6eb82ba2be0472547b04e5a692879535
Gerrit-Change-Number: 19152
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 22 Oct 2022 10:34:22 +0000
Gerrit-HasComments: No
[kudu-CR] [tserver] validate scanner TTL vs RPC connection timeout
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19152 )
Change subject: [tserver] validate scanner TTL vs RPC connection timeout
......................................................................
Patch Set 1: Code-Review+1
LGTM
--
To view, visit http://gerrit.cloudera.org:8080/19152
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If1439dfb6eb82ba2be0472547b04e5a692879535
Gerrit-Change-Number: 19152
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Fri, 21 Oct 2022 04:02:32 +0000
Gerrit-HasComments: No
[kudu-CR] [tserver] validate scanner TTL vs RPC connection timeout
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: [tserver] validate scanner TTL vs RPC connection timeout
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/19152
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: If1439dfb6eb82ba2be0472547b04e5a692879535
Gerrit-Change-Number: 19152
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [tserver] validate scanner TTL vs RPC connection timeout
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19152 )
Change subject: [tserver] validate scanner TTL vs RPC connection timeout
......................................................................
Patch Set 1:
> My immature ideas extends by this patch:
>
> 1. In java client, 'keepAlivePeriodMs' should be set less than
> min(--rpc_default_keepalive_time_ms, --scan_ttl_ms), I think
> sending at least twice keepalive requests is better during
> min(--rpc_default_keepalive_time_ms, --scan_ttl_ms).
> 2. Java client and kudu server's some parameters should match.
> Maybe kudu masters can keep some key parameters such as
> 'rpc_default_keepalive_time_ms', client get them and adjust its
> local parameters.
This patch is an attempt to make the parameters of Kudu tablet servers to be more consistent, so RPC connections of idle scanners aren't closed inadvertently by the server side.
As for the consistency between the client and the server side configuration, there isn't much can be done programmatically unless the client side has enough credentials to call GetFlags(). However, we cannot assume that's the case because GetFlags() require super-user/admin credentials.
--
To view, visit http://gerrit.cloudera.org:8080/19152
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If1439dfb6eb82ba2be0472547b04e5a692879535
Gerrit-Change-Number: 19152
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Sat, 22 Oct 2022 15:54:05 +0000
Gerrit-HasComments: No
[kudu-CR] [tserver] validate scanner TTL vs RPC connection timeout
Posted by "Yuqi Du (Code Review)" <ge...@cloudera.org>.
Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/19152 )
Change subject: [tserver] validate scanner TTL vs RPC connection timeout
......................................................................
Patch Set 1:
My immature ideas extends by this patch:
1. In java client, 'keepAlivePeriodMs' should be set less than min(--rpc_default_keepalive_time_ms, --scan_ttl_ms), I think sending at least twice keepalive requests is better during min(--rpc_default_keepalive_time_ms, --scan_ttl_ms).
2. Java client and kudu server's some parameters should match. Maybe kudu masters can keep some key parameters such as 'rpc_default_keepalive_time_ms', client get them and adjust its local parameters.
--
To view, visit http://gerrit.cloudera.org:8080/19152
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If1439dfb6eb82ba2be0472547b04e5a692879535
Gerrit-Change-Number: 19152
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>
Gerrit-Comment-Date: Fri, 21 Oct 2022 04:15:27 +0000
Gerrit-HasComments: No
[kudu-CR] [tserver] validate scanner TTL vs RPC connection timeout
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19152 )
Change subject: [tserver] validate scanner TTL vs RPC connection timeout
......................................................................
[tserver] validate scanner TTL vs RPC connection timeout
This patch adds a group validator for --scanner_ttl_ms and
--rpc_default_keepalive_time_ms flags. The validator outputs a warning
(not an error though) if the TTL for an idle scanner is greater than the
timeout for an idle RPC connection.
Even if an idle scanner is kept alive at the server side for some time,
Kudu servers periodically close connections that have been idle at least
for --rpc_default_keepalive_time_ms time interval. So, setting the
--rpc_default_keepalive_time_ms flag to a greater or equal value than
--scanner_ttl_ms helps keeping yet-to-be-used connections to idle
scanners open, avoiding inadvertent closure and re-opening connections
to scanners that might yet be sent continuation scan requests. The new
constraint also helps to work around one particular bug [1] in the Kudu
Java client.
I didn't add a test for the newly added validator, but I manually
verified that the warning is output as expected when necessary.
[1] https://issues.apache.org/jira/browse/KUDU-3169
Change-Id: If1439dfb6eb82ba2be0472547b04e5a692879535
Reviewed-on: http://gerrit.cloudera.org:8080/19152
Tested-by: Alexey Serbin <al...@apache.org>
Reviewed-by: Yuqi Du <sh...@gmail.com>
Reviewed-by: Yingchun Lai <ac...@gmail.com>
---
M src/kudu/tserver/scanners.cc
1 file changed, 28 insertions(+), 6 deletions(-)
Approvals:
Alexey Serbin: Verified
Yuqi Du: Looks good to me, but someone else must approve
Yingchun Lai: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/19152
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: If1439dfb6eb82ba2be0472547b04e5a692879535
Gerrit-Change-Number: 19152
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Yuqi Du <sh...@gmail.com>