[kudu-CR] KUDU-3491 Destruct master before creating a new one
Attila Bukor has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/20913 ) Change subject: KUDU-3491 Destruct master before creating a new one .. KUDU-3491 Destruct master before creating a new one ServerBase constructor runs MinidumpExceptionHandler constructor that calls RegisterMinidumpExceptionHandler(). This function increments the static atomic variable current_num_instances_. Then the ServerBase is destructed, a similar process happens and current_num_instances_ gets decremented. If current_num_instances_ is not zero before incrementing or not 1 before decrementing, then it is considered an error. This indicates that only one Server can run at any given time. But in case of multi-master config, the master server is replaced, and without the change it is possible that the second server's constructor precede first server's destructor. This change makes it sure that the destructor is executed before the second one's constructor. Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3 Reviewed-on: http://gerrit.cloudera.org:8080/20913 Reviewed-by: Attila Bukor Reviewed-by: Mahesh Reddy Tested-by: Kudu Jenkins --- M src/kudu/master/dynamic_multi_master-test.cc M src/kudu/master/master_runner.cc 2 files changed, 43 insertions(+), 2 deletions(-) Approvals: Attila Bukor: Looks good to me, approved Mahesh Reddy: Looks good to me, but someone else must approve Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/20913 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3 Gerrit-Change-Number: 20913 Gerrit-PatchSet: 8 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Ádám Bakai
[kudu-CR] KUDU-3491 Destruct master before creating a new one
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/20913 ) Change subject: KUDU-3491 Destruct master before creating a new one .. Patch Set 7: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/20913 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3 Gerrit-Change-Number: 20913 Gerrit-PatchSet: 7 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Ádám Bakai Gerrit-Comment-Date: Wed, 17 Jan 2024 19:14:41 + Gerrit-HasComments: No
[kudu-CR] KUDU-3491 Destruct master before creating a new one
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/20913 ) Change subject: KUDU-3491 Destruct master before creating a new one .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/20913 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3 Gerrit-Change-Number: 20913 Gerrit-PatchSet: 7 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Ádám Bakai Gerrit-Comment-Date: Wed, 17 Jan 2024 19:01:58 + Gerrit-HasComments: No
[kudu-CR] KUDU-3491 Destruct master before creating a new one
Hello Mahesh Reddy, Tidy Bot, Attila Bukor, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20913 to look at the new patch set (#7). Change subject: KUDU-3491 Destruct master before creating a new one .. KUDU-3491 Destruct master before creating a new one ServerBase constructor runs MinidumpExceptionHandler constructor that calls RegisterMinidumpExceptionHandler(). This function increments the static atomic variable current_num_instances_. Then the ServerBase is destructed, a similar process happens and current_num_instances_ gets decremented. If current_num_instances_ is not zero before incrementing or not 1 before decrementing, then it is considered an error. This indicates that only one Server can run at any given time. But in case of multi-master config, the master server is replaced, and without the change it is possible that the second server's constructor precede first server's destructor. This change makes it sure that the destructor is executed before the second one's constructor. Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3 --- M src/kudu/master/dynamic_multi_master-test.cc M src/kudu/master/master_runner.cc 2 files changed, 43 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/20913/7 -- To view, visit http://gerrit.cloudera.org:8080/20913 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3 Gerrit-Change-Number: 20913 Gerrit-PatchSet: 7 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Ádám Bakai
[kudu-CR] KUDU-3491 Destruct master before creating a new one
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/20913 ) Change subject: KUDU-3491 Destruct master before creating a new one .. Patch Set 6: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/20913 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3 Gerrit-Change-Number: 20913 Gerrit-PatchSet: 6 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Ádám Bakai Gerrit-Comment-Date: Wed, 17 Jan 2024 18:23:56 + Gerrit-HasComments: No
[kudu-CR] KUDU-3491 Destruct master before creating a new one
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/20913 ) Change subject: KUDU-3491 Destruct master before creating a new one .. Patch Set 6: Code-Review+2 Thanks -- To view, visit http://gerrit.cloudera.org:8080/20913 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3 Gerrit-Change-Number: 20913 Gerrit-PatchSet: 6 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Ádám Bakai Gerrit-Comment-Date: Wed, 17 Jan 2024 18:10:19 + Gerrit-HasComments: No
[kudu-CR] KUDU-3491 Destruct master before creating a new one
Hello Mahesh Reddy, Tidy Bot, Attila Bukor, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20913 to look at the new patch set (#6). Change subject: KUDU-3491 Destruct master before creating a new one .. KUDU-3491 Destruct master before creating a new one ServerBase constructor runs MinidumpExceptionHandler constructor that calls RegisterMinidumpExceptionHandler(). This function increments the static atomic variable current_num_instances_. Then the ServerBase is destructed, a similar process happens and current_num_instances_ gets decremented. If current_num_instances_ is not zero before incrementing or not 1 before decrementing, then it is considered an error. This indicates that only one Server can run at any given time. But in case of multi-master config, the master server is replaced, and without the change it is possible that the second server's constructor precede first server's destructor. This change makes it sure that the destructor is executed before the second one's constructor. Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3 --- M src/kudu/master/dynamic_multi_master-test.cc M src/kudu/master/master_runner.cc 2 files changed, 40 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/20913/6 -- To view, visit http://gerrit.cloudera.org:8080/20913 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3 Gerrit-Change-Number: 20913 Gerrit-PatchSet: 6 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Ádám Bakai
[kudu-CR] KUDU-3491 Destruct master before creating a new one
Hello Mahesh Reddy, Tidy Bot, Attila Bukor, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20913 to look at the new patch set (#5). Change subject: KUDU-3491 Destruct master before creating a new one .. KUDU-3491 Destruct master before creating a new one ServerBase constructor runs MinidumpExceptionHandler constructor that calls RegisterMinidumpExceptionHandler(). This function increments the static atomic variable current_num_instances_. Then the ServerBase is destructed, a similar process happens and current_num_instances_ gets decremented. If current_num_instances_ is not zero before incrementing or not 1 before decrementing, then it is considered an error. This indicates that only one Server can run at any given time. But in case of multi-master config, the master server is replaced, and without the change it is possible that the second server's constructor precede first server's destructor. This change makes it sure that the destructor is executed before the second one's constructor. Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3 --- M src/kudu/master/dynamic_multi_master-test.cc M src/kudu/master/master_runner.cc 2 files changed, 40 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/20913/5 -- To view, visit http://gerrit.cloudera.org:8080/20913 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3 Gerrit-Change-Number: 20913 Gerrit-PatchSet: 5 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Ádám Bakai
[kudu-CR] KUDU-3491 Destruct master before creating a new one
Ádám Bakai has posted comments on this change. (
http://gerrit.cloudera.org:8080/20913 )
Change subject: KUDU-3491 Destruct master before creating a new one
..
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20913/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20913/4//COMMIT_MSG@20
PS4, Line 20: This patch fixes KUDU-3491 issue.
> nit: no need for this line now as it's in the subject line
Done
http://gerrit.cloudera.org:8080/#/c/20913/3/src/kudu/master/dynamic_multi_master-test.cc
File src/kudu/master/dynamic_multi_master-test.cc:
http://gerrit.cloudera.org:8080/#/c/20913/3/src/kudu/master/dynamic_multi_master-test.cc@1483
PS3, Line 1483:
opts_.extra_master_flags.emplace_back("--enable_minidumps=true");
> Can you add a comment that minidumps are disabled for mini cluster by defau
Done
--
To view, visit http://gerrit.cloudera.org:8080/20913
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3
Gerrit-Change-Number: 20913
Gerrit-PatchSet: 4
Gerrit-Owner: Ádám Bakai
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Ádám Bakai
Gerrit-Comment-Date: Wed, 17 Jan 2024 18:08:44 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3491 Destruct master before creating a new one
Attila Bukor has posted comments on this change. (
http://gerrit.cloudera.org:8080/20913 )
Change subject: KUDU-3491 Destruct master before creating a new one
..
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/20913/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/20913/4//COMMIT_MSG@20
PS4, Line 20: This patch fixes KUDU-3491 issue.
nit: no need for this line now as it's in the subject line
http://gerrit.cloudera.org:8080/#/c/20913/3/src/kudu/master/dynamic_multi_master-test.cc
File src/kudu/master/dynamic_multi_master-test.cc:
http://gerrit.cloudera.org:8080/#/c/20913/3/src/kudu/master/dynamic_multi_master-test.cc@1483
PS3, Line 1483:
opts_.extra_master_flags.emplace_back("--enable_minidumps=true");
Can you add a comment that minidumps are disabled for mini cluster by default
which is the reason this wasn't caught earlier?
--
To view, visit http://gerrit.cloudera.org:8080/20913
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3
Gerrit-Change-Number: 20913
Gerrit-PatchSet: 4
Gerrit-Owner: Ádám Bakai
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Ádám Bakai
Gerrit-Comment-Date: Wed, 17 Jan 2024 17:42:00 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3491 Destruct master before creating a new one
Hello Mahesh Reddy, Tidy Bot, Attila Bukor, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20913 to look at the new patch set (#4). Change subject: KUDU-3491 Destruct master before creating a new one .. KUDU-3491 Destruct master before creating a new one ServerBase constructor runs MinidumpExceptionHandler constructor that calls RegisterMinidumpExceptionHandler(). This function increments the static atomic variable current_num_instances_. Then the ServerBase is destructed, a similar process happens and current_num_instances_ gets decremented. If current_num_instances_ is not zero before incrementing or not 1 before decrementing, then it is considered an error. This indicates that only one Server can run at any given time. But in case of multi-master config, the master server is replaced, and without the change it is possible that the second server's constructor precede first server's destructor. This change makes it sure that the destructor is executed before the second one's constructor. This patch fixes KUDU-3491 issue. Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3 --- M src/kudu/master/dynamic_multi_master-test.cc M src/kudu/master/master_runner.cc 2 files changed, 37 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/20913/4 -- To view, visit http://gerrit.cloudera.org:8080/20913 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3 Gerrit-Change-Number: 20913 Gerrit-PatchSet: 4 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Ádám Bakai
[kudu-CR] KUDU-3491 Destruct master before creating a new one
Ádám Bakai has posted comments on this change. ( http://gerrit.cloudera.org:8080/20913 ) Change subject: KUDU-3491 Destruct master before creating a new one .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/20913/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20913/2//COMMIT_MSG@7 PS2, Line 7: [master_runner] > nit: replace this tag with the ticket ID, and rephrase it to something like Done http://gerrit.cloudera.org:8080/#/c/20913/2/src/kudu/master/master_runner.cc File src/kudu/master/master_runner.cc: PS2: > +1 Done PS2: > Can you add a test that would fail with the old behavior and pass with the Done -- To view, visit http://gerrit.cloudera.org:8080/20913 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3 Gerrit-Change-Number: 20913 Gerrit-PatchSet: 2 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy Gerrit-Reviewer: Ádám Bakai Gerrit-Comment-Date: Wed, 17 Jan 2024 17:28:53 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-3491 Destruct master before creating a new one
Hello Mahesh Reddy, Attila Bukor, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/20913 to look at the new patch set (#3). Change subject: KUDU-3491 Destruct master before creating a new one .. KUDU-3491 Destruct master before creating a new one ServerBase constructor runs MinidumpExceptionHandler constructor that calls RegisterMinidumpExceptionHandler(). This function increments the static atomic variable current_num_instances_. Then the ServerBase is destructed, a similar process happens and current_num_instances_ gets decremented. If current_num_instances_ is not zero before incrementing or not 1 before decrementing, then it is considered an error. This indicates that only one Server can run at any given time. But in case of multi-master config, the master server is replaced, and without the change it is possible that the second server's constructor precede first server's destructor. This change makes it sure that the destructor is executed before the second one's constructor. This patch fixes KUDU-3491 issue. Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3 --- M src/kudu/master/dynamic_multi_master-test.cc M src/kudu/master/master_runner.cc 2 files changed, 39 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/13/20913/3 -- To view, visit http://gerrit.cloudera.org:8080/20913 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3c1019d092bbf9e58f4fc33753a1218bc79735d3 Gerrit-Change-Number: 20913 Gerrit-PatchSet: 3 Gerrit-Owner: Ádám Bakai Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy
