From c74a1b89e3ce69c2c94a9accdf9347d04e1edc6d Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 9 Jan 2013 03:08:00 -0800 Subject: [PATCH 01/19] By careful how many GetObjectByHash requests we send. --- src/cpp/ripple/LedgerAcquire.cpp | 21 +++++++++++++++------ src/cpp/ripple/LedgerAcquire.h | 2 +- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/cpp/ripple/LedgerAcquire.cpp b/src/cpp/ripple/LedgerAcquire.cpp index 462ab33ba..01cf1bfba 100644 --- a/src/cpp/ripple/LedgerAcquire.cpp +++ b/src/cpp/ripple/LedgerAcquire.cpp @@ -74,8 +74,8 @@ void PeerSet::TimerEntry(boost::weak_ptr wptr, const boost::system::err ptr->invokeOnTimer(); } -LedgerAcquire::LedgerAcquire(const uint256& hash) : PeerSet(hash, LEDGER_ACQUIRE_TIMEOUT), - mHaveBase(false), mHaveState(false), mHaveTransactions(false), mAborted(false), mSignaled(false), mAccept(false) +LedgerAcquire::LedgerAcquire(const uint256& hash) : PeerSet(hash, LEDGER_ACQUIRE_TIMEOUT), mHaveBase(false), + mHaveState(false), mHaveTransactions(false), mAborted(false), mSignaled(false), mAccept(false), mByHash(true) { #ifdef LA_DEBUG cLog(lsTRACE) << "Acquiring ledger " << mHash; @@ -138,6 +138,8 @@ void LedgerAcquire::onTimer(bool progress) else trigger(Peer::pointer()); } + else + mByHash = true; } void LedgerAcquire::addPeers() @@ -234,7 +236,7 @@ void LedgerAcquire::trigger(Peer::ref peer) { tmGL.set_querytype(ripple::qtINDIRECT); - if (!isProgress()) + if (!isProgress() && mByHash) { std::vector need = getNeededHashes(); if (!need.empty()) @@ -260,9 +262,6 @@ void LedgerAcquire::trigger(Peer::ref peer) } } PackedMessage::pointer packet = boost::make_shared(tmBH, ripple::mtGET_OBJECTS); - if (peer) - peer->sendPacket(packet); - else { boost::recursive_mutex::scoped_lock sl(mLock); for (boost::unordered_map::iterator it = mPeers.begin(), end = mPeers.end(); @@ -270,10 +269,20 @@ void LedgerAcquire::trigger(Peer::ref peer) { Peer::pointer iPeer = theApp->getConnectionPool().getPeerById(it->first); if (iPeer) + { + mByHash = false; iPeer->sendPacket(packet); + } } } } + else + { + cLog(lsINFO) << "getNeededHashes says acquire is complete"; + mHaveBase = true; + mHaveTransactions = true; + mHaveState = true; + } } } diff --git a/src/cpp/ripple/LedgerAcquire.h b/src/cpp/ripple/LedgerAcquire.h index 2ef1384d7..b5c338612 100644 --- a/src/cpp/ripple/LedgerAcquire.h +++ b/src/cpp/ripple/LedgerAcquire.h @@ -78,7 +78,7 @@ public: protected: Ledger::pointer mLedger; - bool mHaveBase, mHaveState, mHaveTransactions, mAborted, mSignaled, mAccept; + bool mHaveBase, mHaveState, mHaveTransactions, mAborted, mSignaled, mAccept, mByHash; std::vector< boost::function > mOnComplete; From 0778a3ebae5f7b38b5e62912716a2f724646d553 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 9 Jan 2013 03:16:59 -0800 Subject: [PATCH 02/19] Don't blow up if asked to acquire a hash that's not a ledger. --- src/cpp/ripple/LedgerAcquire.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/cpp/ripple/LedgerAcquire.cpp b/src/cpp/ripple/LedgerAcquire.cpp index 01cf1bfba..9941fc067 100644 --- a/src/cpp/ripple/LedgerAcquire.cpp +++ b/src/cpp/ripple/LedgerAcquire.cpp @@ -90,7 +90,11 @@ bool LedgerAcquire::tryLocal() return false; mLedger = boost::make_shared(strCopy(node->getData()), true); - assert(mLedger->getHash() == mHash); + if (mLedger->getHash() != mHash) + { // We know for a fact the ledger can never be acquired + mFailed = true; + return true; + } mHaveBase = true; if (!mLedger->getTransHash()) @@ -236,7 +240,7 @@ void LedgerAcquire::trigger(Peer::ref peer) { tmGL.set_querytype(ripple::qtINDIRECT); - if (!isProgress() && mByHash) + if (!isProgress() && !mFailed && mByHash) { std::vector need = getNeededHashes(); if (!need.empty()) @@ -287,7 +291,7 @@ void LedgerAcquire::trigger(Peer::ref peer) } - if (!mHaveBase) + if (!mHaveBase && !mFailed) { tmGL.set_itype(ripple::liBASE); cLog(lsTRACE) << "Sending base request to " << (peer ? "selected peer" : "all peers"); @@ -299,7 +303,7 @@ void LedgerAcquire::trigger(Peer::ref peer) if (mLedger) tmGL.set_ledgerseq(mLedger->getLedgerSeq()); - if (mHaveBase && !mHaveTransactions) + if (mHaveBase && !mHaveTransactions && !mFailed) { assert(mLedger); if (mLedger->peekTransactionMap()->getHash().isZero()) @@ -340,7 +344,7 @@ void LedgerAcquire::trigger(Peer::ref peer) } } - if (mHaveBase && !mHaveState) + if (mHaveBase && !mHaveState && !mFailed) { assert(mLedger); if (mLedger->peekAccountStateMap()->getHash().isZero()) From b5e78bda342886f6dcb77481e529cb48763f47aa Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 9 Jan 2013 03:28:20 -0800 Subject: [PATCH 03/19] Fix GetObjByHash replies. --- src/cpp/ripple/Peer.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cpp/ripple/Peer.cpp b/src/cpp/ripple/Peer.cpp index 4c66f6e85..7290b89ad 100644 --- a/src/cpp/ripple/Peer.cpp +++ b/src/cpp/ripple/Peer.cpp @@ -1089,7 +1089,7 @@ void Peer::recvGetObjectByHash(ripple::TMGetObjectByHash& packet) { // this is a query ripple::TMGetObjectByHash reply; - reply.clear_query(); + reply.set_query(false); if (packet.has_seq()) reply.set_seq(packet.seq()); reply.set_type(packet.type()); @@ -1117,7 +1117,7 @@ void Peer::recvGetObjectByHash(ripple::TMGetObjectByHash& packet) } } } - cLog(lsTRACE) << "GetObjByHash query: had " << reply.objects_size() << " of " << packet.objects_size() + cLog(lsTRACE) << "GetObjByHash had " << reply.objects_size() << " of " << packet.objects_size() << " for " << getIP(); sendPacket(boost::make_shared(packet, ripple::mtGET_OBJECTS)); } From e714a16b9551f508bd1f9b24ef25007b4c4d3f3e Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 9 Jan 2013 07:14:07 -0800 Subject: [PATCH 04/19] Optimizations. --- src/cpp/ripple/SHAMapSync.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/cpp/ripple/SHAMapSync.cpp b/src/cpp/ripple/SHAMapSync.cpp index 65bd73408..d564eebb9 100644 --- a/src/cpp/ripple/SHAMapSync.cpp +++ b/src/cpp/ripple/SHAMapSync.cpp @@ -32,12 +32,12 @@ void SHAMap::getMissingNodes(std::vector& nodeIDs, std::vector stack; - stack.push(root); + std::stack stack; + stack.push(root.get()); while (!stack.empty()) { - SHAMapTreeNode::pointer node = stack.top(); + SHAMapTreeNode* node = stack.top(); stack.pop(); int base = rand() % 256; @@ -49,10 +49,10 @@ void SHAMap::getMissingNodes(std::vector& nodeIDs, std::vectorgetChildNodeID(branch); const uint256& childHash = node->getChildHash(branch); - SHAMapTreeNode::pointer d; + SHAMapTreeNode* d; try { - d = getNode(childID, childHash, false); + d = getNodePointer(childID, childHash); } catch (SHAMapMissingNode&) { // node is not in the map @@ -61,9 +61,11 @@ void SHAMap::getMissingNodes(std::vector& nodeIDs, std::vector nodeData; if (filter->haveNode(childID, childHash, nodeData)) { - d = boost::make_shared(childID, nodeData, mSeq, snfPREFIX, childHash); + SHAMapTreeNode::pointer ptr = + boost::make_shared(childID, nodeData, mSeq, snfPREFIX, childHash); cLog(lsTRACE) << "Got sync node from cache: " << *d; - mTNByID[*d] = d; + mTNByID[*ptr] = ptr; + d = ptr.get(); } } } From 9a2e2d78c627085baede12ae50072015f84904b8 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 9 Jan 2013 09:21:48 -0800 Subject: [PATCH 05/19] Extra debug --- src/cpp/ripple/LedgerMaster.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cpp/ripple/LedgerMaster.cpp b/src/cpp/ripple/LedgerMaster.cpp index 71602afaa..80e59b655 100644 --- a/src/cpp/ripple/LedgerMaster.cpp +++ b/src/cpp/ripple/LedgerMaster.cpp @@ -383,6 +383,8 @@ void LedgerMaster::checkAccept(const uint256& hash, uint32 seq) if (theApp->getValidations().getTrustedValidationCount(hash) < minVal) // nothing we can do return; + cLog(lsINFO) << "Advancing accepted ledger to " << seq << " with >= " << minVal << " validations"; + mLastValidateHash = hash; mLastValidateSeq = seq; From 4f1619eb5e3aadcc4375fd51f6d021dc99cae6b0 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 9 Jan 2013 09:21:58 -0800 Subject: [PATCH 06/19] Extra debug. --- src/cpp/ripple/LedgerAcquire.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/cpp/ripple/LedgerAcquire.cpp b/src/cpp/ripple/LedgerAcquire.cpp index 9941fc067..f18675a37 100644 --- a/src/cpp/ripple/LedgerAcquire.cpp +++ b/src/cpp/ripple/LedgerAcquire.cpp @@ -92,6 +92,7 @@ bool LedgerAcquire::tryLocal() mLedger = boost::make_shared(strCopy(node->getData()), true); if (mLedger->getHash() != mHash) { // We know for a fact the ledger can never be acquired + cLog(lsWARNING) << mHash << " cannot be a ledger"; mFailed = true; return true; } @@ -130,6 +131,7 @@ void LedgerAcquire::onTimer(bool progress) { if (getTimeouts() > 6) { + cLog(lsWARNING) << "Six timeouts for ledger " << mHash; setFailed(); done(); return; @@ -137,6 +139,7 @@ void LedgerAcquire::onTimer(bool progress) if (!progress) { + cLog(lsDEBUG) << "No progress for ledger " << mHash; if (!getPeerCount()) addPeers(); else @@ -232,7 +235,11 @@ void LedgerAcquire::trigger(Peer::ref peer) } if (!mHaveBase) + { tryLocal(); + if (mFailed) + cLog(lsWARNING) << " failed local for " << mHash; + } ripple::TMGetLedger tmGL; tmGL.set_ledgerhash(mHash.begin(), mHash.size()); @@ -288,7 +295,6 @@ void LedgerAcquire::trigger(Peer::ref peer) mHaveState = true; } } - } if (!mHaveBase && !mFailed) From eadaaaa0fda9c427a49cbca8cd402ed7920c040b Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 9 Jan 2013 09:22:25 -0800 Subject: [PATCH 07/19] Clean up debug. --- src/cpp/ripple/LedgerConsensus.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/cpp/ripple/LedgerConsensus.cpp b/src/cpp/ripple/LedgerConsensus.cpp index fc5ee28cc..ce2446585 100644 --- a/src/cpp/ripple/LedgerConsensus.cpp +++ b/src/cpp/ripple/LedgerConsensus.cpp @@ -1190,7 +1190,7 @@ uint32 LedgerConsensus::roundCloseTime(uint32 closeTime) void LedgerConsensus::accept(SHAMap::ref set, LoadEvent::pointer) { - if (set->getHash().isNonZero()) + if (set->getHash().isNonZero()) // put our set where others can get it later theApp->getOPs().takePosition(mPreviousLedger->getLedgerSeq(), set); boost::recursive_mutex::scoped_lock masterLock(theApp->getMasterLock()); @@ -1198,7 +1198,10 @@ void LedgerConsensus::accept(SHAMap::ref set, LoadEvent::pointer) uint32 closeTime = roundCloseTime(mOurPosition->getCloseTime()); - cLog(lsDEBUG) << "Computing new LCL based on network consensus"; + cLog(lsDEBUG) << "Report: Prop=" << (mProposing ? "yes" : "no") << " val=" << (mValidating ? "yes" : "no") << + " corLCL=" << (mHaveCorrectLCL ? "yes" : "no") << " fail="<< (mConsensusFail ? "yes" : "no"); + cLog(lsDEBUG) << "Report: Prev = " << mPrevLedgerHash << ":" << mPreviousLedger->getLedgerSeq(); + cLog(lsDEBUG) << "Report: TxSt = " << set->getHash() << ", close " << closeTime << (closeTimeCorrect ? "" : "X"); CanonicalTXSet failedTransactions(set->getHash()); @@ -1227,10 +1230,6 @@ void LedgerConsensus::accept(SHAMap::ref set, LoadEvent::pointer) closeTime = mPreviousLedger->getCloseTimeNC() + 1; } - cLog(lsDEBUG) << "Report: Prop=" << (mProposing ? "yes" : "no") << " val=" << (mValidating ? "yes" : "no") << - " corLCL=" << (mHaveCorrectLCL ? "yes" : "no") << " fail="<< (mConsensusFail ? "yes" : "no"); - cLog(lsDEBUG) << "Report: Prev = " << mPrevLedgerHash << ":" << mPreviousLedger->getLedgerSeq(); - cLog(lsDEBUG) << "Report: TxSt = " << set->getHash() << ", close " << closeTime << (closeTimeCorrect ? "" : "X"); cLog(lsDEBUG) << "Report: NewL = " << newLCL->getHash() << ":" << newLCL->getLedgerSeq(); newLCL->setAccepted(closeTime, mCloseResolution, closeTimeCorrect); From e502badd2b58401dca5204d37c992c177b8a92bf Mon Sep 17 00:00:00 2001 From: Stefan Thomas Date: Wed, 9 Jan 2013 18:29:12 +0100 Subject: [PATCH 08/19] Add test case for #7. --- test/offer-test.js | 47 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/test/offer-test.js b/test/offer-test.js index 2c20dafb1..2d59657b1 100644 --- a/test/offer-test.js +++ b/test/offer-test.js @@ -72,6 +72,53 @@ buster.testCase("Offer tests", { }); }, + "offer create then crossing offer, no trust lines" : + function (done) { + var self = this; + + async.waterfall([ + function (callback) { + self.remote.transaction() + .offer_create("root", "500/BTC/root", "100/USD/root") + .on('proposed', function (m) { + // console.log("PROPOSED: offer_create: %s", JSON.stringify(m)); + + callback(m.result !== 'tesSUCCESS'); + }) + .on('final', function (m) { + // console.log("FINAL: offer_create: %s", JSON.stringify(m)); + + buster.assert.equals('tesSUCCESS', m.metadata.TransactionResult); + + callback(); + }) + .submit(); + }, + function (callback) { + self.remote.transaction() + .offer_create("root", "100/USD/root", "500/BTC/root") + .on('proposed', function (m) { + // console.log("PROPOSED: offer_create: %s", JSON.stringify(m)); + + callback(m.result !== 'tesSUCCESS'); + }) + .on('final', function (m) { + // console.log("FINAL: offer_create: %s", JSON.stringify(m)); + + buster.assert.equals('tesSUCCESS', m.metadata.TransactionResult); + + callback(); + }) + .submit(); + } + ], function (error) { + // console.log("result: error=%s", error); + buster.refute(error); + + if (error) done(); + }); + }, + "offer_create then ledger_accept then offer_cancel then ledger_accept." : function (done) { var self = this; From fcabad79ae846de72ab05ec9dbc9404f5325fa63 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 9 Jan 2013 09:29:15 -0800 Subject: [PATCH 09/19] Fix the breakage. --- src/cpp/ripple/Ledger.cpp | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-) diff --git a/src/cpp/ripple/Ledger.cpp b/src/cpp/ripple/Ledger.cpp index dbeb9ab28..c12e1d92d 100644 --- a/src/cpp/ripple/Ledger.cpp +++ b/src/cpp/ripple/Ledger.cpp @@ -538,28 +538,11 @@ Ledger::pointer Ledger::loadByIndex(uint32 ledgerIndex) } Ledger::pointer Ledger::loadByHash(const uint256& ledgerHash) -{ // This is a low-level function with no caching +{ // This is a low-level function with no caching and only gets accepted ledgers std::string sql="SELECT * from Ledgers WHERE LedgerHash='"; sql.append(ledgerHash.GetHex()); sql.append("';"); - Ledger::pointer ret = getSQL(sql); - if (ret) - return ret; - HashedObject::pointer node = theApp->getHashedObjectStore().retrieve(ledgerHash); - if (!node) - return Ledger::pointer(); - try - { - Ledger::pointer ledger = boost::make_shared(strCopy(node->getData()), true); - if (ledger->getHash() == ledgerHash) - return ledger; - } - catch (...) - { - cLog(lsDEBUG) << "Exception trying to load ledger by hash: " << ledgerHash; - return Ledger::pointer(); - } - return Ledger::pointer(); + return getSQL(sql); } Ledger::pointer Ledger::getLastFullLedger() From 1acdad8601131c9293306fdb1fe91c278ec97ce1 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 9 Jan 2013 09:38:41 -0800 Subject: [PATCH 10/19] Make it compile. --- src/cpp/ripple/LedgerConsensus.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/cpp/ripple/LedgerConsensus.cpp b/src/cpp/ripple/LedgerConsensus.cpp index ce2446585..778412a56 100644 --- a/src/cpp/ripple/LedgerConsensus.cpp +++ b/src/cpp/ripple/LedgerConsensus.cpp @@ -1197,6 +1197,12 @@ void LedgerConsensus::accept(SHAMap::ref set, LoadEvent::pointer) assert(set->getHash() == mOurPosition->getCurrentHash()); uint32 closeTime = roundCloseTime(mOurPosition->getCloseTime()); + bool closeTimeCorrect = true; + if (closeTime == 0) + { // we agreed to disagree + closeTimeCorrect = false; + closeTime = mPreviousLedger->getCloseTimeNC() + 1; + } cLog(lsDEBUG) << "Report: Prop=" << (mProposing ? "yes" : "no") << " val=" << (mValidating ? "yes" : "no") << " corLCL=" << (mHaveCorrectLCL ? "yes" : "no") << " fail="<< (mConsensusFail ? "yes" : "no"); @@ -1215,7 +1221,6 @@ void LedgerConsensus::accept(SHAMap::ref set, LoadEvent::pointer) boost::shared_ptr acctNodes = newLCL->peekAccountStateMap()->disarmDirty(); boost::shared_ptr txnNodes = newLCL->peekTransactionMap()->disarmDirty(); - // write out dirty nodes (temporarily done here) Most come before setAccepted int fc; while ((fc = SHAMap::flushDirty(*acctNodes, 256, hotACCOUNT_NODE, newLCL->getLedgerSeq())) > 0) @@ -1223,13 +1228,6 @@ void LedgerConsensus::accept(SHAMap::ref set, LoadEvent::pointer) while ((fc = SHAMap::flushDirty(*txnNodes, 256, hotTRANSACTION_NODE, newLCL->getLedgerSeq())) > 0) { cLog(lsTRACE) << "Flushed " << fc << " dirty transaction nodes"; } - bool closeTimeCorrect = true; - if (closeTime == 0) - { // we agreed to disagree - closeTimeCorrect = false; - closeTime = mPreviousLedger->getCloseTimeNC() + 1; - } - cLog(lsDEBUG) << "Report: NewL = " << newLCL->getHash() << ":" << newLCL->getLedgerSeq(); newLCL->setAccepted(closeTime, mCloseResolution, closeTimeCorrect); From d2d84d2af730c737bad45b14e11a105ad7d90071 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 9 Jan 2013 09:53:39 -0800 Subject: [PATCH 11/19] More conservative check. --- src/cpp/ripple/LedgerMaster.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/ripple/LedgerMaster.cpp b/src/cpp/ripple/LedgerMaster.cpp index 80e59b655..ab0e7a923 100644 --- a/src/cpp/ripple/LedgerMaster.cpp +++ b/src/cpp/ripple/LedgerMaster.cpp @@ -196,7 +196,7 @@ void LedgerMaster::missingAcquireComplete(LedgerAcquire::pointer acq) mMissingLedger.reset(); mMissingSeq = 0; - if (!acq->isFailed()) + if (acq->isComplete()) { setFullLedger(acq->getLedger()); acq->getLedger()->pendSave(false); From 714b3fb0d09b0b3c25f6dc4f680a189ecbd8170d Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 9 Jan 2013 10:14:18 -0800 Subject: [PATCH 12/19] Small fixes. --- src/cpp/ripple/HashedObject.cpp | 4 +++- src/cpp/ripple/LedgerAcquire.cpp | 4 ++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/cpp/ripple/HashedObject.cpp b/src/cpp/ripple/HashedObject.cpp index 288639f63..49a451050 100644 --- a/src/cpp/ripple/HashedObject.cpp +++ b/src/cpp/ripple/HashedObject.cpp @@ -33,7 +33,6 @@ bool HashedObjectStore::store(HashedObjectType type, uint32 index, } assert(hash == Serializer::getSHA512Half(data)); - mNegativeCache.del(hash); HashedObject::pointer object = boost::make_shared(type, index, data, hash); if (!mCache.canonicalize(hash, object)) { @@ -48,6 +47,7 @@ bool HashedObjectStore::store(HashedObjectType type, uint32 index, } // else // cLog(lsTRACE) << "HOS: already had " << hash; + mNegativeCache.del(hash); return true; } @@ -152,6 +152,7 @@ HashedObject::pointer HashedObjectStore::retrieve(const uint256& hash) db->getStr("ObjType", type); if (type.size() == 0) { + assert(false); mNegativeCache.add(hash); return HashedObject::pointer(); } @@ -173,6 +174,7 @@ HashedObject::pointer HashedObjectStore::retrieve(const uint256& hash) case 'A': htype = hotACCOUNT_NODE; break; case 'N': htype = hotTRANSACTION_NODE; break; default: + assert(false); cLog(lsERROR) << "Invalid hashed object"; mNegativeCache.add(hash); return HashedObject::pointer(); diff --git a/src/cpp/ripple/LedgerAcquire.cpp b/src/cpp/ripple/LedgerAcquire.cpp index f18675a37..f046b9ef6 100644 --- a/src/cpp/ripple/LedgerAcquire.cpp +++ b/src/cpp/ripple/LedgerAcquire.cpp @@ -193,13 +193,13 @@ void LedgerAcquire::done() mOnComplete.clear(); mLock.unlock(); - if (isComplete() && mLedger) + if (isComplete() && !isFailed() && mLedger) { if (mAccept) mLedger->setAccepted(); theApp->getLedgerMaster().storeLedger(mLedger); } - else if (isFailed()) + else theApp->getMasterLedgerAcquire().logFailure(mHash); for (unsigned int i = 0; i < triggers.size(); ++i) From 07634b51683b75fef8406c83ea253207a1769755 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 9 Jan 2013 10:29:34 -0800 Subject: [PATCH 13/19] This is probably what was causing all the trouble. --- src/cpp/ripple/SHAMapSync.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/ripple/SHAMapSync.cpp b/src/cpp/ripple/SHAMapSync.cpp index d564eebb9..7124403ac 100644 --- a/src/cpp/ripple/SHAMapSync.cpp +++ b/src/cpp/ripple/SHAMapSync.cpp @@ -49,7 +49,7 @@ void SHAMap::getMissingNodes(std::vector& nodeIDs, std::vectorgetChildNodeID(branch); const uint256& childHash = node->getChildHash(branch); - SHAMapTreeNode* d; + SHAMapTreeNode* d = NULL; try { d = getNodePointer(childID, childHash); From 6cf1b3dbc17067b37e4db9c4b1cec3303b66ebe2 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 9 Jan 2013 10:31:14 -0800 Subject: [PATCH 14/19] Tiny cleanup. --- src/cpp/ripple/LedgerAcquire.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cpp/ripple/LedgerAcquire.cpp b/src/cpp/ripple/LedgerAcquire.cpp index f046b9ef6..5fce7a9ee 100644 --- a/src/cpp/ripple/LedgerAcquire.cpp +++ b/src/cpp/ripple/LedgerAcquire.cpp @@ -238,7 +238,9 @@ void LedgerAcquire::trigger(Peer::ref peer) { tryLocal(); if (mFailed) + { cLog(lsWARNING) << " failed local for " << mHash; + } } ripple::TMGetLedger tmGL; From d8b79aa0ee2e0c44a50fc71557f3f4dac7e8b9b7 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 9 Jan 2013 10:42:23 -0800 Subject: [PATCH 15/19] Fix bad log type. --- src/cpp/ripple/Ledger.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cpp/ripple/Ledger.cpp b/src/cpp/ripple/Ledger.cpp index c12e1d92d..15d03084b 100644 --- a/src/cpp/ripple/Ledger.cpp +++ b/src/cpp/ripple/Ledger.cpp @@ -525,7 +525,7 @@ Ledger::pointer Ledger::getSQL(const std::string& sql) assert(false); return Ledger::pointer(); } - Log(lsTRACE) << "Loaded ledger: " << ledgerHash; + cLog(lsTRACE) << "Loaded ledger: " << ledgerHash; return ret; } From 38af3468813a3f54b806c4d0d865744adfe0ba70 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 9 Jan 2013 10:49:08 -0800 Subject: [PATCH 16/19] Fix some calls that bypass the cache. --- src/cpp/ripple/LedgerMaster.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cpp/ripple/LedgerMaster.cpp b/src/cpp/ripple/LedgerMaster.cpp index ab0e7a923..e3932c3a0 100644 --- a/src/cpp/ripple/LedgerMaster.cpp +++ b/src/cpp/ripple/LedgerMaster.cpp @@ -142,7 +142,7 @@ void LedgerMaster::asyncAccept(Ledger::pointer ledger) if ((ledger->getLedgerSeq() == 0) || mCompleteLedgers.hasValue(ledger->getLedgerSeq() - 1)) break; } - Ledger::pointer prevLedger = Ledger::loadByIndex(ledger->getLedgerSeq() - 1); + Ledger::pointer prevLedger = mLedgerHistory.getLedgerBySeq(ledger->getLedgerSeq() - 1); if (!prevLedger || (prevLedger->getHash() != ledger->getParentHash())) break; ledger = prevLedger; @@ -152,7 +152,7 @@ void LedgerMaster::asyncAccept(Ledger::pointer ledger) bool LedgerMaster::acquireMissingLedger(const uint256& ledgerHash, uint32 ledgerSeq) { // return: false = already gave up recently - Ledger::pointer ledger = Ledger::loadByIndex(ledgerSeq); + Ledger::pointer ledger = mLedgerHistory.getLedgerBySeq(ledgerSeq); if (ledger && (ledger->getHash() == ledgerHash)) { cLog(lsDEBUG) << "Ledger hash found in database"; From d9ab92e88edce196f41e72d71273d6b4e9487b56 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 9 Jan 2013 11:06:07 -0800 Subject: [PATCH 17/19] Cleanup. --- src/cpp/ripple/SHAMapSync.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cpp/ripple/SHAMapSync.cpp b/src/cpp/ripple/SHAMapSync.cpp index 7124403ac..7b16fbeee 100644 --- a/src/cpp/ripple/SHAMapSync.cpp +++ b/src/cpp/ripple/SHAMapSync.cpp @@ -115,10 +115,9 @@ std::vector SHAMap::getNeededHashes(int max) { SHAMapNode childID = node->getChildNodeID(branch); const uint256& childHash = node->getChildHash(branch); - SHAMapTreeNode* d; try { - d = getNodePointer(childID, childHash); + SHAMapTreeNode* d = getNodePointer(childID, childHash); assert(d); if (d->isInner() && !d->isFullBelow()) stack.push(d); From 7ed37066cfb8ea4704c733066b407ca3b04278c9 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 9 Jan 2013 15:55:18 -0800 Subject: [PATCH 18/19] Add some features to the KeyCache code so we can use it for ledger acquire failure tracking too. --- src/cpp/ripple/HashedObject.cpp | 3 +- src/cpp/ripple/KeyCache.h | 54 ++++++++++++++++++++---------- src/cpp/ripple/LedgerAcquire.cpp | 56 +++----------------------------- src/cpp/ripple/LedgerAcquire.h | 13 +++++--- 4 files changed, 53 insertions(+), 73 deletions(-) diff --git a/src/cpp/ripple/HashedObject.cpp b/src/cpp/ripple/HashedObject.cpp index 49a451050..cfa9dcdac 100644 --- a/src/cpp/ripple/HashedObject.cpp +++ b/src/cpp/ripple/HashedObject.cpp @@ -12,7 +12,8 @@ SETUP_LOG(); DECLARE_INSTANCE(HashedObject); HashedObjectStore::HashedObjectStore(int cacheSize, int cacheAge) : - mCache("HashedObjectStore", cacheSize, cacheAge), mWriteGeneration(0), mWritePending(false) + mCache("HashedObjectStore", cacheSize, cacheAge), mNegativeCache("HashedObjectNegativeCache", 0, 120), + mWriteGeneration(0), mWritePending(false) { mWriteSet.reserve(128); } diff --git a/src/cpp/ripple/KeyCache.h b/src/cpp/ripple/KeyCache.h index e5ec5262d..e4a1d8727 100644 --- a/src/cpp/ripple/KeyCache.h +++ b/src/cpp/ripple/KeyCache.h @@ -1,6 +1,8 @@ #ifndef KEY_CACHE__H #define KEY_CACHE__H +#include + #include #include @@ -12,40 +14,58 @@ public: typedef typename map_type::iterator map_iterator; protected: - boost::mutex mNCLock; - map_type mCache; - int mTargetSize, mTargetAge; + const std::string mName; + boost::mutex mNCLock; + map_type mCache; + int mTargetSize, mTargetAge; - uint64_t mHits, mMisses; - public: - KeyCache(int size = 0, int age = 120) : mTargetSize(size), mTargetAge(age), mHits(0), mMisses(0) + KeyCache(const std::string& name, int size = 0, int age = 120) : mName(name), mTargetSize(size), mTargetAge(age) { assert((mTargetSize >= 0) && (mTargetAge > 2)); } - void getStats(int& size, uint64_t& hits, uint64_t& misses) + void getSize() { boost::mutex::scoped_lock sl(mNCLock); - - size = mCache.size(); - hits = mHits; - misses = mMisses; + return mCache.size(); } - bool isPresent(const key_type& key) + void getTargetSize() + { + boost::mutex::scoped_lock sl(mNCLock); + return mTargetSize; + } + + void getTargetAge() + { + boost::mutex::scoped_lock sl(mNCLock); + return mTargetAge; + } + + void setTargets(int size, int age) + { + boost::mutex::scoped_lock sl(mNCLock); + mTargetSize = size; + mTargetAge = age; + assert((mTargetSize >= 0) && (mTargetAge > 2)); + } + + const std::string& getName() + { + return mName; + } + + bool isPresent(const key_type& key, bool refresh = true) { // Check if an entry is cached, refresh it if so boost::mutex::scoped_lock sl(mNCLock); map_iterator it = mCache.find(key); if (it == mCache.end()) - { - ++mMisses; return false; - } - it->second = time(NULL); - ++mHits; + if (refresh) + it->second = time(NULL); return true; } diff --git a/src/cpp/ripple/LedgerAcquire.cpp b/src/cpp/ripple/LedgerAcquire.cpp index 5fce7a9ee..e3ad9a92d 100644 --- a/src/cpp/ripple/LedgerAcquire.cpp +++ b/src/cpp/ripple/LedgerAcquire.cpp @@ -74,8 +74,9 @@ void PeerSet::TimerEntry(boost::weak_ptr wptr, const boost::system::err ptr->invokeOnTimer(); } -LedgerAcquire::LedgerAcquire(const uint256& hash) : PeerSet(hash, LEDGER_ACQUIRE_TIMEOUT), mHaveBase(false), - mHaveState(false), mHaveTransactions(false), mAborted(false), mSignaled(false), mAccept(false), mByHash(true) +LedgerAcquire::LedgerAcquire(const uint256& hash) : PeerSet(hash, LEDGER_ACQUIRE_TIMEOUT), + mHaveBase(false), mHaveState(false), mHaveTransactions(false), mAborted(false), mSignaled(false), mAccept(false), + mByHash(true) { #ifdef LA_DEBUG cLog(lsTRACE) << "Acquiring ledger " << mHash; @@ -724,57 +725,10 @@ SMAddNode LedgerAcquireMaster::gotLedgerData(ripple::TMLedgerData& packet, Peer: return SMAddNode::invalid(); } -void LedgerAcquireMaster::logFailure(const uint256& hash) -{ - time_t now = time(NULL); - boost::mutex::scoped_lock sl(mLock); - - std::map::iterator it = mRecentFailures.begin(); - while (it != mRecentFailures.end()) - { - if (it->first == hash) - { - it->second = now; - return; - } - if (it->second > now) - { // time jump or discontinuity - it->second = now; - ++it; - } - else if ((it->second + 180) < now) - mRecentFailures.erase(it++); - else - ++it; - } - mRecentFailures[hash] = now; -} - -bool LedgerAcquireMaster::isFailure(const uint256& hash) -{ - time_t now = time(NULL); - boost::mutex::scoped_lock sl(mLock); - - std::map::iterator it = mRecentFailures.find(hash); - if (it == mRecentFailures.end()) - return false; - - if (it->second > now) - { - it->second = now; - return true; - } - - if ((it->second + 180) < now) - { - mRecentFailures.erase(it); - return false; - } - return true; -} - void LedgerAcquireMaster::sweep() { + mRecentFailures.sweep(); + time_t now = time(NULL); boost::mutex::scoped_lock sl(mLock); diff --git a/src/cpp/ripple/LedgerAcquire.h b/src/cpp/ripple/LedgerAcquire.h index b5c338612..9c63a5d6b 100644 --- a/src/cpp/ripple/LedgerAcquire.h +++ b/src/cpp/ripple/LedgerAcquire.h @@ -18,6 +18,11 @@ #include "InstanceCounter.h" #include "ripple.pb.h" +// How long before we try again to acquire the same ledger +#ifndef LEDGER_REACQUIRE_INTERVAL +#define LEDGER_REACQUIRE_INTERVAL 180 +#endif + DEFINE_INSTANCE(LedgerAcquire); class PeerSet @@ -123,10 +128,10 @@ class LedgerAcquireMaster protected: boost::mutex mLock; std::map mLedgers; - std::map mRecentFailures; + KeyCache mRecentFailures; public: - LedgerAcquireMaster() { ; } + LedgerAcquireMaster() : mRecentFailures("LedgerAcquireRecentFailures", 0, LEDGER_REACQUIRE_INTERVAL) { ; } LedgerAcquire::pointer findCreate(const uint256& hash); LedgerAcquire::pointer find(const uint256& hash); @@ -134,8 +139,8 @@ public: void dropLedger(const uint256& ledgerHash); SMAddNode gotLedgerData(ripple::TMLedgerData& packet, Peer::ref); - void logFailure(const uint256&); - bool isFailure(const uint256&); + void logFailure(const uint256& h) { mRecentFailures.add(h); } + bool isFailure(const uint256& h) { return mRecentFailures.isPresent(h, false); } void sweep(); }; From cf284897bbd168e3c6061d4fdbece6031b974477 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 9 Jan 2013 15:55:41 -0800 Subject: [PATCH 19/19] Fix a case where we might try too quickly to re-acquire a ledger. --- src/cpp/ripple/LedgerMaster.cpp | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/cpp/ripple/LedgerMaster.cpp b/src/cpp/ripple/LedgerMaster.cpp index e3932c3a0..ffa0ed45d 100644 --- a/src/cpp/ripple/LedgerMaster.cpp +++ b/src/cpp/ripple/LedgerMaster.cpp @@ -444,19 +444,26 @@ void LedgerMaster::tryPublish() } else { - LedgerAcquire::pointer acq = theApp->getMasterLedgerAcquire().findCreate(hash); - if (!acq->isDone()) + if (theApp->getMasterLedgerAcquire().isFailure(hash)) { - acq->setAccept(); - break; - } - else if (acq->isComplete() && !acq->isFailed()) - { - mPubLedger = acq->getLedger(); - mPubLedgers.push_back(mPubLedger); + cLog(lsFATAL) << "Unable to acquire a recent validated ledger"; } else - cLog(lsWARNING) << "Failed to acquire a published ledger"; + { + LedgerAcquire::pointer acq = theApp->getMasterLedgerAcquire().findCreate(hash); + if (!acq->isDone()) + { + acq->setAccept(); + break; + } + else if (acq->isComplete() && !acq->isFailed()) + { + mPubLedger = acq->getLedger(); + mPubLedgers.push_back(mPubLedger); + } + else + cLog(lsWARNING) << "Failed to acquire a published ledger"; + } } } }