Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make DnsResolver better #1443

Closed
wants to merge 15 commits into from
Closed

make DnsResolver better #1443

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
26fdf6f
guard for DnsResolver, at most one request for each (host,port,family…
kedixa Dec 12, 2023
6031684
add address family to DnsCache's key
kedixa Dec 12, 2023
92a93ae
make lines shorter
kedixa Dec 12, 2023
3a68910
Merge branch 'master' into dev
kedixa Dec 12, 2023
b849339
WFDnsResolver use release_guard_safe
kedixa Dec 12, 2023
0eae222
remove null pointer check in DnsCache::get_inner
kedixa Dec 12, 2023
9ed9cdf
set delayed only when needed
kedixa Dec 13, 2023
c19ab2f
rename chost to meaningful cache_host
kedixa Dec 13, 2023
8c8d826
parse resolv.conf according to dns_server_params.address_family
kedixa Dec 13, 2023
9028603
optimize __get_cache_host
kedixa Dec 13, 2023
83d3a9b
exactly one request at same time
kedixa Dec 14, 2023
49c02cd
rename __default_family to __dns_server_family
kedixa Dec 14, 2023
7998b66
make DnsCache::get_inner simple
kedixa Dec 15, 2023
0dd6b1c
move delayed flag into addrinfo::ai_flags
kedixa Dec 15, 2023
21b92ea
revert changes of WFGlobal.cc
kedixa Dec 18, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 24 additions & 5 deletions src/manager/DnsCache.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,34 @@

#define GET_CURRENT_SECOND std::chrono::duration_cast<std::chrono::seconds>(std::chrono::steady_clock::now().time_since_epoch()).count()

#define CONFIDENT_INC 10
#define TTL_INC 10
#define CONFIDENT_INC 5
#define TTL_INC 5

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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个delayed应该用指针的

{
int64_t cur_time = GET_CURRENT_SECOND;
std::lock_guard<std::mutex> lock(mutex_);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里的cache_pool_返回const DnsHandle *，但下面又用const_cast去改value，这里多出的的const设计感觉很突兀

const DnsHandle *handle = cache_pool_.get(host_port);

delayed = false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个为啥在锁里。


if (handle)
{
switch (type)
{
case GET_TYPE_TTL:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里不要用switch了，把代码合并一下，也不需要CONFIDENT_INC了，反正都是5秒。

if (cur_time > handle->value.expire_time)
{
const_cast<DnsHandle *>(handle)->value.expire_time += TTL_INC;
if (!handle->value.expire_delayed)
{
DnsHandle *h = const_cast<DnsHandle *>(handle);
h->value.expire_time += TTL_INC;
h->value.expire_delayed = true;

delayed = cur_time <= h->value.expire_time;
}

cache_pool_.release(handle);
return NULL;
}
Expand All @@ -49,7 +60,15 @@ const DnsCache::DnsHandle *DnsCache::get_inner(const HostPort& host_port, int ty
case GET_TYPE_CONFIDENT:
if (cur_time > handle->value.confident_time)
{
const_cast<DnsHandle *>(handle)->value.confident_time += CONFIDENT_INC;
if (!handle->value.confident_delayed)
{
DnsHandle *h = const_cast<DnsHandle *>(handle);
h->value.confident_time += CONFIDENT_INC;
h->value.confident_delayed = true;

delayed = cur_time <= h->value.confident_time;
}

cache_pool_.release(handle);
return NULL;
}
Expand Down
33 changes: 20 additions & 13 deletions src/manager/DnsCache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ struct DnsCacheValue
struct addrinfo *addrinfo;
int64_t confident_time;
int64_t expire_time;
bool confident_delayed;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

加了一个指针的大小，有一点浪费了。

bool expire_delayed;
};

// RAII: NO. Release handle by user
Expand Down Expand Up @@ -62,34 +64,38 @@ class DnsCache
return get(std::string(host), port);
}

const DnsHandle *get_ttl(const HostPort& host_port)
const DnsHandle *get_ttl(const HostPort& host_port, bool& delayed)
{
return get_inner(host_port, GET_TYPE_TTL);
return get_inner(host_port, GET_TYPE_TTL, delayed);
}

const DnsHandle *get_ttl(const std::string& host, unsigned short port)
const DnsHandle *get_ttl(const std::string& host, unsigned short port,
bool& delayed)
{
return get_ttl(HostPort(host, port));
return get_ttl(HostPort(host, port), delayed);
}

const DnsHandle *get_ttl(const char *host, unsigned short port)
const DnsHandle *get_ttl(const char *host, unsigned short port,
bool& delayed)
{
return get_ttl(std::string(host), port);
return get_ttl(std::string(host), port, delayed);
}

const DnsHandle *get_confident(const HostPort& host_port)
const DnsHandle *get_confident(const HostPort& host_port, bool& delayed)
{
return get_inner(host_port, GET_TYPE_CONFIDENT);
return get_inner(host_port, GET_TYPE_CONFIDENT, delayed);
}

const DnsHandle *get_confident(const std::string& host, unsigned short port)
const DnsHandle *get_confident(const std::string& host, unsigned short port,
bool& delayed)
{
return get_confident(HostPort(host, port));
return get_confident(HostPort(host, port), delayed);
}

const DnsHandle *get_confident(const char *host, unsigned short port)
const DnsHandle *get_confident(const char *host, unsigned short port,
bool &delayed)
{
return get_confident(std::string(host), port);
return get_confident(std::string(host), port, delayed);
}

const DnsHandle *put(const HostPort& host_port,
Expand Down Expand Up @@ -132,7 +138,8 @@ class DnsCache
}

private:
const DnsHandle *get_inner(const HostPort& host_port, int type);
const DnsHandle *get_inner(const HostPort& host_port, int type,
bool &delayed);

std::mutex mutex_;

Expand Down
60 changes: 44 additions & 16 deletions src/nameservice/WFDnsResolver.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,16 @@ static ThreadDnsTask *__create_thread_dns_task(const std::string& host,
return task;
}

static std::string __get_guard_name(const char *host, unsigned short port,
int address_family)
{
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));
Copy link
Contributor

@Barenboim Barenboim Dec 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个命名和dns cache里统一一下吧。例如："0xfffff.github.com4:80"，其中0xfffff是这个dns resolver的地址。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

往ResolverTask里传递一个Resolver地址，较为奇怪；或许可用DnsClient地址，虽然也较为奇怪

return guard_name;
}

void WFResolverTask::dispatch()
{
const ParsedURI& uri = ns_params_.uri;
Expand All @@ -355,11 +365,12 @@ void WFResolverTask::dispatch()
DnsCache *dns_cache = WFGlobal::get_dns_cache();
const DnsCache::DnsHandle *addr_handle;
std::string hostname = host_;
bool dns_delayed;

if (ns_params_.retry_times == 0)
addr_handle = dns_cache->get_ttl(hostname, port_);
addr_handle = dns_cache->get_ttl(hostname, port_, dns_delayed);
else
addr_handle = dns_cache->get_confident(hostname, port_);
addr_handle = dns_cache->get_confident(hostname, port_, dns_delayed);

if (addr_handle)
{
Expand Down Expand Up @@ -441,6 +452,22 @@ void WFResolverTask::dispatch()
}
}

if (!in_guard_ && !dns_delayed)
{
std::string guard_name = __get_guard_name(host_, port_,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

family作为第一个参数吧。

ep_params_.address_family);
WFTimerTask *timer = WFTaskFactory::create_timer_task(0, 0, nullptr);
auto *guard = WFTaskFactory::create_guard(guard_name, timer);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里不会用auto，直接写WFConditional。


series_of(this)->push_front(this);
series_of(this)->push_front(guard);
in_guard_ = true;

this->set_has_next();
this->subtask_done();
return;
}

WFDnsClient *client = WFGlobal::get_dns_client();
if (client)
{
Expand Down Expand Up @@ -519,12 +546,7 @@ SubTask *WFResolverTask::done()
SeriesWork *series = series_of(this);

if (!has_next_)
{
if (this->callback)
this->callback(this);

delete this;
}
task_callback();
else
has_next_ = false;

Expand Down Expand Up @@ -596,10 +618,7 @@ void WFResolverTask::dns_single_callback(void *net_dns_task)
this->error = dns_task->get_error();
}

if (this->callback)
this->callback(this);

delete this;
task_callback();
}

void WFResolverTask::dns_partial_callback(void *net_dns_task)
Expand Down Expand Up @@ -659,10 +678,7 @@ void WFResolverTask::dns_parallel_callback(const void *parallel)

delete[] c4;

if (this->callback)
this->callback(this);

delete this;
task_callback();
}

void WFResolverTask::thread_dns_callback(void *thrd_dns_task)
Expand All @@ -681,6 +697,18 @@ void WFResolverTask::thread_dns_callback(void *thrd_dns_task)
this->error = dns_task->get_error();
}

task_callback();
}

void WFResolverTask::task_callback()
{
if (in_guard_)
{
std::string guard_name = __get_guard_name(host_, port_,
ep_params_.address_family);
WFTaskFactory::release_guard(guard_name);
}

if (this->callback)
this->callback(this);

Expand Down
5 changes: 5 additions & 0 deletions src/nameservice/WFDnsResolver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class WFResolverTask : public WFRouterTask
dns_ttl_default_ = dns_ttl_default;
dns_ttl_min_ = dns_ttl_min;
has_next_ = false;
in_guard_ = false;
}

WFResolverTask(const struct WFNSParams *ns_params,
Expand All @@ -46,6 +47,7 @@ class WFResolverTask : public WFRouterTask
ns_params_(*ns_params)
{
has_next_ = false;
in_guard_ = false;
}

protected:
Expand All @@ -62,6 +64,8 @@ class WFResolverTask : public WFRouterTask
unsigned int ttl_default,
unsigned int ttl_min);

void task_callback();

protected:
struct WFNSParams ns_params_;
unsigned int dns_ttl_default_;
Expand All @@ -72,6 +76,7 @@ class WFResolverTask : public WFRouterTask
const char *host_;
unsigned short port_;
bool has_next_;
bool in_guard_;
};

class WFDnsResolver : public WFNSPolicy
Expand Down