dns: adjust DNS refresh rate respecting to record TTL by yxue · Pull Request #6975 · envoyproxy/envoy

yxue · 2019-05-16T22:38:53Z

Description: Querying record TTL when resolving the host name and adjust DNS refresh rate respecting to record TTL.
Risk Level: Medium
Testing: unit test
Docs Changes: https://github.com/envoyproxy/envoy/pull/6975/files#diff-cc6a4f8b8abf46253b0f3e14392ca6e9

Release Notes: https://github.com/envoyproxy/envoy/pull/6975/files#diff-d8d3e33358b55e6c0466dd1bb5f40c79
Fixes #Issue: #6876

Signed-off-by: Yan Xue <yxyan@google.com>

mattklein123 · 2019-05-16T23:10:47Z

@crazyxy thanks for this. I haven't reviewed this, but a quick drive by that this definitely needs to be configurable and also needs documentation and release notes. Thank you!

Signed-off-by: Yan Xue <yxyan@google.com>

mattklein123

Thanks for working on this. 2 big questions to get started.

/wait

include/envoy/network/address.h

source/common/network/dns_impl.cc

Signed-off-by: Yan Xue <yxyan@google.com>

yxue · 2019-05-17T22:02:48Z

@crazyxy thanks for this. I haven't reviewed this, but a quick drive by that this definitely needs to be configurable and also needs documentation and release notes. Thank you!

Add configurable, documentation and release notes.

mattklein123

Thanks. I still would like to hear from @htuch on the general approach to this change. Given how scary this change is IMO, I wonder if we should runtime guard this new code and allow reverting to the old path (or maybe only do the new code version if the user requests TTL usage)? @htuch WDYT?

/wait

source/common/json/config_schemas.cc

Signed-off-by: Yan Xue <yxyan@google.com>

lambdai

Have some comments in case you select not to change at upstream

include/envoy/network/dns.h

source/common/network/dns_impl.cc

source/extensions/clusters/redis/redis_cluster.cc

source/common/network/dns_impl.cc

include/envoy/network/dns.h

source/common/network/dns_impl.cc

Signed-off-by: Yan Xue <yxyan@google.com>

stale · 2019-05-29T03:24:59Z

This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

stale · 2019-06-05T04:28:11Z

This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

htuch · 2019-06-07T19:29:56Z

Looks like c-ares/c-ares#259 has some progress..

yxue · 2019-06-07T20:18:09Z

Looks like c-ares/c-ares#259 has some progress..

Not actually. c-ares/c-ares#257 hasn't been merged. 😟

source/common/network/dns_impl.h

Signed-off-by: crazyxy <yxyan@google.com>

yxue · 2019-06-25T13:11:19Z

@mattklein123 @htuch I created another small pr to switch to ares_getaddrinfo. I will send the second one after the first one is merged. PTAL, thanks!

Upgrade c-ares. Part of #6975 Description: Upgrade c-ares. Switch from ares_gethostbyname to ares_getaddrinfo Risk Level: Med Testing: Unit test Docs Changes: N/A Release Notes: Signed-off-by: crazyxy <yxyan@google.com>

Signed-off-by: crazyxy <yxyan@google.com>

yxue · 2019-06-28T04:34:08Z

@mattklein123 @htuch I merged the pr from the upstream to upgrade c-ares and updated this pr. Could you please take a look? Thanks!

htuch

Implementation LGTM, I'll leave coverage of tests and final merge to @mattklein123. Thanks!

source/common/upstream/logical_dns_cluster.cc

source/common/upstream/strict_dns_cluster.cc

Signed-off-by: crazyxy <yxyan@google.com>

mattklein123

Thanks, LGTM with some small comments. Great work!

/wait

docs/root/intro/version_history.rst

source/common/network/dns_impl.cc

test/test_common/utility.cc

test/test_common/utility.h

test/common/network/dns_impl_test.cc

Signed-off-by: crazyxy <yxyan@google.com>

Signed-off-by: Yan Xue <yxyan@google.com>

mattklein123

Thank you!

Adjust DNS refresh rate respecting to record TTL

5853fc7

Signed-off-by: Yan Xue <yxyan@google.com>

mattklein123 assigned htuch and mattklein123 May 16, 2019

Add documentation and release notes

77f7f8f

Signed-off-by: Yan Xue <yxyan@google.com>

mattklein123 requested changes May 17, 2019

View reviewed changes

include/envoy/network/address.h Outdated Show resolved Hide resolved

source/common/network/dns_impl.cc Outdated Show resolved Hide resolved

repokitteh-read-only bot added the waiting label May 17, 2019

yxue added 5 commits May 17, 2019 12:40

Remove ttl() from intance interface

c5eaa46

Signed-off-by: Yan Xue <yxyan@google.com>

Add respect_dns_ttl option

8d97f4f

Signed-off-by: Yan Xue <yxyan@google.com>

Update docs and release note

f266965

Signed-off-by: Yan Xue <yxyan@google.com>

clang-tidy

82fd836

Signed-off-by: Yan Xue <yxyan@google.com>

Spelling check

a086565

Signed-off-by: Yan Xue <yxyan@google.com>

repokitteh-read-only bot removed the waiting label May 17, 2019

mattklein123 requested changes May 19, 2019

View reviewed changes

source/common/json/config_schemas.cc Outdated Show resolved Hide resolved

repokitteh-read-only bot added the waiting label May 19, 2019

Remove changes to V1 code

501d092

Signed-off-by: Yan Xue <yxyan@google.com>

repokitteh-read-only bot removed the waiting label May 20, 2019

lambdai reviewed May 20, 2019

View reviewed changes

yxue added 2 commits May 20, 2019 12:03

Code review

df48b1c

Signed-off-by: Yan Xue <yxyan@google.com>

Code review

df3539d

Signed-off-by: Yan Xue <yxyan@google.com>

mattklein123 added the waiting:any label May 21, 2019

repokitteh-read-only bot added waiting and removed waiting:any labels May 22, 2019

stale bot added the stale stalebot believes this issue/PR has not been touched recently label May 29, 2019

stale bot closed this Jun 5, 2019

htuch reviewed Jun 24, 2019

View reviewed changes

source/common/network/dns_impl.h Show resolved Hide resolved

resolve comments

27fa87e

Signed-off-by: crazyxy <yxyan@google.com>

repokitteh-read-only bot removed the waiting label Jun 24, 2019

yxue added 2 commits June 24, 2019 16:33

Fix ci failure

d0393c7

Signed-off-by: crazyxy <yxyan@google.com>

merge upstream and resolve conflict

699d51c

Signed-off-by: crazyxy <yxyan@google.com>

yxue mentioned this pull request Jun 25, 2019

dns: switch from ares_gethostbyname to ares_getaddrinfo #7395

Merged

mattklein123 added the waiting label Jun 25, 2019

htuch mentioned this pull request Jun 26, 2019

[WIP] dns: add support for SRV records in DNS lookup #6379

Closed

Merge remote-tracking branch 'upstream/master' into feature/dnsttl-2

f789752

Signed-off-by: crazyxy <yxyan@google.com>

repokitteh-read-only bot removed the waiting label Jun 27, 2019

yxue added 2 commits June 27, 2019 16:03

remove unused code

0fff440

Signed-off-by: crazyxy <yxyan@google.com>

remove empty line

f62142e

Signed-off-by: crazyxy <yxyan@google.com>

htuch reviewed Jun 28, 2019

View reviewed changes

source/common/upstream/logical_dns_cluster.cc Show resolved Hide resolved

source/common/upstream/strict_dns_cluster.cc Outdated Show resolved Hide resolved

Use explict type

2caa04a

Signed-off-by: crazyxy <yxyan@google.com>

mattklein123 requested changes Jun 29, 2019

View reviewed changes

repokitteh-read-only bot added the waiting label Jun 29, 2019

Resolve comments

c9f63dd

Signed-off-by: crazyxy <yxyan@google.com>

repokitteh-read-only bot removed the waiting label Jul 1, 2019

mattklein123 added the waiting label Jul 1, 2019

resolve comments

3cb5447

Signed-off-by: Yan Xue <yxyan@google.com>

repokitteh-read-only bot removed the waiting label Jul 1, 2019

yxue added 3 commits July 1, 2019 10:08

Remove reference data member

1cf956c

Signed-off-by: Yan Xue <yxyan@google.com>

revert change

c47168b

Signed-off-by: Yan Xue <yxyan@google.com>

Make data member reference

166c062

Signed-off-by: Yan Xue <yxyan@google.com>

mattklein123 approved these changes Jul 1, 2019

View reviewed changes

mattklein123 merged commit e0e7628 into envoyproxy:master Jul 1, 2019

yxue deleted the feature/dnsttl-2 branch July 14, 2019 02:25

Conversation

yxue commented May 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattklein123 commented May 16, 2019

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

yxue commented May 17, 2019

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lambdai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stale bot commented May 29, 2019

Uh oh!

stale bot commented Jun 5, 2019

Uh oh!

htuch commented Jun 7, 2019

Uh oh!

yxue commented Jun 7, 2019

Uh oh!

Uh oh!

yxue commented Jun 25, 2019

Uh oh!

yxue commented Jun 28, 2019

Uh oh!

htuch left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yxue commented May 16, 2019 •

edited

Loading