You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Ádám Bakai (Code Review)" <ge...@cloudera.org> on 2023/03/21 13:18:57 UTC
[kudu-CR] Change order of operations during InitCatalogManagerTask
Ádám Bakai has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19639
Change subject: Change order of operations during InitCatalogManagerTask
......................................................................
Change order of operations during InitCatalogManagerTask
In MasterStartupTest. StartupWebPage test, WaitForCatalogManagerInit is
executed, which waits until catalog_manager_init_status_ is set. It is
set by InitCatalogManagerTask and with the previous order of operations
it was possible (but very rare) that catalog_manager_init_status_ was
set but the timer was not stopped at the time of the startup http
request, which resulted an output where
initialize_master_catalog_status was 0. With this order
initialize_master_catalog_progress_ is always set if
WaitForCatalogManagerInit is finished.
Change-Id: I64c980a8b357c99d4431a02d58b3c03a60bae1ad
---
M src/kudu/master/master.cc
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/39/19639/1
--
To view, visit http://gerrit.cloudera.org:8080/19639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I64c980a8b357c99d4431a02d58b3c03a60bae1ad
Gerrit-Change-Number: 19639
Gerrit-PatchSet: 1
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
[kudu-CR] Change order of operations during InitCatalogManagerTask
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/19639 )
Change subject: Change order of operations during InitCatalogManagerTask
......................................................................
Patch Set 3:
Thanks for addressing this. Is this a part of the flaky test failures? Was the fix re-run with dist-test and did we see any further flakiness?
--
To view, visit http://gerrit.cloudera.org:8080/19639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I64c980a8b357c99d4431a02d58b3c03a60bae1ad
Gerrit-Change-Number: 19639
Gerrit-PatchSet: 3
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 23 May 2023 01:09:53 +0000
Gerrit-HasComments: No
[kudu-CR] Change order of operations during InitCatalogManagerTask
Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/19639 )
Change subject: Change order of operations during InitCatalogManagerTask
......................................................................
Patch Set 5:
> Patch Set 4:
>
> > Thanks for addressing this. Is this a part of the flaky test
> > failures? Was the fix re-run with dist-test and did we see any
> > further flakiness?
>
> It's not part of the flaky tests My idea is that it's pretty sure that this change itself won't introduce new flakiness, and dist-test will check if it breaks any test.
Didn't the previous behavior cause a flaky test? How did this problem manifest if not by a flaky test?
--
To view, visit http://gerrit.cloudera.org:8080/19639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I64c980a8b357c99d4431a02d58b3c03a60bae1ad
Gerrit-Change-Number: 19639
Gerrit-PatchSet: 5
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Ádám Bakai <ab...@cloudera.com>
Gerrit-Comment-Date: Tue, 20 Jun 2023 10:06:48 +0000
Gerrit-HasComments: No
[kudu-CR] Change order of operations during InitCatalogManagerTask
Posted by "Ádám Bakai (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19639
to look at the new patch set (#4).
Change subject: Change order of operations during InitCatalogManagerTask
......................................................................
Change order of operations during InitCatalogManagerTask
In MasterStartupTest.StartupWebPage test, WaitForCatalogManagerInit is
executed, which waits until catalog_manager_init_status_ is set. It is
set by InitCatalogManagerTask and with the previous order of operations
it was possible (but very rare) that catalog_manager_init_status_ was
set but the timer was not stopped at the time of the startup http
request, which resulted an output where
initialize_master_catalog_status was 0. With this order
initialize_master_catalog_progress_ is always set if
WaitForCatalogManagerInit is finished.
Change-Id: I64c980a8b357c99d4431a02d58b3c03a60bae1ad
---
M src/kudu/master/master.cc
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/39/19639/4
--
To view, visit http://gerrit.cloudera.org:8080/19639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I64c980a8b357c99d4431a02d58b3c03a60bae1ad
Gerrit-Change-Number: 19639
Gerrit-PatchSet: 4
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Change order of operations during InitCatalogManagerTask
Posted by "Ádám Bakai (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19639
to look at the new patch set (#2).
Change subject: Change order of operations during InitCatalogManagerTask
......................................................................
Change order of operations during InitCatalogManagerTask
In MasterStartupTest. StartupWebPage test, WaitForCatalogManagerInit is
executed, which waits until catalog_manager_init_status_ is set. It is
set by InitCatalogManagerTask and with the previous order of operations
it was possible (but very rare) that catalog_manager_init_status_ was
set but the timer was not stopped at the time of the startup http
request, which resulted an output where
initialize_master_catalog_status was 0. With this order
initialize_master_catalog_progress_ is always set if
WaitForCatalogManagerInit is finished.
Change-Id: I64c980a8b357c99d4431a02d58b3c03a60bae1ad
---
M src/kudu/master/master.cc
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/39/19639/2
--
To view, visit http://gerrit.cloudera.org:8080/19639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I64c980a8b357c99d4431a02d58b3c03a60bae1ad
Gerrit-Change-Number: 19639
Gerrit-PatchSet: 2
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Change order of operations during InitCatalogManagerTask
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19639 )
Change subject: Change order of operations during InitCatalogManagerTask
......................................................................
Change order of operations during InitCatalogManagerTask
In MasterStartupTest.StartupWebPage test, WaitForCatalogManagerInit is
executed, which waits until catalog_manager_init_status_ is set. It is
set by InitCatalogManagerTask and with the previous order of operations
it was possible (but very rare) that catalog_manager_init_status_ was
set but the timer was not stopped at the time of the startup http
request, which resulted an output where
initialize_master_catalog_status was 0. With this order
initialize_master_catalog_progress_ is always set if
WaitForCatalogManagerInit is finished.
Change-Id: I64c980a8b357c99d4431a02d58b3c03a60bae1ad
Reviewed-on: http://gerrit.cloudera.org:8080/19639
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <al...@apache.org>
---
M src/kudu/master/master.cc
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/19639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I64c980a8b357c99d4431a02d58b3c03a60bae1ad
Gerrit-Change-Number: 19639
Gerrit-PatchSet: 6
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Ádám Bakai <ab...@cloudera.com>
[kudu-CR] Change order of operations during InitCatalogManagerTask
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19639 )
Change subject: Change order of operations during InitCatalogManagerTask
......................................................................
Patch Set 5: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19639/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19639/2//COMMIT_MSG@7
PS2, Line 7: Change order of operations during InitCatalogManagerTask
[master] fix setting ...
--
To view, visit http://gerrit.cloudera.org:8080/19639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I64c980a8b357c99d4431a02d58b3c03a60bae1ad
Gerrit-Change-Number: 19639
Gerrit-PatchSet: 5
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Ádám Bakai <ab...@cloudera.com>
Gerrit-Comment-Date: Thu, 24 Aug 2023 21:47:15 +0000
Gerrit-HasComments: Yes
[kudu-CR] Change order of operations during InitCatalogManagerTask
Posted by "Ádám Bakai (Code Review)" <ge...@cloudera.org>.
Ádám Bakai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19639 )
Change subject: Change order of operations during InitCatalogManagerTask
......................................................................
Patch Set 4:
> Thanks for addressing this. Is this a part of the flaky test
> failures? Was the fix re-run with dist-test and did we see any
> further flakiness?
It's not part of the flaky tests My idea is that it's pretty sure that this change itself won't introduce new flakiness, and dist-test will check if it breaks any test.
--
To view, visit http://gerrit.cloudera.org:8080/19639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I64c980a8b357c99d4431a02d58b3c03a60bae1ad
Gerrit-Change-Number: 19639
Gerrit-PatchSet: 4
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Ádám Bakai <ab...@cloudera.com>
Gerrit-Comment-Date: Fri, 16 Jun 2023 12:06:55 +0000
Gerrit-HasComments: No
[kudu-CR] Change order of operations during InitCatalogManagerTask
Posted by "Ádám Bakai (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19639
to look at the new patch set (#5).
Change subject: Change order of operations during InitCatalogManagerTask
......................................................................
Change order of operations during InitCatalogManagerTask
In MasterStartupTest.StartupWebPage test, WaitForCatalogManagerInit is
executed, which waits until catalog_manager_init_status_ is set. It is
set by InitCatalogManagerTask and with the previous order of operations
it was possible (but very rare) that catalog_manager_init_status_ was
set but the timer was not stopped at the time of the startup http
request, which resulted an output where
initialize_master_catalog_status was 0. With this order
initialize_master_catalog_progress_ is always set if
WaitForCatalogManagerInit is finished.
Change-Id: I64c980a8b357c99d4431a02d58b3c03a60bae1ad
---
M src/kudu/master/master.cc
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/39/19639/5
--
To view, visit http://gerrit.cloudera.org:8080/19639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I64c980a8b357c99d4431a02d58b3c03a60bae1ad
Gerrit-Change-Number: 19639
Gerrit-PatchSet: 5
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Ádám Bakai <ab...@cloudera.com>
[kudu-CR] Change order of operations during InitCatalogManagerTask
Posted by "Ádám Bakai (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19639
to look at the new patch set (#3).
Change subject: Change order of operations during InitCatalogManagerTask
......................................................................
Change order of operations during InitCatalogManagerTask
In MasterStartupTest. StartupWebPage test, WaitForCatalogManagerInit is
executed, which waits until catalog_manager_init_status_ is set. It is
set by InitCatalogManagerTask and with the previous order of operations
it was possible (but very rare) that catalog_manager_init_status_ was
set but the timer was not stopped at the time of the startup http
request, which resulted an output where
initialize_master_catalog_status was 0. With this order
initialize_master_catalog_progress_ is always set if
WaitForCatalogManagerInit is finished.
Change-Id: I64c980a8b357c99d4431a02d58b3c03a60bae1ad
---
M src/kudu/master/master.cc
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/39/19639/3
--
To view, visit http://gerrit.cloudera.org:8080/19639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I64c980a8b357c99d4431a02d58b3c03a60bae1ad
Gerrit-Change-Number: 19639
Gerrit-PatchSet: 3
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] Change order of operations during InitCatalogManagerTask
Posted by "Ádám Bakai (Code Review)" <ge...@cloudera.org>.
Ádám Bakai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19639 )
Change subject: Change order of operations during InitCatalogManagerTask
......................................................................
Patch Set 5:
> > Patch Set 4:
> >
> > > Thanks for addressing this. Is this a part of the flaky test
> > > failures? Was the fix re-run with dist-test and did we see any
> > > further flakiness?
> >
> > It's not part of the flaky tests My idea is that it's pretty sure
> that this change itself won't introduce new flakiness, and
> dist-test will check if it breaks any test.
>
> Didn't the previous behavior cause a flaky test? How did this
> problem manifest if not by a flaky test?
Sorry, I meant like it's not part of the flaky test list. It happens rarely. It can be reproduced on my development environment by adding a sleep like this:
master.cc:392
catalog_manager_init_status_.Set(s);
SleepFor(MonoDelta::FromMilliseconds(10));
startup_path_handler_->initialize_master_catalog_progress()->Stop();
--
To view, visit http://gerrit.cloudera.org:8080/19639
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I64c980a8b357c99d4431a02d58b3c03a60bae1ad
Gerrit-Change-Number: 19639
Gerrit-PatchSet: 5
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Ádám Bakai <ab...@cloudera.com>
Gerrit-Comment-Date: Tue, 01 Aug 2023 15:18:19 +0000
Gerrit-HasComments: No