From: bert hubert Date: Tue, 10 Mar 2015 12:20:54 +0000 (+0100) Subject: simplify StateHolder API and make std::shared_ptr an invisible implementation detail X-Git-Tag: dnsdist-1.0.0-alpha1~248^2~88^2~64 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e5a14b2bdc71fb19384dc39a64ca447f3a5722cf;p=pdns simplify StateHolder API and make std::shared_ptr an invisible implementation detail --- diff --git a/pdns/dnsdist-lua.cc b/pdns/dnsdist-lua.cc index 5ef4048e7..20f510634 100644 --- a/pdns/dnsdist-lua.cc +++ b/pdns/dnsdist-lua.cc @@ -72,8 +72,8 @@ vector> setupLua(bool client) } auto states = g_dstates.getCopy(); - states->push_back(ret); - std::stable_sort(states->begin(), states->end(), [](const decltype(ret)& a, const decltype(ret)& b) { + states.push_back(ret); + std::stable_sort(states.begin(), states.end(), [](const decltype(ret)& a, const decltype(ret)& b) { return a->order < b->order; }); g_dstates.setState(states); @@ -88,19 +88,19 @@ vector> setupLua(bool client) { auto states = g_dstates.getCopy(); if(auto* rem = boost::get>(&var)) - states->erase(remove(states->begin(), states->end(), *rem), states->end()); + states.erase(remove(states.begin(), states.end(), *rem), states.end()); else - states->erase(states->begin() + boost::get(var)); + states.erase(states.begin() + boost::get(var)); g_dstates.setState(states); } ); g_lua.writeFunction("setServerPolicy", [](ServerPolicy policy) { - g_policy.setState(std::make_shared(policy)); + g_policy.setState(policy); }); g_lua.writeFunction("setServerPolicyLua", [](string name, policy_t policy) { - g_policy.setState(std::make_shared(ServerPolicy{name, policy})); + g_policy.setState(ServerPolicy{name, policy}); }); g_lua.writeFunction("showServerPolicy", []() { @@ -116,9 +116,7 @@ vector> setupLua(bool client) g_lua.writeVariable("wrandom", ServerPolicy{"wrandom", wrandom}); g_lua.writeVariable("leastOutstanding", ServerPolicy{"leastOutstanding", leastOutstanding}); g_lua.writeFunction("addACL", [](const std::string& domain) { - auto state=g_ACL.getCopy(); - state->addMask(domain); - g_ACL.setState(state); + g_ACL.modify([domain](NetmaskGroup& nmg) { nmg.addMask(domain); }); }); g_lua.writeFunction("addLocal", [client](const std::string& addr) { @@ -133,16 +131,16 @@ vector> setupLua(bool client) } }); g_lua.writeFunction("setACL", [](const vector>& parts) { - auto nmg = std::make_shared(); + NetmaskGroup nmg; for(const auto& p : parts) { - nmg->addMask(p.second); + nmg.addMask(p.second); } g_ACL.setState(nmg); }); g_lua.writeFunction("showACL", []() { vector vec; - g_ACL.getCopy()->toStringVector(&vec); + g_ACL.getCopy().toStringVector(&vec); for(const auto& s : vec) g_outputBuffer+=s+"\n"; @@ -167,7 +165,7 @@ vector> setupLua(bool client) uint64_t totQPS{0}, totQueries{0}, totDrops{0}; int counter=0; auto states = g_dstates.getCopy(); - for(const auto& s : *states) { + for(const auto& s : states) { string status; if(s->availability == DownstreamState::Availability::Up) status = "UP"; @@ -233,7 +231,7 @@ vector> setupLua(bool client) boost::format fmt("%-3d %-50s %s\n"); g_outputBuffer += (fmt % "#" % "Object" % "Pool").str(); int num=0; - for(const auto& lim : *g_poolrules.getCopy()) { + for(const auto& lim : g_poolrules.getCopy()) { string name; if(auto nmg=boost::get(&lim.first)) { name=nmg->toString(); @@ -285,7 +283,7 @@ vector> setupLua(bool client) boost::format fmt("%-3d %-50s %7d %8d %8d\n"); g_outputBuffer += (fmt % "#" % "Object" % "Lim" % "Passed" % "Blocked").str(); int num=0; - for(const auto& lim : *g_limiters.getCopy()) { + for(const auto& lim : g_limiters.getCopy()) { string name; if(auto nmg=boost::get(&lim.first)) { name=nmg->toString(); @@ -302,13 +300,13 @@ vector> setupLua(bool client) g_lua.writeFunction("getServers", []() { vector > > ret; int count=1; - for(const auto& s : *g_dstates.getCopy()) { + for(const auto& s : g_dstates.getCopy()) { ret.push_back(make_pair(count++, s)); } return ret; }); - g_lua.writeFunction("getServer", [](int i) { return (*g_dstates.getCopy())[i]; }); + g_lua.writeFunction("getServer", [](int i) { return g_dstates.getCopy().at(i); }); g_lua.registerFunction("setQPS", [](DownstreamState& s, int lim) { s.qps = lim ? QPSLimiter(lim, lim) : QPSLimiter(); }); g_lua.registerFunction("addPool", [](DownstreamState& s, string pool) { s.pools.insert(pool);}); diff --git a/pdns/dnsdist.cc b/pdns/dnsdist.cc index 40a9008de..44644aeba 100644 --- a/pdns/dnsdist.cc +++ b/pdns/dnsdist.cc @@ -42,8 +42,10 @@ /* Known sins: No centralized statistics We neglect to do recvfromto() on 0.0.0.0 - Receiver is currently singlethreaded (not that bad actually) + Receiver is currently singlethreaded + not *that* bad actually, but now that we are thread safe, might want to scale lack of help() + lack of autocomplete */ namespace po = boost::program_options; @@ -254,7 +256,7 @@ ComboAddress g_serverControl{"127.0.0.1:5199"}; servers_t getDownstreamCandidates(const std::string& pool) { servers_t ret; - for(const auto& s : *g_dstates.getCopy()) + for(const auto& s : g_dstates.getCopy()) if((pool.empty() && s->pools.empty()) || s->pools.count(pool)) ret.push_back(s); @@ -444,11 +446,11 @@ catch(...) int getTCPDownstream(DownstreamState** ds, const ComboAddress& remote, const DNSName& qname, uint16_t qtype, dnsheader* dh) { - auto policy=g_policy.getCopy()->policy; + auto policy=g_policy.getCopy().policy; { std::lock_guard lock(g_luamutex); - *ds = policy(*g_dstates.getCopy(), remote, qname, qtype, dh).get(); // XXX I think this misses pool selection! + *ds = policy(g_dstates.getCopy(), remote, qname, qtype, dh).get(); // XXX I think this misses pool selection! } vinfolog("TCP connecting to downstream %s", (*ds)->remote.toStringWithPort()); @@ -660,14 +662,11 @@ void* maintThread() if(g_tcpclientthreads.d_queued > 1 && g_tcpclientthreads.d_numthreads < 10) g_tcpclientthreads.addTCPClientThread(); - for(auto& dss : *(g_dstates.getCopy())) { // this points to the actual shared_ptrs! + for(auto& dss : g_dstates.getCopy()) { // this points to the actual shared_ptrs! if(dss->availability==DownstreamState::Availability::Auto) { bool newState=upCheck(dss->remote); if(newState != dss->upStatus) { - cout<remote.toStringWithPort(), newState ? "up" : "down"); - cout<<"> "; - cout.flush(); } dss->upStatus = newState; } @@ -954,7 +953,7 @@ try ServerPolicy leastOutstandingPol{"leastOutstanding", leastOutstanding}; - g_policy.setState(std::make_shared(leastOutstandingPol)); + g_policy.setState(leastOutstandingPol); if(g_vm.count("client") || g_vm.count("command")) { setupLua(true); doClient(g_serverControl); @@ -998,7 +997,7 @@ try auto acl = g_ACL.getCopy(); for(auto& addr : {"127.0.0.0/8", "10.0.0.0/8", "100.64.0.0/10", "169.254.0.0/16", "192.168.0.0/16", "172.16.0.0/12", "::1/128", "fc00::/7", "fe80::/10"}) - acl->addMask(addr); + acl.addMask(addr); g_ACL.setState(acl); if(g_vm.count("remotes")) { @@ -1009,7 +1008,7 @@ try } } - for(auto& dss : *g_dstates.getCopy()) { + for(auto& dss : g_dstates.getCopy()) { // it is a copy, but the internal shared_ptrs are the real deal if(dss->availability==DownstreamState::Availability::Auto) { bool newState=upCheck(dss->remote); warnlog("Marking downstream %s as '%s'", dss->remote.toStringWithPort(), newState ? "up" : "down"); diff --git a/pdns/sholder.hh b/pdns/sholder.hh index ef57cd33e..3fa17eb49 100644 --- a/pdns/sholder.hh +++ b/pdns/sholder.hh @@ -35,7 +35,7 @@ public: explicit LocalStateHolder(GlobalStateHolder* source) : d_source(source) {} - const T* operator->() // only const access, but see "read-only" above + const T* operator->() // fast const-only access, but see "read-only" above { if(d_source->getGeneration() != d_generation) { d_source->getState(&d_state, & d_generation); @@ -43,13 +43,9 @@ public: return d_state.get(); } - const T& operator*() // only const access, but see "read-only" above + const T& operator*() // fast const-only access, but see "read-only" above { - if(d_source->getGeneration() != d_generation) { - d_source->getState(&d_state, &d_generation); - } - - return *d_state; + return *operator->(); } void reset() @@ -73,29 +69,21 @@ public: { return LocalStateHolder(this); } - void setState(std::shared_ptr state) + + void setState(T state) //!< Safely & slowly change the global state { std::lock_guard l(d_lock); - d_state = state; + d_state = std::make_shared(T(state)); d_generation++; } - unsigned int getGeneration() const - { - return d_generation; - } - void getState(std::shared_ptr* state, unsigned int* generation) const - { - std::lock_guard l(d_lock); - *state=d_state; - *generation = d_generation; - } - std::shared_ptr getCopy() const + + T getCopy() const //!< Safely & slowly get a copy of the global state { std::lock_guard l(d_lock); - shared_ptr ret = shared_ptr(new T(*d_state)); - return d_state; + return *d_state; } + //! Safely & slowly modify the global state template void modify(F act) { std::lock_guard l(d_lock); @@ -106,6 +94,17 @@ public: typedef T value_type; private: + unsigned int getGeneration() const + { + return d_generation; + } + void getState(std::shared_ptr* state, unsigned int* generation) const + { + std::lock_guard l(d_lock); + *state=d_state; + *generation = d_generation; + } + friend class LocalStateHolder; mutable std::mutex d_lock; std::shared_ptr d_state; std::atomic d_generation{1};