You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org> on 2021/05/25 00:57:25 UTC

[kudu-CR] WIP [master] KUDU-2181 Fix duplicate master address and remove unsafe flag tag

Bankim Bhavsar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17500


Change subject: WIP [master] KUDU-2181 Fix duplicate master address and remove unsafe flag tag
......................................................................

WIP [master] KUDU-2181 Fix duplicate master address and remove unsafe flag tag

While testing add master on a physical cluster, observed that
supplying only hostnames without port resulted in duplicates
being supplied to bring up new master which in turn leads
to failure in creating distributed Raft config on startup.

For e.g. kudu master add hp1,hp2 h2

Reason being hp2 is compared as hp2:7051 whereas in the vector
of strings "master_addresses", it contains hp1,hp2.

This changes adds a new function ParseAddresses() in HostPort class
that's a variant of existing ParseStrings() function and takes
a vector of strings instead. This new function is used
in the duplicate detection logic.

This change also removes the unsafe tag from
--master_addr_add_new_master as the feature is ready but keeps
it hidden as it's only meant to be used by the add master orchestration
tool.

WIP because of missing test.
With UNIQUE_LOOPBACK on Linux, if masters are assigned same default
port then I can skip supplying port and simulate the issue.
Without UNIQUE_LOOPBACK on Mac, I'll have to disable the test.

Change-Id: Icf29730e3a6b225adb24ff161cac2ad777b46b81
---
M src/kudu/master/sys_catalog.cc
M src/kudu/tools/tool_action_master.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
4 files changed, 27 insertions(+), 12 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Icf29730e3a6b225adb24ff161cac2ad777b46b81
Gerrit-Change-Number: 17500
Gerrit-PatchSet: 1
Gerrit-Owner: Bankim Bhavsar <ba...@cloudera.com>

[kudu-CR] [master] KUDU-2181 Fix duplicate master address and remove unsafe flag tag

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

Change subject: [master] KUDU-2181 Fix duplicate master address and remove unsafe flag tag
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17500/2/src/kudu/tools/tool_action_master.cc
File src/kudu/tools/tool_action_master.cc:

http://gerrit.cloudera.org:8080/#/c/17500/2/src/kudu/tools/tool_action_master.cc@360
PS2, Line 360: contain duplicates.
> I hope the input strings doesn't contain some strange notations on the port
That looks intentional.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf29730e3a6b225adb24ff161cac2ad777b46b81
Gerrit-Change-Number: 17500
Gerrit-PatchSet: 2
Gerrit-Owner: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 26 May 2021 19:16:43 +0000
Gerrit-HasComments: Yes

[kudu-CR] [master] KUDU-2181 Fix duplicate master address and remove unsafe flag tag

Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

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

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

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

Change subject: [master] KUDU-2181 Fix duplicate master address and remove unsafe flag tag
......................................................................

[master] KUDU-2181 Fix duplicate master address and remove unsafe flag tag

While testing add master on a physical cluster, observed that
supplying only hostnames without port resulted in duplicates
being supplied to bring up new master which in turn leads
to failure in creating distributed Raft config on startup.

For e.g. kudu master add hp1,hp2 h2

Reason being hp2 is compared as hp2:7051 whereas in the vector
of strings "master_addresses", it contains hp1,hp2.

This changes adds a new function ParseAddresses() in HostPort class
that's a variant of existing ParseStrings() function and takes
a vector of strings instead. This new function is used
in the duplicate detection logic.

This change also removes the unsafe tag from
--master_addr_add_new_master as the feature is ready but keeps
it hidden as it's only meant to be used by the add master orchestration
tool.

This one is tricky to test locally and write a test because
we need to specify ports for starting masters/tservers.
Verified the fix on physical cluster. Along with CM integration
we can add a systest later.

Change-Id: Icf29730e3a6b225adb24ff161cac2ad777b46b81
---
M src/kudu/master/sys_catalog.cc
M src/kudu/tools/tool_action_master.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
4 files changed, 27 insertions(+), 12 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Icf29730e3a6b225adb24ff161cac2ad777b46b81
Gerrit-Change-Number: 17500
Gerrit-PatchSet: 2
Gerrit-Owner: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [master] KUDU-2181 Fix duplicate master address and remove unsafe flag tag

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

Change subject: [master] KUDU-2181 Fix duplicate master address and remove unsafe flag tag
......................................................................

[master] KUDU-2181 Fix duplicate master address and remove unsafe flag tag

While testing add master on a physical cluster, observed that
supplying only hostnames without port resulted in duplicates
being supplied to bring up new master which in turn leads
to failure in creating distributed Raft config on startup.

For e.g. kudu master add hp1,hp2 h2

Reason being hp2 is compared as hp2:7051 whereas in the vector
of strings "master_addresses", it contains hp1,hp2.

This changes adds a new function ParseAddresses() in HostPort class
that's a variant of existing ParseStrings() function and takes
a vector of strings instead. This new function is used
in the duplicate detection logic.

This change also removes the unsafe tag from
--master_addr_add_new_master as the feature is ready but keeps
it hidden as it's only meant to be used by the add master orchestration
tool.

This one is tricky to test locally and write a test because
we need to specify ports for starting masters/tservers.
Verified the fix on physical cluster. Along with CM integration
we can add a systest later.

Change-Id: Icf29730e3a6b225adb24ff161cac2ad777b46b81
Reviewed-on: http://gerrit.cloudera.org:8080/17500
Tested-by: Bankim Bhavsar <ba...@cloudera.com>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/master/sys_catalog.cc
M src/kudu/tools/tool_action_master.cc
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
4 files changed, 27 insertions(+), 12 deletions(-)

Approvals:
  Bankim Bhavsar: Verified
  Alexey Serbin: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Icf29730e3a6b225adb24ff161cac2ad777b46b81
Gerrit-Change-Number: 17500
Gerrit-PatchSet: 3
Gerrit-Owner: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [master] KUDU-2181 Fix duplicate master address and remove unsafe flag tag

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

Change subject: [master] KUDU-2181 Fix duplicate master address and remove unsafe flag tag
......................................................................


Patch Set 2: Verified+1

> Patch Set 2: Verified-1
> 
> Build Failed 
> 
> http://jenkins.kudu.apache.org/job/kudu-gerrit/23941/ : FAILURE

Unrelated test failure in MultiThreadedTabletTest/2.DeleteAndReinsert


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf29730e3a6b225adb24ff161cac2ad777b46b81
Gerrit-Change-Number: 17500
Gerrit-PatchSet: 2
Gerrit-Owner: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 26 May 2021 16:47:59 +0000
Gerrit-HasComments: No

[kudu-CR] [master] KUDU-2181 Fix duplicate master address and remove unsafe flag tag

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

Change subject: [master] KUDU-2181 Fix duplicate master address and remove unsafe flag tag
......................................................................


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17500/2/src/kudu/tools/tool_action_master.cc
File src/kudu/tools/tool_action_master.cc:

http://gerrit.cloudera.org:8080/#/c/17500/2/src/kudu/tools/tool_action_master.cc@360
PS2, Line 360: contain duplicates.
I hope the input strings doesn't contain some strange notations on the port number, like host1.local:07050 and host1.local:7050 both present.  At least, it's not likely that somebody is going to supply such an input other than intentionally.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Icf29730e3a6b225adb24ff161cac2ad777b46b81
Gerrit-Change-Number: 17500
Gerrit-PatchSet: 2
Gerrit-Owner: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 26 May 2021 18:36:07 +0000
Gerrit-HasComments: Yes

[kudu-CR] [master] KUDU-2181 Fix duplicate master address and remove unsafe flag tag

Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has removed a vote on this change.

Change subject: [master] KUDU-2181 Fix duplicate master address and remove unsafe flag tag
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/17500
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Icf29730e3a6b225adb24ff161cac2ad777b46b81
Gerrit-Change-Number: 17500
Gerrit-PatchSet: 2
Gerrit-Owner: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)