Fix some account_tx issues: (RIPD-1035)

* Sanely handled specified ledger in account_tx
* Reject un-validated ledger in account_tx
* Wait to publish a ledger until it's indexed
* Add unit test for PendingSaves
This commit is contained in:
JoelKatz
2015-09-28 16:29:11 -07:00
committed by Nik Bougalis
parent caa4ed31de
commit 5ee94f8928
11 changed files with 202 additions and 36 deletions

View File

@@ -2382,6 +2382,10 @@
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
</ClCompile>
<ClCompile Include="..\..\src\ripple\ledger\tests\PendingSaves_test.cpp">
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>
</ClCompile>
<ClCompile Include="..\..\src\ripple\ledger\tests\SkipList_test.cpp">
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='debug|x64'">True</ExcludedFromBuild>
<ExcludedFromBuild Condition="'$(Configuration)|$(Platform)'=='release|x64'">True</ExcludedFromBuild>

View File

@@ -3102,6 +3102,9 @@
<ClCompile Include="..\..\src\ripple\ledger\tests\PaymentSandbox_test.cpp">
<Filter>ripple\ledger\tests</Filter>
</ClCompile>
<ClCompile Include="..\..\src\ripple\ledger\tests\PendingSaves_test.cpp">
<Filter>ripple\ledger\tests</Filter>
</ClCompile>
<ClCompile Include="..\..\src\ripple\ledger\tests\SkipList_test.cpp">
<Filter>ripple\ledger\tests</Filter>
</ClCompile>

View File

@@ -928,8 +928,16 @@ void Ledger::updateSkipList ()
static bool saveValidatedLedger (
Application& app, std::shared_ptr<Ledger> const& ledger, bool current)
{
// TODO(tom): Fix this hard-coded SQL!
auto j = app.journal ("Ledger");
if (! app.pendingSaves().startWork (ledger->info().seq))
{
// The save was completed synchronously
JLOG (j.debug) << "Save aborted";
return true;
}
// TODO(tom): Fix this hard-coded SQL!
JLOG (j.trace)
<< "saveValidatedLedger "
<< (current ? "" : "fromAcquire ") << ledger->info().seq;
@@ -998,7 +1006,7 @@ static bool saveValidatedLedger (
app.getLedgerMaster().failedSave(seq, ledger->info().hash);
// Clients can now trust the database for information about this
// ledger sequence.
app.pendingSaves().erase(seq);
app.pendingSaves().finishWork(seq);
return false;
}
@@ -1098,7 +1106,7 @@ static bool saveValidatedLedger (
// Clients can now trust the database for
// information about this ledger sequence.
app.pendingSaves().erase(seq);
app.pendingSaves().finishWork(seq);
return true;
}
@@ -1110,18 +1118,28 @@ bool pendSaveValidated (Application& app,
{
if (! app.getHashRouter ().setFlags (ledger->info().hash, SF_SAVED))
{
// We have tried to save this ledger recently
JLOG (app.journal ("Ledger").debug) << "Double pend save for "
<< ledger->info().seq;
return true;
if (! isSynchronous ||
! app.pendingSaves().pending (ledger->info().seq))
{
// Either we don't need it to be finished
// or it is finished
return true;
}
}
assert (ledger->isImmutable ());
if (! app.pendingSaves().insert (ledger->info().seq))
if (! app.pendingSaves().shouldWork (ledger->info().seq, isSynchronous))
{
JLOG (app.journal ("Ledger").debug)
<< "Pend save with seq in pending saves "
<< ledger->info().seq;
return true;
}

View File

@@ -21,8 +21,9 @@
#define RIPPLE_APP_PENDINGSAVES_H_INCLUDED
#include <ripple/protocol/Protocol.h>
#include <boost/container/flat_set.hpp>
#include <map>
#include <mutex>
#include <condition_variable>
namespace ripple {
@@ -36,49 +37,111 @@ class PendingSaves
{
private:
std::mutex mutable mutex_;
boost::container::flat_set<LedgerIndex> set_;
std::map <LedgerIndex, bool> map_;
std::condition_variable await_;
public:
/** Add a ledger to the list.
This is called when the ledger is built but before
we have updated the SQLite indexes. Clients querying
the indexes will not see results from this ledger.
/** Start working on a ledger
@return `true` If the ledger indexes was not
already in the list.
This is called prior to updating the SQLite indexes.
@return 'true' if work should be done
*/
bool
insert (LedgerIndex seq)
startWork (LedgerIndex seq)
{
std::lock_guard<
std::mutex> lock(mutex_);
return set_.insert(seq).second;
std::lock_guard <std::mutex> lock(mutex_);
auto it = map_.find (seq);
if ((it == map_.end()) || it->second)
{
// Work done or another thread is doing it
return false;
}
it->second = true;
return true;
}
/** Remove a ledger from the list.
/** Finish working on a ledger
This is called after the ledger has been fully saved,
indicating that the SQLite indexes will produce correct
results in response to client requests.
This is called after updating the SQLite indexes.
The tracking of the work in progress is removed and
threads awaiting completion are notified.
*/
void
erase (LedgerIndex seq)
finishWork (LedgerIndex seq)
{
std::lock_guard<
std::mutex> lock(mutex_);
set_.erase(seq);
std::lock_guard <std::mutex> lock(mutex_);
map_.erase (seq);
await_.notify_all();
}
/** Returns a copy of the current set. */
auto
getSnapshot() const ->
decltype(set_)
/** Return `true` if a ledger is in the progress of being saved. */
bool
pending (LedgerIndex seq)
{
std::lock_guard<
std::mutex> lock(mutex_);
return set_;
std::lock_guard <std::mutex> lock(mutex_);
return map_.find(seq) != map_.end();
}
/** Check if a ledger should be dispatched
Called to determine whether work should be done or
dispatched. If work is already in progress and the
call is synchronous, wait for work to be completed.
@return 'true' if work should be done or dispatched
*/
bool
shouldWork (LedgerIndex seq, bool isSynchronous)
{
std::unique_lock <std::mutex> lock(mutex_);
do
{
auto it = map_.find (seq);
if (it == map_.end())
{
map_.emplace(seq, false);
return true;
}
if (! isSynchronous)
{
// Already dispatched
return false;
}
if (! it->second)
{
// Scheduled, but not dispatched
return true;
}
// Already in progress, just need to wait
await_.wait (lock);
} while (true);
}
/** Get a snapshot of the pending saves
Each entry in the returned map corresponds to a ledger
that is in progress or dispatched. The boolean indicates
whether work is currently in progress.
*/
std::map <LedgerIndex, bool>
getSnapshot () const
{
std::lock_guard <std::mutex> lock(mutex_);
return map_;
}
};
}

View File

@@ -480,12 +480,12 @@ public:
// Best effort for remaining exclusions
for(auto v : pendingSaves)
{
if ((v >= minVal) && (v <= maxVal))
if ((v.first >= minVal) && (v.first <= maxVal))
{
if (v > ((minVal + maxVal) / 2))
maxVal = v - 1;
if (v.first > ((minVal + maxVal) / 2))
maxVal = v.first - 1;
else
minVal = v + 1;
minVal = v.first + 1;
}
}

View File

@@ -0,0 +1,61 @@
//------------------------------------------------------------------------------
/*
This file is part of rippled: https://github.com/ripple/rippled
Copyright (c) 2012, 2013 Ripple Labs Inc.
Permission to use, copy, modify, and/or distribute this software for any
purpose with or without fee is hereby granted, provided that the above
copyright notice and this permission notice appear in all copies.
THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
//==============================================================================
#include <BeastConfig.h>
#include <ripple/app/ledger/PendingSaves.h>
#include <beast/unit_test/suite.h>
namespace ripple {
namespace test {
struct PendingSaves_test : public beast::unit_test::suite
{
void testSaves()
{
PendingSaves ps;
// Basic test
expect (!ps.pending (0));
expect (!ps.startWork(0));
expect (ps.shouldWork (0, true));
expect (ps.startWork (0));
expect (ps.pending (0));
expect (! ps.shouldWork (0, false));
ps.finishWork(0);
expect (!ps.pending (0));
// Test work stealing
expect (ps.shouldWork (0, false));
expect (ps.pending (0));
expect (ps.shouldWork (0, true));
expect (ps.pending (0));
expect (ps.startWork (0));
expect (!ps.startWork (0));
ps.finishWork(0);
expect (! ps.pending (0));
}
void run() override
{
testSaves();
}
};
BEAST_DEFINE_TESTSUITE(PendingSaves,ledger,ripple);
} // test
} // ripple

View File

@@ -62,6 +62,7 @@ enum error_code_i
rpcACT_NOT_FOUND,
rpcINSUF_FUNDS,
rpcLGR_NOT_FOUND,
rpcLGR_NOT_VALIDATED,
rpcMASTER_DISABLED,
rpcNO_ACCOUNT,
rpcNO_PATH,

View File

@@ -77,6 +77,7 @@ public:
add (rpcLGR_IDXS_INVALID, "lgrIdxsInvalid", "Ledger indexes invalid.");
add (rpcLGR_IDX_MALFORMED, "lgrIdxMalformed", "Ledger index malformed.");
add (rpcLGR_NOT_FOUND, "lgrNotFound", "Ledger not found.");
add (rpcLGR_NOT_VALIDATED, "lgrNotValidated", "Ledger not validated.");
add (rpcLOAD_FAILED, "loadFailed", "Load failed");
add (rpcMASTER_DISABLED, "masterDisabled", "Master key is disabled.");
add (rpcNOT_ENABLED, "notEnabled", "Not enabled in configuration.");

View File

@@ -99,6 +99,13 @@ Json::Value doAccountTx (RPC::Context& context)
if (! ledger)
return ret;
if (! ret[jss::validated].asBool() ||
(ledger->info().seq > uValidatedMax) ||
(ledger->info().seq < uValidatedMin))
{
return rpcError (rpcLGR_NOT_VALIDATED);
}
uLedgerMin = uLedgerMax = ledger->info().seq;
}

View File

@@ -119,6 +119,13 @@ Json::Value doAccountTxOld (RPC::Context& context)
if (!ledger)
return ret;
if (! ret[jss::validated].asBool() ||
(ledger->info().seq > uValidatedMax) ||
(ledger->info().seq < uValidatedMin))
{
return rpcError (rpcLGR_NOT_VALIDATED);
}
uLedgerMin = uLedgerMax = ledger->info().seq;
}

View File

@@ -38,3 +38,4 @@
#include <ripple/ledger/tests/PaymentSandbox_test.cpp>
#include <ripple/ledger/tests/SkipList_test.cpp>
#include <ripple/ledger/tests/View_test.cpp>
#include <ripple/ledger/tests/PendingSaves_test.cpp>