From 4a160f880ce46153acb23b137f30fd588df5fb9d Mon Sep 17 00:00:00 2001 From: Peter Iannucci Date: Fri, 11 Oct 2013 17:00:49 -0400 Subject: [PATCH] Clean-ups --- rpc/connection.cc | 3 +- rpc/rpc.cc | 137 +++++++++++++++++---------------------- rpc/rpc.h | 185 ++++++++++++++++++++++++----------------------------- 3 files changed, 144 insertions(+), 181 deletions(-) diff --git a/rpc/connection.cc b/rpc/connection.cc index 6c406a4..315a82e 100644 --- a/rpc/connection.cc +++ b/rpc/connection.cc @@ -208,8 +208,7 @@ bool connection::readpdu() { tcpsconn::tcpsconn(connection_delegate *m1, in_port_t port, int lossytest) : tcp_(socket(AF_INET, SOCK_STREAM, 0)), mgr_(m1), lossy_(lossytest) { - sockaddr_in sin; - memset(&sin, 0, sizeof(sin)); + sockaddr_in sin{}; // zero initialize sin.sin_family = AF_INET; sin.sin_port = hton(port); diff --git a/rpc/rpc.cc b/rpc/rpc.cc index abbe470..80e429e 100644 --- a/rpc/rpc.cc +++ b/rpc/rpc.cc @@ -70,7 +70,7 @@ rpcc::rpcc(const string & d, bool retrans) : dst_(make_sockaddr(d)), srv_nonce_(0), bind_done_(false), xid_(1), lossytest_(0), retrans_(retrans), reachable_(true), chan_(), destroy_wait_ (false), xid_rep_done_(-1) { - if(retrans){ + if (retrans) { set_rand_seed(); clt_nonce_ = (unsigned int)random(); } else { @@ -81,7 +81,7 @@ rpcc::rpcc(const string & d, bool retrans) : } char *loss_env = getenv("RPC_LOSSY"); - if(loss_env) + if (loss_env) lossytest_ = atoi(loss_env); // xid starts with 1 and latest received reply starts with 0 @@ -95,7 +95,7 @@ rpcc::rpcc(const string & d, bool retrans) : rpcc::~rpcc() { cancel(); IF_LEVEL(2) LOG("delete nonce " << clt_nonce_ << " channo=" << (chan_?chan_->channo():-1)); - if(chan_) + if (chan_) chan_->closeconn(); VERIFY(calls_.size() == 0); } @@ -103,7 +103,7 @@ rpcc::~rpcc() { int rpcc::bind(milliseconds to) { unsigned int r; int ret = call_timeout(rpc_const::bind, to, r, 0); - if(ret == 0){ + if (ret == 0) { lock ml(m_); bind_done_ = true; srv_nonce_ = r; @@ -118,22 +118,21 @@ void rpcc::cancel(void) { lock ml(m_); if (calls_.size()) { LOG("force callers to fail"); - for(auto &p : calls_){ + for (auto &p : calls_) { caller *ca = p.second; IF_LEVEL(2) LOG("force caller to fail"); - { - lock cl(ca->m); - ca->done = true; - ca->intret = rpc_const::cancel_failure; - ca->c.notify_one(); - } + + lock cl(ca->m); + ca->done = true; + ca->intret = rpc_const::cancel_failure; + ca->c.notify_one(); } - while (calls_.size () > 0){ - destroy_wait_ = true; + destroy_wait_ = true; + while (calls_.size () > 0) destroy_wait_c_.wait(ml); - } + LOG("done"); } } @@ -145,15 +144,13 @@ int rpcc::call1(proc_t proc, marshall &req, string &rep, milliseconds to) { { lock ml(m_); - if((proc != rpc_const::bind && !bind_done_) || - (proc == rpc_const::bind && bind_done_)){ + if ((proc != rpc_const::bind && !bind_done_) || (proc == rpc_const::bind && bind_done_)) { IF_LEVEL(1) LOG("rpcc has not been bound to dst or binding twice"); return rpc_const::bind_failure; } - if(destroy_wait_){ - return rpc_const::cancel_failure; - } + if (destroy_wait_) + return rpc_const::cancel_failure; ca.xid = xid_++; calls_[ca.xid] = &ca; @@ -163,13 +160,13 @@ int rpcc::call1(proc_t proc, marshall &req, string &rep, milliseconds to) { } milliseconds curr_to = rpc::to_min; - auto finaldeadline = steady_clock::now() + to, nextdeadline = finaldeadline; + auto finaldeadline = steady_clock::now() + to; bool transmit = true; shared_ptr ch; while (1) { - if(transmit) { + if (transmit) { get_refconn(ch); if (ch) { if (reachable_) { @@ -192,36 +189,32 @@ int rpcc::call1(proc_t proc, marshall &req, string &rep, milliseconds to) { transmit = false; // only send once on a given channel } - if(finaldeadline == time_point::min()) - break; - - nextdeadline = steady_clock::now() + curr_to; - if(nextdeadline > finaldeadline) { - nextdeadline = finaldeadline; - finaldeadline = time_point::min(); - } + auto nextdeadline = min(steady_clock::now() + curr_to, finaldeadline); + curr_to *= 2; { lock cal(ca.m); - while (!ca.done){ + while (!ca.done) { IF_LEVEL(2) LOG("wait"); - if(ca.c.wait_until(cal, nextdeadline) == cv_status::timeout){ + if (ca.c.wait_until(cal, nextdeadline) == cv_status::timeout) { IF_LEVEL(2) LOG("timeout"); break; } } - if(ca.done){ + if (ca.done) { IF_LEVEL(2) LOG("reply received"); break; } } - if(retrans_ && (!ch || ch->isdead())) { + if (nextdeadline >= finaldeadline) + break; + + if (retrans_ && (!ch || ch->isdead())) { // since connection is dead, retransmit // on the new connection transmit = true; } - curr_to *= 2; } { @@ -233,9 +226,8 @@ int rpcc::call1(proc_t proc, marshall &req, string &rep, milliseconds to) { // I don't think there's any harm in maybe doing it twice update_xid_rep(ca.xid); - if(destroy_wait_){ - destroy_wait_c_.notify_one(); - } + if (destroy_wait_) + destroy_wait_c_.notify_one(); } if (ca.done && lossytest_) @@ -259,9 +251,7 @@ int rpcc::call1(proc_t proc, marshall &req, string &rep, milliseconds to) { return (ca.done? ca.intret : rpc_const::timeout_failure); } -void -rpcc::get_refconn(shared_ptr & ch) -{ +void rpcc::get_refconn(shared_ptr & ch) { lock ml(chan_m_); if (!chan_ || chan_->isdead()) chan_ = connection::to_dst(dst_, this, lossytest_); @@ -282,7 +272,7 @@ rpcc::got_pdu(const shared_ptr &, const string & b) reply_header h; rep.unpack_header(h); - if(!rep.ok()){ + if (!rep.ok()) { IF_LEVEL(1) LOG("unmarshall header failed!!!"); return true; } @@ -291,17 +281,17 @@ rpcc::got_pdu(const shared_ptr &, const string & b) update_xid_rep(h.xid); - if(calls_.find(h.xid) == calls_.end()){ + if (calls_.find(h.xid) == calls_.end()) { IF_LEVEL(2) LOG("xid " << h.xid << " no pending request"); return true; } caller *ca = calls_[h.xid]; lock cl(ca->m); - if(!ca->done){ + if (!ca->done) { *ca->rep = b; ca->intret = h.ret; - if(ca->intret < 0){ + if (ca->intret < 0) { IF_LEVEL(2) LOG("RPC reply error for xid " << h.xid << " intret " << ca->intret); } ca->done = 1; @@ -314,12 +304,11 @@ rpcc::got_pdu(const shared_ptr &, const string & b) void rpcc::update_xid_rep(int xid) { - if(xid <= xid_rep_window_.front()){ + if (xid <= xid_rep_window_.front()) return; - } - for (auto it = xid_rep_window_.begin(); it != xid_rep_window_.end(); it++){ - if(*it > xid){ + for (auto it = xid_rep_window_.begin(); it != xid_rep_window_.end(); it++) { + if (*it > xid) { xid_rep_window_.insert(it, xid); goto compress; } @@ -328,7 +317,7 @@ rpcc::update_xid_rep(int xid) compress: auto it = xid_rep_window_.begin(); - for (it++; it != xid_rep_window_.end(); it++){ + for (it++; it != xid_rep_window_.end(); it++) { while (xid_rep_window_.front() + 1 == *it) xid_rep_window_.pop_front(); } @@ -350,18 +339,15 @@ void rpcs::start() { listener_ = unique_ptr(new tcpsconn(this, port_, loss_env ? atoi(loss_env) : 0)); } -rpcs::~rpcs() -{ +rpcs::~rpcs() { // must delete listener before dispatchpool listener_ = nullptr; dispatchpool_ = nullptr; free_reply_window(); } -bool -rpcs::got_pdu(const shared_ptr & c, const string & b) -{ - if(!reachable_){ +bool rpcs::got_pdu(const shared_ptr & c, const string & b) { + if (!reachable_) { IF_LEVEL(1) LOG("not reachable"); return true; } @@ -369,22 +355,18 @@ rpcs::got_pdu(const shared_ptr & c, const string & b) return dispatchpool_->addJob(bind(&rpcs::dispatch, this, c, b)); } -void -rpcs::reg1(proc_t proc, handler *h) -{ +void rpcs::reg1(proc_t proc, handler *h) { lock pl(procs_m_); VERIFY(procs_.count(proc) == 0); procs_[proc] = h; VERIFY(procs_.count(proc) >= 1); } -void -rpcs::updatestat(proc_t proc) -{ +void rpcs::updatestat(proc_t proc) { lock cl(count_m_); counts_[proc]++; curr_counts_--; - if(curr_counts_ == 0){ + if (curr_counts_ == 0) { LOG("RPC STATS: "); for (auto i = counts_.begin(); i != counts_.end(); i++) LOG(hex << i->first << ":" << dec << i->second); @@ -394,7 +376,7 @@ rpcs::updatestat(proc_t proc) size_t totalrep = 0, maxrep = 0; for (auto clt : reply_window_) { totalrep += clt.second.size(); - if(clt.second.size() > maxrep) + if (clt.second.size() > maxrep) maxrep = clt.second.size(); } IF_LEVEL(1) LOG("REPLY WINDOW: clients " << (reply_window_.size()-1) << " total reply " << @@ -422,7 +404,7 @@ void rpcs::dispatch(shared_ptr c, const string & buf) { reply_header rh{h.xid,0}; // is client sending to an old instance of server? - if(h.srv_nonce != 0 && h.srv_nonce != nonce_){ + if (h.srv_nonce != 0 && h.srv_nonce != nonce_) { IF_LEVEL(2) LOG("rpc for an old server instance " << h.srv_nonce << " (current " << nonce_ << ") proc " << hex << h.proc); rh.ret = rpc_const::oldsrv_failure; @@ -435,7 +417,7 @@ void rpcs::dispatch(shared_ptr c, const string & buf) { // is RPC proc a registered procedure? { lock pl(procs_m_); - if(procs_.count(proc) < 1){ + if (procs_.count(proc) < 1) { LOG("unknown proc 0x" << hex << proc << " with h.srv_nonce=" << h.srv_nonce << ", my srv_nonce=" << nonce_); VERIFY(0); return; @@ -447,12 +429,12 @@ void rpcs::dispatch(shared_ptr c, const string & buf) { rpcs::rpcstate_t stat; string b1; - if(h.clt_nonce){ + if (h.clt_nonce) { // have i seen this client before? { lock rwl(reply_window_m_); // if we don't know about this clt_nonce, create a cleanup object - if(reply_window_.find(h.clt_nonce) == reply_window_.end()){ + if (reply_window_.find(h.clt_nonce) == reply_window_.end()) { VERIFY (reply_window_[h.clt_nonce].size() == 0); // create reply_window_[h.clt_nonce].push_back(reply_t(-1)); // store starting reply xid IF_LEVEL(2) LOG("new client " << h.clt_nonce << " xid " << h.xid << @@ -465,7 +447,7 @@ void rpcs::dispatch(shared_ptr c, const string & buf) { lock rwl(conns_m_); if (conns_.find(h.clt_nonce) == conns_.end()) conns_[h.clt_nonce] = c; - else if(conns_[h.clt_nonce]->create_time() < c->create_time()) + else if (conns_[h.clt_nonce]->create_time() < c->create_time()) conns_[h.clt_nonce] = c; } @@ -614,24 +596,21 @@ int rpcs::rpcbind(unsigned int &r, int) { return 0; } -static sockaddr_in make_sockaddr(const string &host, const string &port); - static sockaddr_in make_sockaddr(const string &hostandport) { + string host = "127.0.0.1"; + string port = hostandport; auto colon = hostandport.find(':'); - if (colon == string::npos) - return make_sockaddr("127.0.0.1", hostandport); - else - return make_sockaddr(hostandport.substr(0, colon), hostandport.substr(colon+1)); -} + if (colon != string::npos) { + host = hostandport.substr(0, colon); + port = hostandport.substr(colon+1); + } -static sockaddr_in make_sockaddr(const string &host, const string &port) { - sockaddr_in dst; - bzero(&dst, sizeof(dst)); + sockaddr_in dst{}; // zero initialize dst.sin_family = AF_INET; struct in_addr a{inet_addr(host.c_str())}; - if(a.s_addr != INADDR_NONE) + if (a.s_addr != INADDR_NONE) dst.sin_addr.s_addr = a.s_addr; else { struct hostent *hp = gethostbyname(host.c_str()); diff --git a/rpc/rpc.h b/rpc/rpc.h index 4f9a231..84c12f3 100644 --- a/rpc/rpc.h +++ b/rpc/rpc.h @@ -30,10 +30,10 @@ class rpc_const { // rpc client endpoint. // manages a xid space per destination socket // threaded: multiple threads can be sending RPCs, -class rpcc : public connection_delegate { +class rpcc : private connection_delegate { private: - //manages per rpc info + // manages per rpc info struct caller { caller(int _xid, string *_rep) : xid(_xid), rep(_rep) {} @@ -81,7 +81,23 @@ class rpcc : public connection_delegate { int call1(proc_t proc, marshall &req, string &rep, milliseconds to); template - int call_m(proc_t proc, marshall &req, R & r, milliseconds to); + int call_m(proc_t proc, marshall &req, R & r, milliseconds to) { + string rep; + int intret = call1(proc, req, rep, to); + unmarshall u(rep, true); + if (intret < 0) return intret; + u >> r; + if (u.okdone() != true) { + cerr << "rpcc::call_m: failed to unmarshall the reply. You are probably " << + "calling RPC 0x" << hex << proc << " with the wrong return type." << endl; + VERIFY(0); + return rpc_const::unmarshal_reply_failure; + } + return intret; + } + + bool got_pdu(const shared_ptr & c, const string & b); + public: rpcc(const string & d, bool retrans=true); @@ -95,136 +111,105 @@ class rpcc : public connection_delegate { void cancel(); - bool got_pdu(const shared_ptr & c, const string & b); - template - inline int call(proc_t proc, R & r, const Args&... args); + inline int call(proc_t proc, R & r, const Args&... args) { + return call_timeout(proc, rpc::to_max, r, args...); + } template - inline int call_timeout(proc_t proc, milliseconds to, R & r, const Args&... args); + inline int call_timeout(proc_t proc, milliseconds to, R & r, const Args&... args) { + marshall m{args...}; + return call_m(proc, m, r, to); + } }; -template int -rpcc::call_m(proc_t proc, marshall &req, R & r, milliseconds to) -{ - string rep; - int intret = call1(proc, req, rep, to); - unmarshall u(rep, true); - if (intret < 0) return intret; - u >> r; - if (u.okdone() != true) { - cerr << "rpcc::call_m: failed to unmarshall the reply. You are probably " << - "calling RPC 0x" << hex << proc << " with the wrong return type." << endl; - VERIFY(0); - return rpc_const::unmarshal_reply_failure; - } - return intret; -} - -template inline int -rpcc::call(proc_t proc, R & r, const Args&... args) -{ - return call_timeout(proc, rpc::to_max, r, args...); -} - -template inline int -rpcc::call_timeout(proc_t proc, const milliseconds to, R & r, const Args&... args) -{ - marshall m{args...}; - return call_m(proc, m, r, to); -} - // rpc server endpoint. -class rpcs : public connection_delegate { - - typedef enum { - NEW, // new RPC, not a duplicate - INPROGRESS, // duplicate of an RPC we're still processing - DONE, // duplicate of an RPC we already replied to (have reply) - FORGOTTEN, // duplicate of an old RPC whose reply we've forgotten - } rpcstate_t; - +class rpcs : private connection_delegate { private: + typedef enum { + NEW, // new RPC, not a duplicate + INPROGRESS, // duplicate of an RPC we're still processing + DONE, // duplicate of an RPC we already replied to (have reply) + FORGOTTEN, // duplicate of an old RPC whose reply we've forgotten + } rpcstate_t; + // state about an in-progress or completed RPC, for at-most-once. // if cb_present is true, then the RPC is complete and a reply // has been sent; in that case buf points to a copy of the reply, // and sz holds the size of the reply. - struct reply_t { - reply_t (int _xid) : xid(_xid), cb_present(false) {} - reply_t (int _xid, const string & _buf) : xid(_xid), cb_present(true), buf(_buf) {} - int xid; - bool cb_present; // whether the reply buffer is valid - string buf; // the reply buffer - }; - - in_port_t port_; - unsigned int nonce_; - - // provide at most once semantics by maintaining a window of replies - // per client that that client hasn't acknowledged receiving yet. + struct reply_t { + reply_t (int _xid) : xid(_xid), cb_present(false) {} + reply_t (int _xid, const string & _buf) : xid(_xid), cb_present(true), buf(_buf) {} + int xid; + bool cb_present; // whether the reply buffer is valid + string buf; // the reply buffer + }; + + in_port_t port_; + unsigned int nonce_; + + // provide at most once semantics by maintaining a window of replies + // per client that that client hasn't acknowledged receiving yet. // indexed by client nonce. - map > reply_window_; + map> reply_window_; - void free_reply_window(void); - void add_reply(unsigned int clt_nonce, int xid, const string & b); + void free_reply_window(void); + void add_reply(unsigned int clt_nonce, int xid, const string & b); - rpcstate_t checkduplicate_and_update(unsigned int clt_nonce, - int xid, int rep_xid, string & b); + rpcstate_t checkduplicate_and_update(unsigned int clt_nonce, + int xid, int rep_xid, string & b); - void updatestat(proc_t proc); + void updatestat(proc_t proc); - // latest connection to the client - map> conns_; + // latest connection to the client + map> conns_; - // counting - const size_t counting_; - size_t curr_counts_; - map counts_; + // counting + const size_t counting_; + size_t curr_counts_; + map counts_; - bool reachable_; + bool reachable_; - // map proc # to function - map procs_; + // map proc # to function + map procs_; - mutex procs_m_; // protect insert/delete to procs[] - mutex count_m_; //protect modification of counts - mutex reply_window_m_; // protect reply window et al - mutex conns_m_; // protect conns_ + mutex procs_m_; // protect insert/delete to procs[] + mutex count_m_; // protect modification of counts + mutex reply_window_m_; // protect reply window et al + mutex conns_m_; // protect conns_ + void dispatch(shared_ptr c, const string & buf); - protected: + // internal handler registration + void reg1(proc_t proc, handler *); - void dispatch(shared_ptr c, const string & buf); + unique_ptr dispatchpool_; + unique_ptr listener_; - // internal handler registration - void reg1(proc_t proc, handler *); + // RPC handler for clients binding + int rpcbind(unsigned int &r, int a); - unique_ptr dispatchpool_; - unique_ptr listener_; + bool got_pdu(const shared_ptr & c, const string & b); public: - rpcs(in_port_t port, size_t counts=0); - ~rpcs(); - inline in_port_t port() { return listener_->port(); } - //RPC handler for clients binding - int rpcbind(unsigned int &r, int a); - void set_reachable(bool r) { reachable_ = r; } + rpcs(in_port_t port, size_t counts=0); + ~rpcs(); - bool got_pdu(const shared_ptr & c, const string & b); + void set_reachable(bool r) { reachable_ = r; } - struct ReturnOnFailure { - static inline int unmarshall_args_failure() { - return rpc_const::unmarshal_args_failure; + template void reg(proc_t proc, F f, C *c=nullptr) { + struct ReturnOnFailure { + static inline int unmarshall_args_failure() { + return rpc_const::unmarshal_args_failure; + } + }; + reg1(proc, marshalled_func::wrap(f, c)); } - }; - - template void reg(proc_t proc, F f, C *c=nullptr) { - reg1(proc, marshalled_func::wrap(f, c)); - } - void start(); + void start(); }; #endif -- 1.7.9.5