You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org> on 2021/11/02 05:27:02 UTC
[kudu-CR] KUDU-1959 - Implement tests for the startup progress page for tablet servers
Abhishek Chennaka has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17990
Change subject: KUDU-1959 - Implement tests for the startup progress page for tablet servers
......................................................................
KUDU-1959 - Implement tests for the startup progress page for tablet servers
This patch implements the tests for the startup page using mini tablet
server.
- We inject latency to bootstrap tablets while reading the webpage every
10 milliseconds and validating the status for each step.
- Fail a data directory and validate the status of each startup step.
We also validate the below startup metrics in the above scenarios (
log_block_manager* metrics in the case of using log block manager):
- log_block_manager_total_containers_startup
- log_block_manager_processed_containers_startup
- log_block_manager_containers_processing_time_startup
- tablets_num_total_startup
- tablets_num_opened_startup
- tablets_opening_time_startup
Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
---
M src/kudu/mini-cluster/webui_checker.h
M src/kudu/tserver/tablet_server-test.cc
M src/kudu/util/thread.cc
3 files changed, 238 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/17990/1
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
[kudu-CR] KUDU-1959 - Add tests for /startup page and metrics for tservers
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17990 )
Change subject: KUDU-1959 - Add tests for /startup page and metrics for tservers
......................................................................
KUDU-1959 - Add tests for /startup page and metrics for tservers
This patch implements the tests for the startup page using mini tablet
server.
- We inject latency to bootstrap tablets while reading the webpage every
10 milliseconds and validating the status for each step.
- Fail a data directory and validate the status of each startup step.
We also validate the below startup metrics in the above scenarios
(log_block_manager* metrics in the case of using log block manager):
- log_block_manager_total_containers_startup
- log_block_manager_processed_containers_startup
- log_block_manager_containers_processing_time_startup
- tablets_num_total_startup
- tablets_num_opened_startup
- tablets_opening_time_startup
Additionally we also fix a race condition in the Kudu tablet server
WebUI. This race condition occurs if the tablet server is started while
the WebUI is continuously curled. The reason appears to be starting up
of the webserver before registering the path handlers as a part of
the change https://gerrit.cloudera.org/#/c/17730/
Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Reviewed-on: http://gerrit.cloudera.org:8080/17990
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Tested-by: Andrew Wong <aw...@cloudera.com>
---
M src/kudu/mini-cluster/webui_checker.h
M src/kudu/server/webserver.cc
M src/kudu/tserver/tablet_server-test.cc
3 files changed, 219 insertions(+), 4 deletions(-)
Approvals:
Andrew Wong: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 16
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1959 - Add tests for /startup page and metrics for tservers
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17990 )
Change subject: KUDU-1959 - Add tests for /startup page and metrics for tservers
......................................................................
Patch Set 11:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17990/11//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17990/11//COMMIT_MSG@14
PS11, Line 14: We also validate the below startup metrics in the above scenarios (
nit: add a new line to separate the two lists + parenthesis should be moved to next line
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 11
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 15 Nov 2021 18:30:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Implement tests for the startup progress page for tablet servers
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17990
to look at the new patch set (#3).
Change subject: KUDU-1959 - Implement tests for the startup progress page for tablet servers
......................................................................
KUDU-1959 - Implement tests for the startup progress page for tablet servers
This patch implements the tests for the startup page using mini tablet
server.
- We inject latency to bootstrap tablets while reading the webpage every
10 milliseconds and validating the status for each step.
- Fail a data directory and validate the status of each startup step.
We also validate the below startup metrics in the above scenarios (
log_block_manager* metrics in the case of using log block manager):
- log_block_manager_total_containers_startup
- log_block_manager_processed_containers_startup
- log_block_manager_containers_processing_time_startup
- tablets_num_total_startup
- tablets_num_opened_startup
- tablets_opening_time_startup
Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
---
M src/kudu/mini-cluster/webui_checker.h
M src/kudu/tserver/tablet_server-test.cc
2 files changed, 234 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/17990/3
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1959 - Add tests for /startup page and metrics for tservers
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17990
to look at the new patch set (#14).
Change subject: KUDU-1959 - Add tests for /startup page and metrics for tservers
......................................................................
KUDU-1959 - Add tests for /startup page and metrics for tservers
This patch implements the tests for the startup page using mini tablet
server.
- We inject latency to bootstrap tablets while reading the webpage every
10 milliseconds and validating the status for each step.
- Fail a data directory and validate the status of each startup step.
We also validate the below startup metrics in the above scenarios
(log_block_manager* metrics in the case of using log block manager):
- log_block_manager_total_containers_startup
- log_block_manager_processed_containers_startup
- log_block_manager_containers_processing_time_startup
- tablets_num_total_startup
- tablets_num_opened_startup
- tablets_opening_time_startup
Additionally we also fix a race condition in the Kudu tablet server
WebUI. This race condition occurs if the tablet server is started while
the WebUI is continuously curled. The reason appears to be starting up
of the webserver before registering the path handlers as a part of
the change https://gerrit.cloudera.org/#/c/17730/
Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
---
M src/kudu/mini-cluster/webui_checker.h
M src/kudu/server/webserver.cc
M src/kudu/tserver/tablet_server-test.cc
3 files changed, 222 insertions(+), 7 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/17990/14
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 14
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1959 - Add tests for /startup page for tservers
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17990 )
Change subject: KUDU-1959 - Add tests for /startup page for tservers
......................................................................
Patch Set 6:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/17990/6/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/17990/6/src/kudu/tserver/tablet_server-test.cc@775
PS6, Line 775: FLAGS_block_manager = GetParam();
IMO it doesn't seem worth parameterizing this test. We already get limited mileage because the FBM isn't used very much, but also, the differences in code paths are mostly unrelated to this parameterization in the first place -- only the LBM counts really change, and the change is the skipping of an entire test; the rest of the metric codepaths are basically the same across parameterizations.
Instead, how about we just run all of these tests with the default block manager, and in TestLogBlockContainerMetrics, if the default block manager is "file" e.g. in MacOS, we change the things that we check for -- instead of checking for the presence of LBM metrics, we check for the _absence_ of the LBM metrics.
http://gerrit.cloudera.org:8080/#/c/17990/6/src/kudu/tserver/tablet_server-test.cc@795
PS6, Line 795: current_status
nit: maybe call this "metrics_page" or something, so it's a bit more obvious what it is expected to be, without having to read the rest of this file
Same below
http://gerrit.cloudera.org:8080/#/c/17990/6/src/kudu/tserver/tablet_server-test.cc@921
PS6, Line 921: CheckNonZeroMetricIsStable
nit: for void functions that may raise assertions, surround them in NO_FATALS(), so the error message will point to this line, rather than to a line in CheckNonZeroMetricIsStable's defn
Same elsewhere
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 6
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 08 Nov 2021 23:26:52 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Add tests for /startup page and metrics for tservers
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/17990 )
Change subject: KUDU-1959 - Add tests for /startup page and metrics for tservers
......................................................................
Patch Set 14:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17990/13/src/kudu/server/webserver.cc
File src/kudu/server/webserver.cc:
http://gerrit.cloudera.org:8080/#/c/17990/13/src/kudu/server/webserver.cc@815
PS13, Line 815: paths_and_handlers.reserve(path_handlers_.size());
: for (const auto& [path, handler] : path_handlers_) {
: paths_and_handlers.emplace_back(path, handler);
: }
: }
: EasyJson path_handlers = ej.Set("path_handlers", EasyJson::kArray);
: for (const auto& [path, handler] : paths_and_handlers) {
: if (handler->is_on_nav_bar()) {
: EasyJson path_handler = path_handlers.PushBack(EasyJson::kObject);
: path_handler["path"] = path;
:
> Introducing a new static lock has some code smell. It's true, there's typic
Done
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 14
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 29 Nov 2021 17:33:44 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Add tests for /startup page for tservers
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17990
to look at the new patch set (#5).
Change subject: KUDU-1959 - Add tests for /startup page for tservers
......................................................................
KUDU-1959 - Add tests for /startup page for tservers
This patch implements the tests for the startup page using mini tablet
server.
- We inject latency to bootstrap tablets while reading the webpage every
10 milliseconds and validating the status for each step.
- Fail a data directory and validate the status of each startup step.
We also validate the below startup metrics in the above scenarios (
log_block_manager* metrics in the case of using log block manager):
- log_block_manager_total_containers_startup
- log_block_manager_processed_containers_startup
- log_block_manager_containers_processing_time_startup
- tablets_num_total_startup
- tablets_num_opened_startup
- tablets_opening_time_startup
Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
---
M src/kudu/mini-cluster/webui_checker.h
M src/kudu/tserver/tablet_server-test.cc
2 files changed, 213 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/17990/5
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1959 - Add tests for /startup page and metrics for tservers
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17990 )
Change subject: KUDU-1959 - Add tests for /startup page and metrics for tservers
......................................................................
Patch Set 14: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17990/14/src/kudu/server/webserver.cc
File src/kudu/server/webserver.cc:
http://gerrit.cloudera.org:8080/#/c/17990/14/src/kudu/server/webserver.cc@821
PS14, Line 821:
nit: spacing
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 14
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 29 Nov 2021 18:07:35 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Add tests for /startup page and metrics for tservers
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17990
to look at the new patch set (#12).
Change subject: KUDU-1959 - Add tests for /startup page and metrics for tservers
......................................................................
KUDU-1959 - Add tests for /startup page and metrics for tservers
This patch implements the tests for the startup page using mini tablet
server.
- We inject latency to bootstrap tablets while reading the webpage every
10 milliseconds and validating the status for each step.
- Fail a data directory and validate the status of each startup step.
We also validate the below startup metrics in the above scenarios
(log_block_manager* metrics in the case of using log block manager):
- log_block_manager_total_containers_startup
- log_block_manager_processed_containers_startup
- log_block_manager_containers_processing_time_startup
- tablets_num_total_startup
- tablets_num_opened_startup
- tablets_opening_time_startup
Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
---
M src/kudu/mini-cluster/webui_checker.h
M src/kudu/tserver/tablet_server-test.cc
2 files changed, 210 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/17990/12
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 12
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1959 - Implement tests for the startup progress page for tablet servers
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/17990 )
Change subject: KUDU-1959 - Implement tests for the startup progress page for tablet servers
......................................................................
Patch Set 3:
Appears the failure is unrelated and is being addressed by Andrew already in https://gerrit.cloudera.org/#/c/17993/
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 03 Nov 2021 15:46:14 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1959 - Add tests for /startup page for tservers
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17990
to look at the new patch set (#7).
Change subject: KUDU-1959 - Add tests for /startup page for tservers
......................................................................
KUDU-1959 - Add tests for /startup page for tservers
This patch implements the tests for the startup page using mini tablet
server.
- We inject latency to bootstrap tablets while reading the webpage every
10 milliseconds and validating the status for each step.
- Fail a data directory and validate the status of each startup step.
We also validate the below startup metrics in the above scenarios (
log_block_manager* metrics in the case of using log block manager):
- log_block_manager_total_containers_startup
- log_block_manager_processed_containers_startup
- log_block_manager_containers_processing_time_startup
- tablets_num_total_startup
- tablets_num_opened_startup
- tablets_opening_time_startup
Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
---
M src/kudu/mini-cluster/webui_checker.h
M src/kudu/tserver/tablet_server-test.cc
2 files changed, 212 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/17990/7
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1959 - Add tests for /startup page and metrics for tservers
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17990
to look at the new patch set (#15).
Change subject: KUDU-1959 - Add tests for /startup page and metrics for tservers
......................................................................
KUDU-1959 - Add tests for /startup page and metrics for tservers
This patch implements the tests for the startup page using mini tablet
server.
- We inject latency to bootstrap tablets while reading the webpage every
10 milliseconds and validating the status for each step.
- Fail a data directory and validate the status of each startup step.
We also validate the below startup metrics in the above scenarios
(log_block_manager* metrics in the case of using log block manager):
- log_block_manager_total_containers_startup
- log_block_manager_processed_containers_startup
- log_block_manager_containers_processing_time_startup
- tablets_num_total_startup
- tablets_num_opened_startup
- tablets_opening_time_startup
Additionally we also fix a race condition in the Kudu tablet server
WebUI. This race condition occurs if the tablet server is started while
the WebUI is continuously curled. The reason appears to be starting up
of the webserver before registering the path handlers as a part of
the change https://gerrit.cloudera.org/#/c/17730/
Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
---
M src/kudu/mini-cluster/webui_checker.h
M src/kudu/server/webserver.cc
M src/kudu/tserver/tablet_server-test.cc
3 files changed, 219 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/17990/15
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 15
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1959 - Implement tests for the startup progress page for tablet servers
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17990
to look at the new patch set (#2).
Change subject: KUDU-1959 - Implement tests for the startup progress page for tablet servers
......................................................................
KUDU-1959 - Implement tests for the startup progress page for tablet servers
This patch implements the tests for the startup page using mini tablet
server.
- We inject latency to bootstrap tablets while reading the webpage every
10 milliseconds and validating the status for each step.
- Fail a data directory and validate the status of each startup step.
We also validate the below startup metrics in the above scenarios (
log_block_manager* metrics in the case of using log block manager):
- log_block_manager_total_containers_startup
- log_block_manager_processed_containers_startup
- log_block_manager_containers_processing_time_startup
- tablets_num_total_startup
- tablets_num_opened_startup
- tablets_opening_time_startup
Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
---
M src/kudu/mini-cluster/webui_checker.h
M src/kudu/tserver/tablet_server-test.cc
2 files changed, 234 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/17990/2
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1959 - Add tests for /startup page for tservers
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17990
to look at the new patch set (#6).
Change subject: KUDU-1959 - Add tests for /startup page for tservers
......................................................................
KUDU-1959 - Add tests for /startup page for tservers
This patch implements the tests for the startup page using mini tablet
server.
- We inject latency to bootstrap tablets while reading the webpage every
10 milliseconds and validating the status for each step.
- Fail a data directory and validate the status of each startup step.
We also validate the below startup metrics in the above scenarios (
log_block_manager* metrics in the case of using log block manager):
- log_block_manager_total_containers_startup
- log_block_manager_processed_containers_startup
- log_block_manager_containers_processing_time_startup
- tablets_num_total_startup
- tablets_num_opened_startup
- tablets_opening_time_startup
Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
---
M src/kudu/mini-cluster/webui_checker.h
M src/kudu/tserver/tablet_server-test.cc
2 files changed, 212 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/17990/6
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 6
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1959 - Add tests for /startup page and metrics for tservers
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/17990 )
Change subject: KUDU-1959 - Add tests for /startup page and metrics for tservers
......................................................................
Patch Set 12:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/17990/11//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17990/11//COMMIT_MSG@14
PS11, Line 14:
> nit: add a new line to separate the two lists + parenthesis should be moved
Done
http://gerrit.cloudera.org:8080/#/c/17990/6/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/17990/6/src/kudu/tserver/tablet_server-test.cc@775
PS6, Line 775: NO_FATALS(StartTabletServer(kNumD
> IMO it doesn't seem worth parameterizing this test. We already get limited
Done
http://gerrit.cloudera.org:8080/#/c/17990/6/src/kudu/tserver/tablet_server-test.cc@795
PS6, Line 795: e_status, "\"r
> nit: maybe call this "metrics_page" or something, so it's a bit more obviou
Done
http://gerrit.cloudera.org:8080/#/c/17990/6/src/kudu/tserver/tablet_server-test.cc@921
PS6, Line 921:
> nit: for void functions that may raise assertions, surround them in NO_FATA
Done
http://gerrit.cloudera.org:8080/#/c/17990/8/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/17990/8/src/kudu/tserver/tablet_server-test.cc@793
PS8, Line 793: g&
> nit: move the ampersand over, same below
Done
http://gerrit.cloudera.org:8080/#/c/17990/8/src/kudu/tserver/tablet_server-test.cc@798
PS8, Line 798:
> nit: align spaces
Done
http://gerrit.cloudera.org:8080/#/c/17990/8/src/kudu/tserver/tablet_server-test.cc@829
PS8, Line 829: const string url = Substitute("http://$0/startup", mini_s
> nit: seems like this is only used in the context of the full URL. How about
Done
http://gerrit.cloudera.org:8080/#/c/17990/8/src/kudu/tserver/tablet_server-test.cc@833
PS8, Line 833: url, &b
> nit: there is a using directive for this, so we can drop the "strings::"
Done
http://gerrit.cloudera.org:8080/#/c/17990/8/src/kudu/tserver/tablet_server-test.cc@852
PS8, Line 852: t
> nit: can drop this outer parentheses
Done
http://gerrit.cloudera.org:8080/#/c/17990/8/src/kudu/tserver/tablet_server-test.cc@896
PS8, Line 896: // Validate populated metrics in case of zero tablets dur
> nit: same here w.r.t the URL
Done
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 12
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 15 Nov 2021 21:49:00 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Implement tests for the startup progress page for tablet servers
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17990 )
Change subject: KUDU-1959 - Implement tests for the startup progress page for tablet servers
......................................................................
Patch Set 3:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc@31
PS3, Line 31:
nit: remove the extra newline
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc@46
PS3, Line 46:
nit: remove the extra newlines
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc@782
PS3, Line 782: FLAGS_tablet_bootstrap_inject_latency_ms
I'm not sure there's much value in adding a test that has no latency (especially if there is already a test that exercises the code paths with latency). Consider just injecting latency for all of these tests?
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc@805
PS3, Line 805:
nit: adjust spacing to align the quotes
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc@803
PS3, Line 803: void IsStatusValid(const string current_status) {
: ASSERT_STR_MATCHES(current_status, "\"init_status\":[100|0]"
: ".*\"read_filesystem_status\":[100|0]"
: ".*\"read_instance_metadatafiles_status\":[100|0]"
: ".*\"read_data_directories_status\":"
: "([0-9]|[1-9][0-9]|100)"
: ".*\"start_tablets_status\":([0-9]|[1-9][0-9]|100)"
: ".*\"start_rpc_server_status\":[100|0]");
: }
:
: void IsStatusComplete(const string current_status) {
: ASSERT_STR_MATCHES(current_status, "\"init_status\":100"
: ".*\"read_filesystem_status\":100"
: ".*\"read_instance_metadatafiles_status\":100"
: ".*\"read_data_directories_status\":100"
: ".*\"start_tablets_status\":100"
: ".*\"start_rpc_server_status\":100");
: }
nit: adjust spacing since these are in the scope of a class
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc@901
PS3, Line 901:
nit: remove the extra whitespace
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc@926
PS3, Line 926: &bu
nit: add a space, same below, and same in other places with the pattern ",&"
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc@981
PS3, Line 981: ASSERT_OK(c.FetchURL(
: Substitute("http://$0/metrics?metrics=log_block_manager_containers_processing_time_startup",
: addr),&buf));
: time_elapsed = buf.ToString();
: ASSERT_STR_NOT_CONTAINS(time_elapsed, "\"value\": 0");
: SleepFor(MonoDelta::FromMilliseconds(10));
: ASSERT_OK(c.FetchURL(
: Substitute("http://$0/metrics?metrics=log_block_manager_containers_processing_time_startup",
: addr),&buf));
: ASSERT_EQ(time_elapsed, buf.ToString());
nit: this pattern seems to be used a lot. Consider defining a reusable method, e.g.
void CheckNonZeroMetricIsStable(const string& metric_name) {
const auto url = Substitute("http://$0/metrics?metrics=$1", addr, metric_name);
ASSERT_OK(c.FetchURL(url, &buf));
const auto orig_buf = buf.ToString();
ASSERT_STR_NOT_CONTAINS(orig_buf, "\"value\": 0");
SleepFor(MonoDelta::FromMilliseconds(10));
ASSERT_OK(c.FetchURL(url, &buf));
ASSERT_EQ(orig_buf, buf.ToString());
}
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 03 Nov 2021 21:43:40 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Add tests for /startup page for tservers
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17990
to look at the new patch set (#4).
Change subject: KUDU-1959 - Add tests for /startup page for tservers
......................................................................
KUDU-1959 - Add tests for /startup page for tservers
This patch implements the tests for the startup page using mini tablet
server.
- We inject latency to bootstrap tablets while reading the webpage every
10 milliseconds and validating the status for each step.
- Fail a data directory and validate the status of each startup step.
We also validate the below startup metrics in the above scenarios (
log_block_manager* metrics in the case of using log block manager):
- log_block_manager_total_containers_startup
- log_block_manager_processed_containers_startup
- log_block_manager_containers_processing_time_startup
- tablets_num_total_startup
- tablets_num_opened_startup
- tablets_opening_time_startup
Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
---
M src/kudu/mini-cluster/webui_checker.h
M src/kudu/tserver/tablet_server-test.cc
2 files changed, 205 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/17990/4
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1959 - Add tests for /startup page and metrics for tservers
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17990
to look at the new patch set (#9).
Change subject: KUDU-1959 - Add tests for /startup page and metrics for tservers
......................................................................
KUDU-1959 - Add tests for /startup page and metrics for tservers
This patch implements the tests for the startup page using mini tablet
server.
- We inject latency to bootstrap tablets while reading the webpage every
10 milliseconds and validating the status for each step.
- Fail a data directory and validate the status of each startup step.
We also validate the below startup metrics in the above scenarios (
log_block_manager* metrics in the case of using log block manager):
- log_block_manager_total_containers_startup
- log_block_manager_processed_containers_startup
- log_block_manager_containers_processing_time_startup
- tablets_num_total_startup
- tablets_num_opened_startup
- tablets_opening_time_startup
Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
---
M src/kudu/mini-cluster/webui_checker.h
M src/kudu/tserver/tablet_server-test.cc
2 files changed, 210 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/17990/9
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1959 - Add tests for /startup page and metrics for tservers
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17990 )
Change subject: KUDU-1959 - Add tests for /startup page and metrics for tservers
......................................................................
Patch Set 15: Verified+1
Test flakes are unrelated to this patch.
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 15
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 29 Nov 2021 23:41:24 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1959 - Add tests for /startup page and metrics for tservers
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has removed a vote on this change.
Change subject: KUDU-1959 - Add tests for /startup page and metrics for tservers
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 15
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1959 - Add tests for /startup page and metrics for tservers
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17990
to look at the new patch set (#13).
Change subject: KUDU-1959 - Add tests for /startup page and metrics for tservers
......................................................................
KUDU-1959 - Add tests for /startup page and metrics for tservers
This patch implements the tests for the startup page using mini tablet
server.
- We inject latency to bootstrap tablets while reading the webpage every
10 milliseconds and validating the status for each step.
- Fail a data directory and validate the status of each startup step.
We also validate the below startup metrics in the above scenarios
(log_block_manager* metrics in the case of using log block manager):
- log_block_manager_total_containers_startup
- log_block_manager_processed_containers_startup
- log_block_manager_containers_processing_time_startup
- tablets_num_total_startup
- tablets_num_opened_startup
- tablets_opening_time_startup
Additionally we also fix a race condition in the Kudu tablet server
WebUI. This race condition occurs if the tablet server is started while
the WebUI is continuously curled. The reason appears to be starting up
of the webserver before registering the path handlers as a part of
the change https://gerrit.cloudera.org/#/c/17730/
Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
---
M src/kudu/mini-cluster/webui_checker.h
M src/kudu/server/webserver.cc
M src/kudu/tserver/tablet_server-test.cc
3 files changed, 218 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/17990/13
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 13
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1959 - Add tests for /startup page and metrics for tservers
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17990 )
Change subject: KUDU-1959 - Add tests for /startup page and metrics for tservers
......................................................................
Patch Set 15: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 15
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 29 Nov 2021 19:58:14 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1959 - Add tests for /startup page and metrics for tservers
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17990
to look at the new patch set (#8).
Change subject: KUDU-1959 - Add tests for /startup page and metrics for tservers
......................................................................
KUDU-1959 - Add tests for /startup page and metrics for tservers
This patch implements the tests for the startup page using mini tablet
server.
- We inject latency to bootstrap tablets while reading the webpage every
10 milliseconds and validating the status for each step.
- Fail a data directory and validate the status of each startup step.
We also validate the below startup metrics in the above scenarios (
log_block_manager* metrics in the case of using log block manager):
- log_block_manager_total_containers_startup
- log_block_manager_processed_containers_startup
- log_block_manager_containers_processing_time_startup
- tablets_num_total_startup
- tablets_num_opened_startup
- tablets_opening_time_startup
Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
---
M src/kudu/mini-cluster/webui_checker.h
M src/kudu/tserver/tablet_server-test.cc
2 files changed, 212 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/17990/8
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 8
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1959 - Add tests for /startup page and metrics for tservers
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Attila Bukor, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17990
to look at the new patch set (#10).
Change subject: KUDU-1959 - Add tests for /startup page and metrics for tservers
......................................................................
KUDU-1959 - Add tests for /startup page and metrics for tservers
This patch implements the tests for the startup page using mini tablet
server.
- We inject latency to bootstrap tablets while reading the webpage every
10 milliseconds and validating the status for each step.
- Fail a data directory and validate the status of each startup step.
We also validate the below startup metrics in the above scenarios (
log_block_manager* metrics in the case of using log block manager):
- log_block_manager_total_containers_startup
- log_block_manager_processed_containers_startup
- log_block_manager_containers_processing_time_startup
- tablets_num_total_startup
- tablets_num_opened_startup
- tablets_opening_time_startup
Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
---
M src/kudu/mini-cluster/webui_checker.h
M src/kudu/tserver/tablet_server-test.cc
2 files changed, 210 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/90/17990/10
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 10
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-1959 - Add tests for /startup page and metrics for tservers
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17990 )
Change subject: KUDU-1959 - Add tests for /startup page and metrics for tservers
......................................................................
Patch Set 12:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17990/12/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/17990/12/src/kudu/tserver/tablet_server-test.cc@851
PS12, Line 851: SleepFor(MonoDelta::FromMilliseconds(10));
We chatted about this offline. Seems like this delay is added to test around a real race that is happening in the webserver, where we're receiving a request as the templates are being registered. We should instead address the race
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 12
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 16 Nov 2021 19:36:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Implement tests for the startup progress page for tablet servers
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/17990 )
Change subject: KUDU-1959 - Implement tests for the startup progress page for tablet servers
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17990/1/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/17990/1/src/kudu/tserver/tablet_server-test.cc@45
PS1, Line 45: #include <regex.h>
: #include <kudu/integration-tests/cluster_itest_util.h>
We don't need this and line 31 too.
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 02 Nov 2021 05:34:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Implement tests for the startup progress page for tablet servers
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17990 )
Change subject: KUDU-1959 - Implement tests for the startup progress page for tablet servers
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/17990/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17990/3//COMMIT_MSG@7
PS3, Line 7: KUDU-1959 - Implement tests for the startup progress page for tablet servers
This is 76 characters which seems a bit too long. Generally, we wrap lines in the body at 72 characters (except for log messages, etc), and try to keep the subjects even shorter, preferably at 50, but that is often impossible. Still, please consider rephrasing and reformatting this (e.g. "KUDU-1959 Add tests for /startup page in tservers" is 49 characters).
The subject of a commit message is similar to the subject of an email, the goal is to provide a very brief summary so that the reader knows what it's about at a first glance.
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
PS3:
Some of the concerns raised in the parent apply here as well.
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc@784
PS3, Line 784: /*num_data_dirs=*/
nit: no need for this as you're using a constant which describes what this parameter is.
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 05 Nov 2021 09:42:54 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Implement tests for the startup progress page for tablet servers
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/17990 )
Change subject: KUDU-1959 - Implement tests for the startup progress page for tablet servers
......................................................................
Patch Set 3:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/17990/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/17990/3//COMMIT_MSG@7
PS3, Line 7: KUDU-1959 - Implement tests for the startup progress page for tablet servers
> This is 76 characters which seems a bit too long. Generally, we wrap lines
Done
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
PS3:
> Some of the concerns raised in the parent apply here as well.
Got it.
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc@31
PS3, Line 31:
> nit: remove the extra newline
Done
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc@46
PS3, Line 46:
> nit: remove the extra newlines
Done
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc@782
PS3, Line 782: FLAGS_tablet_bootstrap_inject_latency_ms
> I'm not sure there's much value in adding a test that has no latency (espec
Makes sense. Done
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc@784
PS3, Line 784: /*num_data_dirs=*/
> nit: no need for this as you're using a constant which describes what this
Done
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc@805
PS3, Line 805:
> nit: adjust spacing to align the quotes
Done
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc@803
PS3, Line 803: void IsStatusValid(const string current_status) {
: ASSERT_STR_MATCHES(current_status, "\"init_status\":[100|0]"
: ".*\"read_filesystem_status\":[100|0]"
: ".*\"read_instance_metadatafiles_status\":[100|0]"
: ".*\"read_data_directories_status\":"
: "([0-9]|[1-9][0-9]|100)"
: ".*\"start_tablets_status\":([0-9]|[1-9][0-9]|100)"
: ".*\"start_rpc_server_status\":[100|0]");
: }
:
: void IsStatusComplete(const string current_status) {
: ASSERT_STR_MATCHES(current_status, "\"init_status\":100"
: ".*\"read_filesystem_status\":100"
: ".*\"read_instance_metadatafiles_status\":100"
: ".*\"read_data_directories_status\":100"
: ".*\"start_tablets_status\":100"
: ".*\"start_rpc_server_status\":100");
: }
> nit: adjust spacing since these are in the scope of a class
Done
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc@901
PS3, Line 901:
> nit: remove the extra whitespace
Done
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc@926
PS3, Line 926: &bu
> nit: add a space, same below, and same in other places with the pattern ",&
Done
http://gerrit.cloudera.org:8080/#/c/17990/3/src/kudu/tserver/tablet_server-test.cc@981
PS3, Line 981: ASSERT_OK(c.FetchURL(
: Substitute("http://$0/metrics?metrics=log_block_manager_containers_processing_time_startup",
: addr),&buf));
: time_elapsed = buf.ToString();
: ASSERT_STR_NOT_CONTAINS(time_elapsed, "\"value\": 0");
: SleepFor(MonoDelta::FromMilliseconds(10));
: ASSERT_OK(c.FetchURL(
: Substitute("http://$0/metrics?metrics=log_block_manager_containers_processing_time_startup",
: addr),&buf));
: ASSERT_EQ(time_elapsed, buf.ToString());
> nit: this pattern seems to be used a lot. Consider defining a reusable meth
Done
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 08 Nov 2021 03:40:19 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Add tests for /startup page and metrics for tservers
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17990 )
Change subject: KUDU-1959 - Add tests for /startup page and metrics for tservers
......................................................................
Patch Set 8: Code-Review+1
(6 comments)
Overall looks good, just some style nits.
Also the lint build seems to be complaining.
http://gerrit.cloudera.org:8080/#/c/17990/8/src/kudu/tserver/tablet_server-test.cc
File src/kudu/tserver/tablet_server-test.cc:
http://gerrit.cloudera.org:8080/#/c/17990/8/src/kudu/tserver/tablet_server-test.cc@793
PS8, Line 793: g &
nit: move the ampersand over, same below
http://gerrit.cloudera.org:8080/#/c/17990/8/src/kudu/tserver/tablet_server-test.cc@798
PS8, Line 798:
nit: align spaces
http://gerrit.cloudera.org:8080/#/c/17990/8/src/kudu/tserver/tablet_server-test.cc@829
PS8, Line 829: string addr = mini_server_->bound_http_addr().ToString();
nit: seems like this is only used in the context of the full URL. How about we instead define this as
const string url = Substitute("http://$0/startup", mini_server_->bound_http_addr().ToString());
http://gerrit.cloudera.org:8080/#/c/17990/8/src/kudu/tserver/tablet_server-test.cc@833
PS8, Line 833: strings
nit: there is a using directive for this, so we can drop the "strings::"
http://gerrit.cloudera.org:8080/#/c/17990/8/src/kudu/tserver/tablet_server-test.cc@852
PS8, Line 852: (
nit: can drop this outer parentheses
http://gerrit.cloudera.org:8080/#/c/17990/8/src/kudu/tserver/tablet_server-test.cc@896
PS8, Line 896: string addr = mini_server_->bound_http_addr().ToString();
nit: same here w.r.t the URL
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 8
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 11 Nov 2021 01:30:33 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Add tests for /startup page and metrics for tservers
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17990 )
Change subject: KUDU-1959 - Add tests for /startup page and metrics for tservers
......................................................................
Patch Set 13:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17990/13/src/kudu/server/webserver.cc
File src/kudu/server/webserver.cc:
http://gerrit.cloudera.org:8080/#/c/17990/13/src/kudu/server/webserver.cc@815
PS13, Line 815: {
: static simple_spinlock pathHandlerLock_;
: std::lock_guard<simple_spinlock> l(pathHandlerLock_);
: for (const PathHandlerMap::value_type &handler: path_handlers_) {
: if (handler.second->is_on_nav_bar()) {
: EasyJson path_handler = path_handlers.PushBack(EasyJson::kObject);
: path_handler["path"] = handler.first;
: path_handler["alias"] = handler.second->alias();
: }
: }
: }
Introducing a new static lock has some code smell. It's true, there's typically only one webserver per Kudu process, but it's pretty surprising to have such a limitation that all Webserver instances share this lock.
Instead, how about we copy the pointers in 'path_handlers_' while under the lock we already have, something like
...
vector<pair<string, PathHandler*> paths_and_handlers;
{
shared_lock<RWMutex> l(lock_);
ej["footer_html"] = footer_html_;
paths_and_handlers.reserve(path_handlers.size());
for (const auto& [path, handler] : path_handlers_) {
paths_and_handlers.emplace_back(path, handler);
}
}
EasyJson path_handlers = ej.Set("path_handlers", EasyJson::kArray);
for (const auto& [path, handler] : paths_and_handlers) {
if (handler.second->is_on_nav_bar()) {
EasyJson path_handler = path_handlers.PushBack(EasyJson::kObject);
path_handler["path"] = path;
path_handler["alias"] = handler->alias();
}
}
--
To view, visit http://gerrit.cloudera.org:8080/17990
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f432b4eb813e51214b4d6b3c5b7b4c89426f47f
Gerrit-Change-Number: 17990
Gerrit-PatchSet: 13
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 29 Nov 2021 06:00:04 +0000
Gerrit-HasComments: Yes