You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2021/09/16 05:53:48 UTC
[kudu-CR] [util] allow DnsResolver to refresh DNS addresses
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17849
Change subject: [util] allow DnsResolver to refresh DNS addresses
......................................................................
[util] allow DnsResolver to refresh DNS addresses
This patch introduces functionality to the DnsResolver to refresh an
address, rather than looking it up in the cache. It does this by
removing any cached entry and performing the lookup.
This will be used in a follow-up change to refresh the address on
certain transient failures.
A new --dns_addr_resolution_override flag is also introduced for testing
purposes.
Change-Id: I0616f3e6fb50aba271f106b05d1926fc46a53ed0
---
M src/kudu/util/net/dns_resolver-test.cc
M src/kudu/util/net/dns_resolver.cc
M src/kudu/util/net/dns_resolver.h
M src/kudu/util/net/net_util.cc
M src/kudu/util/ttl_cache.h
5 files changed, 144 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/17849/1
--
To view, visit http://gerrit.cloudera.org:8080/17849
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0616f3e6fb50aba271f106b05d1926fc46a53ed0
Gerrit-Change-Number: 17849
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
[kudu-CR] [util] allow DnsResolver to refresh DNS addresses
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17849 )
Change subject: [util] allow DnsResolver to refresh DNS addresses
......................................................................
[util] allow DnsResolver to refresh DNS addresses
This patch introduces functionality to the DnsResolver to refresh an
address, rather than looking it up in the cache. It does this by
removing any cached entry and performing the lookup.
This will be used in a follow-up change to refresh the address on
certain transient failures.
A new --dns_addr_resolution_override flag is also introduced for testing
purposes.
Change-Id: I0616f3e6fb50aba271f106b05d1926fc46a53ed0
Reviewed-on: http://gerrit.cloudera.org:8080/17849
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M src/kudu/util/net/dns_resolver-test.cc
M src/kudu/util/net/dns_resolver.cc
M src/kudu/util/net/dns_resolver.h
M src/kudu/util/net/net_util.cc
M src/kudu/util/ttl_cache.h
5 files changed, 145 insertions(+), 2 deletions(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/17849
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I0616f3e6fb50aba271f106b05d1926fc46a53ed0
Gerrit-Change-Number: 17849
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@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] [util] allow DnsResolver to refresh DNS addresses
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17849 )
Change subject: [util] allow DnsResolver to refresh DNS addresses
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/17849
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0616f3e6fb50aba271f106b05d1926fc46a53ed0
Gerrit-Change-Number: 17849
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@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: Tue, 21 Sep 2021 21:22:43 +0000
Gerrit-HasComments: No
[kudu-CR] [util] allow DnsResolver to refresh DNS addresses
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Bankim Bhavsar,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/17849
to look at the new patch set (#2).
Change subject: [util] allow DnsResolver to refresh DNS addresses
......................................................................
[util] allow DnsResolver to refresh DNS addresses
This patch introduces functionality to the DnsResolver to refresh an
address, rather than looking it up in the cache. It does this by
removing any cached entry and performing the lookup.
This will be used in a follow-up change to refresh the address on
certain transient failures.
A new --dns_addr_resolution_override flag is also introduced for testing
purposes.
Change-Id: I0616f3e6fb50aba271f106b05d1926fc46a53ed0
---
M src/kudu/util/net/dns_resolver-test.cc
M src/kudu/util/net/dns_resolver.cc
M src/kudu/util/net/dns_resolver.h
M src/kudu/util/net/net_util.cc
M src/kudu/util/ttl_cache.h
5 files changed, 145 insertions(+), 2 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/49/17849/2
--
To view, visit http://gerrit.cloudera.org:8080/17849
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0616f3e6fb50aba271f106b05d1926fc46a53ed0
Gerrit-Change-Number: 17849
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@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] [util] allow DnsResolver to refresh DNS addresses
Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17849 )
Change subject: [util] allow DnsResolver to refresh DNS addresses
......................................................................
Patch Set 1: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17849/1/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/17849/1/src/kudu/util/net/net_util.cc@211
PS1, Line 211: port_
Okay so we are summing the override flag doesn't pass port?
--
To view, visit http://gerrit.cloudera.org:8080/17849
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0616f3e6fb50aba271f106b05d1926fc46a53ed0
Gerrit-Change-Number: 17849
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 17 Sep 2021 22:21:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] [util] allow DnsResolver to refresh DNS addresses
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17849 )
Change subject: [util] allow DnsResolver to refresh DNS addresses
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/17849/1/src/kudu/util/net/dns_resolver-test.cc
File src/kudu/util/net/dns_resolver-test.cc:
http://gerrit.cloudera.org:8080/#/c/17849/1/src/kudu/util/net/dns_resolver-test.cc@89
PS1, Line 89: lookups
> nit: lookups
Done
http://gerrit.cloudera.org:8080/#/c/17849/1/src/kudu/util/net/dns_resolver-test.cc@121
PS1, Line 121: ASSERT_FALSE
> nit: switch to ASSERT_FALSE() here -- if the container with addresses is em
Done
http://gerrit.cloudera.org:8080/#/c/17849/1/src/kudu/util/net/dns_resolver-test.cc@142
PS1, Line 142: ASSERT_OK
> Switch to ASSERT_OK here -- if ResolveAddresses() fails, there is no sense
Done
http://gerrit.cloudera.org:8080/#/c/17849/1/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/17849/1/src/kudu/util/net/net_util.cc@209
PS1, Line 209:
> nit: should this comparison be performed in a case-insensitive manner to re
Done
--
To view, visit http://gerrit.cloudera.org:8080/17849
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0616f3e6fb50aba271f106b05d1926fc46a53ed0
Gerrit-Change-Number: 17849
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@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: Mon, 20 Sep 2021 23:13:07 +0000
Gerrit-HasComments: Yes
[kudu-CR] [util] allow DnsResolver to refresh DNS addresses
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17849 )
Change subject: [util] allow DnsResolver to refresh DNS addresses
......................................................................
Patch Set 1: Verified+1
(4 comments)
http://gerrit.cloudera.org:8080/#/c/17849/1/src/kudu/util/net/dns_resolver-test.cc
File src/kudu/util/net/dns_resolver-test.cc:
http://gerrit.cloudera.org:8080/#/c/17849/1/src/kudu/util/net/dns_resolver-test.cc@89
PS1, Line 89: looksup
nit: lookups
http://gerrit.cloudera.org:8080/#/c/17849/1/src/kudu/util/net/dns_resolver-test.cc@121
PS1, Line 121: EXPECT_FALSE
nit: switch to ASSERT_FALSE() here -- if the container with addresses is empty, there is no sense to continue with further checks below (even though it's turns to be an empty for() loop)?
http://gerrit.cloudera.org:8080/#/c/17849/1/src/kudu/util/net/dns_resolver-test.cc@142
PS1, Line 142: EXPECT_OK
Switch to ASSERT_OK here -- if ResolveAddresses() fails, there is no sense to continue and try to call validate_addrs() below?
http://gerrit.cloudera.org:8080/#/c/17849/1/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/17849/1/src/kudu/util/net/net_util.cc@209
PS1, Line 209: host_and_addr[0] == host_
nit: should this comparison be performed in a case-insensitive manner to reflect that DNS names are note case-sensitive (at least when using ASCII symbols)?
--
To view, visit http://gerrit.cloudera.org:8080/17849
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0616f3e6fb50aba271f106b05d1926fc46a53ed0
Gerrit-Change-Number: 17849
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@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: Sat, 18 Sep 2021 23:54:12 +0000
Gerrit-HasComments: Yes
[kudu-CR] [util] allow DnsResolver to refresh DNS addresses
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17849 )
Change subject: [util] allow DnsResolver to refresh DNS addresses
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/17849/1/src/kudu/util/net/net_util.cc
File src/kudu/util/net/net_util.cc:
http://gerrit.cloudera.org:8080/#/c/17849/1/src/kudu/util/net/net_util.cc@211
PS1, Line 211: port_
> Okay so we are summing the override flag doesn't pass port?
Yeah, that's mentioned in the flag description. It's ignored otherwise.
--
To view, visit http://gerrit.cloudera.org:8080/17849
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0616f3e6fb50aba271f106b05d1926fc46a53ed0
Gerrit-Change-Number: 17849
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 18 Sep 2021 04:34:06 +0000
Gerrit-HasComments: Yes