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 2020/09/19 01:22:36 UTC
[kudu-CR] [server] --rpc default keepalive time ms is not runtime
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16476
Change subject: [server] --rpc_default_keepalive_time_ms is not runtime
......................................................................
[server] --rpc_default_keepalive_time_ms is not runtime
The --rpc_default_keepalive_time_ms flag is not runtime: the messenger
for a Kudu server created in ServerBase::Init() using current value of
the flag and the parameter cannot be changed later on.
Change-Id: Id79bb22d7ce424a37b0a61b75c054906e7de0d1c
---
M src/kudu/server/server_base.cc
1 file changed, 0 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/76/16476/1
--
To view, visit http://gerrit.cloudera.org:8080/16476
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id79bb22d7ce424a37b0a61b75c054906e7de0d1c
Gerrit-Change-Number: 16476
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] [server] --rpc default keepalive time ms is not runtime
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16476 )
Change subject: [server] --rpc_default_keepalive_time_ms is not runtime
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16476/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16476/1//COMMIT_MSG@9
PS1, Line 9: --rpc_default_keepalive_time_ms flag is not runtime
> Yes, I think it's worth fixing the inaccuracy. This flag is not run-time f
What I mean is that it used to be runtime by the behavior. Instead of marking the flag as no-longer runtime should we fix the behavior? Is it useful to be runtime?
--
To view, visit http://gerrit.cloudera.org:8080/16476
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id79bb22d7ce424a37b0a61b75c054906e7de0d1c
Gerrit-Change-Number: 16476
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 21 Sep 2020 17:05:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] [server] --rpc default keepalive time ms is not runtime
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16476 )
Change subject: [server] --rpc_default_keepalive_time_ms is not runtime
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16476
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id79bb22d7ce424a37b0a61b75c054906e7de0d1c
Gerrit-Change-Number: 16476
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 22 Sep 2020 16:07:44 +0000
Gerrit-HasComments: No
[kudu-CR] [server] --rpc default keepalive time ms is not runtime
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16476 )
Change subject: [server] --rpc_default_keepalive_time_ms is not runtime
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16476/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16476/1//COMMIT_MSG@9
PS1, Line 9: --rpc_default_keepalive_time_ms flag is not runtime
> What I mean is that it used to be runtime by the behavior. Instead of marki
Yep, it wouldn't hurt to make this flag being run-time.
I think fixing the behavior of this flag back to run-time would be a separate review item, here I just want to fix the declaration so the users of kudu CLI and the readers of the code wouldn't be confused.
BTW, this is the first time I took closer look at this in a couple years :)
--
To view, visit http://gerrit.cloudera.org:8080/16476
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id79bb22d7ce424a37b0a61b75c054906e7de0d1c
Gerrit-Change-Number: 16476
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 21 Sep 2020 18:46:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] [server] --rpc default keepalive time ms is not runtime
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16476 )
Change subject: [server] --rpc_default_keepalive_time_ms is not runtime
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16476/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16476/1//COMMIT_MSG@9
PS1, Line 9: --rpc_default_keepalive_time_ms flag is not runtime
It looks like it was runtime before migrated to server_base in https://github.com/apache/kudu/commit/b357fa9b729ce52627a862e08ebac822ae470bc9
Do you think we should fix that behavior change/regression?
--
To view, visit http://gerrit.cloudera.org:8080/16476
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id79bb22d7ce424a37b0a61b75c054906e7de0d1c
Gerrit-Change-Number: 16476
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 21 Sep 2020 13:12:50 +0000
Gerrit-HasComments: Yes
[kudu-CR] [server] --rpc default keepalive time ms is not runtime
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16476 )
Change subject: [server] --rpc_default_keepalive_time_ms is not runtime
......................................................................
[server] --rpc_default_keepalive_time_ms is not runtime
The --rpc_default_keepalive_time_ms flag is not runtime: the messenger
for a Kudu server created in ServerBase::Init() using current value of
the flag and the parameter cannot be changed later on.
Change-Id: Id79bb22d7ce424a37b0a61b75c054906e7de0d1c
Reviewed-on: http://gerrit.cloudera.org:8080/16476
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke <gr...@apache.org>
---
M src/kudu/server/server_base.cc
1 file changed, 0 insertions(+), 1 deletion(-)
Approvals:
Kudu Jenkins: Verified
Grant Henke: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/16476
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id79bb22d7ce424a37b0a61b75c054906e7de0d1c
Gerrit-Change-Number: 16476
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [server] --rpc default keepalive time ms is not runtime
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16476 )
Change subject: [server] --rpc_default_keepalive_time_ms is not runtime
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16476/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16476/1//COMMIT_MSG@9
PS1, Line 9: --rpc_default_keepalive_time_ms flag is not runtime
> It looks like it was runtime before migrated to server_base in https://gith
Yes, I think it's worth fixing the inaccuracy. This flag is not run-time flag by the behavior de facto.
--
To view, visit http://gerrit.cloudera.org:8080/16476
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id79bb22d7ce424a37b0a61b75c054906e7de0d1c
Gerrit-Change-Number: 16476
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 21 Sep 2020 17:01:23 +0000
Gerrit-HasComments: Yes