make DnsResolver better #1443
Closed
make DnsResolver better #1443
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…) tuple at the same time
Barenboim
reviewed
Dec 12, 2023
src/manager/DnsCache.cc
Outdated
| { | ||
| int64_t cur_time = GET_CURRENT_SECOND; | ||
| std::lock_guard<std::mutex> lock(mutex_); | ||
| const DnsHandle *handle = cache_pool_.get(host_port); | ||
|
|
||
| delayed = false; |
Barenboim
reviewed
Dec 12, 2023
src/manager/DnsCache.cc
Outdated
|
|
||
| const DnsCache::DnsHandle *DnsCache::get_inner(const HostPort& host_port, int type) | ||
| const DnsCache::DnsHandle *DnsCache::get_inner(const HostPort& host_port, | ||
| int type, bool& delayed) |
Barenboim
reviewed
Dec 12, 2023
src/nameservice/WFDnsResolver.cc
Outdated
| @@ -441,6 +452,22 @@ void WFResolverTask::dispatch() | |||
| } | |||
| } | |||
|
|
|||
| if (!in_guard_ && !dns_delayed) | |||
| { | |||
| std::string guard_name = __get_guard_name(host_, port_, | |||
Barenboim
reviewed
Dec 12, 2023
src/nameservice/WFDnsResolver.cc
Outdated
| std::string guard_name = __get_guard_name(host_, port_, | ||
| ep_params_.address_family); | ||
| WFTimerTask *timer = WFTaskFactory::create_timer_task(0, 0, nullptr); | ||
| auto *guard = WFTaskFactory::create_guard(guard_name, timer); |
There was a problem hiding this comment.
这里不会用auto，直接写WFConditional。
Barenboim
reviewed
Dec 12, 2023
src/manager/DnsCache.h
Outdated
| @@ -35,6 +35,8 @@ struct DnsCacheValue | |||
| struct addrinfo *addrinfo; | |||
| int64_t confident_time; | |||
| int64_t expire_time; | |||
| bool confident_delayed; | |||
Barenboim
reviewed
Dec 12, 2023
src/nameservice/WFDnsResolver.cc
Outdated
| std::string guard_name("dns:"); | ||
| guard_name.append(host ? host : "").append(":"); | ||
| guard_name.append(std::to_string(port)).append(":"); | ||
| guard_name.append(std::to_string(address_family)); |
There was a problem hiding this comment.
这个命名和dns cache里统一一下吧。例如："0xfffff.github.com4:80"，其中0xfffff是这个dns resolver的地址。
There was a problem hiding this comment.
往ResolverTask里传递一个Resolver地址，较为奇怪；或许可用DnsClient地址，虽然也较为奇怪
Barenboim
reviewed
Dec 12, 2023
src/manager/DnsCache.cc
Outdated
| h->value.expire_time += TTL_INC; | ||
| h->value.expire_delayed = true; | ||
|
|
||
| if (delayed) |
Barenboim
reviewed
Dec 12, 2023
src/manager/DnsCache.cc
Outdated
| { | ||
| int64_t cur_time = GET_CURRENT_SECOND; | ||
| *delayed = false; |
There was a problem hiding this comment.
delayed是在 return NULL的时候，才有意义吗？好像return一个正常handler的时候，这个值是无意义的。这样的话，是不是可以在return NULL的时候才给这个指针内容赋值呢？
Barenboim
reviewed
Dec 13, 2023
src/nameservice/WFDnsResolver.cc
Outdated
| switch (family) | ||
| { | ||
| case AF_UNSPEC: | ||
| cache_host.push_back('*'); |
There was a problem hiding this comment.
这里直接return hostname+'*'就可以了吧？
kedixa
commented
Dec 14, 2023
| @@ -27,7 +27,7 @@ | |||
| #define TTL_INC 5 | |||
|
|
|||
| const DnsCache::DnsHandle *DnsCache::get_inner(const HostPort& host_port, | |||
| int type, bool *delayed) | |||
| int type) | |||
| { | |||
| int64_t cur_time = GET_CURRENT_SECOND; | |||
| std::lock_guard<std::mutex> lock(mutex_); | |||
There was a problem hiding this comment.
这里的cache_pool_返回const DnsHandle *，但下面又用const_cast去改value，这里多出的的const设计感觉很突兀
Barenboim
reviewed
Dec 14, 2023
src/manager/DnsCache.cc
Outdated
| @@ -39,7 +40,13 @@ const DnsCache::DnsHandle *DnsCache::get_inner(const HostPort& host_port, int ty | |||
| case GET_TYPE_TTL: | |||
There was a problem hiding this comment.
这里不要用switch了，把代码合并一下，也不需要CONFIDENT_INC了，反正都是5秒。
Barenboim
reviewed
Dec 14, 2023
src/manager/WFGlobal.cc
Outdated
| @@ -478,7 +478,46 @@ inline ExecQueue *__ExecManager::get_exec_queue(const std::string& queue_name) | |||
| return queue; | |||
| } | |||
|
|
|||
| static void __dns_server_family(bool *v4, bool *v6) | |||
There was a problem hiding this comment.
这个函数最好不要假设外部已经把v4，v6都初始化为false了。
|
分成两个PR重新提交吧。这个有点太乱了。 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.