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:01 UTC
[kudu-CR] KUDU-1959 - Implement tests for the startup web page for Kudu Master
Abhishek Chennaka has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17989
Change subject: KUDU-1959 - Implement tests for the startup web page for Kudu Master
......................................................................
KUDU-1959 - Implement tests for the startup web page for Kudu Master
This patch contains a test for the Kudu Master Startup Webpage.
We start up a mini master and validate the status of each startup step.
We also fix the startup timers for the mini master.
Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
---
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
2 files changed, 47 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/17989/1
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 1
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
[kudu-CR] KUDU-1959 - Add tests for /startup page for the Master
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17989 )
Change subject: KUDU-1959 - Add tests for /startup page for the Master
......................................................................
Patch Set 5:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@390
PS4, Line 390:
> I felt calling the function once with a longer string to match would be cheaper than calling the function multiple times with a shorter string?
That makes sense, but I think readability and flexibility is more important than performance in test code, at least to a degree.
It's possible, for example, that someone decides that these should be in alphabetical order, or wants to put add a new metric somewhere between these lines as their logical place. I'm not sure if a change like that should break this test.
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@394
PS4, Line 394: *\"read_data_directo
> Yes, I thought about that approach too but we would want it to be strictly
Right after these you have .*, so this would match 1000 as well, so I still don't really see the upside of having the more complicated regex.
http://gerrit.cloudera.org:8080/#/c/17989/5/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/17989/5/src/kudu/master/master-test.cc@407
PS5, Line 407: run_status_reader = true;
If the above two assertions fail, this line will be never reached and the thread read_startup_page thread will loop infinitely. I think it's best to copy this statement to scoped_cleanup as well before the join.
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 08 Nov 2021 16:01:29 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Add test for /startup page for the Master
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17989
to look at the new patch set (#8).
Change subject: KUDU-1959 - Add test for /startup page for the Master
......................................................................
KUDU-1959 - Add test for /startup page for the Master
This patch contains a test for the Kudu Master Startup Webpage.
We start up a mini master and validate the status of each startup step.
We also fix the startup timers for the mini master.
Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
---
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
2 files changed, 48 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/17989/8
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 8
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1959 - Add test for /startup page for the Master
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17989 )
Change subject: KUDU-1959 - Add test for /startup page for the Master
......................................................................
Patch Set 7: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@405
PS4, Line 405:
: ASSERT_OK(mini_master_->Restart());
: ASSERT_OK(mini_master_->WaitForCatalogManagerInit());
: run
> Guess Alexey meant to use something like:
Whatever works best to join the thread in case of assertion triggers. Current version in PS7 looks good to me.
If I understood correctly, Attila suggested to use different boolean flags for different conditions leading to the necessity to join the thread. That could work as well. As is, the current version looks good and simple enough to allow the thread to join if any of the assertions after line 404 in PS7 are triggered.
http://gerrit.cloudera.org:8080/#/c/17989/7/src/kudu/master/master.cc
File src/kudu/master/master.cc:
http://gerrit.cloudera.org:8080/#/c/17989/7/src/kudu/master/master.cc@300
PS7, Line 300: Status s = catalog_manager_init_status_.Get();
: return s;
nit: it could be as simple as
return catalog_manager_init_status_.Get();
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 10 Nov 2021 20:06:44 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Add test for /startup page for the Master
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has removed a vote on this change.
Change subject: KUDU-1959 - Add test for /startup page for the Master
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1959 - Add tests for /startup page for the Master
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17989 )
Change subject: KUDU-1959 - Add tests for /startup page for the Master
......................................................................
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/17989/5/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/17989/5/src/kudu/master/master-test.cc@382
PS5, Line 382: const int wait_time = 10;
nit: constant values are usually named per the GSG https://google.github.io/styleguide/cppguide.html#Constant_Names, i.e.
kWaitTime
That said, it does seems odd to have a separate constant for something used just once. In general we do prefer to not introduce "magic numbers" in tests, especially when used in several places. But in this case it's used just once, and a reader could infer its value is arbitrarily short. I.e. if the value had to be something more specific, a constant variable and/or a comment would be warranted. But to avoid a tight loop, IMO it's fairly self-explanatory.
http://gerrit.cloudera.org:8080/#/c/17989/5/src/kudu/master/master-test.cc@407
PS5, Line 407: run_status_reader = true;
> If the above two assertions fail, this line will be never reached and the t
+1
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 08 Nov 2021 23:04:34 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Implement tests for the startup web page for Kudu Master
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17989 )
Change subject: KUDU-1959 - Implement tests for the startup web page for Kudu Master
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master.cc
File src/kudu/master/master.cc:
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master.cc@265
PS4, Line 265: startup_path_handler_->initialize_master_catalog_progress()->Start();
Is it possible to move both Start() and Stop() for this timer into Master::InitCatalogManagerTask()? Call Start() in the very begin of the method, and call Stop() after calling Set() on the 'catalog_manager_init_status_' promise?
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 04 Nov 2021 01:53:17 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Implement tests for the startup web page for Kudu Master
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17989 )
Change subject: KUDU-1959 - Implement tests for the startup web page for Kudu Master
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@405
PS4, Line 405: SCOPED_CLEANUP({
: is_started = true;
: read_startup_page.join();
: });
> When an ASSERT_* fails, it aborts program execution. I wonder what happens
What does 'aborts' mean here? Doesn't the test framework continues running if there other test scenarios?
The point here is to let the thread to join.
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 05 Nov 2021 19:20:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Implement tests for the startup web page for Kudu Master
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/17989 )
Change subject: KUDU-1959 - Implement tests for the startup web page for Kudu Master
......................................................................
Patch Set 4:
(10 comments)
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@381
PS4, Line 381: is_started
> nit: taking into consideration what the thread is doing, maybe rename this
Done
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@387
PS4, Line 387: 10
> nit: move these to a constant
Done
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@387
PS4, Line 387: SleepFor(MonoDelta::FromMilliseconds(10));
> nit: given the actual start of the mini_master happens after this thread is
Done
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@390
PS4, Line 390: 100
> Is it possible that this ill ever be 0? Or does it get set strictly before
Yes, it is possible. Great catch there. Must be an error on my side. Fixed it.
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@390
PS4, Line 390: 100
> As Andrew pointed, it would be great to clarify on the init_status (maybe,
Done
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@390
PS4, Line 390: 100
> Assuming the order doesn't matter, you could also move these all to separat
I felt calling the function once with a longer string to match would be cheaper than calling the function multiple times with a shorter string?
Maybe similar to the usage in src/kudu/tools/kudu-txn-cli-test.cc
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@394
PS4, Line 394: [0-9]|[1-9][0-9]|100
> Any particular reason why not simply [0-9]{1,3} for all of these?
Yes, I thought about that approach too but we would want it to be strictly between [0,100] and not more than 100, hence the above approach.
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@405
PS4, Line 405: SCOPED_CLEANUP({
: is_started = true;
: read_startup_page.join();
: });
> What happens if one of the two ASSERT_OK() above triggers? In other words,
Done
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@405
PS4, Line 405: SCOPED_CLEANUP({
: is_started = true;
: read_startup_page.join();
: });
> What does 'aborts' mean here? Doesn't the test framework continues running
Guess Alexey meant to use something like:
SCOPED_CLEANUP({
read_startup_page.join();
});
ASSERT_OK(mini_master_->Restart());
ASSERT_OK(mini_master_->WaitForCatalogManagerInit());
run_status_reader = true;
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master.cc
File src/kudu/master/master.cc:
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master.cc@265
PS4, Line 265: startup_path_handler_->initialize_master_catalog_progress()->Start();
> Is it possible to move both Start() and Stop() for this timer into Master::
Makes sense. Done.
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 08 Nov 2021 03:40:14 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Implement tests for the startup web page for Kudu Master
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/17989
to look at the new patch set (#2).
Change subject: KUDU-1959 - Implement tests for the startup web page for Kudu Master
......................................................................
KUDU-1959 - Implement tests for the startup web page for Kudu Master
This patch contains a test for the Kudu Master Startup Webpage.
We start up a mini master and validate the status of each startup step.
We also fix the startup timers for the mini master.
Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
---
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
2 files changed, 48 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/17989/2
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1959 - Implement tests for the startup web page for Kudu Master
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17989 )
Change subject: KUDU-1959 - Implement tests for the startup web page for Kudu Master
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@390
PS4, Line 390: 100
Is it possible that this ill ever be 0? Or does it get set strictly before the webserver comes up?
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 03 Nov 2021 21:19:29 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Add tests for /startup page for the Master
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/17989 )
Change subject: KUDU-1959 - Add tests for /startup page for the Master
......................................................................
Patch Set 5:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@390
PS4, Line 390:
> > I felt calling the function once with a longer string to match would be c
I see, makes sense. Agreed.
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@394
PS4, Line 394: *\"read_data_directo
> Right after these you have .*, so this would match 1000 as well, so I still
Nice catch there. Will try to put in a "( |,)" to match the next character and handle that case. The only upside would be to get a strictly valid number as the status.
http://gerrit.cloudera.org:8080/#/c/17989/5/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/17989/5/src/kudu/master/master-test.cc@382
PS5, Line 382: const int wait_time = 10;
> nit: constant values are usually named per the GSG https://google.github.io
Makes sense, removing the use of const variable.
http://gerrit.cloudera.org:8080/#/c/17989/5/src/kudu/master/master-test.cc@407
PS5, Line 407: run_status_reader = true;
> If the above two assertions fail, this line will be never reached and the t
Done
http://gerrit.cloudera.org:8080/#/c/17989/5/src/kudu/master/master-test.cc@407
PS5, Line 407: run_status_reader = true;
> +1
Done
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 09 Nov 2021 06:07:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Add tests for /startup page for the Master
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17989
to look at the new patch set (#6).
Change subject: KUDU-1959 - Add tests for /startup page for the Master
......................................................................
KUDU-1959 - Add tests for /startup page for the Master
This patch contains a test for the Kudu Master Startup Webpage.
We start up a mini master and validate the status of each startup step.
We also fix the startup timers for the mini master.
Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
---
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
2 files changed, 49 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/17989/6
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 6
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1959 - Add test for /startup page for the Master
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/17989 )
Change subject: KUDU-1959 - Add test for /startup page for the Master
......................................................................
Patch Set 9:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/17989/7/src/kudu/master/master.cc
File src/kudu/master/master.cc:
http://gerrit.cloudera.org:8080/#/c/17989/7/src/kudu/master/master.cc@300
PS7, Line 300: return catalog_manager_init_status_.Get();
: }
> nit: it could be as simple as
Done
http://gerrit.cloudera.org:8080/#/c/17989/8/src/kudu/master/master.cc
File src/kudu/master/master.cc:
http://gerrit.cloudera.org:8080/#/c/17989/8/src/kudu/master/master.cc@300
PS8, Line 300:
> nit: double semi-colon
Done
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 15 Nov 2021 15:53:27 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Implement tests for the startup web page for Kudu Master
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17989 )
Change subject: KUDU-1959 - Implement tests for the startup web page for Kudu Master
......................................................................
Patch Set 4:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@387
PS4, Line 387: 10
nit: move these to a constant
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@390
PS4, Line 390: 100
> As Andrew pointed, it would be great to clarify on the init_status (maybe,
Assuming the order doesn't matter, you could also move these all to separate ASSERT_STR_MATCHES calls, one for each metric, similar to the ASSERT_STR_CONTAINS calls below.
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@394
PS4, Line 394: [0-9]|[1-9][0-9]|100
Any particular reason why not simply [0-9]{1,3} for all of these?
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@405
PS4, Line 405: SCOPED_CLEANUP({
: is_started = true;
: read_startup_page.join();
: });
> What happens if one of the two ASSERT_OK() above triggers? In other words,
When an ASSERT_* fails, it aborts program execution. I wonder what happens if there are more asserts in the scoped cleanup in this case and if it's safe to use. If it isn't, you could split the OK and the failure case to two different flags like is_started and is_failed and in the failure case simply break out of the loop in the read_startup_page thread.
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 05 Nov 2021 09:18:56 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Add test for /startup page for the Master
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17989 )
Change subject: KUDU-1959 - Add test for /startup page for the Master
......................................................................
Patch Set 8: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 8
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 12 Nov 2021 19:35:02 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1959 - Add test for /startup page for the Master
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17989 )
Change subject: KUDU-1959 - Add test for /startup page for the Master
......................................................................
Patch Set 7: Code-Review+2
LGTM, though seems there's still feedback from others
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 11 Nov 2021 01:20:49 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1959 - Add test for /startup page for the Master
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17989 )
Change subject: KUDU-1959 - Add test for /startup page for the Master
......................................................................
KUDU-1959 - Add test for /startup page for the Master
This patch contains a test for the Kudu Master Startup Webpage.
We start up a mini master and validate the status of each startup step.
We also fix the startup timers for the mini master.
Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Reviewed-on: http://gerrit.cloudera.org:8080/17989
Reviewed-by: Attila Bukor <ab...@apache.org>
Tested-by: Attila Bukor <ab...@apache.org>
---
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
2 files changed, 47 insertions(+), 4 deletions(-)
Approvals:
Attila Bukor: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 10
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1959 - Implement tests for the startup web page for Kudu Master
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/17989
to look at the new patch set (#3).
Change subject: KUDU-1959 - Implement tests for the startup web page for Kudu Master
......................................................................
KUDU-1959 - Implement tests for the startup web page for Kudu Master
This patch contains a test for the Kudu Master Startup Webpage.
We start up a mini master and validate the status of each startup step.
We also fix the startup timers for the mini master.
Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
---
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
2 files changed, 48 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/17989/3
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1959 - Implement tests for the startup web page for Kudu Master
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17989 )
Change subject: KUDU-1959 - Implement tests for the startup web page for Kudu Master
......................................................................
Patch Set 4:
(4 comments)
A few more nits.
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@381
PS4, Line 381: is_started
nit: taking into consideration what the thread is doing, maybe rename this variable into 'run_status_reader' and negate current values?
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@387
PS4, Line 387: SleepFor(MonoDelta::FromMilliseconds(10));
nit: given the actual start of the mini_master happens after this thread is created and started, you could move this SleepFor() to be the very first statement in the while() loop and remove the extra SleepFor() in the very end of the cycle.
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@390
PS4, Line 390: 100
> Is it possible that this ill ever be 0? Or does it get set strictly before
As Andrew pointed, it would be great to clarify on the init_status (maybe, add a comment?).
One more readability nit for the ASSERT_STR_MATCHES pattern: it would be great if the line breaks corresponded to the status/percentage pairs
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@405
PS4, Line 405: SCOPED_CLEANUP({
: is_started = true;
: read_startup_page.join();
: });
What happens if one of the two ASSERT_OK() above triggers? In other words, consider moving this scoped cleanup right after instantiating 'read_startup_page' above and setting 'is_started' right after calling WaitForCatalogManagerInit() as well.
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 04 Nov 2021 02:25:08 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1959 - Add test for /startup page for the Master
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17989
to look at the new patch set (#9).
Change subject: KUDU-1959 - Add test for /startup page for the Master
......................................................................
KUDU-1959 - Add test for /startup page for the Master
This patch contains a test for the Kudu Master Startup Webpage.
We start up a mini master and validate the status of each startup step.
We also fix the startup timers for the mini master.
Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
---
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
2 files changed, 47 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/17989/9
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1959 - Add test for /startup page for the Master
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17989 )
Change subject: KUDU-1959 - Add test for /startup page for the Master
......................................................................
Patch Set 9: Verified+1 Code-Review+2
There was an unrelated flaky test failure in TSAN
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 9
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 15 Nov 2021 18:28:24 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1959 - Add tests for /startup page for the Master
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17989
to look at the new patch set (#5).
Change subject: KUDU-1959 - Add tests for /startup page for the Master
......................................................................
KUDU-1959 - Add tests for /startup page for the Master
This patch contains a test for the Kudu Master Startup Webpage.
We start up a mini master and validate the status of each startup step.
We also fix the startup timers for the mini master.
Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
---
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
2 files changed, 48 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/17989/5
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 5
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1959 - Add test for /startup page for the Master
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17989
to look at the new patch set (#7).
Change subject: KUDU-1959 - Add test for /startup page for the Master
......................................................................
KUDU-1959 - Add test for /startup page for the Master
This patch contains a test for the Kudu Master Startup Webpage.
We start up a mini master and validate the status of each startup step.
We also fix the startup timers for the mini master.
Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
---
M src/kudu/master/master-test.cc
M src/kudu/master/master.cc
2 files changed, 49 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/17989/7
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 7
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1959 - Add test for /startup page for the Master
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17989 )
Change subject: KUDU-1959 - Add test for /startup page for the Master
......................................................................
Patch Set 8:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc
File src/kudu/master/master-test.cc:
http://gerrit.cloudera.org:8080/#/c/17989/4/src/kudu/master/master-test.cc@405
PS4, Line 405:
: ASSERT_OK(mini_master_->Restart());
: ASSERT_OK(mini_master_->WaitForCatalogManagerInit());
: run
> Whatever works best to join the thread in case of assertion triggers. Curr
Yea, my concern was about running assertions in a different thread after an assertion fails on the main thread. It shouldn't be a problem though.
http://gerrit.cloudera.org:8080/#/c/17989/8/src/kudu/master/master.cc
File src/kudu/master/master.cc:
http://gerrit.cloudera.org:8080/#/c/17989/8/src/kudu/master/master.cc@300
PS8, Line 300: ;
nit: double semi-colon
--
To view, visit http://gerrit.cloudera.org:8080/17989
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I823a59df215cfeb579f1593a63ef6133de37271c
Gerrit-Change-Number: 17989
Gerrit-PatchSet: 8
Gerrit-Owner: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 12 Nov 2021 20:58:02 +0000
Gerrit-HasComments: Yes