You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Mahesh Reddy (Code Review)" <ge...@cloudera.org> on 2020/08/25 23:06:15 UTC
[kudu-CR] KUDU-3184: Added null check
Mahesh Reddy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16364
Change subject: KUDU-3184: Added null check
......................................................................
KUDU-3184: Added null check
Issue occurred if hostname didn't have a separator (ex. a string of
numbers), ai_canonname wasn't assigned the hostname and stayed null
causing an issue when hostname was assigned the canonical name later
Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
---
M src/kudu/util/net/net_util.cc
1 file changed, 8 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/16364/1
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 1
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
[kudu-CR] [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16364 )
Change subject: [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16364/5/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/16364/5/src/kudu/util/net/net_util.cc@418
PS5, Line 418: result->ai_canonname != nullptr
Should we return non-OK status if esult->ai_canonname == nullptr?
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 1
Gerrit-Owner: Mahesh Reddy <mr...@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-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 26 Aug 2020 23:26:33 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3184:[net] Fix GetFQDN() when canonical name returns null
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16364 )
Change subject: KUDU-3184:[net] Fix GetFQDN() when canonical name returns null
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16364/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16364/4//COMMIT_MSG@7
PS4, Line 7: KUDU-3184
> For future reference, it'd be nice if you could add more details to the tic
Yeh I should probably add more details to the ticket, I'll do that now
http://gerrit.cloudera.org:8080/#/c/16364/4//COMMIT_MSG@7
PS4, Line 7: :[net]
> nit: add a space, e.g.
Done
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 4
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 26 Aug 2020 20:37:40 +0000
Gerrit-HasComments: Yes
[kudu-CR] [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
Posted by "Mahesh Reddy (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/16364
to look at the new patch set (#5).
Change subject: [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
......................................................................
[net] KUDU-3184: Fix GetFQDN() when canonical name returns null
Issue occured if FQDN didn't have domain name (ex .local).
Canonical name wasn't assigned FQDN and returned null.
Ran into this issue with macOS version 10.15.6.
Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
---
M src/kudu/util/net/net_util.cc
1 file changed, 7 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/16364/5
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 5
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-3184:Fix GetFQDN() when canonical name returns null [net]
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16364 )
Change subject: KUDU-3184:Fix GetFQDN() when canonical name returns null [net]
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/16364/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16364/1//COMMIT_MSG@7
PS1, Line 7: ix GetFQDN() whe
> Rather "Fix GetFQDN() when canonical name returned is null"
Done
http://gerrit.cloudera.org:8080/#/c/16364/1//COMMIT_MSG@11
PS1, Line 11:
> end sentence with a period.
Will add macOS comment after this build
http://gerrit.cloudera.org:8080/#/c/16364/1/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/16364/1/src/kudu/util/net/net_util.cc@36
PS1, Line 36: #include <boost/functional/hash/hash.hpp>
> Was this file recommended by iwyu on your Mac?
discarded this endian.h change, kept other changes
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 26 Aug 2020 00:24:02 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3184:[net] Fix GetFQDN() when canonical name returns null
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Bankim Bhavsar,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16364
to look at the new patch set (#4).
Change subject: KUDU-3184:[net] Fix GetFQDN() when canonical name returns null
......................................................................
KUDU-3184:[net] Fix GetFQDN() when canonical name returns null
Issue occured if FQDN didn't have domain name (ex .local).
Canonical name wasn't assigned FQDN and returned null.
Ran into this issue with macOS version 10.15.6.
Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
---
M src/kudu/util/net/net_util.cc
1 file changed, 7 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/16364/4
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 4
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16364 )
Change subject: [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
......................................................................
[net] KUDU-3184: Fix GetFQDN() when canonical name returns null
Issue occured if FQDN didn't have domain name (ex .local).
Canonical name wasn't assigned FQDN and returned null.
Ran into this issue with macOS version 10.15.6.
Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Reviewed-on: http://gerrit.cloudera.org:8080/16364
Reviewed-by: Bankim Bhavsar <ba...@cloudera.com>
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
2 files changed, 8 insertions(+), 4 deletions(-)
Approvals:
Bankim Bhavsar: Looks good to me, approved
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 7
Gerrit-Owner: Mahesh Reddy <mr...@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-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16364 )
Change subject: [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16364/5/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/16364/5/src/kudu/util/net/net_util.cc@418
PS5, Line 418: result->ai_canonname != nullptr
> Related to Alexey's point, what happens if GetFQDN() returns an error statu
I would have to test that but hostname should match the FQDN when the canonical name is null so master bring should succeed. I guess I'm not sure how returning an error status would impact that but I can test that.
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 5
Gerrit-Owner: Mahesh Reddy <mr...@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-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 27 Aug 2020 00:45:46 +0000
Gerrit-HasComments: Yes
[kudu-CR] [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
Posted by "Mahesh Reddy (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/16364
to look at the new patch set (#6).
Change subject: [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
......................................................................
[net] KUDU-3184: Fix GetFQDN() when canonical name returns null
Issue occured if FQDN didn't have domain name (ex .local).
Canonical name wasn't assigned FQDN and returned null.
Ran into this issue with macOS version 10.15.6.
Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
---
M src/kudu/util/net/net_util.cc
M src/kudu/util/net/net_util.h
2 files changed, 8 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/16364/6
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 6
Gerrit-Owner: Mahesh Reddy <mr...@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-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16364 )
Change subject: [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 6
Gerrit-Owner: Mahesh Reddy <mr...@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-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 27 Aug 2020 22:13:29 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3184:Fix GetFQDN() when canonical name returns null [net]
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Bankim Bhavsar,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16364
to look at the new patch set (#2).
Change subject: KUDU-3184:Fix GetFQDN() when canonical name returns null [net]
......................................................................
KUDU-3184:Fix GetFQDN() when canonical name returns null [net]
Issue occured if hostname didn't have separator (ex. a string of
numbers without a period), canonical name stayed null and wasn't
assigned hostname.
Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
---
M src/kudu/util/net/net_util.cc
1 file changed, 7 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/16364/2
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 2
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-3184:[net] Fix GetFQDN() when canonical name returns null
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16364 )
Change subject: KUDU-3184:[net] Fix GetFQDN() when canonical name returns null
......................................................................
Patch Set 4:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/16364/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16364/3//COMMIT_MSG@7
PS3, Line 7: null
> The context [net] goes at the beginning of the summary line, either before
Done
http://gerrit.cloudera.org:8080/#/c/16364/3/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/16364/3/src/kudu/util/net/net_util.cc@21
PS3, Line 21: #include <ifaddrs.
> It's recommended to use C++ header file when available.
Done
http://gerrit.cloudera.org:8080/#/c/16364/3/src/kudu/util/net/net_util.cc@417
PS3, Line 417: // On macOS ai_cannonname returns null when FQDN doesn't have domain name (ex .local)
> I should have mentioned this earlier. Could you add a comment when/why ai_c
Done
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 4
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 26 Aug 2020 19:06:45 +0000
Gerrit-HasComments: Yes
[kudu-CR] [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16364 )
Change subject: [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16364/5/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/16364/5/src/kudu/util/net/net_util.cc@418
PS5, Line 418: result->ai_canonname != nullptr
> In the case that the canonical name is null it's my understanding that FQDN
Related to Alexey's point, what happens if GetFQDN() returns an error status instead of simply returning back hostname?
Does master bring up succeed?
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 5
Gerrit-Owner: Mahesh Reddy <mr...@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-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 27 Aug 2020 00:14:38 +0000
Gerrit-HasComments: Yes
[kudu-CR] [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16364 )
Change subject: [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 6
Gerrit-Owner: Mahesh Reddy <mr...@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-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 27 Aug 2020 23:06:17 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3184:Fix GetFQDN() when canonical name returns null [net]
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Kudu Jenkins, Bankim Bhavsar,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16364
to look at the new patch set (#3).
Change subject: KUDU-3184:Fix GetFQDN() when canonical name returns null [net]
......................................................................
KUDU-3184:Fix GetFQDN() when canonical name returns null [net]
Issue occured if hostname didn't have separator (ex. a string of
numbers without a period), canonical name stayed null and wasn't
assigned hostname. Ran into this issue with macOS version 10.15.6.
Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
---
M src/kudu/util/net/net_util.cc
1 file changed, 7 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/64/16364/3
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 3
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] KUDU-3184: Added null check
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16364 )
Change subject: KUDU-3184: Added null check
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/16364/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16364/1//COMMIT_MSG@7
PS1, Line 7: Added null check
Rather "Fix GetFQDN() when canonical name returned is null"
Also add [net] context.
http://gerrit.cloudera.org:8080/#/c/16364/1//COMMIT_MSG@11
PS1, Line 11: later
end sentence with a period.
Mention about hitting this issue on Mac OS.
http://gerrit.cloudera.org:8080/#/c/16364/1/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/16364/1/src/kudu/util/net/net_util.cc@36
PS1, Line 36: #include "i386/endian.h"
> error: 'i386/endian.h' file not found [clang-diagnostic-error]
Was this file recommended by iwyu on your Mac?
It looks architecture specific best avoided or wrapped under some arch #ifdefs.
Something seems off about these includes suggested. I'd suggest revert all header file changes and let iwyu on gerrit make any recommendations, if needed.
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 1
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 25 Aug 2020 23:18:32 +0000
Gerrit-HasComments: Yes
[kudu-CR] [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16364 )
Change subject: [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16364/5/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/16364/5/src/kudu/util/net/net_util.cc@418
PS5, Line 418: result->ai_canonname != nullptr
> I would have to test that but hostname should match the FQDN when the canon
Yep, that makes sense. If you find that the best option is returning Status::OK() when result->ai_canonname is nullptr, maybe at least update the in-line doc/comment for this function in net_util.h to document the new behavior. Thanks!
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 5
Gerrit-Owner: Mahesh Reddy <mr...@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-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 27 Aug 2020 15:13:52 +0000
Gerrit-HasComments: Yes
[kudu-CR] [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16364 )
Change subject: [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 5
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 26 Aug 2020 22:39:12 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3184:Fix GetFQDN() when canonical name returns null [net]
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16364 )
Change subject: KUDU-3184:Fix GetFQDN() when canonical name returns null [net]
......................................................................
Patch Set 3:
(3 comments)
Almost ready to do. Minor comments.
http://gerrit.cloudera.org:8080/#/c/16364/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16364/3//COMMIT_MSG@7
PS3, Line 7: [net]
The context [net] goes at the beginning of the summary line, either before or after the jira.
http://gerrit.cloudera.org:8080/#/c/16364/3/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/16364/3/src/kudu/util/net/net_util.cc@21
PS3, Line 21: #include <errno.h>
It's recommended to use C++ header file when available.
Try putting the cerrno file back to its original place. I bet IWYU won't complain then.
http://gerrit.cloudera.org:8080/#/c/16364/3/src/kudu/util/net/net_util.cc@417
PS3, Line 417:
I should have mentioned this earlier. Could you add a comment when/why ai_canonname could be null?
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 3
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 26 Aug 2020 17:04:55 +0000
Gerrit-HasComments: Yes
[kudu-CR] [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16364 )
Change subject: [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16364/5/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/16364/5/src/kudu/util/net/net_util.cc@418
PS5, Line 418: result->ai_canonname != nullptr
> Should we return non-OK status if esult->ai_canonname == nullptr?
In the case that the canonical name is null it's my understanding that FQDN doesn't have a domain name. The call on line 404 to GetHostname would return the hostname which in this case is same as FQDN since there's no domain name. Therefore, even if the canonical name was null, hostname would already be the FQDN.
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 5
Gerrit-Owner: Mahesh Reddy <mr...@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-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 26 Aug 2020 23:46:13 +0000
Gerrit-HasComments: Yes
[kudu-CR] [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
Posted by "Mahesh Reddy (Code Review)" <ge...@cloudera.org>.
Mahesh Reddy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16364 )
Change subject: [net] KUDU-3184: Fix GetFQDN() when canonical name returns null
......................................................................
Patch Set 5:
(1 comment)
> Patch Set 5:
>
> (1 comment)
http://gerrit.cloudera.org:8080/#/c/16364/5/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/16364/5/src/kudu/util/net/net_util.cc@418
PS5, Line 418: result->ai_canonname != nullptr
> Yep, that makes sense. If you find that the best option is returning Statu
Returning Status::NotFound if the canonical name was null caused the master bring up to fail. I'll leave the code as is and update the comment in net_util.h.
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 5
Gerrit-Owner: Mahesh Reddy <mr...@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-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Thu, 27 Aug 2020 21:59:06 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3184:[net] Fix GetFQDN() when canonical name returns null
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16364 )
Change subject: KUDU-3184:[net] Fix GetFQDN() when canonical name returns null
......................................................................
Patch Set 4: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16364/4//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16364/4//COMMIT_MSG@7
PS4, Line 7: :[net]
nit: add a space, e.g.
[net] KUDU-3184: fix GetFQDN() when...
http://gerrit.cloudera.org:8080/#/c/16364/4//COMMIT_MSG@7
PS4, Line 7: KUDU-3184
For future reference, it'd be nice if you could add more details to the ticket or the commit message for a fix. For instance, it seems unlikely that every failure to startup a Master (i.e. "Unable to start Master at index 0") is caused by this issue. Through debugging, there were probably some stack traces you found that could have been included that would help other users determine whether they hit this same issue vs some other Master failure.
--
To view, visit http://gerrit.cloudera.org:8080/16364
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6bfc1a39175761e3a2f19280066cb1c8343fe79d
Gerrit-Change-Number: 16364
Gerrit-PatchSet: 4
Gerrit-Owner: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mahesh Reddy <mr...@cloudera.com>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 26 Aug 2020 19:09:57 +0000
Gerrit-HasComments: Yes