From: Remi Gacogne Date: Tue, 21 Feb 2017 15:14:09 +0000 (+0100) Subject: rec: Speed up the packet cache X-Git-Tag: rec-4.1.0-alpha1~229^2~3 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e9f63d47cfb9a494af17f1c23c954d0f67700ae1;p=pdns rec: Speed up the packet cache * Don't parse the response's qname for every call to `getResponsePacket()`, this leads to a ~15% speed up on pure retrieval * Only hash once, keep the hash result around, leading to a ~40% speed up on insertion --- diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc index 1776ccf15..048f4b5ff 100644 --- a/pdns/pdns_recursor.cc +++ b/pdns/pdns_recursor.cc @@ -205,6 +205,7 @@ struct DNSComboWriter { bool d_tcp; int d_socket; int d_tag{0}; + uint32_t d_qhash{0}; string d_query; shared_ptr d_tcpConnection; vector > d_ednsOpts; @@ -1162,7 +1163,7 @@ void startDoResolve(void *p) if(sendmsg(dc->d_socket, &msgh, 0) < 0 && g_logCommonErrors) L<d_remote.toStringWithPort()<<" failed with: "<insertResponsePacket(dc->d_tag, dc->d_mdp.d_qname, dc->d_mdp.d_qtype, dc->d_query, + t_packetCache->insertResponsePacket(dc->d_tag, dc->d_qhash, dc->d_mdp.d_qname, dc->d_mdp.d_qtype, dc->d_mdp.d_qclass, string((const char*)&*packet.begin(), packet.size()), g_now.tv_sec, pw.getHeader()->rcode == RCode::ServFail ? SyncRes::s_packetcacheservfailttl : @@ -1540,6 +1541,7 @@ string* doProcessUDPQuestion(const std::string& question, const ComboAddress& fr string response; const struct dnsheader* dh = (struct dnsheader*)question.c_str(); unsigned int ctag=0; + uint32_t qhash; bool needECS = false; std::vector policyTags; std::unordered_map data; @@ -1605,7 +1607,7 @@ string* doProcessUDPQuestion(const std::string& question, const ComboAddress& fr } #endif /* HAVE_PROTOBUF */ - cacheHit = (!SyncRes::s_nopacketcache && t_packetCache->getResponsePacket(ctag, question, g_now.tv_sec, &response, &age, &pbMessage)); + cacheHit = (!SyncRes::s_nopacketcache && t_packetCache->getResponsePacket(ctag, question, g_now.tv_sec, &response, &age, &qhash, &pbMessage)); if (cacheHit) { #ifdef HAVE_PROTOBUF if(luaconfsLocal->protobufServer && (!luaconfsLocal->protobufTaggedOnly || !pbMessage.getAppliedPolicy().empty() || !pbMessage.getPolicyTags().empty())) { @@ -1669,6 +1671,7 @@ string* doProcessUDPQuestion(const std::string& question, const ComboAddress& fr DNSComboWriter* dc = new DNSComboWriter(question.c_str(), question.size(), g_now); dc->setSocket(fd); dc->d_tag=ctag; + dc->d_qhash=qhash; dc->d_query = question; dc->setRemote(&fromaddr); dc->setLocal(destaddr); diff --git a/pdns/recpacketcache.cc b/pdns/recpacketcache.cc index 1d2483179..dd71567a4 100644 --- a/pdns/recpacketcache.cc +++ b/pdns/recpacketcache.cc @@ -31,7 +31,7 @@ int RecursorPacketCache::doWipePacketCache(const DNSName& name, uint16_t qtype, if(iter->d_name != name) break; } - + if(qtype==0xffff || iter->d_type == qtype) { iter=idx.erase(iter); count++; @@ -42,14 +42,12 @@ int RecursorPacketCache::doWipePacketCache(const DNSName& name, uint16_t qtype, return count; } -// one day this function could be really fast by doing only a case insensitive compare -static bool qrMatch(const std::string& query, const std::string& response) +static bool qrMatch(const std::string& query, const DNSName& rname, uint16_t rtype, uint16_t rclass) { - uint16_t rqtype, rqclass, qqtype, qqclass; - DNSName queryname(query.c_str(), query.length(), sizeof(dnsheader), false, &qqtype, &qqclass, 0); - DNSName respname(response.c_str(), response.length(), sizeof(dnsheader), false, &rqtype, &rqclass, 0); + uint16_t qtype, qclass; + DNSName qname(query.c_str(), query.length(), sizeof(dnsheader), false, &qtype, &qclass, 0); // this ignores checking on the EDNS subnet flags! - return queryname==respname && rqtype == qqtype && rqclass == qqclass; + return qname==rname && rtype == qtype && rclass == qclass; } uint32_t RecursorPacketCache::canHashPacket(const std::string& origPacket) @@ -96,17 +94,17 @@ uint32_t RecursorPacketCache::canHashPacket(const std::string& origPacket) } bool RecursorPacketCache::getResponsePacket(unsigned int tag, const std::string& queryPacket, time_t now, - std::string* responsePacket, uint32_t* age) + std::string* responsePacket, uint32_t* age, uint32_t* qhash) { - return getResponsePacket(tag, queryPacket, now, responsePacket, age, nullptr); + return getResponsePacket(tag, queryPacket, now, responsePacket, age, qhash, nullptr); } bool RecursorPacketCache::getResponsePacket(unsigned int tag, const std::string& queryPacket, time_t now, - std::string* responsePacket, uint32_t* age, RecProtoBufMessage* protobufMessage) + std::string* responsePacket, uint32_t* age, uint32_t* qhash, RecProtoBufMessage* protobufMessage) { - uint32_t h = canHashPacket(queryPacket); + *qhash = canHashPacket(queryPacket); auto& idx = d_packetCache.get(); - auto range = idx.equal_range(tie(tag,h)); + auto range = idx.equal_range(tie(tag,*qhash)); if(range.first == range.second) { d_misses++; @@ -115,7 +113,7 @@ bool RecursorPacketCache::getResponsePacket(unsigned int tag, const std::string& for(auto iter = range.first ; iter != range.second ; ++ iter) { // the possibility is VERY real that we get hits that are not right - birthday paradox - if(!qrMatch(queryPacket, iter->d_packet)) + if(!qrMatch(queryPacket, iter->d_name, iter->d_type, iter->d_class)) continue; if((uint32_t)now < iter->d_ttd) { // it is right, it is fresh! *age = now - iter->d_creation; @@ -153,20 +151,19 @@ bool RecursorPacketCache::getResponsePacket(unsigned int tag, const std::string& } -void RecursorPacketCache::insertResponsePacket(unsigned int tag, const DNSName& qname, uint16_t qtype, const std::string& queryPacket, const std::string& responsePacket, time_t now, uint32_t ttl) +void RecursorPacketCache::insertResponsePacket(unsigned int tag, uint32_t qhash, const DNSName& qname, uint16_t qtype, uint16_t qclass, const std::string& responsePacket, time_t now, uint32_t ttl) { - insertResponsePacket(tag, qname, qtype, queryPacket, responsePacket, now, ttl, nullptr); + insertResponsePacket(tag, qhash, qname, qtype, qclass, responsePacket, now, ttl, nullptr); } -void RecursorPacketCache::insertResponsePacket(unsigned int tag, const DNSName& qname, uint16_t qtype, const std::string& queryPacket, const std::string& responsePacket, time_t now, uint32_t ttl, const RecProtoBufMessage* protobufMessage) +void RecursorPacketCache::insertResponsePacket(unsigned int tag, uint32_t qhash, const DNSName& qname, uint16_t qtype, uint16_t qclass, const std::string& responsePacket, time_t now, uint32_t ttl, const RecProtoBufMessage* protobufMessage) { - auto qhash = canHashPacket(queryPacket); auto& idx = d_packetCache.get(); auto range = idx.equal_range(tie(tag,qhash)); auto iter = range.first; for( ; iter != range.second ; ++iter) { - if(iter->d_type != qtype) + if(iter->d_type != qtype || iter->d_class != qclass) continue; // this only happens on insert which is relatively rare and does not need to be super fast DNSName respname(iter->d_packet.c_str(), iter->d_packet.length(), sizeof(dnsheader), false, 0, 0, 0); @@ -191,6 +188,7 @@ void RecursorPacketCache::insertResponsePacket(unsigned int tag, const DNSName& e.d_name = qname; e.d_qhash = qhash; e.d_type = qtype; + e.d_class = qclass; e.d_ttd = now+ttl; e.d_creation = now; e.d_tag = tag; diff --git a/pdns/recpacketcache.hh b/pdns/recpacketcache.hh index 65a6baad9..8c198feb6 100644 --- a/pdns/recpacketcache.hh +++ b/pdns/recpacketcache.hh @@ -50,10 +50,10 @@ class RecursorPacketCache { public: RecursorPacketCache(); - bool getResponsePacket(unsigned int tag, const std::string& queryPacket, time_t now, std::string* responsePacket, uint32_t* age); - void insertResponsePacket(unsigned int tag, const DNSName& qname, uint16_t qtype, const std::string& queryPacket, const std::string& responsePacket, time_t now, uint32_t ttd); - bool getResponsePacket(unsigned int tag, const std::string& queryPacket, time_t now, std::string* responsePacket, uint32_t* age, RecProtoBufMessage* protobufMessage); - void insertResponsePacket(unsigned int tag, const DNSName& qname, uint16_t qtype, const std::string& queryPacket, const std::string& responsePacket, time_t now, uint32_t ttd, const RecProtoBufMessage* protobufMessage); + bool getResponsePacket(unsigned int tag, const std::string& queryPacket, time_t now, std::string* responsePacket, uint32_t* age, uint32_t* qhash); + bool getResponsePacket(unsigned int tag, const std::string& queryPacket, time_t now, std::string* responsePacket, uint32_t* age, uint32_t* qhash, RecProtoBufMessage* protobufMessage); + void insertResponsePacket(unsigned int tag, uint32_t qhash, const DNSName& qname, uint16_t qtype, uint16_t qclass, const std::string& responsePacket, time_t now, uint32_t ttd); + void insertResponsePacket(unsigned int tag, uint32_t qhash, const DNSName& qname, uint16_t qtype, uint16_t qclass, const std::string& responsePacket, time_t now, uint32_t ttd, const RecProtoBufMessage* protobufMessage); void doPruneTo(unsigned int maxSize=250000); uint64_t doDump(int fd); int doWipePacketCache(const DNSName& name, uint16_t qtype=0xffff, bool subtree=false); @@ -72,6 +72,7 @@ private: mutable uint32_t d_creation; // so we can 'age' our packets DNSName d_name; uint16_t d_type; + uint16_t d_class; mutable std::string d_packet; // "I know what I am doing" #ifdef HAVE_PROTOBUF mutable RecProtoBufMessage d_protobufMessage; diff --git a/pdns/test-recpacketcache_cc.cc b/pdns/test-recpacketcache_cc.cc index 8d723253b..bdd9addc4 100644 --- a/pdns/test-recpacketcache_cc.cc +++ b/pdns/test-recpacketcache_cc.cc @@ -17,6 +17,11 @@ BOOST_AUTO_TEST_SUITE(recpacketcache_cc) BOOST_AUTO_TEST_CASE(test_recPacketCacheSimple) { RecursorPacketCache rpc; + string fpacket; + int tag=0; + uint32_t age=0; + uint32_t qhash=0; + uint32_t ttd=3600; BOOST_CHECK_EQUAL(rpc.size(), 0); DNSName qname("www.powerdns.com"); @@ -26,27 +31,29 @@ BOOST_AUTO_TEST_CASE(test_recPacketCacheSimple) { pw.getHeader()->qr=false; pw.getHeader()->id=random(); string qpacket((const char*)&packet[0], packet.size()); - pw.startRecord(qname, QType::A, 3600); + pw.startRecord(qname, QType::A, ttd); + + BOOST_CHECK_EQUAL(rpc.getResponsePacket(tag, qpacket, time(nullptr), &fpacket, &age, &qhash), false); ARecordContent ar("127.0.0.1"); ar.toPacket(pw); pw.commit(); string rpacket((const char*)&packet[0], packet.size()); - rpc.insertResponsePacket(0,qname, QType::A, qpacket, rpacket, time(0), 3600); + rpc.insertResponsePacket(tag, qhash, qname, QType::A, QClass::IN, rpacket, time(0), ttd); BOOST_CHECK_EQUAL(rpc.size(), 1); rpc.doPruneTo(0); BOOST_CHECK_EQUAL(rpc.size(), 0); - rpc.insertResponsePacket(0,qname, QType::A, qpacket, rpacket, time(0), 3600); + rpc.insertResponsePacket(tag, qhash, qname, QType::A, QClass::IN, rpacket, time(0), ttd); BOOST_CHECK_EQUAL(rpc.size(), 1); rpc.doWipePacketCache(qname); BOOST_CHECK_EQUAL(rpc.size(), 0); - rpc.insertResponsePacket(0,qname, QType::A, qpacket, rpacket, time(0), 3600); - uint32_t age=0; - string fpacket; - bool found = rpc.getResponsePacket(0, qpacket, time(0), &fpacket, &age); - BOOST_CHECK_EQUAL(found, 1); + rpc.insertResponsePacket(tag, qhash, qname, QType::A, QClass::IN, rpacket, time(0), ttd); + uint32_t qhash2 = 0; + bool found = rpc.getResponsePacket(tag, qpacket, time(nullptr), &fpacket, &age, &qhash2); + BOOST_CHECK_EQUAL(found, true); + BOOST_CHECK_EQUAL(qhash, qhash2); BOOST_CHECK_EQUAL(fpacket, rpacket); packet.clear(); @@ -57,14 +64,12 @@ BOOST_AUTO_TEST_CASE(test_recPacketCacheSimple) { pw2.getHeader()->qr=false; pw2.getHeader()->id=random(); qpacket.assign((const char*)&packet[0], packet.size()); - found = rpc.getResponsePacket(0, qpacket, time(0), &fpacket, &age); - BOOST_CHECK_EQUAL(found, 0); + + found = rpc.getResponsePacket(tag, qpacket, time(nullptr), &fpacket, &age, &qhash); + BOOST_CHECK_EQUAL(found, false); rpc.doWipePacketCache(DNSName("com"), 0xffff, true); BOOST_CHECK_EQUAL(rpc.size(), 0); - - - } BOOST_AUTO_TEST_SUITE_END()