You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Anonymous Coward (Code Review)" <ge...@cloudera.org> on 2021/10/22 18:21:54 UTC
[kudu-CR] KUDU-3308: Add health check endpoint to webserver
zchovan@cloudera.com has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17962
Change subject: KUDU-3308: Add health check endpoint to webserver
......................................................................
KUDU-3308: Add health check endpoint to webserver
[server] This change adds a '/healthz' endpoint to the kudu-master and
tablet-server webinterfaces.
This endpoint only returns the HTTPStatus 200 and an 'OK' message, for
liveliness probe usage. As the Webserver only starts after the RPC
Server has been started there is no need to add an initial wait time for
the first check.
Change-Id: Ic735e210d95aeec1dfd1335ee972318f12d52475
---
M src/kudu/server/default_path_handlers.cc
1 file changed, 9 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/17962/1
--
To view, visit http://gerrit.cloudera.org:8080/17962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic735e210d95aeec1dfd1335ee972318f12d52475
Gerrit-Change-Number: 17962
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <zc...@cloudera.com>
[kudu-CR] KUDU-3308: Add health check endpoint to webserver
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17962 )
Change subject: KUDU-3308: Add health check endpoint to webserver
......................................................................
Patch Set 1:
Could you add how did you test this change in the commit description?
--
To view, visit http://gerrit.cloudera.org:8080/17962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic735e210d95aeec1dfd1335ee972318f12d52475
Gerrit-Change-Number: 17962
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 27 Oct 2021 19:39:40 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3308: Add health check endpoint to webserver
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17962 )
Change subject: KUDU-3308: Add health check endpoint to webserver
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic735e210d95aeec1dfd1335ee972318f12d52475
Gerrit-Change-Number: 17962
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 28 Oct 2021 22:32:49 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3308: Add health check endpoint to webserver
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17962 )
Change subject: KUDU-3308: Add health check endpoint to webserver
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/17962/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17962/1//COMMIT_MSG@9
PS1, Line 9: This change adds a '/healthz' endpoint to the kudu-master and
: tablet-server webinterfaces.
It would be great if you added a small test for the newly introduced end-point. I guess WebserverTest.TestDefaultPaths is a good place (or a good reference) for such a new test scenario.
http://gerrit.cloudera.org:8080/#/c/17962/1//COMMIT_MSG@12
PS1, Line 12: As the Webserver only starts after the RPC
: Server has been started there is no need to add an initial wait time for
: the first check
I guess this is no longer true since https://github.com/apache/kudu/commit/bd5f72e0ed03b614d49b8125160bf555d11dcf9c is committed.
http://gerrit.cloudera.org:8080/#/c/17962/1/src/kudu/server/default_path_handlers.cc
File src/kudu/server/default_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/17962/1/src/kudu/server/default_path_handlers.cc@452
PS1, Line 452: healthz
Any specific consideration on using 'healthz' instead of just 'health'? From my experience, usually that's '/health' endpoint.
--
To view, visit http://gerrit.cloudera.org:8080/17962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic735e210d95aeec1dfd1335ee972318f12d52475
Gerrit-Change-Number: 17962
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 27 Oct 2021 20:28:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3308: Add health check endpoint to webserver
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17962 )
Change subject: KUDU-3308: Add health check endpoint to webserver
......................................................................
Patch Set 1: Code-Review+1
Is this API easily extendable to add a request param to return health like ksck output?
--
To view, visit http://gerrit.cloudera.org:8080/17962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic735e210d95aeec1dfd1335ee972318f12d52475
Gerrit-Change-Number: 17962
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 25 Oct 2021 17:23:42 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3308: Add health check endpoint to webserver
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
zchovan@cloudera.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/17962 )
Change subject: KUDU-3308: Add health check endpoint to webserver
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/17962/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17962/1//COMMIT_MSG@9
PS1, Line 9: This change adds a '/healthz' endpoint to the kudu-master and
: tablet-server webinterfaces.
> It would be great if you added a small test for the newly introduced end-po
you're right, I missed that this test exists, I'll amend it
http://gerrit.cloudera.org:8080/#/c/17962/1//COMMIT_MSG@12
PS1, Line 12: As the Webserver only starts after the RPC
: Server has been started there is no need to add an initial wait time for
: the first check
> I guess this is no longer true since https://github.com/apache/kudu/commit/
Yes, based on the commit msg there, moving it to "AddPostInitializedDefaultPathHandlers" should ensure that the RPC server is up and running.
http://gerrit.cloudera.org:8080/#/c/17962/1/src/kudu/server/default_path_handlers.cc
File src/kudu/server/default_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/17962/1/src/kudu/server/default_path_handlers.cc@452
PS1, Line 452: is_styl
> Kubernetes normally uses /healthz. Also, we have /varz and /memz too.
What Attila said, and also the jira called for /healthz, but I'm fine with /health too.
--
To view, visit http://gerrit.cloudera.org:8080/17962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic735e210d95aeec1dfd1335ee972318f12d52475
Gerrit-Change-Number: 17962
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 28 Oct 2021 13:51:19 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3308: Add health check endpoint to webserver
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17962 )
Change subject: KUDU-3308: Add health check endpoint to webserver
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic735e210d95aeec1dfd1335ee972318f12d52475
Gerrit-Change-Number: 17962
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 27 Oct 2021 19:39:03 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3308: Add health check endpoint to webserver
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17962 )
Change subject: KUDU-3308: Add health check endpoint to webserver
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17962/1/src/kudu/server/default_path_handlers.cc
File src/kudu/server/default_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/17962/1/src/kudu/server/default_path_handlers.cc@452
PS1, Line 452: healthz
> Any specific consideration on using 'healthz' instead of just 'health'? Fr
Kubernetes normally uses /healthz. Also, we have /varz and /memz too.
--
To view, visit http://gerrit.cloudera.org:8080/17962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic735e210d95aeec1dfd1335ee972318f12d52475
Gerrit-Change-Number: 17962
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 28 Oct 2021 10:24:07 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3308: Add health check endpoint to webserver
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Bankim Bhavsar,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17962
to look at the new patch set (#2).
Change subject: KUDU-3308: Add health check endpoint to webserver
......................................................................
KUDU-3308: Add health check endpoint to webserver
[server] This change adds a '/healthz' endpoint to the kudu-master and
tablet-server webinterfaces.
This endpoint only returns the HTTPStatus 200 and an 'OK' message, for
liveliness probe usage. As the Webserver only starts after the RPC
Server has been started there is no need to add an initial wait time for
the first check.
Change-Id: Ic735e210d95aeec1dfd1335ee972318f12d52475
---
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/webserver-test.cc
2 files changed, 13 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/62/17962/2
--
To view, visit http://gerrit.cloudera.org:8080/17962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic735e210d95aeec1dfd1335ee972318f12d52475
Gerrit-Change-Number: 17962
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3308: Add health check endpoint to webserver
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17962 )
Change subject: KUDU-3308: Add health check endpoint to webserver
......................................................................
Patch Set 2: Code-Review+2
(2 comments)
http://gerrit.cloudera.org:8080/#/c/17962/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17962/1//COMMIT_MSG@12
PS1, Line 12: As the Webserver only starts after the RPC
: Server has been started there is no need to add an initial wait time for
: the first check
> Yes, based on the commit msg there, moving it to "AddPostInitializedDefault
Makes sense, indeed.
http://gerrit.cloudera.org:8080/#/c/17962/1/src/kudu/server/default_path_handlers.cc
File src/kudu/server/default_path_handlers.cc:
http://gerrit.cloudera.org:8080/#/c/17962/1/src/kudu/server/default_path_handlers.cc@452
PS1, Line 452: is_styl
> What Attila said, and also the jira called for /healthz, but I'm fine with
I'm OK with both as well, actually.
I guess using those 'z' suffixes follows google's conventions: https://stackoverflow.com/questions/43380939/where-does-the-convention-of-using-healthz-for-application-health-checks-come-f
I used alternative to etcd solution: consul; some other non-google things used simply /health endpoints as well.
Speaking to k8s, would it make to switch from /healthz to /livez /readyz instead (not necessarily in this changelist, but maybe later on): https://kubernetes.io/docs/reference/using-api/health-checks/ ?
That might be inline with the recent development that Abhishek in that area. Just as an extra note.
--
To view, visit http://gerrit.cloudera.org:8080/17962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic735e210d95aeec1dfd1335ee972318f12d52475
Gerrit-Change-Number: 17962
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 28 Oct 2021 19:18:33 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3308: Add health check endpoint to webserver
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17962 )
Change subject: KUDU-3308: Add health check endpoint to webserver
......................................................................
KUDU-3308: Add health check endpoint to webserver
[server] This change adds a '/healthz' endpoint to the kudu-master and
tablet-server webinterfaces.
This endpoint only returns the HTTPStatus 200 and an 'OK' message, for
liveliness probe usage. As the Webserver only starts after the RPC
Server has been started there is no need to add an initial wait time for
the first check.
Change-Id: Ic735e210d95aeec1dfd1335ee972318f12d52475
Reviewed-on: http://gerrit.cloudera.org:8080/17962
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Bankim Bhavsar <ba...@cloudera.com>
---
M src/kudu/server/default_path_handlers.cc
M src/kudu/server/webserver-test.cc
2 files changed, 13 insertions(+), 0 deletions(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
Bankim Bhavsar: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/17962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic735e210d95aeec1dfd1335ee972318f12d52475
Gerrit-Change-Number: 17962
Gerrit-PatchSet: 3
Gerrit-Owner: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3308: Add health check endpoint to webserver
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
zchovan@cloudera.com has posted comments on this change. ( http://gerrit.cloudera.org:8080/17962 )
Change subject: KUDU-3308: Add health check endpoint to webserver
......................................................................
Patch Set 1:
> Patch Set 1: Code-Review+1
>
> Is this API easily extendable to add a request param to return health like ksck output?
It should be possible to pass additional arguments via HTTP Get params and change the output according to that. However that might prove limited use for HTTP request based Liveness Probe purposes. From the kubernetes docs:
"Any code greater than or equal to 200 and less than 400 indicates success. Any other code indicates failure."
If we need a more comprehensive check for liveness probe, then most likely a command based Liveness Probe.
--
To view, visit http://gerrit.cloudera.org:8080/17962
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic735e210d95aeec1dfd1335ee972318f12d52475
Gerrit-Change-Number: 17962
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward <zc...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 26 Oct 2021 18:09:43 +0000
Gerrit-HasComments: No