From f7cbbfd209d8df2dec21da907e48bb09c5429484 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 2 Jul 2016 16:05:22 +0900 Subject: [PATCH] Retain peers obtained earlier --- src/DefaultPeerStorage.cc | 63 +++++++++++++++++++++------------- test/DefaultPeerStorageTest.cc | 48 ++++++++++++++------------ 2 files changed, 66 insertions(+), 45 deletions(-) diff --git a/src/DefaultPeerStorage.cc b/src/DefaultPeerStorage.cc index dc5353f6..34b1fe89 100644 --- a/src/DefaultPeerStorage.cc +++ b/src/DefaultPeerStorage.cc @@ -54,7 +54,6 @@ namespace aria2 { namespace { const size_t MAX_PEER_LIST_SIZE = 512; -const size_t MAX_PEER_LIST_UPDATE = 100; } // namespace @@ -89,6 +88,14 @@ void DefaultPeerStorage::addUniqPeer(const std::shared_ptr& peer) bool DefaultPeerStorage::addPeer(const std::shared_ptr& peer) { + if (unusedPeers_.size() >= maxPeerListSize_) { + A2_LOG_DEBUG(fmt("Adding %s:%u is rejected, since unused peer list is full " + "(%lu peers > %lu)", + peer->getIPAddress().c_str(), peer->getPort(), + static_cast(unusedPeers_.size()), + static_cast(maxPeerListSize_))); + return false; + } if (isPeerAlreadyAdded(peer)) { A2_LOG_DEBUG(fmt("Adding %s:%u is rejected because it has been already" " added.", @@ -104,7 +111,7 @@ bool DefaultPeerStorage::addPeer(const std::shared_ptr& peer) if (peerListSize >= maxPeerListSize_) { deleteUnusedPeer(peerListSize - maxPeerListSize_ + 1); } - unusedPeers_.push_front(peer); + unusedPeers_.push_back(peer); addUniqPeer(peer); A2_LOG_DEBUG(fmt("Now unused peer list contains %lu peers", static_cast(unusedPeers_.size()))); @@ -114,29 +121,36 @@ bool DefaultPeerStorage::addPeer(const std::shared_ptr& peer) void DefaultPeerStorage::addPeer( const std::vector>& peers) { - size_t added = 0; - size_t addMax = std::min(maxPeerListSize_, MAX_PEER_LIST_UPDATE); - for (auto itr = std::begin(peers), eoi = std::end(peers); - itr != eoi && added < addMax; ++itr) { - auto& peer = *itr; - if (isPeerAlreadyAdded(peer)) { - A2_LOG_DEBUG(fmt("Adding %s:%u is rejected because it has been already" - " added.", - peer->getIPAddress().c_str(), peer->getPort())); - continue; + if (unusedPeers_.size() < maxPeerListSize_) { + for (auto& peer : peers) { + if (isPeerAlreadyAdded(peer)) { + A2_LOG_DEBUG(fmt("Adding %s:%u is rejected because it has been already" + " added.", + peer->getIPAddress().c_str(), peer->getPort())); + continue; + } + else if (isBadPeer(peer->getIPAddress())) { + A2_LOG_DEBUG(fmt("Adding %s:%u is rejected because it is marked bad.", + peer->getIPAddress().c_str(), peer->getPort())); + continue; + } + else { + A2_LOG_DEBUG(fmt(MSG_ADDING_PEER, peer->getIPAddress().c_str(), + peer->getPort())); + } + unusedPeers_.push_back(peer); + addUniqPeer(peer); } - else if (isBadPeer(peer->getIPAddress())) { - A2_LOG_DEBUG(fmt("Adding %s:%u is rejected because it is marked bad.", - peer->getIPAddress().c_str(), peer->getPort())); - continue; - } - else { + } + else { + for (auto& peer : peers) { A2_LOG_DEBUG( - fmt(MSG_ADDING_PEER, peer->getIPAddress().c_str(), peer->getPort())); + fmt("Adding %s:%u is rejected, since unused peer list is full " + "(%lu peers > %lu)", + peer->getIPAddress().c_str(), peer->getPort(), + static_cast(unusedPeers_.size()), + static_cast(maxPeerListSize_))); } - unusedPeers_.push_front(peer); - addUniqPeer(peer); - ++added; } const size_t peerListSize = unusedPeers_.size(); if (peerListSize > maxPeerListSize_) { @@ -220,7 +234,10 @@ void DefaultPeerStorage::addBadPeer(const std::string& ipaddr) void DefaultPeerStorage::deleteUnusedPeer(size_t delSize) { for (; delSize > 0 && !unusedPeers_.empty(); --delSize) { - onErasingPeer(unusedPeers_.back()); + auto& peer = unusedPeers_.back(); + onErasingPeer(peer); + A2_LOG_DEBUG(fmt("Remove peer %s:%u", peer->getIPAddress().c_str(), + peer->getOrigPort())); unusedPeers_.pop_back(); } } diff --git a/test/DefaultPeerStorageTest.cc b/test/DefaultPeerStorageTest.cc index 7249458f..ddb870ac 100644 --- a/test/DefaultPeerStorageTest.cc +++ b/test/DefaultPeerStorageTest.cc @@ -72,9 +72,9 @@ void DefaultPeerStorageTest::testDeleteUnusedPeer() { DefaultPeerStorage ps; - std::shared_ptr peer1(new Peer("192.168.0.1", 6889)); - std::shared_ptr peer2(new Peer("192.168.0.2", 6889)); - std::shared_ptr peer3(new Peer("192.168.0.3", 6889)); + auto peer1 = std::make_shared("192.168.0.1", 6889); + auto peer2 = std::make_shared("192.168.0.2", 6889); + auto peer3 = std::make_shared("192.168.0.3", 6889); CPPUNIT_ASSERT(ps.addPeer(peer1)); CPPUNIT_ASSERT(ps.addPeer(peer2)); @@ -83,7 +83,7 @@ void DefaultPeerStorageTest::testDeleteUnusedPeer() ps.deleteUnusedPeer(2); CPPUNIT_ASSERT_EQUAL((size_t)1, ps.getUnusedPeers().size()); - CPPUNIT_ASSERT_EQUAL(std::string("192.168.0.3"), + CPPUNIT_ASSERT_EQUAL(std::string("192.168.0.1"), ps.getUnusedPeers()[0]->getIPAddress()); ps.deleteUnusedPeer(100); @@ -97,20 +97,20 @@ void DefaultPeerStorageTest::testAddPeer() ps.setMaxPeerListSize(2); ps.setBtRuntime(btRuntime); - std::shared_ptr peer1(new Peer("192.168.0.1", 6889)); - std::shared_ptr peer2(new Peer("192.168.0.2", 6889)); - std::shared_ptr peer3(new Peer("192.168.0.3", 6889)); + auto peer1 = std::make_shared("192.168.0.1", 6889); + auto peer2 = std::make_shared("192.168.0.2", 6889); + auto peer3 = std::make_shared("192.168.0.3", 6889); CPPUNIT_ASSERT(ps.addPeer(peer1)); CPPUNIT_ASSERT(ps.addPeer(peer2)); - CPPUNIT_ASSERT(ps.addPeer(peer3)); + CPPUNIT_ASSERT(!ps.addPeer(peer3)); CPPUNIT_ASSERT_EQUAL((size_t)2, ps.getUnusedPeers().size()); - CPPUNIT_ASSERT_EQUAL(std::string("192.168.0.3"), + CPPUNIT_ASSERT_EQUAL(std::string("192.168.0.1"), ps.getUnusedPeers()[0]->getIPAddress()); CPPUNIT_ASSERT(!ps.addPeer(peer2)); - CPPUNIT_ASSERT(ps.addPeer(peer1)); + CPPUNIT_ASSERT(!ps.addPeer(peer3)); CPPUNIT_ASSERT_EQUAL((size_t)2, ps.getUnusedPeers().size()); CPPUNIT_ASSERT_EQUAL(std::string("192.168.0.1"), @@ -137,20 +137,24 @@ void DefaultPeerStorageTest::testIsPeerAvailable() void DefaultPeerStorageTest::testCheckoutPeer() { DefaultPeerStorage ps; - std::shared_ptr peers[] = { - std::shared_ptr(new Peer("192.168.0.1", 1000)), - std::shared_ptr(new Peer("192.168.0.2", 1000)), - std::shared_ptr(new Peer("192.168.0.3", 1000))}; - int len = arraySize(peers); - for (int i = 0; i < len; ++i) { - ps.addPeer(peers[i]); + + auto peers = { + std::make_shared("192.168.0.1", 1000), + std::make_shared("192.168.0.2", 1000), + std::make_shared("192.168.0.3", 1000), + }; + + for (auto& peer : peers) { + ps.addPeer(peer); } - for (int i = 0; i < len; ++i) { - std::shared_ptr peer = ps.checkoutPeer(i + 1); - CPPUNIT_ASSERT_EQUAL(peers[len - i - 1]->getIPAddress(), - peer->getIPAddress()); + + int i = 0; + for (auto& peer : peers) { + auto p = ps.checkoutPeer(i + 1); + ++i; + CPPUNIT_ASSERT_EQUAL(peer->getIPAddress(), p->getIPAddress()); } - CPPUNIT_ASSERT(!ps.checkoutPeer(len + 1)); + CPPUNIT_ASSERT(!ps.checkoutPeer(peers.size() + 1)); } void DefaultPeerStorageTest::testReturnPeer()