This is an automated email from the ASF dual-hosted git repository.
oknet pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push:
new 2906f16 Optimize: rewrite getbyname_imm and getSRVbyname_imm as
wrappers for getby
2906f16 is described below
commit 2906f166c0f02d5efd731819a121092d5c8fae49
Author: Oknet Xu <xuc...@skyguard.com.cn>
AuthorDate: Sat Feb 2 23:17:55 2019 +0800
Optimize: rewrite getbyname_imm and getSRVbyname_imm as wrappers for getby
---
iocore/hostdb/HostDB.cc | 291 +++++++++++++++-----------------------
iocore/hostdb/I_HostDBProcessor.h | 17 +--
proxy/http/HttpSM.cc | 8 +-
3 files changed, 124 insertions(+), 192 deletions(-)
diff --git a/iocore/hostdb/HostDB.cc b/iocore/hostdb/HostDB.cc
index fbf410c..5b81c85 100644
--- a/iocore/hostdb/HostDB.cc
+++ b/iocore/hostdb/HostDB.cc
@@ -605,57 +605,77 @@ HostDBContinuation::insert(unsigned int attl)
// Get an entry by either name or IP
//
Action *
-HostDBProcessor::getby(Continuation *cont, const char *hostname, int len,
sockaddr const *ip, bool aforce_dns,
- HostResStyle host_res_style, int dns_lookup_timeout)
+HostDBProcessor::getby(Continuation *cont, cb_process_result_pfn
cb_process_result, HostDBHash &hash, Options const &opt)
{
- HostDBHash hash;
+ bool trylock = (cb_process_result == nullptr);
+ bool force_dns = false;
EThread *thread = this_ethread();
Ptr<ProxyMutex> mutex = thread->mutex;
ip_text_buffer ipb;
+ if (opt.flags & HOSTDB_FORCE_DNS_ALWAYS) {
+ force_dns = true;
+ } else if (opt.flags & HOSTDB_FORCE_DNS_RELOAD) {
+ force_dns = hostdb_re_dns_on_reload;
+ if (force_dns) {
+ HOSTDB_INCREMENT_DYN_STAT(hostdb_re_dns_on_reload_stat);
+ }
+ }
+
HOSTDB_INCREMENT_DYN_STAT(hostdb_total_lookups_stat);
- if ((!hostdb_enable || (hostname && !*hostname)) ||
(hostdb_disable_reverse_lookup && ip)) {
- MUTEX_TRY_LOCK(lock, cont->mutex, thread);
- if (!lock.is_locked()) {
- goto Lretry;
+ if (!hostdb_enable || // if the HostDB
is disabled,
+ (hash.host_name && !*hash.host_name) || // or host_name
is empty string
+ (hostdb_disable_reverse_lookup && hash.ip.isValid())) { // or try to
lookup by ip address when the reverse lookup disabled
+ if (cb_process_result) {
+ (cont->*cb_process_result)(nullptr);
+ } else {
+ MUTEX_TRY_LOCK(lock, cont->mutex, thread);
+ if (!lock.is_locked()) {
+ goto Lretry;
+ }
+ cont->handleEvent(EVENT_HOST_DB_LOOKUP, nullptr);
}
- cont->handleEvent(EVENT_HOST_DB_LOOKUP, nullptr);
return ACTION_RESULT_DONE;
}
- // Load the hash data.
- hash.set_host(hostname, hostname ? (len ? len : strlen(hostname)) : 0);
- hash.ip.assign(ip);
- hash.port = ip ? ats_ip_port_host_order(ip) : 0;
- hash.db_mark = db_mark_for(host_res_style);
- hash.refresh();
-
// Attempt to find the result in-line, for level 1 hits
- //
- if (!aforce_dns) {
+ if (!force_dns) {
MUTEX_TRY_LOCK(lock, cont->mutex, thread);
bool loop = lock.is_locked();
while (loop) {
loop = false; // Only loop on explicit set for retry.
// find the partition lock
- //
Ptr<ProxyMutex> bucket_mutex =
hostDB.refcountcache->lock_for_key(hash.hash.fold());
MUTEX_TRY_LOCK(lock2, bucket_mutex, thread);
-
+ if (!lock2.is_locked() && !trylock) {
+ // Refer to: [TS-374](http://issues.apache.org/jira/browse/TS-374)
+ lock2.acquire(thread);
+ }
if (lock2.is_locked()) {
// If we can get the lock and a level 1 probe succeeds, return
- Ptr<HostDBInfo> r = probe(bucket_mutex, hash, aforce_dns);
+ Ptr<HostDBInfo> r = probe(bucket_mutex, hash, false);
if (r) {
- if (r->is_failed() && hostname) {
- loop = check_for_retry(hash.db_mark, host_res_style);
+ // fail, see if we should retry with alternate
+ if (hash.db_mark != HOSTDB_MARK_SRV && r->is_failed() &&
hash.host_name) {
+ loop = check_for_retry(hash.db_mark, opt.host_res_style);
}
if (!loop) {
// No retry -> final result. Return it.
- Debug("hostdb", "immediate answer for %s",
- hostname ? hostname : ats_is_ip(ip) ? ats_ip_ntop(ip, ipb,
sizeof ipb) : "<null>");
+ if (hash.db_mark == HOSTDB_MARK_SRV) {
+ Debug("hostdb", "immediate SRV answer for %.*s from hostdb",
hash.host_len, hash.host_name);
+ Debug("dns_srv", "immediate SRV answer for %.*s from hostdb",
hash.host_len, hash.host_name);
+ } else if (hash.host_name) {
+ Debug("hostdb", "immediate answer for %.*s", hash.host_len,
hash.host_name);
+ } else {
+ Debug("hostdb", "immediate answer for %s", hash.ip.isValid() ?
hash.ip.toString(ipb, sizeof ipb) : "<null>");
+ }
HOSTDB_INCREMENT_DYN_STAT(hostdb_total_hits_stat);
- reply_to_cont(cont, r.get());
+ if (cb_process_result) {
+ (cont->*cb_process_result)(r.get());
+ } else {
+ reply_to_cont(cont, r.get());
+ }
return ACTION_RESULT_DONE;
}
hash.refresh(); // only on reloop, because we've changed the family.
@@ -663,19 +683,28 @@ HostDBProcessor::getby(Continuation *cont, const char
*hostname, int len, sockad
}
}
}
- Debug("hostdb", "delaying force %d answer for %s", aforce_dns,
- hostname ? hostname : ats_is_ip(ip) ? ats_ip_ntop(ip, ipb, sizeof ipb)
: "<null>");
+ if (hash.db_mark == HOSTDB_MARK_SRV) {
+ Debug("hostdb", "delaying (force=%d) SRV answer for %.*s [timeout = %d]",
force_dns, hash.host_len, hash.host_name,
+ opt.timeout);
+ Debug("dns_srv", "delaying (force=%d) SRV answer for %.*s [timeout = %d]",
force_dns, hash.host_len, hash.host_name,
+ opt.timeout);
+ } else if (hash.host_name) {
+ Debug("hostdb", "delaying (force=%d) answer for %.*s [timeout %d]",
force_dns, hash.host_len, hash.host_name, opt.timeout);
+ } else {
+ Debug("hostdb", "delaying (force=%d) answer for %s [timeout %d]",
force_dns,
+ hash.ip.isValid() ? hash.ip.toString(ipb, sizeof ipb) : "<null>",
opt.timeout);
+ }
Lretry:
// Otherwise, create a continuation to do a deeper probe in the background
//
HostDBContinuation *c = hostDBContAllocator.alloc();
- HostDBContinuation::Options opt;
- opt.timeout = dns_lookup_timeout;
- opt.force_dns = aforce_dns;
- opt.cont = cont;
- opt.host_res_style = host_res_style;
- c->init(hash, opt);
+ HostDBContinuation::Options copt;
+ copt.timeout = opt.timeout;
+ copt.force_dns = force_dns;
+ copt.cont = cont;
+ copt.host_res_style = (hash.db_mark == HOSTDB_MARK_SRV) ? HOST_RES_NONE :
opt.host_res_style;
+ c->init(hash, copt);
SET_CONTINUATION_HANDLER(c,
(HostDBContHandler)&HostDBContinuation::probeEvent);
thread->schedule_in(c, MUTEX_RETRY_DELAY);
@@ -688,188 +717,96 @@ Lretry:
Action *
HostDBProcessor::getbyname_re(Continuation *cont, const char *ahostname, int
len, Options const &opt)
{
- bool force_dns = false;
- EThread *thread = this_ethread();
- ProxyMutex *mutex = thread->mutex.get();
+ HostDBHash hash;
- if (opt.flags & HOSTDB_FORCE_DNS_ALWAYS) {
- force_dns = true;
- } else if (opt.flags & HOSTDB_FORCE_DNS_RELOAD) {
- force_dns = (hostdb_re_dns_on_reload ? true : false);
- if (force_dns) {
- HOSTDB_INCREMENT_DYN_STAT(hostdb_re_dns_on_reload_stat);
- }
- }
- return getby(cont, ahostname, len, nullptr, force_dns, opt.host_res_style,
opt.timeout);
+ ink_assert(nullptr != ahostname);
+
+ // Load the hash data.
+ hash.set_host(ahostname, ahostname ? (len ? len : strlen(ahostname)) : 0);
+ // Leave hash.ip invalid
+ hash.port = 0;
+ hash.db_mark = db_mark_for(opt.host_res_style);
+ hash.refresh();
+
+ return getby(cont, nullptr, hash, opt);
}
Action *
HostDBProcessor::getbynameport_re(Continuation *cont, const char *ahostname,
int len, Options const &opt)
{
- bool force_dns = false;
- EThread *thread = this_ethread();
- ProxyMutex *mutex = thread->mutex.get();
+ HostDBHash hash;
- if (opt.flags & HOSTDB_FORCE_DNS_ALWAYS) {
- force_dns = true;
- } else if (opt.flags & HOSTDB_FORCE_DNS_RELOAD) {
- force_dns = (hostdb_re_dns_on_reload ? true : false);
- if (force_dns) {
- HOSTDB_INCREMENT_DYN_STAT(hostdb_re_dns_on_reload_stat);
- }
- }
- sockaddr sa;
- ats_ip4_set(&sa, INADDR_ANY, htons(opt.port));
- return getby(cont, ahostname, len, &sa, force_dns, opt.host_res_style,
opt.timeout);
+ ink_assert(nullptr != ahostname);
+
+ // Load the hash data.
+ hash.set_host(ahostname, ahostname ? (len ? len : strlen(ahostname)) : 0);
+ // Leave hash.ip invalid
+ hash.port = opt.port;
+ hash.db_mark = db_mark_for(opt.host_res_style);
+ hash.refresh();
+
+ return getby(cont, nullptr, hash, opt);
}
-/* Support SRV records */
+// Lookup Hostinfo by addr
Action *
-HostDBProcessor::getSRVbyname_imm(Continuation *cont, process_srv_info_pfn
process_srv_info, const char *hostname, int len,
- Options const &opt)
+HostDBProcessor::getbyaddr_re(Continuation *cont, sockaddr const *aip)
{
- ink_assert(cont->mutex->thread_holding == this_ethread());
- bool force_dns = false;
- EThread *thread = cont->mutex->thread_holding;
- ProxyMutex *mutex = thread->mutex.get();
-
- ink_assert(nullptr != hostname);
-
- if (opt.flags & HOSTDB_FORCE_DNS_ALWAYS) {
- force_dns = true;
- } else if (opt.flags & HOSTDB_FORCE_DNS_RELOAD) {
- force_dns = (hostdb_re_dns_on_reload ? true : false);
- if (force_dns) {
- HOSTDB_INCREMENT_DYN_STAT(hostdb_re_dns_on_reload_stat);
- }
- }
-
HostDBHash hash;
- HOSTDB_INCREMENT_DYN_STAT(hostdb_total_lookups_stat);
+ ink_assert(nullptr != aip);
- if (!hostdb_enable || !*hostname) {
- (cont->*process_srv_info)(nullptr);
- return ACTION_RESULT_DONE;
- }
+ HostDBProcessor::Options opt;
+ opt.host_res_style = HOST_RES_NONE;
- hash.host_name = hostname;
- hash.host_len = len ? len : strlen(hostname);
- hash.port = 0;
- hash.db_mark = HOSTDB_MARK_SRV;
+ // Leave hash.host_name as nullptr
+ hash.ip.assign(aip);
+ hash.port = ats_ip_port_host_order(aip);
+ hash.db_mark = db_mark_for(opt.host_res_style);
hash.refresh();
- // Attempt to find the result in-line, for level 1 hits
- if (!force_dns) {
- // find the partition lock
- Ptr<ProxyMutex> bucket_mutex =
hostDB.refcountcache->lock_for_key(hash.hash.fold());
- MUTEX_TRY_LOCK(lock, bucket_mutex, thread);
-
- // If we can get the lock and a level 1 probe succeeds, return
- if (lock.is_locked()) {
- Ptr<HostDBInfo> r = probe(bucket_mutex, hash, false);
- if (r) {
- Debug("hostdb", "immediate SRV answer for %s from hostdb", hostname);
- Debug("dns_srv", "immediate SRV answer for %s from hostdb", hostname);
- HOSTDB_INCREMENT_DYN_STAT(hostdb_total_hits_stat);
- (cont->*process_srv_info)(r.get());
- return ACTION_RESULT_DONE;
- }
- }
- }
+ return getby(cont, nullptr, hash, opt);
+}
- Debug("dns_srv", "delaying (force=%d) SRV answer for %.*s [timeout = %d]",
force_dns, hash.host_len, hash.host_name, opt.timeout);
+/* Support SRV records */
+Action *
+HostDBProcessor::getSRVbyname_imm(Continuation *cont, cb_process_result_pfn
process_srv_info, const char *hostname, int len,
+ Options const &opt)
+{
+ ink_assert(cont->mutex->thread_holding == this_ethread());
+ HostDBHash hash;
- // Otherwise, create a continuation to do a deeper probe in the background
- HostDBContinuation *c = hostDBContAllocator.alloc();
- HostDBContinuation::Options copt;
- copt.timeout = opt.timeout;
- copt.cont = cont;
- copt.force_dns = force_dns;
- c->init(hash, copt);
- SET_CONTINUATION_HANDLER(c,
(HostDBContHandler)&HostDBContinuation::probeEvent);
+ ink_assert(nullptr != hostname);
- thread->schedule_in(c, MUTEX_RETRY_DELAY);
+ hash.set_host(hostname, len ? len : strlen(hostname));
+ // Leave hash.ip invalid
+ hash.port = 0;
+ hash.db_mark = HOSTDB_MARK_SRV;
+ hash.refresh();
- return &c->action;
+ return getby(cont, process_srv_info, hash, opt);
}
// Wrapper from getbyname to getby
//
Action *
-HostDBProcessor::getbyname_imm(Continuation *cont, process_hostdb_info_pfn
process_hostdb_info, const char *hostname, int len,
+HostDBProcessor::getbyname_imm(Continuation *cont, cb_process_result_pfn
process_hostdb_info, const char *hostname, int len,
Options const &opt)
{
ink_assert(cont->mutex->thread_holding == this_ethread());
- bool force_dns = false;
- EThread *thread = cont->mutex->thread_holding;
- ProxyMutex *mutex = thread->mutex.get();
HostDBHash hash;
ink_assert(nullptr != hostname);
- if (opt.flags & HOSTDB_FORCE_DNS_ALWAYS) {
- force_dns = true;
- } else if (opt.flags & HOSTDB_FORCE_DNS_RELOAD) {
- force_dns = (hostdb_re_dns_on_reload ? true : false);
- if (force_dns) {
- HOSTDB_INCREMENT_DYN_STAT(hostdb_re_dns_on_reload_stat);
- }
- }
-
- HOSTDB_INCREMENT_DYN_STAT(hostdb_total_lookups_stat);
-
- if (!hostdb_enable || !*hostname) {
- (cont->*process_hostdb_info)(nullptr);
- return ACTION_RESULT_DONE;
- }
-
hash.set_host(hostname, len ? len : strlen(hostname));
+ // Leave hash.ip invalid
+ // TODO: May I rename the wrapper name to getbynameport_imm ? - oknet
+ // By comparing getbyname_re and getbynameport_re, the hash.port should be
0 if only get hostinfo by name.
hash.port = opt.port;
hash.db_mark = db_mark_for(opt.host_res_style);
hash.refresh();
- // Attempt to find the result in-line, for level 1 hits
- if (!force_dns) {
- bool loop;
- do {
- loop = false; // loop only on explicit set for retry
- // find the partition lock
- Ptr<ProxyMutex> bucket_mutex =
hostDB.refcountcache->lock_for_key(hash.hash.fold());
- SCOPED_MUTEX_LOCK(lock, bucket_mutex, thread);
- // do a level 1 probe for immediate result.
- Ptr<HostDBInfo> r = probe(bucket_mutex, hash, false);
- if (r) {
- if (r->is_failed()) { // fail, see if we should retry with alternate
- loop = check_for_retry(hash.db_mark, opt.host_res_style);
- }
- if (!loop) {
- // No retry -> final result. Return it.
- Debug("hostdb", "immediate answer for %.*s", hash.host_len,
hash.host_name);
- HOSTDB_INCREMENT_DYN_STAT(hostdb_total_hits_stat);
- (cont->*process_hostdb_info)(r.get());
- return ACTION_RESULT_DONE;
- }
- hash.refresh(); // Update for retry.
- }
- } while (loop);
- }
-
- Debug("hostdb", "delaying force %d answer for %.*s [timeout %d]", force_dns,
hash.host_len, hash.host_name, opt.timeout);
-
- // Otherwise, create a continuation to do a deeper probe in the background
- HostDBContinuation *c = hostDBContAllocator.alloc();
- HostDBContinuation::Options copt;
- copt.cont = cont;
- copt.force_dns = force_dns;
- copt.timeout = opt.timeout;
- copt.host_res_style = opt.host_res_style;
- c->init(hash, copt);
- SET_CONTINUATION_HANDLER(c,
(HostDBContHandler)&HostDBContinuation::probeEvent);
-
- thread->schedule_in(c, HOST_DB_RETRY_PERIOD);
-
- return &c->action;
+ return getby(cont, process_hostdb_info, hash, opt);
}
Action *
diff --git a/iocore/hostdb/I_HostDBProcessor.h
b/iocore/hostdb/I_HostDBProcessor.h
index a9f5d0c..347f1a9 100644
--- a/iocore/hostdb/I_HostDBProcessor.h
+++ b/iocore/hostdb/I_HostDBProcessor.h
@@ -393,11 +393,11 @@ struct HostDBRoundRobin {
};
struct HostDBCache;
+struct HostDBHash;
// Prototype for inline completion functionf or
// getbyname_imm()
-typedef void (Continuation::*process_hostdb_info_pfn)(HostDBInfo *r);
-typedef void (Continuation::*process_srv_info_pfn)(HostDBInfo *r);
+typedef void (Continuation::*cb_process_result_pfn)(HostDBInfo *r);
Action *iterate(Continuation *cont);
@@ -448,20 +448,16 @@ struct HostDBProcessor : public Processor {
Action *getbynameport_re(Continuation *cont, const char *hostname, int len,
Options const &opt = DEFAULT_OPTIONS);
- Action *getSRVbyname_imm(Continuation *cont, process_srv_info_pfn
process_srv_info, const char *hostname, int len,
+ Action *getSRVbyname_imm(Continuation *cont, cb_process_result_pfn
process_srv_info, const char *hostname, int len,
Options const &opt = DEFAULT_OPTIONS);
- Action *getbyname_imm(Continuation *cont, process_hostdb_info_pfn
process_hostdb_info, const char *hostname, int len,
+ Action *getbyname_imm(Continuation *cont, cb_process_result_pfn
process_hostdb_info, const char *hostname, int len,
Options const &opt = DEFAULT_OPTIONS);
Action *iterate(Continuation *cont);
/** Lookup Hostinfo by addr */
- Action *
- getbyaddr_re(Continuation *cont, sockaddr const *aip)
- {
- return getby(cont, nullptr, 0, aip, false, HOST_RES_NONE, 0);
- }
+ Action *getbyaddr_re(Continuation *cont, sockaddr const *aip);
/** Set the application information (fire-and-forget). */
void
@@ -500,8 +496,7 @@ struct HostDBProcessor : public Processor {
HostDBCache *cache();
private:
- Action *getby(Continuation *cont, const char *hostname, int len, sockaddr
const *ip, bool aforce_dns, HostResStyle host_res_style,
- int timeout);
+ Action *getby(Continuation *cont, cb_process_result_pfn cb_process_result,
HostDBHash &hash, Options const &opt);
public:
/** Set something.
diff --git a/proxy/http/HttpSM.cc b/proxy/http/HttpSM.cc
index 3aa0cee..8f2c3d0 100644
--- a/proxy/http/HttpSM.cc
+++ b/proxy/http/HttpSM.cc
@@ -2249,7 +2249,7 @@ HttpSM::state_hostdb_lookup(int event, void *data)
opt.host_res_style = ua_txn->get_host_res_style();
Action *dns_lookup_action_handle =
- hostDBProcessor.getbyname_imm(this,
(process_hostdb_info_pfn)&HttpSM::process_hostdb_info, host_name, 0, opt);
+ hostDBProcessor.getbyname_imm(this,
(cb_process_result_pfn)&HttpSM::process_hostdb_info, host_name, 0, opt);
if (dns_lookup_action_handle != ACTION_RESULT_DONE) {
ink_assert(!pending_action);
pending_action = dns_lookup_action_handle;
@@ -4047,7 +4047,7 @@ HttpSM::do_hostdb_lookup()
opt.timeout = t_state.api_txn_dns_timeout_value;
}
Action *srv_lookup_action_handle =
- hostDBProcessor.getSRVbyname_imm(this,
(process_srv_info_pfn)&HttpSM::process_srv_info, d, 0, opt);
+ hostDBProcessor.getSRVbyname_imm(this,
(cb_process_result_pfn)&HttpSM::process_srv_info, d, 0, opt);
if (srv_lookup_action_handle != ACTION_RESULT_DONE) {
ink_assert(!pending_action);
@@ -4064,7 +4064,7 @@ HttpSM::do_hostdb_lookup()
opt.host_res_style = ua_txn->get_host_res_style();
Action *dns_lookup_action_handle =
- hostDBProcessor.getbyname_imm(this,
(process_hostdb_info_pfn)&HttpSM::process_hostdb_info, host_name, 0, opt);
+ hostDBProcessor.getbyname_imm(this,
(cb_process_result_pfn)&HttpSM::process_hostdb_info, host_name, 0, opt);
if (dns_lookup_action_handle != ACTION_RESULT_DONE) {
ink_assert(!pending_action);
pending_action = dns_lookup_action_handle;
@@ -4098,7 +4098,7 @@ HttpSM::do_hostdb_lookup()
opt.timeout = (t_state.api_txn_dns_timeout_value != -1) ?
t_state.api_txn_dns_timeout_value : 0;
opt.host_res_style = ua_txn->get_host_res_style();
- Action *dns_lookup_action_handle = hostDBProcessor.getbyname_imm(this,
(process_hostdb_info_pfn)&HttpSM::process_hostdb_info,
+ Action *dns_lookup_action_handle = hostDBProcessor.getbyname_imm(this,
(cb_process_result_pfn)&HttpSM::process_hostdb_info,
t_state.dns_info.lookup_name, 0, opt);
if (dns_lookup_action_handle != ACTION_RESULT_DONE) {
