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/11/17 13:55:17 UTC

[kudu-CR] [ExternalMiniCluster] Implement leader master function

Ádám Bakai has uploaded this change for review. ( http://gerrit.cloudera.org:8080/20714


Change subject: [ExternalMiniCluster] Implement leader_master function
......................................................................

[ExternalMiniCluster] Implement leader_master function

Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
---
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
2 files changed, 6 insertions(+), 5 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
Gerrit-Change-Number: 20714
Gerrit-PatchSet: 1
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>

[kudu-CR] [ExternalMiniCluster] improve leader master()

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

Change subject: [ExternalMiniCluster] improve leader_master()
......................................................................


Patch Set 8:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/20714/8/src/kudu/integration-tests/master_cert_authority-itest.cc
File src/kudu/integration-tests/master_cert_authority-itest.cc:

http://gerrit.cloudera.org:8080/#/c/20714/8/src/kudu/integration-tests/master_cert_authority-itest.cc@366
PS8, Line 366: cluster_opts_.num_tablet_servers = 0;
Please add a comment to explain why to override the default for the 'num_tablet_servers' field.


http://gerrit.cloudera.org:8080/#/c/20714/8/src/kudu/integration-tests/security-itest.cc
File src/kudu/integration-tests/security-itest.cc:

http://gerrit.cloudera.org:8080/#/c/20714/8/src/kudu/integration-tests/security-itest.cc@1446
PS8, Line 1446: cluster_opts_.num_tablet_servers = 0;
Please add a comment to explain why to override the default for the 'num_tablet_servers' field.


http://gerrit.cloudera.org:8080/#/c/20714/5/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:

http://gerrit.cloudera.org:8080/#/c/20714/5/src/kudu/mini-cluster/external_mini_cluster.cc@1043
PS5, Line 1043:         found 
> Then "found" variable will be true and it will exit from the outer loop as 
Ah, indeed -- I missed that '!found' addition for the outer 'for' cycle.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
Gerrit-Change-Number: 20714
Gerrit-PatchSet: 8
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Ádám Bakai <ab...@cloudera.com>
Gerrit-Comment-Date: Tue, 28 Nov 2023 20:16:00 +0000
Gerrit-HasComments: Yes

[kudu-CR] [ExternalMiniCluster] Implement leader master function

Posted by "Ádám Bakai (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/20714

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

Change subject: [ExternalMiniCluster] Implement leader_master function
......................................................................

[ExternalMiniCluster] Implement leader_master function

Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
---
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
2 files changed, 6 insertions(+), 5 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
Gerrit-Change-Number: 20714
Gerrit-PatchSet: 2
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] [ExternalMiniCluster] improve leader master()

Posted by "Ádám Bakai (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/20714

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

Change subject: [ExternalMiniCluster] improve leader_master()
......................................................................

[ExternalMiniCluster] improve leader_master()

ExternalMiniCluster::leader_master() function was improved to use the
RPC. However during the dns_alias_itest cases, the host that was stored
in the masters_[]->bound_rpc_hostport was a domain name, and the RPC
returns IP address and they are compared as strings. This made
leader_master() check in ExternalMiniCluster::AddTabletServer() fail.
The solution is that bound_rpc_hostport values are converted to Sockaddr
and compared to the leader_master_addr that is retrieved from the
ConnectToClusterRpc call.

Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
---
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
2 files changed, 21 insertions(+), 13 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
Gerrit-Change-Number: 20714
Gerrit-PatchSet: 5
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] [ExternalMiniCluster] improve leader master()

Posted by "Ádám Bakai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, 

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

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

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

Change subject: [ExternalMiniCluster] improve leader_master()
......................................................................

[ExternalMiniCluster] improve leader_master()

ExternalMiniCluster::leader_master() function was improved to use the
RPC. However during the dns_alias_itest cases, the host that was stored
in the masters_[]->bound_rpc_hostport was a domain name, and the RPC
returns IP address and they are compared as strings. This made
leader_master() check in ExternalMiniCluster::AddTabletServer() fail.
The solution is that bound_rpc_hostport values are converted to Sockaddr
and compared to the leader_master_addr that is retrieved from the
ConnectToClusterRpc call.

ConnectToFollowerMasterTest::AuthnTokenVerifierHaveKeys was changed so
it doesn't start any tservers, since leader_master() check can't find
leader during tablet server initialization.

In master_cert_authority-itest, num of tablet servers was set to zero as
well because tablet servers are not needed for the test and waiting for
leader made the test flaky.

Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
---
M src/kudu/integration-tests/master_cert_authority-itest.cc
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
4 files changed, 26 insertions(+), 13 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
Gerrit-Change-Number: 20714
Gerrit-PatchSet: 8
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Ádám Bakai <ab...@cloudera.com>

[kudu-CR] [ExternalMiniCluster] improve leader master()

Posted by "Ádám Bakai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, 

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

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

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

Change subject: [ExternalMiniCluster] improve leader_master()
......................................................................

[ExternalMiniCluster] improve leader_master()

ExternalMiniCluster::leader_master() function was improved to use the
RPC. However during the dns_alias_itest cases, the host that was stored
in the masters_[]->bound_rpc_hostport was a domain name, and the RPC
returns IP address and they are compared as strings. This made
leader_master() check in ExternalMiniCluster::AddTabletServer() fail.
The solution is that bound_rpc_hostport values are converted to Sockaddr
and compared to the leader_master_addr that is retrieved from the
ConnectToClusterRpc call.

ConnectToFollowerMasterTest::AuthnTokenVerifierHaveKeys was changed so
it doesn't start any tservers, since leader_master() check can't find
leader during tablet server initialization.

In master_cert_authority-itest, num of tablet servers was set to zero as
well because tablet servers are not needed for the test and waiting for
leader made the test flaky.

Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
---
M src/kudu/integration-tests/master_cert_authority-itest.cc
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
4 files changed, 25 insertions(+), 13 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
Gerrit-Change-Number: 20714
Gerrit-PatchSet: 7
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Ádám Bakai <ab...@cloudera.com>

[kudu-CR] [ExternalMiniCluster] improve leader master()

Posted by "Ádám Bakai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, 

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

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

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

Change subject: [ExternalMiniCluster] improve leader_master()
......................................................................

[ExternalMiniCluster] improve leader_master()

ExternalMiniCluster::leader_master() function was improved to use the
RPC. However during the dns_alias_itest cases, the host that was stored
in the masters_[]->bound_rpc_hostport was a domain name, and the RPC
returns IP address and they are compared as strings. This made
leader_master() check in ExternalMiniCluster::AddTabletServer() fail.
The solution is that bound_rpc_hostport values are converted to Sockaddr
and compared to the leader_master_addr that is retrieved from the
ConnectToClusterRpc call.

ConnectToFollowerMasterTest::AuthnTokenVerifierHaveKeys was changed so
it doesn't start any tservers, since leader_master() check can't find
leader during tablet server initialization.

Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
3 files changed, 25 insertions(+), 14 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
Gerrit-Change-Number: 20714
Gerrit-PatchSet: 6
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Ádám Bakai <ab...@cloudera.com>

[kudu-CR] [ExternalMiniCluster] improve leader master()

Posted by "Ádám Bakai (Code Review)" <ge...@cloudera.org>.
Ádám Bakai has posted comments on this change. ( http://gerrit.cloudera.org:8080/20714 )

Change subject: [ExternalMiniCluster] improve leader_master()
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20714/5/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:

http://gerrit.cloudera.org:8080/#/c/20714/5/src/kudu/mini-cluster/external_mini_cluster.cc@1043
PS5, Line 1043:         break;
> This 'break' is to bail out of the embedded 'for' cycle, but what about exi
Then "found" variable will be true and it will exit from the outer loop as well.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
Gerrit-Change-Number: 20714
Gerrit-PatchSet: 5
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Ádám Bakai <ab...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Nov 2023 11:45:44 +0000
Gerrit-HasComments: Yes

[kudu-CR] [ExternalMiniCluster] Improve leader master retrieval

Posted by "Ádám Bakai (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/20714

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

Change subject: [ExternalMiniCluster] Improve leader master retrieval
......................................................................

[ExternalMiniCluster] Improve leader master retrieval

ExternalMiniCluster::leader_master() function was improved to use the RPC.
However during the dns_alias_itest cases, the host that was stored in the
masters_[]->bound_rpc_hostport was a domain name, and the RPC returns IP
address and they are compared as strings. This made leader_master() check
in ExternalMiniCluster::AddTabletServer() fail. The solution is that
bound_rpc_hostport values are converted to Sockaddr and compared to the
leader_master_addr that is retrieved from the ConnectToClusterRpc.

Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
---
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
2 files changed, 21 insertions(+), 13 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
Gerrit-Change-Number: 20714
Gerrit-PatchSet: 4
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] [ExternalMiniCluster] improve leader master()

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

Change subject: [ExternalMiniCluster] improve leader_master()
......................................................................


Patch Set 9: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
Gerrit-Change-Number: 20714
Gerrit-PatchSet: 9
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Ádám Bakai <ab...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Nov 2023 19:17:03 +0000
Gerrit-HasComments: No

[kudu-CR] [ExternalMiniCluster] Implement leader master function

Posted by "Ádám Bakai (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/20714

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

Change subject: [ExternalMiniCluster] Implement leader_master function
......................................................................

[ExternalMiniCluster] Implement leader_master function

Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
---
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
2 files changed, 9 insertions(+), 5 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
Gerrit-Change-Number: 20714
Gerrit-PatchSet: 3
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)

[kudu-CR] [ExternalMiniCluster] improve leader master()

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

Change subject: [ExternalMiniCluster] improve leader_master()
......................................................................

[ExternalMiniCluster] improve leader_master()

ExternalMiniCluster::leader_master() function was improved to use the
RPC. However during the dns_alias_itest cases, the host that was stored
in the masters_[]->bound_rpc_hostport was a domain name, and the RPC
returns IP address and they are compared as strings. This made
leader_master() check in ExternalMiniCluster::AddTabletServer() fail.
The solution is that bound_rpc_hostport values are converted to Sockaddr
and compared to the leader_master_addr that is retrieved from the
ConnectToClusterRpc call.

ConnectToFollowerMasterTest::AuthnTokenVerifierHaveKeys was changed so
it doesn't start any tservers, since leader_master() check can't find
leader during tablet server initialization.

In master_cert_authority-itest, num of tablet servers was set to zero as
well because tablet servers are not needed for the test and waiting for
leader made the test flaky.

Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
Reviewed-on: http://gerrit.cloudera.org:8080/20714
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <al...@apache.org>
---
M src/kudu/integration-tests/master_cert_authority-itest.cc
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
4 files changed, 31 insertions(+), 13 deletions(-)

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

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
Gerrit-Change-Number: 20714
Gerrit-PatchSet: 10
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Ádám Bakai <ab...@cloudera.com>

[kudu-CR] [ExternalMiniCluster] improve leader master()

Posted by "Ádám Bakai (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Alexey Serbin, Kudu Jenkins, 

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

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

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

Change subject: [ExternalMiniCluster] improve leader_master()
......................................................................

[ExternalMiniCluster] improve leader_master()

ExternalMiniCluster::leader_master() function was improved to use the
RPC. However during the dns_alias_itest cases, the host that was stored
in the masters_[]->bound_rpc_hostport was a domain name, and the RPC
returns IP address and they are compared as strings. This made
leader_master() check in ExternalMiniCluster::AddTabletServer() fail.
The solution is that bound_rpc_hostport values are converted to Sockaddr
and compared to the leader_master_addr that is retrieved from the
ConnectToClusterRpc call.

ConnectToFollowerMasterTest::AuthnTokenVerifierHaveKeys was changed so
it doesn't start any tservers, since leader_master() check can't find
leader during tablet server initialization.

In master_cert_authority-itest, num of tablet servers was set to zero as
well because tablet servers are not needed for the test and waiting for
leader made the test flaky.

Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
---
M src/kudu/integration-tests/master_cert_authority-itest.cc
M src/kudu/integration-tests/security-itest.cc
M src/kudu/mini-cluster/external_mini_cluster.cc
M src/kudu/mini-cluster/external_mini_cluster.h
4 files changed, 31 insertions(+), 13 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/14/20714/9
-- 
To view, visit http://gerrit.cloudera.org:8080/20714
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
Gerrit-Change-Number: 20714
Gerrit-PatchSet: 9
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Ádám Bakai <ab...@cloudera.com>

[kudu-CR] [ExternalMiniCluster] improve leader master()

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

Change subject: [ExternalMiniCluster] improve leader_master()
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/20714/5/src/kudu/mini-cluster/external_mini_cluster.cc
File src/kudu/mini-cluster/external_mini_cluster.cc:

http://gerrit.cloudera.org:8080/#/c/20714/5/src/kudu/mini-cluster/external_mini_cluster.cc@997
PS5, Line 997: ExternalMaster* ExternalMiniCluster::leader_master() {
             :   int idx = 0;
             :   Status s = GetLeaderMasterIndex(&idx);
             :   if (!s.ok()) return nullptr;
             :   return master(idx);
             : }
I guess there might be tests that used this method in its original incarnation in a single-master test cluster, and that usage wasn't assuming the master already bootstrapped and established its own Raft leadership so ConnectToMaster call would work.  You'd need to find those and update them correspondingly, otherwise there might be a lot of failures and flakiness introduced due to this change.


http://gerrit.cloudera.org:8080/#/c/20714/5/src/kudu/mini-cluster/external_mini_cluster.cc@1043
PS5, Line 1043:         break;
This 'break' is to bail out of the embedded 'for' cycle, but what about exiting earlier from the outer 'for' cycle as well?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4a40fa8e9513066d08b42eefc57a04ba90bded82
Gerrit-Change-Number: 20714
Gerrit-PatchSet: 5
Gerrit-Owner: Ádám Bakai <ab...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 21 Nov 2023 21:22:23 +0000
Gerrit-HasComments: Yes