ARCHIVE/xahaud
3
0
Fork 0
mirror of https://github.com/Xahau/xahaud.git synced 2025-12-06 17:27:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

Remove dead code and fix a race condition in the node back end.

Browse Source 
Race was between a store and a fetch, as follows:
1) Fetch: Search for the node, it is not found.
2) Store: Add the node to the database, remove the node from the negative cache.
3) Fetch: Add the node to the negative cache.
This would leave a node in the DB, cache, and negative cache.
This commit is contained in:
David Schwartz
2013-12-03 16:01:08 -08:00
committed by JoelKatz
parent 0f2a657196
commit 0e53105ab5
3 changed files with 52 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
src/ripple_core/nodestore/impl/BatchWriter.cpp
View File

@@ -23,7 +23,6 @@ namespace NodeStore
BatchWriter::BatchWriter (Callback& callback, Scheduler& scheduler)
: m_callback (callback)
, m_scheduler (scheduler)
, mWriteGeneration (0)
, mWriteLoad (0)
, mWritePending (false)
{
@@ -63,8 +62,6 @@ void BatchWriter::performScheduledTask ()
void BatchWriter::writeBatch ()
{
int setSize = 0;
for (;;)
{
std::vector< boost::shared_ptr<NodeObject> > set;
@@ -76,21 +73,17 @@ void BatchWriter::writeBatch ()
mWriteSet.swap (set);
assert (mWriteSet.empty ());
++mWriteGeneration;
mWriteCondition.notify_all ();
mWriteLoad = set.size ();
if (set.empty ())
{
mWritePending = false;
mWriteLoad = 0;
mWriteCondition.notify_all ();
// VFALCO NOTE Fix this function to not return from the middle
return;
}
int newSetSize = mWriteSet.size();
mWriteLoad = std::max (setSize, newSetSize);
newSetSize = set.size ();
}
m_callback.writeBatch (set);
@@ -100,9 +93,8 @@ void BatchWriter::writeBatch ()
void BatchWriter::waitForWriting ()
{
LockType::scoped_lock sl (mWriteMutex);
int gen = mWriteGeneration;
while (mWritePending && (mWriteGeneration == gen))
while (mWritePending)
mWriteCondition.wait (sl);
}

1
src/ripple_core/nodestore/impl/BatchWriter.h
View File

@@ -76,7 +76,6 @@ private:
Scheduler& m_scheduler;
LockType mWriteMutex;
CondvarType mWriteCondition;
int mWriteGeneration;
int mWriteLoad;
bool mWritePending;
Batch mWriteSet;

116
src/ripple_core/nodestore/impl/DatabaseImp.h
View File

@@ -34,7 +34,6 @@ public:
, m_fastBackend ((fastBackendParameters.size () > 0)
? createBackend (fastBackendParameters, scheduler) : nullptr)
, m_cache ("NodeStore", 16384, 300)
, m_negativeCache ("NoteStoreNegativeCache", 0, 120)
{
}
@@ -57,75 +56,61 @@ public:
if (obj == nullptr)
{
// It's not in the cache, see if we can skip checking the db.
// There's still a chance it could be in one of the databases.
bool foundInFastBackend = false;
// Check the fast backend database if we have one
//
if (! m_negativeCache.isPresent (hash))
if (m_fastBackend != nullptr)
{
// There's still a chance it could be in one of the databases.
obj = fetchInternal (m_fastBackend, hash);
bool foundInFastBackend = false;
// Check the fast backend database if we have one
//
if (m_fastBackend != nullptr)
{
obj = fetchInternal (m_fastBackend, hash);
// If we found the object, avoid storing it again later.
if (obj != nullptr)
foundInFastBackend = true;
}
// Are we still without an object?
//
if (obj == nullptr)
{
// Yes so at last we will try the main database.
//
{
// Monitor this operation's load since it is expensive.
//
// VFALCO TODO Why is this an autoptr? Why can't it just be a plain old object?
//
// VFALCO NOTE Commented this out because it breaks the unit test!
//
//LoadEvent::autoptr event (getApp().getJobQueue ().getLoadEventAP (jtHO_READ, "HOS::retrieve"));
obj = fetchInternal (m_backend, hash);
}
// If it's not in the main database, remember that so we
// can skip the lookup for the same object again later.
//
if (obj == nullptr)
m_negativeCache.add (hash);
}
// Did we finally get something?
//
// If we found the object, avoid storing it again later.
if (obj != nullptr)
{
// Yes it so canonicalize. This solves the problem where
// more than one thread has its own copy of the same object.
//
m_cache.canonicalize (hash, obj);
if (! foundInFastBackend)
{
// If we have a fast back end, store it there for later.
//
if (m_fastBackend != nullptr)
m_fastBackend->store (obj);
// Since this was a 'hard' fetch, we will log it.
//
WriteLog (lsTRACE, NodeObject) << "HOS: " << hash << " fetch: in db";
}
}
foundInFastBackend = true;
}
else
// Are we still without an object?
//
if (obj == nullptr)
{
// hash is known not to be in the database
// Yes so at last we will try the main database.
//
{
// Monitor this operation's load since it is expensive.
//
// VFALCO TODO Why is this an autoptr? Why can't it just be a plain old object?
//
// VFALCO NOTE Commented this out because it breaks the unit test!
//
//LoadEvent::autoptr event (getApp().getJobQueue ().getLoadEventAP (jtHO_READ, "HOS::retrieve"));
obj = fetchInternal (m_backend, hash);
}
}
// Did we finally get something?
//
if (obj != nullptr)
{
// Yes it so canonicalize. This solves the problem where
// more than one thread has its own copy of the same object.
//
m_cache.canonicalize (hash, obj);
if (! foundInFastBackend)
{
// If we have a fast back end, store it there for later.
//
if (m_fastBackend != nullptr)
m_fastBackend->store (obj);
// Since this was a 'hard' fetch, we will log it.
//
WriteLog (lsTRACE, NodeObject) << "HOS: " << hash << " fetch: in db";
}
}
}
else
@@ -192,7 +177,6 @@ public:
m_fastBackend->store (object);
}
m_negativeCache.del (hash);
}
}
@@ -212,7 +196,6 @@ public:
void sweep ()
{
m_cache.sweep ();
m_negativeCache.sweep ();
}
int getWriteLoad ()
@@ -316,7 +299,6 @@ private:
// VFALCO NOTE What are these things for? We need comments.
TaggedCacheType <uint256, NodeObject, UptimeTimerAdapter> m_cache;
KeyCache <uint256, UptimeTimerAdapter> m_negativeCache;
};
//------------------------------------------------------------------------------