You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2020/09/02 14:14:50 UTC

[kudu-CR] KUDU-2574: Add a unique cluster ID 

Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16403


Change subject: KUDU-2574: Add a unique cluster ID
......................................................................

KUDU-2574: Add a unique cluster ID

This patch adds a uniquie cluster ID to the cluster. The ID is
a UUID that is automatically generated and stored in the
sys_catalog if missing (on fresh startup or upgrade).

This patch also exposes the cluster ID via the masters web-ui
and the masters list tool.

I plan to use this cluster ID to enhance the HMS sync
functionality, but it can be enhanced and exposed further as
needed for other uses as well.

Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
---
M src/kudu/common/wire_protocol.proto
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.cc
M src/kudu/master/master.h
M src/kudu/master/master.proto
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/master_service.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/tool_action_master.cc
M www/masters.mustache
13 files changed, 188 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/16403/1
-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16403 )

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/16403/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16403/3//COMMIT_MSG@9
PS3, Line 9: uniquie
> unique
Done


http://gerrit.cloudera.org:8080/#/c/16403/3/src/kudu/master/catalog_manager.h
File src/kudu/master/catalog_manager.h:

http://gerrit.cloudera.org:8080/#/c/16403/3/src/kudu/master/catalog_manager.h@903
PS3, Line 903: InitClusterID
> nit: InitClusterId() to be consistent with other method names
Done


http://gerrit.cloudera.org:8080/#/c/16403/3/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/16403/3/src/kudu/master/catalog_manager.cc@903
PS3, Line 903:     cluster_id = oid_generator.Next();
> Could we use GenerateId() here?
Done


http://gerrit.cloudera.org:8080/#/c/16403/3/src/kudu/master/sys_catalog-test.cc
File src/kudu/master/sys_catalog-test.cc:

PS3: 
> Could you also add a test that we get the same uuid when we restart a maste
I added a check for restart, but this test only uses a single master so it doesn't handle leadership change. I look for a natural place to add that test.


http://gerrit.cloudera.org:8080/#/c/16403/3/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

http://gerrit.cloudera.org:8080/#/c/16403/3/src/kudu/master/sys_catalog.cc@761
PS3, Line 761:   entries.front().Swap(entry);
> nit: might be clearer as
This is the same as `GetCertAuthorityEntry` below so I changed both.



-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 02 Sep 2020 19:48:48 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16403 )

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................


Patch Set 8: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 8
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 03 Sep 2020 18:06:37 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16403 )

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................

KUDU-2574: Add a unique cluster ID

This patch adds a unique cluster ID to the cluster. The ID is
a UUID that is automatically generated and stored in the
sys_catalog if missing (on fresh startup or upgrade).

This patch also exposes the cluster ID via the masters web-ui
and the `kudu master list` tool.

I plan to use this cluster ID to enhance the HMS sync
functionality, but it can be exposed further as
needed for other uses as well.

Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Reviewed-on: http://gerrit.cloudera.org:8080/16403
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
Tested-by: Kudu Jenkins
---
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/master_failover-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.cc
M src/kudu/master/master.h
M src/kudu/master/master.proto
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/master_service.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/tool_action_master.cc
M www/masters.mustache
16 files changed, 271 insertions(+), 2 deletions(-)

Approvals:
  Alexey Serbin: Looks good to me, approved
  Andrew Wong: Looks good to me, approved
  Kudu Jenkins: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 9
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16403 )

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................


Patch Set 4: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16403/3/src/kudu/master/sys_catalog-test.cc
File src/kudu/master/sys_catalog-test.cc:

PS3: 
> I added a check for restart, but this test only uses a single master so it 
integration-tests/master_failover-itest.cc might be a good candidate



-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 02 Sep 2020 19:59:04 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16403 )

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16403/5/src/kudu/integration-tests/master_failover-itest.cc
File src/kudu/integration-tests/master_failover-itest.cc:

http://gerrit.cloudera.org:8080/#/c/16403/5/src/kudu/integration-tests/master_failover-itest.cc@515
PS5, Line 515:   ASSERT_OK(cluster_->GetLeaderMasterIndex(&new_leader_idx));
             :   ASSERT_NE(leader_idx, new_leader_idx);
> Is it possible that the newly elected master will be of the same index agai
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 03 Sep 2020 17:19:20 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16403 )

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................


Patch Set 2: Code-Review+2

(3 comments)

http://gerrit.cloudera.org:8080/#/c/16403/2/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/16403/2/src/kudu/master/catalog_manager.cc@910
PS2, Line 910:  
Nit: extra space.


http://gerrit.cloudera.org:8080/#/c/16403/2/src/kudu/master/catalog_manager.cc@1042
PS2, Line 1042:   *cluster_id = entry.cluster_id();
Debug check for entry.has_cluster_id() since it's optional?


http://gerrit.cloudera.org:8080/#/c/16403/2/src/kudu/master/master.h
File src/kudu/master/master.h:

http://gerrit.cloudera.org:8080/#/c/16403/2/src/kudu/master/master.h@78
PS2, Line 78: std::string
nit: Return a const reference instead of a copy.
Also a debug check for cluster_id_ being not empty will be good.



-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 02 Sep 2020 16:40:48 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16403 )

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................


Patch Set 5:

(11 comments)

http://gerrit.cloudera.org:8080/#/c/16403/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16403/4//COMMIT_MSG@14
PS4, Line 14: `kudu master
> nit: 'kudu master list'
Done


http://gerrit.cloudera.org:8080/#/c/16403/4//COMMIT_MSG@17
PS4, Line 17: exposed furth
> nit: drop ?
Done


http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/common/wire_protocol.proto
File src/kudu/common/wire_protocol.proto:

http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/common/wire_protocol.proto@123
PS4, Line 123: masters.
> masters
Done


http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/common/wire_protocol.proto@124
PS4, Line 124: optional string cluster_id = 5;
> Is it possible to avoid putting it here and keep cluster_id only in GetMast
Not without a good amount of change because ListMasters returns a list of ServerEntryPB. I think this is okay, given I expect to add the cluster ID to tablet servers at some point as well.


http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/catalog_manager.cc@1042
PS4, Line 1042: *cluster_i
> *cluster_id
Done


http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/master.h
File src/kudu/master/master.h:

http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/master.h@78
PS4, Line 78:   const std::string& cluster_id() const { return cluster_id_; }
> nit: add 'const' specifier -- this is a truly constant method
Done


http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/master.cc
File src/kudu/master/master.cc:

http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/master.cc@337
PS4, Line 337: local_entry.set_cluster_id(cluster_id_);
> Is it possible that this method is called when cluster_id_ isn't populated 
I don't think it's possible.


http://gerrit.cloudera.org:8080/#/c/16403/3/src/kudu/master/sys_catalog-test.cc
File src/kudu/master/sys_catalog-test.cc:

PS3: 
> integration-tests/master_failover-itest.cc might be a good candidate
Done


http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/sys_catalog-test.cc
File src/kudu/master/sys_catalog-test.cc:

http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/sys_catalog-test.cc@443
PS4, Line 443:   const string init_id = master_->cluster_id();
> nit: add 'const' to express it's immutable in this scenario?
Done


http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/sys_catalog-test.cc@457
PS4, Line 457: init_id, cluster_id_entry.cluster_id()
> nit: could you put 'init_id' as the first argument?  The reason is that it'
Done


http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/sys_catalog.cc@761
PS4, Line 761: entries.fr
> nit: it might be entries.front() :)
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 03 Sep 2020 16:37:14 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16403 )

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................


Patch Set 7: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 03 Sep 2020 17:51:58 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16403 )

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/16403/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16403/3//COMMIT_MSG@9
PS3, Line 9: uniquie
unique


http://gerrit.cloudera.org:8080/#/c/16403/3/src/kudu/master/catalog_manager.h
File src/kudu/master/catalog_manager.h:

http://gerrit.cloudera.org:8080/#/c/16403/3/src/kudu/master/catalog_manager.h@903
PS3, Line 903: InitClusterID
nit: InitClusterId() to be consistent with other method names


http://gerrit.cloudera.org:8080/#/c/16403/3/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/16403/3/src/kudu/master/catalog_manager.cc@903
PS3, Line 903:     cluster_id = oid_generator.Next();
Could we use GenerateId() here?


http://gerrit.cloudera.org:8080/#/c/16403/3/src/kudu/master/sys_catalog-test.cc
File src/kudu/master/sys_catalog-test.cc:

PS3: 
Could you also add a test that we get the same uuid when we restart a master, and when a new master leader is elected?


http://gerrit.cloudera.org:8080/#/c/16403/3/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

http://gerrit.cloudera.org:8080/#/c/16403/3/src/kudu/master/sys_catalog.cc@761
PS3, Line 761:   entries.front().Swap(entry);
nit: might be clearer as

 *entry = std::move(entries[0]);



-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 02 Sep 2020 18:52:54 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16403 )

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16403/6/src/kudu/integration-tests/master_failover-itest.cc
File src/kudu/integration-tests/master_failover-itest.cc:

http://gerrit.cloudera.org:8080/#/c/16403/6/src/kudu/integration-tests/master_failover-itest.cc@513
PS6, Line 513:   // Validate the cluster ID of the new leader matches.
             :   int new_leader_idx;
             :   ASSERT_OK(cluster_->GetLeaderMasterIndex(&new_leader_idx));
             :   ASSERT_NE(leader_idx, new_leader_idx);
> You could stick this in ASSERT_EVENTUALLY(), since shutting down the server
I just removed it and left the old master shutdown per Alexey's suggestion.



-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 03 Sep 2020 17:27:15 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Grant Henke (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/16403

to look at the new patch set (#2).

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................

KUDU-2574: Add a unique cluster ID

This patch adds a uniquie cluster ID to the cluster. The ID is
a UUID that is automatically generated and stored in the
sys_catalog if missing (on fresh startup or upgrade).

This patch also exposes the cluster ID via the masters web-ui
and the masters list tool.

I plan to use this cluster ID to enhance the HMS sync
functionality, but it can be enhanced and exposed further as
needed for other uses as well.

Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
---
M src/kudu/common/wire_protocol.proto
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.cc
M src/kudu/master/master.h
M src/kudu/master/master.proto
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/master_service.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/tool_action_master.cc
M www/masters.mustache
13 files changed, 189 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/16403/2
-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16403

to look at the new patch set (#7).

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................

KUDU-2574: Add a unique cluster ID

This patch adds a unique cluster ID to the cluster. The ID is
a UUID that is automatically generated and stored in the
sys_catalog if missing (on fresh startup or upgrade).

This patch also exposes the cluster ID via the masters web-ui
and the `kudu master list` tool.

I plan to use this cluster ID to enhance the HMS sync
functionality, but it can be exposed further as
needed for other uses as well.

Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
---
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/master_failover-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.cc
M src/kudu/master/master.h
M src/kudu/master/master.proto
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/master_service.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/tool_action_master.cc
M www/masters.mustache
16 files changed, 270 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/16403/7
-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16403 )

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................


Patch Set 8: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 8
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 03 Sep 2020 19:41:53 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16403

to look at the new patch set (#8).

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................

KUDU-2574: Add a unique cluster ID

This patch adds a unique cluster ID to the cluster. The ID is
a UUID that is automatically generated and stored in the
sys_catalog if missing (on fresh startup or upgrade).

This patch also exposes the cluster ID via the masters web-ui
and the `kudu master list` tool.

I plan to use this cluster ID to enhance the HMS sync
functionality, but it can be exposed further as
needed for other uses as well.

Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
---
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/master_failover-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.cc
M src/kudu/master/master.h
M src/kudu/master/master.proto
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/master_service.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/tool_action_master.cc
M www/masters.mustache
16 files changed, 271 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/16403/8
-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 8
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16403

to look at the new patch set (#5).

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................

KUDU-2574: Add a unique cluster ID

This patch adds a unique cluster ID to the cluster. The ID is
a UUID that is automatically generated and stored in the
sys_catalog if missing (on fresh startup or upgrade).

This patch also exposes the cluster ID via the masters web-ui
and the `kudu master list` tool.

I plan to use this cluster ID to enhance the HMS sync
functionality, but it can be exposed further as
needed for other uses as well.

Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
---
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/master_failover-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.cc
M src/kudu/master/master.h
M src/kudu/master/master.proto
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/master_service.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/tool_action_master.cc
M www/masters.mustache
16 files changed, 272 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/16403/5
-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16403 )

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16403/6/src/kudu/integration-tests/master_failover-itest.cc
File src/kudu/integration-tests/master_failover-itest.cc:

http://gerrit.cloudera.org:8080/#/c/16403/6/src/kudu/integration-tests/master_failover-itest.cc@513
PS6, Line 513:   // Validate the cluster ID of the new leader matches.
             :   int new_leader_idx;
             :   ASSERT_OK(cluster_->GetLeaderMasterIndex(&new_leader_idx));
             :   ASSERT_NE(leader_idx, new_leader_idx);
> Shutting down guarantees us a new leader (vs restarting we may have ended u
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 03 Sep 2020 18:00:21 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16403 )

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................


Patch Set 5: Code-Review+1

(2 comments)

IWYU is not happy, but otherwise LGTM.

>>> Fixing #includes in '/home/jenkins-slave/workspace/kudu-master/2/src/kudu/integration-tests/master_failover-itest.cc'
@@ -36,7 +36,7 @@
 #include "kudu/gutil/strings/strip.h" // IWYU pragma: keep
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/integration-tests/cluster_itest_util.h"
-#include "kudu/master/master.proxy.h"
+#include "kudu/master/master.pb.h"
 #include "kudu/master/sys_catalog.h" // IWYU pragma: keep
 #include "kudu/mini-cluster/external_mini_cluster.h"
 #include "kudu/util/metrics.h"
IWYU would have edited 1 files on your behalf.

http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/common/wire_protocol.proto
File src/kudu/common/wire_protocol.proto:

http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/common/wire_protocol.proto@124
PS4, Line 124: optional string cluster_id = 5;
> Not without a good amount of change because ListMasters returns a list of S
SGTM.  Thank you for the clarification.


http://gerrit.cloudera.org:8080/#/c/16403/5/src/kudu/integration-tests/master_failover-itest.cc
File src/kudu/integration-tests/master_failover-itest.cc:

http://gerrit.cloudera.org:8080/#/c/16403/5/src/kudu/integration-tests/master_failover-itest.cc@515
PS5, Line 515:   ASSERT_OK(cluster_->GetLeaderMasterIndex(&new_leader_idx));
             :   ASSERT_NE(leader_idx, new_leader_idx);
Is it possible that the newly elected master will be of the same index again?  Maybe, just keep the former leader shut down?



-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 5
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 03 Sep 2020 17:13:44 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16403

to look at the new patch set (#4).

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................

KUDU-2574: Add a unique cluster ID

This patch adds a unique cluster ID to the cluster. The ID is
a UUID that is automatically generated and stored in the
sys_catalog if missing (on fresh startup or upgrade).

This patch also exposes the cluster ID via the masters web-ui
and the masters list tool.

I plan to use this cluster ID to enhance the HMS sync
functionality, but it can be enhanced and exposed further as
needed for other uses as well.

Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
---
M src/kudu/common/wire_protocol.proto
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.cc
M src/kudu/master/master.h
M src/kudu/master/master.proto
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/master_service.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/tool_action_master.cc
M www/masters.mustache
13 files changed, 197 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/16403/4
-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 4
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16403 )

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................


Patch Set 2:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/16403/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16403/4//COMMIT_MSG@14
PS4, Line 14: masters list
nit: 'kudu master list'


http://gerrit.cloudera.org:8080/#/c/16403/4//COMMIT_MSG@17
PS4, Line 17: enhanced and 
nit: drop ?


http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/common/wire_protocol.proto
File src/kudu/common/wire_protocol.proto:

http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/common/wire_protocol.proto@123
PS4, Line 123: master servers
masters


http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/common/wire_protocol.proto@124
PS4, Line 124: optional string cluster_id = 5;
Is it possible to avoid putting it here and keep cluster_id only in GetMasterRegistrationResponsePB ?


http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/catalog_manager.cc@1042
PS4, Line 1042: 
*cluster_id

Otherwise an address would be printed, I guess.


http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/master.h
File src/kudu/master/master.h:

http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/master.h@78
PS4, Line 78:   std::string cluster_id() { return cluster_id_; }
nit: add 'const' specifier -- this is a truly constant method


http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/master.cc
File src/kudu/master/master.cc:

http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/master.cc@337
PS4, Line 337: local_entry.set_cluster_id(cluster_id_);
Is it possible that this method is called when cluster_id_ isn't populated yet?  If so, maybe put this under 'if (!cluster_id_.empty())' clause?


http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/sys_catalog-test.cc
File src/kudu/master/sys_catalog-test.cc:

http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/sys_catalog-test.cc@443
PS4, Line 443: 
nit: add 'const' to express it's immutable in this scenario?


http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/sys_catalog-test.cc@457
PS4, Line 457: 
nit: could you put 'init_id' as the first argument?  The reason is that it's easier to read an error messages when this assertion triggers (if ever).


http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/sys_catalog.cc
File src/kudu/master/sys_catalog.cc:

http://gerrit.cloudera.org:8080/#/c/16403/4/src/kudu/master/sys_catalog.cc@761
PS4, Line 761: p(entry);
nit: it might be entries.front() :)

entries.front() is a bit safer because it would generate out-of-bounds exception if the container is empty, but if you prefer entries[0] for other reason, it's fine with me.



-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 02 Sep 2020 23:37:49 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16403 )

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16403/6/src/kudu/integration-tests/master_failover-itest.cc
File src/kudu/integration-tests/master_failover-itest.cc:

http://gerrit.cloudera.org:8080/#/c/16403/6/src/kudu/integration-tests/master_failover-itest.cc@513
PS6, Line 513:   int new_leader_idx;
             :   ASSERT_OK(cluster_->GetLeaderMasterIndex(&new_leader_idx));
             :   string new_cluster_id = GetClusterId(new_leader_idx);
             :   ASSERT_TRUE(!new_cluster_id.empty());
You could stick this in ASSERT_EVENTUALLY(), since shutting down the server guarantees us a new leader.



-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 03 Sep 2020 17:24:53 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16403 )

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16403/6/src/kudu/integration-tests/master_failover-itest.cc
File src/kudu/integration-tests/master_failover-itest.cc:

http://gerrit.cloudera.org:8080/#/c/16403/6/src/kudu/integration-tests/master_failover-itest.cc@513
PS6, Line 513:   int new_leader_idx;
             :   ASSERT_OK(cluster_->GetLeaderMasterIndex(&new_leader_idx));
             :   string new_cluster_id = GetClusterId(new_leader_idx);
             :   ASSERT_TRUE(!new_cluster_id.empty());
> I just removed it and left the old master shutdown per Alexey's suggestion.
Shutting down guarantees us a new leader (vs restarting we may have ended up with the same leader), so why not leave the assert in?



-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 7
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 03 Sep 2020 17:42:54 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, Andrew Wong, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16403

to look at the new patch set (#6).

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................

KUDU-2574: Add a unique cluster ID

This patch adds a unique cluster ID to the cluster. The ID is
a UUID that is automatically generated and stored in the
sys_catalog if missing (on fresh startup or upgrade).

This patch also exposes the cluster ID via the masters web-ui
and the `kudu master list` tool.

I plan to use this cluster ID to enhance the HMS sync
functionality, but it can be exposed further as
needed for other uses as well.

Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
---
M src/kudu/common/wire_protocol.proto
M src/kudu/integration-tests/cluster_itest_util.cc
M src/kudu/integration-tests/cluster_itest_util.h
M src/kudu/integration-tests/master_failover-itest.cc
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.cc
M src/kudu/master/master.h
M src/kudu/master/master.proto
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/master_service.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/tool_action_master.cc
M www/masters.mustache
16 files changed, 272 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/16403/6
-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 6
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16403 )

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/16403/2/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:

http://gerrit.cloudera.org:8080/#/c/16403/2/src/kudu/master/catalog_manager.cc@910
PS2, Line 910:  
> Nit: extra space.
Done


http://gerrit.cloudera.org:8080/#/c/16403/2/src/kudu/master/catalog_manager.cc@1042
PS2, Line 1042:   *cluster_id = entry.cluster_id();
> Debug check for entry.has_cluster_id() since it's optional?
GetClusterIdEntry has that check already.


http://gerrit.cloudera.org:8080/#/c/16403/2/src/kudu/master/master.h
File src/kudu/master/master.h:

http://gerrit.cloudera.org:8080/#/c/16403/2/src/kudu/master/master.h@78
PS2, Line 78: std::string
> nit: Return a const reference instead of a copy.
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 02 Sep 2020 18:48:25 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2574: Add a unique cluster ID

Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/16403

to look at the new patch set (#3).

Change subject: KUDU-2574: Add a unique cluster ID
......................................................................

KUDU-2574: Add a unique cluster ID

This patch adds a uniquie cluster ID to the cluster. The ID is
a UUID that is automatically generated and stored in the
sys_catalog if missing (on fresh startup or upgrade).

This patch also exposes the cluster ID via the masters web-ui
and the masters list tool.

I plan to use this cluster ID to enhance the HMS sync
functionality, but it can be enhanced and exposed further as
needed for other uses as well.

Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
---
M src/kudu/common/wire_protocol.proto
M src/kudu/master/catalog_manager.cc
M src/kudu/master/catalog_manager.h
M src/kudu/master/master.cc
M src/kudu/master/master.h
M src/kudu/master/master.proto
M src/kudu/master/master_path_handlers.cc
M src/kudu/master/master_service.cc
M src/kudu/master/sys_catalog-test.cc
M src/kudu/master/sys_catalog.cc
M src/kudu/master/sys_catalog.h
M src/kudu/tools/tool_action_master.cc
M www/masters.mustache
13 files changed, 189 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/03/16403/3
-- 
To view, visit http://gerrit.cloudera.org:8080/16403
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4df1d3f7d100336f52563f3008cacf6d9e328fae
Gerrit-Change-Number: 16403
Gerrit-PatchSet: 3
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)